Hi Mahesh, Thanks for the review. Version -27 has been uploaded to address your comments. Please see my detailed answers below inline.
Thanks, Yingzhen On Mon, Mar 31, 2025 at 4:28 PM Mahesh Jethanandani via Datatracker < nore...@ietf.org> wrote: > Mahesh Jethanandani has entered the following ballot position for > draft-ietf-isis-sr-yang-25: No Objection > > 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/ > > > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > Section 1, paragraph 0 > > This document defines a YANG data model [RFC7950] that can be used to > > configure and manage IS-IS Segment Routing [RFC8667] for MPLS data > > plane and it is an augmentation to the IS-IS YANG data model > > [RFC9130]. > > There seems to be some confusion on the use of term "YANG module" and "YANG > data model" in this document. A "YANG data model" refers to a collection of > YANG modules and submodules that together define a structured > representation > configuration, operational data, notifications, and RPCs for a given > system or > protocol, while a "YANG module" refers to a specific YANG file (.yang) > defining > a set of nodes (container, list, leaf, etc.) that represent configuration > or > state data. Moreover, a YANG module can be independent and augment other > modules. > > Based on that definition what you seem to be defining is a YANG module more > than a YANG data model. Can that be reflected consistently in this > document? > > [Yingzhen]: Fixed. > Section 4, paragraph 7 > > This version of this YANG module is part of RFC XXXX > > (https://www.rfc-editor.org/info/rfcXXXX); see the RFC itself > > for full legal notices. > > BTW, is there an instruction for the RFC Editor on what to do with RFC > XXXX? > > [Yingzhen]: fixed . > Section 4, paragraph 35 > > 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."; > > } > > } > > } > > I do agree with Med that the definition in YANG for a management attribute > does > not have to follow the format on the wire. This would be better defined as > an > explicit type. > > [Yingzhen]: fixed. > Section 4, paragraph 35 > > grouping sr-capability { > > description > > "SR capability grouping."; > > reference > > "RFC 8667 - IS-IS Extensions for Segment Routing, Section 3"; > > A grouping makes sense when it is used multiple times in the module. I > only see > one 'uses' statement for this grouping. If this is true, then this and > other > single used groupings should be inlined for easier reading of the module. > > Section 4, paragraph 34 > > container sr-capability { > > leaf-list sr-capability-flags { > > type identityref { > > base sr-capability; > > } > > description > > "SR Capability sub-TLV flags."; > > } > > description > > "SR Capability Flags."; > > } > > Why a 'container sr-capability' within another 'container sr-capability'? > Can > this be renamed or removed? > > [Yingzhen]: removed. > Section 5, paragraph 0 > > The YANG module specified in this document defines a schema for data > > that is designed to be accessed via network management protocols such > > as NETCONF [RFC6241] or RESTCONF [RFC8040]. The lowest NETCONF layer > > is the secure transport layer, and the mandatory-to-implement secure > > transport is Secure Shell (SSH) [RFC6242]. The lowest RESTCONF layer > > is HTTPS, and the mandatory-to-implement secure transport is TLS > > [RFC8446]. > > Please make sure that this template follows the latest template in > rfc8407bis. > > [Yingzhen]: updated. > No reference entries found for these items, which were mentioned in the > text: > [draft-ietf-rtgwg-segment-routing-ti-lfa] > > [Yingzhen]: this is referenced in the module. > Found IP block or address not inside RFC5737/RFC3849 example ranges: > "1.1.1.1". > > [Yingzhen]: this is a router-id, not an ip address. > > ------------------------------------------------------------------------------- > NIT > > ------------------------------------------------------------------------------- > > All comments below are about very minor potential issues that you may > choose to > address in some way - or ignore - as you see fit. Some were flagged by > automated tools (via https://github.com/larseggert/ietf-reviewtool), so > there > will likely be some false positives. There is no need to let me know what > you > did with these suggestions. > > Section 4, paragraph 9 > + s/RFC XXXX/RFC XXXX: A YANG Data Model for IS-IS Segment Routing for the > MPLS > Data Plane/ > > Section 3, paragraph 0 > > The figure below describes the overall structure of the isis-sr-mpls > > YANG module: > > s/isis-sr-mpls/ietf-isis-sr-mpls/ > Section 4, paragraph 8 > > reference > > "RFC XXXX"; > > s/RFC XXXX/RFC XXXX: A YANG Data Model for IS-IS Segment Routing for the > MPLS > Data Plane/ > > Section 2, paragraph 1 > > iguration parameters that have been setup using the base segment routing > modu > > ^^^^^ > The verb "set up" is spelled as two words. The noun "setup" is spelled as > one. > > > [Yingzhen]: all fixed.
_______________________________________________ Lsr mailing list -- lsr@ietf.org To unsubscribe send an email to lsr-le...@ietf.org
