Re: [yang-doctors] Yangdoctors last call review of draft-ietf-lsr-ospfv3-extended-lsa-yang-14
Acee Lindem <acee.ietf@gmail.com> Mon, 26 June 2023 20:44 UTC
Return-Path: <acee.ietf@gmail.com>
X-Original-To: yang-doctors@ietfa.amsl.com
Delivered-To: yang-doctors@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id A1E7BC151707; Mon, 26 Jun 2023 13:44:01 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.095
X-Spam-Level:
X-Spam-Status: No, score=-2.095 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001, URIBL_DBL_BLOCKED_OPENDNS=0.001, URIBL_ZEN_BLOCKED_OPENDNS=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([50.223.129.194]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id td5BwLjd60zg; Mon, 26 Jun 2023 13:43:57 -0700 (PDT)
Received: from mail-qk1-x733.google.com (mail-qk1-x733.google.com [IPv6:2607:f8b0:4864:20::733]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id D4148C1516E9; Mon, 26 Jun 2023 13:43:54 -0700 (PDT)
Received: by mail-qk1-x733.google.com with SMTP id af79cd13be357-7659cb9c42aso140477585a.3; Mon, 26 Jun 2023 13:43:54 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1687812234; x=1690404234; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=eAOvS2ywt7aFCfgDZhFi2D5b9KSg+DTXcQYM2bOZQLk=; b=AxGCtueXC2YpAo8YxocZsoxGZ/XycHdKjNMN8M/3i48HoNxduZR2E97swb0o+2DJVv J2gVdKbNijgmV7X/Hqd+04htbZyCurks0Zn2bM5/WeTYvqm7qA44+50AkwDEV63KlPgf HkvIFu/SkVCiF1D5dkTRbsOpKm3PFQpGLYfMohl23b5DTPNZiJPGx/OesjGKKq5tdApv lkLwPT1RPUXfLZOQjtjwnifas2vfa3nn2OW1MSmDF5vfwjZbkt01Grp6EJOMsXgXq07V BiEUFIGXDzfrr1lX6goWc7I17+Q8CsZmlo+pb7NygoAJZSul5ohbKXK9P1RaI51ul28Z 5MYQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687812234; x=1690404234; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=eAOvS2ywt7aFCfgDZhFi2D5b9KSg+DTXcQYM2bOZQLk=; b=T+PYI2kxHL7pgXvGFexTmQYZa0zGE+74oovjiN0IzCL9rtD0Khzwz0zfvbrlW0eSkW 9ai42v+qczzFSZAOW+CLujbr83e76xhu7qqaACr6A2b4krAHdI/KX8+14iElQZi7wJuD K79AWwpOiNHKosE0xSck00rNi5z3mOh3U1i35nVNQ5wQXUjsJ+UwujkYhxSnrguOqHHN 9tjkSv3wOOK9arGwoOfyHOb7wxW5lz2+SUUWyYgbCcmm28juIvEtDOFIysuREkqP+qLo usFy0sU5wEVgW9GBHFjiJYwrSHFYXGa6n0vutsLRJX4BWnyp7J9zAhaeLrxNf8UAOzcC FaOg==
X-Gm-Message-State: AC+VfDztIz/WRgitZ1hRNG17zQ/n/ktJCi5wNBUsDfQa9fXvq5fj2F+9 KzJlwJh9NSBJnLivADc7dK+yy8imDes=
X-Google-Smtp-Source: ACHHUZ4ZD257vJA/dkLwcB9dbcVxfc96eYTZC5wRsUfWYtDU/twUoVfbIreMFqEYf9f0o9fjhtz/dA==
X-Received: by 2002:a05:620a:2889:b0:75b:23a1:366f with SMTP id j9-20020a05620a288900b0075b23a1366fmr35686103qkp.48.1687812233767; Mon, 26 Jun 2023 13:43:53 -0700 (PDT)
Received: from smtpclient.apple ([136.54.43.175]) by smtp.gmail.com with ESMTPSA id o16-20020a05620a111000b0076548310f04sm3039746qkk.127.2023.06.26.13.43.52 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 26 Jun 2023 13:43:53 -0700 (PDT)
Content-Type: text/plain; charset="utf-8"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.600.7\))
From: Acee Lindem <acee.ietf@gmail.com>
In-Reply-To: <168686390127.21246.1020447065154658425@ietfa.amsl.com>
Date: Mon, 26 Jun 2023 16:43:42 -0400
Cc: yang-doctors@ietf.org, draft-ietf-lsr-ospfv3-extended-lsa-yang.all@ietf.org, last-call@ietf.org, lsr@ietf.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <2AC4934D-05FC-47E4-98A2-478445AAC976@gmail.com>
References: <168686390127.21246.1020447065154658425@ietfa.amsl.com>
To: Mahesh Jethanandani <mjethanandani@gmail.com>
X-Mailer: Apple Mail (2.3731.600.7)
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/riT5CKJWazARWY3IerYCqHAlAYk>
Subject: Re: [yang-doctors] Yangdoctors last call review of draft-ietf-lsr-ospfv3-extended-lsa-yang-14
X-BeenThere: yang-doctors@ietf.org
X-Mailman-Version: 2.1.39
Precedence: list
List-Id: Email list of the yang-doctors directorate <yang-doctors.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/yang-doctors/>
List-Post: <mailto:yang-doctors@ietf.org>
List-Help: <mailto:yang-doctors-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 26 Jun 2023 20:44:01 -0000
Hi Mahesh, Thanks for the review - a lot of good comments. See inline and -16 version. > On Jun 15, 2023, at 5:18 PM, Mahesh Jethanandani via Datatracker <noreply@ietf.org> wrote: > > Reviewer: Mahesh Jethanandani > Review result: On the Right Track > > Document reviewed: draft-ietf-lsr-ospfv3-extended-lsa-yang > > Status: On the right track > > I have marked it as On the Right Track, because of some of the points discussed > below. > > Summary: > > This document defines a YANG data model augmenting the IETF OSPF YANG model to > provide support for OSPFv3 Link State Advertisement (LSA) Extensibility as > defined in RFC 8362. OSPFv3 Extended LSAs provide extensible TLV-based LSAs for > the base LSA types defined in RFC 5340. > > Nits > > Please add a section on Instructions to RFC editors stating what they should do > with references such as RFC XXXX. > > It would be nice to have some consistency between having description and > reference statements start on a new line or on the same line as the statement. > Right now, they are all over the place. > > Some of the descriptions are very cryptic. E.g. > > leaf forwarding-address { > type inet:ipv4-address; > description > "Forwarding address"; I updated the ones that were brief and cryptic. Note that you almost have to have knowledge of RFC 5340 and RFC 8362 to understand the encodings. > > s/Description/description in the YANG model. Actually, I was surprised that > pyang did not complain, but yanglint did. > > libyang err : Invalid character sequence "Description", expected a keyword. > (Line number 318.) libyang err : Parsing module "ietf-ospfv3-extended-lsa" > failed. YANGLINT[E]: Parsing schema module > "ietf-ospfv3-extended-lsa@2023-06-08.yang" failed. Fixed - I’m surprised pyang didn’t complain as well. > > s/Addrss/Address/ Fixed. > > s/E-/Extended / in all descriptions. When referring to the actual LSAs, it is should be “E-“. For example, E-Router-LSA. In other cases, it is spelled out. See RFC 8362. > > Comments: > > The grouping such as ospfv3-e-lsa-as, ospfv3-e-lsa-area, ipv6-fwd-addr-sub-tlv > etc. are used in one place only. Is there a reason why this has not been pulled > inline where it is used? Did not check for all groupings, but if there is only > one use of them, ideally they should be inlined. I consolidated these for the link, area, and AS scoped LSDBs. I left the fowarding-address Sub-TLV in its own grouping consistent with the other Sub-TLVs. > > No need to repeat parent name in the child. Just length will do in the > following. See Section 4.3.1 of RFC 8407. E.g. > > container route-tag-sub-tlv { > description > "Route Tag Sub-TLV"; > leaf route-tag-sub-tlv-length { Fixed. > > Why a double -- in container unknown--tlv {? Fixed. > > A pyang compilation of the model with —ietf and —lint option was clean. > > There are no examples of configuration instance data in the draft. It would be > helpful not only to validate the model, but also help folks who want to use the > model. There are only two booleans that are config=true. We can look at this though. > > A idnits run of the draft reveals a few issues. Please address them. > > Checking boilerplate required by RFC 5378 and the IETF Trust (see > https://trustee.ietf.org/license-info): > --------------------------------------------------------------------- > > No issues found here. > > Checking nits according to > https://www.ietf.org/id-info/1id-guidelines.txt: > --------------------------------------------------------------------- > > No issues found here. > > Checking nits according to https://www.ietf.org/id-info/checklist : > --------------------------------------------------------------------- > > No issues found here. > > Miscellaneous warnings: > --------------------------------------------------------------------- > > == The copyright year in the IETF Trust and authors Copyright Line > does not match the current year > > == Line 1266 has weird spacing: '... allows a rou...' > > -- The document date (October 17, 2019) is 1337 days in the past. > Is this intentional? > > Checking references for intended status: Proposed Standard > --------------------------------------------------------------------- > > (See RFCs 3967 and 4897 for information about using normative > references to lower-maturity documents in RFCs) > > == Outdated reference: draft-ietf-bfd-yang has been published as RFC > 9127 > > ** Downref: Normative reference to an Experimental RFC: RFC 1765 > > ** Downref: Normative reference to an Experimental RFC: RFC 4973 > > ** Downref: Normative reference to an Informational RFC: RFC 5309 > > ** Downref: Normative reference to an Informational RFC: RFC 5714 > > ** Downref: Normative reference to an Informational RFC: RFC 6987 These idnits are fixed. Thanks, Acee > > Summary: 5 errors (**), 0 flaws (~~), 3 warnings (==), 1 comment > (--). > > >
- [yang-doctors] Yangdoctors last call review of dr… Mahesh Jethanandani via Datatracker
- Re: [yang-doctors] Yangdoctors last call review o… Acee Lindem
- Re: [yang-doctors] Yangdoctors last call review o… Yingzhen Qu
- Re: [yang-doctors] Yangdoctors last call review o… Mahesh Jethanandani
- Re: [yang-doctors] [Lsr] Yangdoctors last call re… Renato Westphal
- Re: [yang-doctors] [Lsr] Yangdoctors last call re… Acee Lindem