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 < [email protected]> 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 > > > >
_______________________________________________ Lsr mailing list -- [email protected] To unsubscribe send an email to [email protected]
