Re: [Lsr] [yang-doctors] Yangdoctors last call review of draft-ietf-isis-yang-is-is-cfg-29
Ladislav Lhotka <lhotka@nic.cz> Mon, 07 January 2019 15:18 UTC
Return-Path: <lhotka@nic.cz>
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 5FC52130E97; Mon, 7 Jan 2019 07:18:59 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.9
X-Spam-Level:
X-Spam-Status: No, score=-1.9 tagged_above=-999 required=5 tests=[BAYES_00=-1.9] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id cVL9nHdBvW8d; Mon, 7 Jan 2019 07:18:55 -0800 (PST)
Received: from trail.lhotka.name (trail.lhotka.name [77.48.224.143]) by ietfa.amsl.com (Postfix) with ESMTP id E96691276D0; Mon, 7 Jan 2019 07:18:54 -0800 (PST)
Received: by trail.lhotka.name (Postfix, from userid 109) id 095761820593; Mon, 7 Jan 2019 16:27:53 +0100 (CET)
Received: from localhost (unknown [195.113.220.121]) by trail.lhotka.name (Postfix) with ESMTPSA id D83B6182015B; Mon, 7 Jan 2019 16:27:48 +0100 (CET)
From: Ladislav Lhotka <lhotka@nic.cz>
To: Juergen Schoenwaelder <j.schoenwaelder@jacobs-university.de>, stephane.litkowski@orange.com
Cc: tom petch <ietfc@btconnect.com>, Ebben Aries <exa@juniper.net>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>, "draft-ietf-isis-yang-isis-cfg.all@ietf.org" <draft-ietf-isis-yang-isis-cfg.all@ietf.org>, "lsr@ietf.org" <lsr@ietf.org>
In-Reply-To: <20190107101523.as7u7y64trth5cro@anna.jacobs.jacobs-university.de>
References: <154025553381.13801.5009678921928527816@ietfa.amsl.com> <03ff01d48641$8f8d8600$4001a8c0@gateway.2wire.net> <19850_1543334259_5BFD6973_19850_302_6_9E32478DFA9976438E7A22F69B08FF924B7768B5@OPEXCLILMA4.corporate.adroot.infra.ftgroup> <009101d4a135$a59c2780$4001a8c0@gateway.2wire.net> <009101d4a28c$905b2300$4001a8c0@gateway.2wire.net> <10843_1546855502_5C33244E_10843_420_6_9E32478DFA9976438E7A22F69B08FF924B78A451@OPEXCLILMA4.corporate.adroot.infra.ftgroup> <20190107101523.as7u7y64trth5cro@anna.jacobs.jacobs-university.de>
Date: Mon, 07 Jan 2019 16:18:47 +0100
Message-ID: <87va30ts14.fsf@nic.cz>
MIME-Version: 1.0
Content-Type: text/plain
Archived-At: <https://mailarchive.ietf.org/arch/msg/lsr/-i3nMREDjn6JSF36TOfdIrZc-8o>
Subject: Re: [Lsr] [yang-doctors] Yangdoctors last call review of draft-ietf-isis-yang-is-is-cfg-29
X-BeenThere: lsr@ietf.org
X-Mailman-Version: 2.1.29
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: Mon, 07 Jan 2019 15:18:59 -0000
Juergen Schoenwaelder <j.schoenwaelder@jacobs-university.de> writes: > Defining and standardizing arbitrary limits is, well, arbitrary. What > we often want, I think, is 'implementations should at least handle a > length of x characters' but we have no way to say that in YANG today > (other than description statements). Also note that # characters != # IMO this is not a big issue here because the leaf in question is state data. We have to assume that the client doesn't connect to a rogue server, and that legitimate servers behave reasonably. Lada > bytes with UTF-8/Unicode character sets. > > /js > > On Mon, Jan 07, 2019 at 10:05:00AM +0000, stephane.litkowski@orange.com wrote: >> Hi Tom, >> >> Regarding the length enforcement for operational state leaves that are using string, do we need to always do it as a best practice ? There are plenty of strings with no enforcement today like the "non-best-reason" that you have pointed. >> >> Brgds, >> >> >> -----Original Message----- >> From: tom petch [mailto:ietfc@btconnect.com] >> Sent: Wednesday, January 02, 2019 12:17 >> To: LITKOWSKI Stephane OBS/OINIS; Ebben Aries; yang-doctors@ietf.org >> Cc: draft-ietf-isis-yang-isis-cfg.all@ietf.org; lsr@ietf.org >> Subject: Re: [Lsr] Yangdoctors last call review of draft-ietf-isis-yang-is-is-cfg-29 >> >> Here are the rest of my comments on -29 with a slight tweak to the >> subject line. I would regard most of these (but not the first two) as >> non-discussable, ie I won't complain if you disagree:-) >> >> RFC1195 is in the YANG module but not the references of the I-D >> >> RFC5029 is in the YANG module but not the references of the I-D >> >> PSNPs, CSNPs >> expand on first use - LSP I think ok >> >> leaf best { type boolean; >> what is true and what false? I can guess from the English semantics of >> the name but would rather not guess. >> >> leaf non-best-reason { type string; >> suggest a maximum length, perhaps 127 or 255 ( unless you expect >> screenshots or packet traces to be attached). As it stands, you could >> validly receive >> a length of 18446744073709551615. >> >> You have a mixture of >> System-id system-id System id System ID System Id system id system ID >> suggest consistency; system-id wfm >> >> You have a mixture of >> lsp-id LSPID LSP ID >> here, perhaps lsp-id for the names and LSP ID in the text >> >> case password { leaf key { type string; >> perhaps better with a minimum length >> >> leaf i-e { type boolean; >> what is true and what false? here I am reluctant even to guess >> >> /"Authentication keyto/ "Authentication key to/ >> >> " the authentication key MUST NOT be presented in" >> RFC2119 language means that RFC2119 boilerplate should be in the YANG >> module (but without the [..] ie the reference must be plain text not an >> anchor). >> >> >> It is recommended to use an MD5 >> hash to present the authentication-key."; >> Mmm I think that this may be a red flag to security AD or directorate as >> being too vague as well as MD5 too weak; and I think this should be >> explicitly called out in Security Considerations. >> >> list level-db { key level; leaf level { >> A common convention is for a list of leaf thing to be named things i.e. >> list levels { key level; leaf level { >> >> rpc clear-adjacency { >> "Name of the IS-IS protocol instance whose IS-IS >> information is being queried. >> queried or cleared? >> >> Tom Petch >> >> >> ----- Original Message ----- >> From: "tom petch" <ietfc@btconnect.com> >> Sent: Monday, December 31, 2018 6:21 PM >> Subject: Re: [Lsr] Yangdoctors last call review of >> draft-ietf-isis-yang-isis-cfg-24 >> >> >> > Stephane >> > >> > A new and different comment. >> > >> > grouping neighbor-gmpls-extensions { >> > >> > has a text reference to RFC5307 which does not appear in the >> references >> > for the I-D. However, before adding it, I would like to know why it >> is >> > a good reference for switching capabilities (which is part of this >> > grouping). I think that the reference for switching capabilities >> should >> > be RFC7074 (which this I-D does not currently reference and should >> IMO). >> > >> > And that begs the question, why is switching-capability an >> unrestricted >> > uint8 when only 12 values are valid and three are deprecated? >> > >> > So why not use >> > >> > draft-ietf-teas-yang-te-types? >> > >> > I have a number of additional comments on cfg-29 but this is the one >> > that may take some discussion. >> > >> > Tom Petch >> > >> > >> > ----- Original Message ----- >> > From: <stephane.litkowski@orange.com> >> > >> > Hi Tom, >> > >> > Thanks for your comments. I will fix them asap. >> > Regarding: >> > " Line length is within the RFC limit but the effect is to spread many >> > of the description clauses over multiple lines with indentation of 56 >> > characters, not user friendly e.g. >> > description >> > "List of max LSP >> > bandwidths for different >> > priorities."; >> > " >> > What's your suggestion on this one ? >> > >> > Brgds, >> > >> > -----Original Message----- >> > From: tom petch [mailto:ietfc@btconnect.com] >> > Sent: Tuesday, November 27, 2018 12:11 >> > To: Ebben Aries; yang-doctors@ietf.org >> > Cc: draft-ietf-isis-yang-isis-cfg.all@ietf.org; lsr@ietf.org; Tarek >> Saad >> > (tsaad) >> > Subject: Re: [Lsr] Yangdoctors last call review of >> > draft-ietf-isis-yang-isis-cfg-24 >> > >> > Some quirks in-25 >> > >> > I see lots of YANG reference statements - good - but no mention of >> them >> > in the I-D references - not so good. My list is >> > >> > 5130 >> > 5305 >> > 5306 >> > 5880 >> > 5881 >> > 6119 >> > 6232 >> > 7794 >> > 7810 >> > 7917 >> > 8405 >> > >> > Also perhaps >> > OLD >> > reference "RFC XXXX - YANG Data Model for Bidirectional >> > Forwarding Detection (BFD).Please replace YYYY with >> > published RFC >> > number for draft-ietf-bfd-yang."; >> > >> > NEW >> > reference "RFC YYYY - YANG Data Model for Bidirectional >> > Forwarding Detection (BFD). >> > >> > -- Note to RFC Editor Please replace YYYY with published RFC >> > number for draft-ietf-bfd-yang."; >> > >> > OLD >> > reference "draft-ietf-bfd-yang-xx.txt: >> > YANG Data Model for Bidirectional Forwarding >> > Detection (BFD)"; >> > NEW >> > reference "RFC YYYY - YANG Data Model for Bidirectional >> > Forwarding Detection (BFD). >> > >> > -- Note to RFC Editor Please replace YYYY with published RFC >> > number for draft-ietf-bfd-yang."; >> > >> > >> > Line length is within the RFC limit but the effect is to spread many >> of >> > the description clauses over multiple lines with indentation of 56 >> > characters, not user friendly >> > e.g. >> > description >> > "List of max LSP >> > bandwidths for different >> > priorities."; >> > >> > >> > Acknowledgements is TBD. I note that the editor list of the YANG >> module >> > is somewhat longer than the editor list of the I-D. >> > >> > I note that the augmentation of interfaces seems to have no >> conditional >> > and so will augment all interfaces. I think that this is a generic >> issue >> > but do not see it being addressed anywhere. >> > >> > In a similar vein, you are defining MPLS objects and I am unclear >> > whether or not those should be conditional, or part of the MPLS YANG >> > modules or both (copying Tarek for this) >> > >> > Tom Petch >> > >> > ----- Original Message ----- >> > From: "Ebben Aries" <exa@juniper.net> >> > Sent: Tuesday, October 23, 2018 12:45 AM >> > >> > > Reviewer: Ebben Aries >> > > Review result: On the Right Track >> > > >> > > 1 module in this draft: >> > > - ietf-isis@2018-08-09.yang >> > > >> > > No YANG compiler errors or warnings (from pyang 1.7.5 and yanglint >> > 0.16.54) >> > > >> > > "ietf-isis@2018-08-09" module is compatible with the NMDA >> > architecture. >> > > >> > > Module ietf-isis@2018-08-09.yang: >> > > - Both the description and the draft name reference that this module >> > is >> > > specific to configuration but contains operational state nodes in >> > addition >> > > to RPCs and notifications. Any wording suggesting this is only >> > > configuration should be changed >> > > - Module description must contain most recent copyright notice per >> > > >> > >> https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.1 >> > > - Module description reads "common across all of the vendor >> > implementations". >> > > I don't think this needs to be called out as such as that is the >> > overall >> > > intention of *all* IETF models >> > > - This module contains '22' features (and the respective OSPF module >> > currently >> > > contains '26'). While it is understood the purpose of these >> > features in the >> > > module, take precaution as to complexity for clients if they need >> to >> > > understand >= quantity of features per module in use on a >> > > network-element. We are going to end up w/ feature explosion to >> > convey >> > > *all* possible features of each network-element leading to >> > divergence back >> > > towards native models at the end of the day. A large amount of >> > these >> > > feature names could be defined within a more global namespace >> (e.g. >> > nsr) but >> > > this gives us a granular yet cumbersome approach (e.g. feature >> > isis:nsr, >> > > ospf:nsr, etc..) >> > > - RPC 'clear-adjacency' does not have any input leaf that covers >> > clearing a >> > > specific neighbor/adjacency (See comments below as well regarding >> > RPC >> > > alignment w/ the OSPF model) >> > > - RPC 'clear-adjacency' has an input node of 'interface' however >> this >> > is just >> > > a string type. Is there any reason this is not a >> > leafref/if:interface-ref >> > > (much like in the OSPF model) >> > > - Child nodes within a container or list SHOULD NOT replicate the >> > parent >> > > identifier per >> > > >> > >> https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-4.3. >> > 1. >> > > >> > > A case in point is the list /afs/af that has a leaf of 'af' >> > > <afs> >> > > <af> >> > > <af>ipv4</af> >> > > <enable>true</enable> >> > > </af> >> > > </afs> >> > > >> > > Not only is this replication, but we should likely not abbreviate >> > 'afs' if >> > > we are using the expanded 'address-family' in other IETF models >> such >> > as >> > > ietf-i2rs-rib >> > > >> > > >> > > General comments on the draft + nits: >> > > - Since YANG tree diagrams are used, please include an informative >> > reference >> > > per >> > >> https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.4 >> > > - Section 1.1 does not need to exist since this would be covered by >> > the >> > > reference mentioned above >> > > - Reference to NMDA compliance should be contained within Section 1 >> > (vs. >> > > Section 2) per >> > > >> > >> https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.5 >> > > - Section 2: It seems reference should be given to the location of >> > where the >> > > ietf-routing module is defined (As well as reference to NMDA RFC >> in >> > the >> > > above reference) >> > > - Section 2.1: "Additional modules may be created this to >> support..." >> > needs >> > > slight rewording adjustment >> > > - Section 3: The RPC operations are named 'clear-adjacency' and >> > > 'clear-database' rather w/ reliance off namespacing for >> uniqueness. >> > This >> > > section refers to 'clear-isis-database' and 'clear-isis-adjacency' >> > > - Section 4: Notification name mismatch in this section from actual >> > naming >> > > within the module (e.g. 'adjacency-change' should rather be >> > > 'adjacency-state-change') >> > > - Section 7: Security Considerations will need updating to be >> > patterned after >> > > the latest version of the template at >> > > https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines per >> > > >> > >> https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.7 >> > > - Section 12: All modules imported within this module MUST be >> > referenced >> > > within this section per >> > > >> > >> https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.9. >> > > There are quite a few missing from this section right now >> > > - Appendix A: Some of the XML elements are off in alignment >> > > - Appendix A: Examples must be validated. The example given has the >> > following >> > > issues: >> > > - /routing[name='SLI'] and /routing/description are invalid data >> > nodes and >> > > do not exist. I'm not sure why they are in the XML example here >> > > - The example is meant to reference configuration however >> > > /routing/interfaces is a r/o container >> > > - The control-plane-protocol 'type' needs to be qualified - e.g. >> > > <type >> > xmlns:isis="urn:ietf:params:xml:ns:yang:ietf-isis">isis:isis</type> >> > > - The area-address does not validate against the pattern regex and >> > must end >> > > with a '.' e.g. >> > > <area-address>49.0001.0000.</area-address> >> > > - metric-type/value is set to 'wide' which is invalid. This >> should >> > rather >> > > be 'wide-only' >> > > - isis/afs/af/af is set to 'ipv4-unicast' which is invalid. This >> > should >> > > rather be 'ipv4' per iana-routing-types >> > > - /interfaces/interface/type must be populated and is invalid. >> This >> > should >> > > rather be qualified as such: >> > > <type >> > >> xmlns:ianaift="urn:ietf:params:xml:ns:yang:iana-if-type">ianaift:softwar >> > eLoopback</type> >> > > <type >> > >> xmlns:ianaift="urn:ietf:params:xml:ns:yang:iana-if-type">ianaift:etherne >> > tCsmacd</type> >> > > - /interfaces/interface/link-up-down-trap-enable must have a value >> > > associated as such: >> > > <link-up-down-trap-enable>enabled</link-up-down-trap-enable> >> > > - NP container 'priority' has a must statement checking if an >> > interface-type >> > > is set to 'broadcast' however if you take the XML example from >> > this >> > > section, it will fail to validate even if <priority> is not >> > defined >> > > underneath an interface-type of 'point-to-point'. It seems to >> me >> > that >> > > this logic may need to be readjusted or not exist at all >> (priority >> > can >> > > still be set on implementations on loopback interfaces - which >> > would >> > > default to 'broadcast' in the example here). Could you not >> solve >> > this >> > > with use of 'when' vs. 'must' as such: >> > > >> > > when '../interface-type = "broadcast"' { >> > > description "Priority can only be set for broadcast >> > interfaces."; >> > > } >> > > >> > > >> > >> https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-4.18 >> > ..2. >> > > >> > > - /interfaces/interface/ipv4/mtu must contain a valid value (and >> > likely not >> > > need to be defined for Loopback0) >> > > - 'isis/mpls-te/ipv4-router-id' is invalid and should rather be >> > > 'isis/mpls/te-rid/ipv4-router-id' >> > > - 'isis/afs/af/enabled' is invalid and should rather be >> > 'isis/afs/af/enable' >> > > - Examples should use IPv6 addresses where appropriate per >> > > >> > >> https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.12 >> > >> > >> ________________________________________________________________________ >> > _________________________________________________ >> > >> > Ce message et ses pieces jointes peuvent contenir des informations >> > confidentielles ou privilegiees et ne doivent donc >> > pas etre diffuses, exploites ou copies sans autorisation. Si vous avez >> > recu ce message par erreur, veuillez le signaler >> > a l'expediteur et le detruire ainsi que les pieces jointes. Les >> messages >> > electroniques etant susceptibles d'alteration, >> > Orange decline toute responsabilite si ce message a ete altere, >> deforme >> > ou falsifie. Merci. >> > >> > This message and its attachments may contain confidential or >> privileged >> > information that may be protected by law; >> > they should not be distributed, used or copied without authorisation. >> > If you have received this email in error, please notify the sender and >> > delete this message and its attachments. >> > As emails may be altered, Orange is not liable for messages that have >> > been modified, changed or falsified. >> > Thank you. >> > >> > _______________________________________________ >> > Lsr mailing list >> > Lsr@ietf.org >> > https://www.ietf.org/mailman/listinfo/lsr >> >> >> _________________________________________________________________________________________________________________________ >> >> Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc >> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler >> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, >> Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. >> >> This message and its attachments may contain confidential or privileged information that may be protected by law; >> they should not be distributed, used or copied without authorisation. >> If you have received this email in error, please notify the sender and delete this message and its attachments. >> As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. >> Thank you. >> >> _______________________________________________ >> yang-doctors mailing list >> yang-doctors@ietf.org >> https://www.ietf.org/mailman/listinfo/yang-doctors > > -- > Juergen Schoenwaelder Jacobs University Bremen gGmbH > Phone: +49 421 200 3587 Campus Ring 1 | 28759 Bremen | Germany > Fax: +49 421 200 3103 <https://www.jacobs-university.de/> -- Ladislav Lhotka Head, CZ.NIC Labs PGP Key ID: 0xB8F92B08A9F76C67
- [Lsr] Yangdoctors last call review of draft-ietf-… Ebben Aries
- Re: [Lsr] Yangdoctors last call review of draft-i… stephane.litkowski
- Re: [Lsr] Yangdoctors last call review of draft-i… tom petch
- Re: [Lsr] Yangdoctors last call review of draft-i… stephane.litkowski
- Re: [Lsr] Yangdoctors last call review of draft-i… tom petch
- Re: [Lsr] Yangdoctors last call review of draft-i… Acee Lindem (acee)
- Re: [Lsr] Yangdoctors last call review of draft-i… tom petch
- Re: [Lsr] Yangdoctors last call review of draft-i… tom petch
- Re: [Lsr] Yangdoctors last call review of draft-i… stephane.litkowski
- Re: [Lsr] Yangdoctors last call review of draft-i… stephane.litkowski
- Re: [Lsr] Yangdoctors last call review of draft-i… stephane.litkowski
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… Juergen Schoenwaelder
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… Ladislav Lhotka
- Re: [Lsr] Yangdoctors last call review of draft-i… tom petch
- Re: [Lsr] Yangdoctors last call review of draft-i… stephane.litkowski
- Re: [Lsr] Yangdoctors last call review of draft-i… tom petch
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… Juergen Schoenwaelder
- Re: [Lsr] Yangdoctors last call review of draft-i… stephane.litkowski
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… Juergen Schoenwaelder
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… stephane.litkowski
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… stephane.litkowski
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… Juergen Schoenwaelder
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… stephane.litkowski
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… Juergen Schoenwaelder
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… Andy Bierman
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… Juergen Schoenwaelder
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… Acee Lindem (acee)
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… Juergen Schoenwaelder
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… stephane.litkowski
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… Andy Bierman
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… Acee Lindem (acee)
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… Andy Bierman
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… tom petch
- Re: [Lsr] [yang-doctors] Yangdoctors last call re… stephane.litkowski