Re: [Lsr] Yangdoctors last call review of draft-ietf-lsr-ospfv3-extended-lsa-yang-14
Yingzhen Qu <yingzhen.ietf@gmail.com> Tue, 27 June 2023 20:23 UTC
Return-Path: <yingzhen.ietf@gmail.com>
X-Original-To: lsr@ietfa.amsl.com
Delivered-To: lsr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id ECD62C151092; Tue, 27 Jun 2023 13:23:24 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.094
X-Spam-Level:
X-Spam-Status: No, score=-2.094 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, HTML_MESSAGE=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 OXtsDq3G1ukR; Tue, 27 Jun 2023 13:23:24 -0700 (PDT)
Received: from mail-vk1-xa30.google.com (mail-vk1-xa30.google.com [IPv6:2607:f8b0:4864:20::a30]) (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 1C8AAC14CE22; Tue, 27 Jun 2023 13:23:24 -0700 (PDT)
Received: by mail-vk1-xa30.google.com with SMTP id 71dfb90a1353d-47dcf42d3a0so177286e0c.1; Tue, 27 Jun 2023 13:23:24 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1687897403; x=1690489403; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=zXlKVp8v4fqTMCwdGbZsdjD4vzqAsVCSCZqSiDStWEA=; b=pUMHSgTHiNBJoTXIMjVydvSnZdwbJpc9M/pKGJ/V1+MgylWz9cTHZKqyVKjO9E2hC8 jlVZWseWVFS3UHA/XEZzrfzMiDDvsylLVe+qstvXNf6Mk8/zpbkgUDhNp1jk9NQwlUwS N7qSGTez0TvPHoiV8l46mIPWIHtBNaT0eCEAnDggM3bWaf+oz3mMh+oNnoU5pAiXSrF5 vqWjt5miXtoLjh8jV9MuVDv2Y/dab/h7kMcgsVU9kelioLvfzjYK/jNxtOL/ZYqx5Z3W LwbxQBtia/TVsj0O3bedytgHwb8IkpmzkEe+IPRJMlIEU0vOnlq/K5FYiHxjVNAa+mM+ CGRg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687897403; x=1690489403; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=zXlKVp8v4fqTMCwdGbZsdjD4vzqAsVCSCZqSiDStWEA=; b=Ngyc4lr90THcuXHUM3LRKYsZdxpOZI5CmdwuWKlARLtsWZgOJlUpFN+QCADCPNDfMH QtYXBrA+4pS5M8EStmUJk+8PniTlLLwLtk44NIAgU8k1HdgxbaY+TnExozsuaNo+9v+X BjnqCyRAu9KmoPJeaL3IPuZraJCF64dAvzWuPFynmvq/1/bPgrLuxGKx5Y8BLOGXdodr GxNkHASuPNOy5m01PhPhFTBN5YmcJr6JcGpqJ0ZZaHfKj172jQ3SUOKqnAZL9ua/3TbL ovT38Am4/J3D9+lPGMEgfx7x9lB2nPQI7WuFcCwSa8auK6hwi/UjR+ZbA1qFbTt1rL/0 H7OQ==
X-Gm-Message-State: AC+VfDz/qA15/jHuOsHvGdiy1Y1hg4ItAQ6XWPX08OFQ/3i3GQ+av3rL +kJoY6s+yCga3tJ+cpvduNrXTRRoBIzF+PoybqAsLic=
X-Google-Smtp-Source: ACHHUZ4SyeKbEEQr8wTMgLTSGTZyoxF676kG3xeYhP8OTDFOLlLpy0T3E9AMMv6OzSHUSZLIdcxemDdMRMhcfjjkaPI=
X-Received: by 2002:a1f:608e:0:b0:471:cb70:637d with SMTP id u136-20020a1f608e000000b00471cb70637dmr70794vkb.1.1687897403102; Tue, 27 Jun 2023 13:23:23 -0700 (PDT)
MIME-Version: 1.0
References: <168686390127.21246.1020447065154658425@ietfa.amsl.com> <2AC4934D-05FC-47E4-98A2-478445AAC976@gmail.com>
In-Reply-To: <2AC4934D-05FC-47E4-98A2-478445AAC976@gmail.com>
From: Yingzhen Qu <yingzhen.ietf@gmail.com>
Date: Tue, 27 Jun 2023 13:23:12 -0700
Message-ID: <CABY-gOMcBzv04NWXw9O7VOHdC3fXwmYut7PJaYNCzwyuOC_r5A@mail.gmail.com>
To: Acee Lindem <acee.ietf@gmail.com>
Cc: Mahesh Jethanandani <mjethanandani@gmail.com>, yang-doctors@ietf.org, draft-ietf-lsr-ospfv3-extended-lsa-yang.all@ietf.org, last-call@ietf.org, lsr@ietf.org
Content-Type: multipart/alternative; boundary="0000000000002119a305ff223d8a"
Archived-At: <https://mailarchive.ietf.org/arch/msg/lsr/JJ6ZqyFv9-tZKpWFPRFjfirlb90>
Subject: Re: [Lsr] Yangdoctors last call review of draft-ietf-lsr-ospfv3-extended-lsa-yang-14
X-BeenThere: lsr@ietf.org
X-Mailman-Version: 2.1.39
Precedence: list
List-Id: Link State Routing Working Group <lsr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/lsr>, <mailto:lsr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/lsr/>
List-Post: <mailto:lsr@ietf.org>
List-Help: <mailto:lsr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/lsr>, <mailto:lsr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 27 Jun 2023 20:23:25 -0000
Hi Mahesh, We just uploaded version -17 and added a configuration example. Please let us know if you have any other comments. Thanks, Yingzhen On Mon, Jun 26, 2023 at 1:44 PM Acee Lindem <acee.ietf@gmail.com> wrote: > 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 > > (--). > > > > > > > >
- [Lsr] Yangdoctors last call review of draft-ietf-… Mahesh Jethanandani via Datatracker
- Re: [Lsr] Yangdoctors last call review of draft-i… Acee Lindem
- Re: [Lsr] Yangdoctors last call review of draft-i… Yingzhen Qu
- Re: [Lsr] Yangdoctors last call review of draft-i… Mahesh Jethanandani
- Re: [Lsr] Yangdoctors last call review of draft-i… Renato Westphal
- Re: [Lsr] Yangdoctors last call review of draft-i… Acee Lindem