Hi Med, Thanks for the quick response, really appreciate it.
I've uploaded version -29 and moved RFC9000 from normative to informative. Please let me know if you have any other questions. Thanks, Yingzhen On Fri, Apr 18, 2025 at 3:29 AM <mohamed.boucad...@orange.com> wrote: > Hi Yingzhen, > > > > Thank you for the update. Glad you found the PR with yang changes useful > :-) > > > > I checked the diff draft-ietf-isis-sr-yang-25 vs. > draft-ietf-isis-sr-yang-28 and also sid encoding in the OSPF spec. All > looks good, except one easy-to-fix point: please move RFC9000 from > normative to informative. FYI (as this may be useful for future document), > the guidance on that matter is as follows: > > > > RFC8407bis: > > > > Note: [RFC8341] (or a future RFC that replaces it) MUST be listed as > > normative references. > > > > By default, [RFC4252], [RFC6241], [RFC8040], [RFC8446], [RFC9000], > > and RFC AAAA (or future RFCs that replace any of them) are listed > > as informative references unless normatively cited in other > > sections of the document that specifies the YANG module. > > > > Please let me know when a new version is available so that I clear my > DISCUSS. > > > > Cheers, > > Med > > > > *De :* Yingzhen Qu <yingzhen.i...@gmail.com> > *Envoyé :* mercredi 16 avril 2025 23:41 > *À :* BOUCADAIR Mohamed INNOV/NET <mohamed.boucad...@orange.com> > *Cc :* The IESG <i...@ietf.org>; draft-ietf-isis-sr-y...@ietf.org; > lsr-cha...@ietf.org; lsr@ietf.org; Christian Hopps <cho...@chopps.org> > *Objet :* Re: Mohamed Boucadair's Discuss on draft-ietf-isis-sr-yang-25: > (with DISCUSS and COMMENT) > > > > > > Hi Med, > > > > Sorry for the late reply. We focused on the changes of OSPF SR YANG first, > now we made corresponding changes in the ISIS model as well. Version -27 > has been published to address your comments. > > > > Please see my detailed response below. > > > > Thanks, > > Yingzhen > > > > On Tue, Mar 25, 2025 at 7:18 AM Mohamed Boucadair via Datatracker < > nore...@ietf.org> wrote: > > Mohamed Boucadair has entered the following ballot position for > draft-ietf-isis-sr-yang-25: Discuss > > When responding, please keep the subject line intact and reply to all > email addresses included in the To and CC lines. (Feel free to cut this > introductory paragraph, however.) > > > Please refer to > https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ > for more information about how to handle DISCUSS and COMMENT positions. > > > The document, along with other ballot positions, can be found here: > https://datatracker.ietf.org/doc/draft-ietf-isis-sr-yang/ > > > > ---------------------------------------------------------------------- > DISCUSS: > ---------------------------------------------------------------------- > > Hi Stéphane, Yingzhen, Pushpasis, Helen, and Jeff, > > Thank you for the effort put into this well-written document. > > I appreciate your perseverance to push this specification for almost 10 > years. > > I have a DISCUSS point and a set of comments that I’d like we address. Main > comments are tagged with (*). > > # DISCUSS > > CURRENT: > leaf length { > type uint8; > description > "Length of the SID value. YANG model specification > is necessary since it dictates the semantics of the > SID."; > } > leaf sid { > type uint32; > description > "Segment Identifier (SID) - A 20 bit label or 32 bit SID. > If the length is set to 3, then the 20 rightmost bits > represent an MPLS label. If the length is set to 4, then > the value is a 32-bit index."; > } > > This construct mirrors how the object is encoded on the wire, but when it > comes > to management a type is better instead fo inferring its from the length. I > suggest to use an explicit type here instead of an implicit type. > > [Yingzhen]: an explicit type is defined, the same as changes done is the > OSPF module. > > > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > # Abstract > > OLD: > This document defines a YANG data module that can be used to > configured and manage IS-IS Segment Routing for MPLS data plane. > > NEW: > This document defines a YANG data module that can be used to > manage IS-IS Segment Routing for MPLS data plane. > > Delete “configure” as this is covered by “manage”. Remember the FCAPS ;-) > > [Yingzhen]: fixed. > > > # Section 2 > > Clarify this is a device model + Cite where the base model is defined when > first cited + avoid restricting the model to configuration. We claim anyway > that the module is NMDA compliant. > > OLD: > This document defines a YANG model for IS-IS Segment Routing for the > MPLS data plane. It is an augmentation of the IS-IS base model. > > The IS-IS SR MPLS YANG module requires support for the base segment > routing module [RFC9020], which defines the global segment routing > configuration independent of any specific routing protocol > configuration, and support of IS-IS base model [RFC9130] which > defines basic IS-IS configuration and state. > > NEW: > This document defines a device YANG model for IS-IS Segment Routing for > the > MPLS data plane. It is an augmentation of the IS-IS base model > [RFC9130]. > > The IS-IS SR MPLS YANG module requires support for the base segment > routing module [RFC9020], which defines the global SR > management structure independent of any specific routing protocol, > and support of IS-IS base model [RFC9130] which > defines basic IS-IS configuration and state. > > [Yingzhen]: fixed. > > > > # Section 3 (*) > > I would split the long tree into small snippets and help readers go > through to > understand the rationale for each augment. > > [Yingzhen]: The tree is moved to Appendix B as OSPF. > > > > # Section 4 (*) > > I have several comments about the module: > > * Please run "pyang -f yang --yang-canonical” as the current order does not > follow a canonical order > > [Yingzhen]: done. > > > * Not sure we can claim that the module covers extension that is “which is > common across all of the vendors”. I would simply delete that mention. > > [Yingzhen]: done. > > > > * The boilerplate is not needed > > * It is unfortunate that the naming in the module reasons about TLV objects > > * There are missing reference statements > > * Need to clarify the mapping with the flags as defined in the base spec > (e.g., > af-flag/F-flag, sf-flag/S-Flag) > > * There are several layers without clear explanation (e.g., > sr-capability/sr-capability may question why we need two layers, do we > need the > tree levels prefix-sid-sub-tlvs/prefix-sid-sub-tlvs/prefix-sid-sub-tlv?) > > * No need to repeat the parent prefix > > * Use of plural for lists and leaf-list, while singular is recommended in > the > guidelines. > > * There are no reference to justify the default values > > [Yingzhen]: all the above all fixed. > > > > * Weird to have “Configuration” while we claim the module is NMDA > compliant. > > A version that fixes many of these can be found at: > > https://github.com/boucadair/IETF-Drafts-Reviews/blob/master/YANG/ietf-isis-sr-mpls.yang > . > Please double check/complete. Thanks. > > [Yingzhen]: thanks for providing this. really helpful! > > > > A diff to track the changes can be seen here: > > https://github.com/boucadair/IETF-Drafts-Reviews/commit/395b6ece9e7939c33af7d150a71bfd3948bb5ba1 > > # Section 5 > > ## Please update this section to follow the template defined in 8407bis (*) > > ## I don’t find “node-msd-tlv” defined in the model > > CURRENT: > Some of the readable data nodes in the modules may be considered > sensitive or vulnerable in some network environments. It is thus > important to control read access (e.g., via get, get-config, or > notification) to these data nodes. > > /isis:router-capabilities/sr-capability > > /isis:router-capabilities/sr-algorithms > > /isis:router-capabilities/local-blocks > > /isis:router-capabilities/srms-preference > > /isis:router-capabilities/node-msd-tlv > > [Yingzhen] : this was from module ietf-isis-msd.yang, which used to be in > this file as well. Removed now. > > > ## nit > > OLD: > Unauthorized access to any data node of these subtrees can disclose > the operational state information of IS-IS protocol on this device. > > NEW: > Unauthorized access to any data node of these subtrees can disclose > the operational state information of IS-IS protocol on a device. > > [Yingzhen] : fixed. > > > > # Section 7 > > Consider updating as follows > > OLD: > The IANA is requested to assign one new URI from the IETF XML > registry ([RFC3688]). Authors are suggesting the following URI: > > URI: urn:ietf:params:xml:ns:yang:ietf-isis-sr-mpls > Registrant Contact: The IESG. > XML: N/A, the requested URI is an XML namespace > > This document also requests one new YANG module name in the YANG > Module Names registry ([RFC6020]) with the following suggestion : > > name: ietf-isis-sr-mpls > namespace: urn:ietf:params:xml:ns:yang:ietf-isis-sr-mpls > prefix: isis-sr-mpls > reference: RFC XXXX > > NEW: > The IANA is requested to assign a new URI from the IETF XML > registry [RFC3688]: > > URI: urn:ietf:params:xml:ns:yang:ietf-isis-sr-mpls > Registrant Contact: The IESG. > XML: N/A, the requested URI is an XML namespace > > This document also requests a new YANG module name in the YANG > Module Names registry ([RFC6020]): > > name: ietf-isis-sr-mpls > namespace: urn:ietf:params:xml:ns:yang:ietf-isis-sr-mpls > prefix: isis-sr-mpls > maintained by IANA? N > reference: RFC XXXX > > [Yingzhen]: fixed. > > > > # References (*) > > RFC8446, RFC8340, RFC8040, RFC6242, and RFC6241 are listed as Normative, > while > they shouldn’t. Please move those to be listed as Informative. > > [Yingzhen]: fixed. > > > > # Appendix A (*) > > The JSON example is broken. Please update and validate using yanglint or > yangson, for example. > > OLD: > { > "routing": { > "router-id": "1.1.1.1", > "control-plane-protocols": { > "control-plane-protocol": { > "type": "isis:isis", > "name": "isis", > "isis": { > "system-id": "1111.2222.3333", > "interfaces": { > "interface": { > "name": "", > "segment-routing": { > "adjacency-sid": { > "adj-sids": { > "value": 38888 > } > } > } > } > }, > "segment-routing": { > "enabled": true > }, > "protocol-srgb": { > "srgb": { > "lower-bound": 4000, > "upper-bound": 5000 > } > } > } > } > } > } > } > > NEW: > { > "ietf-routing:routing": { > "router-id": "1.1.1.1", > "control-plane-protocols": { > "control-plane-protocol": { > "type": "ietf-isis:isis", > "name": "isis", > "ietf-isis": { > "system-id": "1111.2222.3333", > "interfaces": { > "interface": { > "name": "", > "ietf-isis-sr-mpls:segment-routing": { > "adjacency-sid": { > "adj-sids": { > "value": 38888 > } > } > } > } > }, > "ietf-isis-sr-mpls:segment-routing": { > "enabled": true > }, > "ietf-isis-sr-mpls:protocol-srgb": { > "srgb": { > "lower-bound": 4000, > "upper-bound": 5000 > } > } > } > } > } > } > } > > [Yingzhen]: the example is fixed. > > > > Cheers, > Med > > > ____________________________________________________________________________________________________________ > 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 To unsubscribe send an email to lsr-le...@ietf.org
