Re: [Lsr] Yangdoctors last call review of draft-ietf-isis-yang-isis-cfg-24
<stephane.litkowski@orange.com> Mon, 07 January 2019 08:59 UTC
Return-Path: <stephane.litkowski@orange.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 F0AFB130E73; Mon, 7 Jan 2019 00:59:50 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.6
X-Spam-Level:
X-Spam-Status: No, score=-2.6 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001, UNPARSEABLE_RELAY=0.001] 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 s-sbyvtpxO1Q; Mon, 7 Jan 2019 00:59:49 -0800 (PST)
Received: from orange.com (mta241.mail.business.static.orange.com [80.12.66.41]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 99896130E11; Mon, 7 Jan 2019 00:59:48 -0800 (PST)
Received: from opfedar01.francetelecom.fr (unknown [xx.xx.xx.2]) by opfedar20.francetelecom.fr (ESMTP service) with ESMTP id 43Y8Tt4rJQz8vHf; Mon, 7 Jan 2019 09:59:46 +0100 (CET)
Received: from Exchangemail-eme2.itn.ftgroup (unknown [xx.xx.31.61]) by opfedar01.francetelecom.fr (ESMTP service) with ESMTP id 43Y8Tt3BlNzBrLG; Mon, 7 Jan 2019 09:59:46 +0100 (CET)
Received: from OPEXCLILMA4.corporate.adroot.infra.ftgroup ([fe80::65de:2f08:41e6:ebbe]) by OPEXCLILM7E.corporate.adroot.infra.ftgroup ([fe80::b91c:ea2c:ac8a:7462%19]) with mapi id 14.03.0415.000; Mon, 7 Jan 2019 09:59:46 +0100
From: stephane.litkowski@orange.com
To: tom petch <ietfc@btconnect.com>, Ebben Aries <exa@juniper.net>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>
CC: "draft-ietf-isis-yang-isis-cfg.all@ietf.org" <draft-ietf-isis-yang-isis-cfg.all@ietf.org>, "lsr@ietf.org" <lsr@ietf.org>, "Tarek Saad (tsaad)" <tsaad@cisco.com>
Thread-Topic: [Lsr] Yangdoctors last call review of draft-ietf-isis-yang-isis-cfg-24
Thread-Index: AQHUhkHafKiWVvfkhUKDIB9kEUVys6WjwLVw
Date: Mon, 07 Jan 2019 08:59:45 +0000
Message-ID: <4665_1546851586_5C331502_4665_135_1_9E32478DFA9976438E7A22F69B08FF924B78A363@OPEXCLILMA4.corporate.adroot.infra.ftgroup>
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>
In-Reply-To: <009101d4a135$a59c2780$4001a8c0@gateway.2wire.net>
Accept-Language: fr-FR, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.168.234.4]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/lsr/8JADxokZfYSDNvz8IQRQSeK7pUg>
Subject: Re: [Lsr] Yangdoctors last call review of draft-ietf-isis-yang-isis-cfg-24
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 08:59:51 -0000
Hi Tom, The IS-IS extension for the Interface Switching Capability descriptor is defined in RFC5307 and this is what the model describes. And the neighbor-gmpls-extensions grouping contains all the extensions defined in RFC5307. What could make sense regarding RFC7074, would be to modify the description and "when" statement of the psc-specific container which matches the PSC2,PSC3,PSC4 that are deprecated. Regarding the uint8, IMO, there is no need for enforcement as this is just an operational state (LSDB description). Brgds, -----Original Message----- From: tom petch [mailto:ietfc@btconnect.com] Sent: Monday, December 31, 2018 19:22 To: LITKOWSKI Stephane OBS/OINIS; 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 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. _________________________________________________________________________________________________________________________ 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] 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