YANG Data Model for Maximum SID Depth Types and MPLS Maximum SID Depth
draft-ietf-mpls-msd-yang-12

I reviewed version draft-ietf-mpls-msd-yang-12

My main observation is that the MSD leaf is read-only instead of read-write.

A consideration for making the leaf read-write instead of read-only is that future MSDs may emerge that benefit more significantly from write capabilities, unlike existing ERLD and BMI. With the currently proposed yang model, updating would be necessary. While this change appears to be backward compatible and should not pose major issues, it will nonetheless require processing.

I agree with you that my observation pertains more to the current MSD drafts than to the proposed YANG model.

My believe is that RW seems more future proof, but that is a discussion i'll leave in the hands of the WG and document authors.

"MPLS", paragraph 0
>  YANG Data Model for MPLS Maximum Segment Identifier (SID) Depth (MSD)
>                       draft-ietf-mpls-msd-yang-10
I support Eric's DISCUSS on the naming of the draft. If the draft is defining identities for both MPLS and SRH, shouldn't the title reflect it?

Section 1, paragraph 1
>    YANG [RFC7950] is a data modeling language used to define the
>    contents of a conceptual data store that allows networked devices to
>    be managed using NETCONF [RFC6241] or RESTCONF [RFC8040].

A redundant paragraph. Do you really need it?

Section 4.1, paragraph 17
>      identity msd-base-mpls {
>        base msd-base;
>        description
>          "Identity for MSD types for MPLS data plane.";
>      }

Would it help to say in the description "Base identity of MSD types for MPLS data plane"?

Section 4.1, paragraph 18
>      identity erld-mpls {
>        base msd-base-mpls;
>        description
>          "msd-erld is defined to advertise the Entropy Readable
>           Label Depth (ERLD).";
>        reference
>          "RFC 8662: Entropy Label for Source Packet Routing in
>                     Networking (SPRING) Tunnels
>           RFC 9088: Signaling Entropy Label Capability and Entropy
>                     Readable Label Depth Using IS-IS";
>      }

I agree with Med that the name of the identity does not match the description. Can this be fixed?

Section 4.1, paragraph 18
>      identity msd-base-srh {
>        base msd-base;
>        description
>          "Identity for MSD types for Segment Routing Header (SRH).";
>      }

Much like above, would it help to say in the description "Base identity of MSD types for SRH"?

Section 4.1, paragraph 18
>      identity srh-max-sl {
>        base msd-base-srh;
>        description
>          "The Maximum Segment Left MSD type.";
>        reference
>          "RFC 9352: IS-IS Extensions to Support Segment Routing
>                     over the IPv6 Data Plane";
>      }

I know that you had a discussion with Med about these identities.

My observation is slightly different. identity 'srh-max-sl' is derived from an identity 'msd-base-srh', which in turn is derived from 'msd-base'. You are suggesting that identity 'srh-max-sl' is a transitive identity of 'msd-base'. This would make sense if there is a need to compare an identity to 'msd-base' without regard to whether the identity is used in MPLS or SRH data planes. The naming of identities suggests otherwise. Is there such a use case?

Section 4.2, paragraph 12
>      grouping msd-type-value {
>        description
>          "Grouping for MSD type and value.";
>        leaf msd-type {
>          type identityref {
>            base iana-msd-types:msd-base-mpls;
>          }
>          mandatory true;
>          description
>            "MSD types. The MSD type is defined in IANA IGP
>             MSD-Types registry.";
>        }

If the nodes defined here are read-only nodes, why the 'mandatory true' statement? Note, if you do decide to drop the mandatory keyword, remember to update the tree diagram.

Section 6.1, paragraph 2
>       URI: urn:ietf:params:xml:ns:yang:iana-msd-types
>       Registrant Contact: IANA.
>       XML: N/A, the requested URI is an XML namespace.

rfc8407bis, Section 5.1 example seems to indicate that when the module is an IANA module, the registrant contact is still "The IESG".

The IANA review of this document seems to not have concluded yet.

No reference entries found for these items, which were mentioned in the text:
[RFC9088], and [RFC9352].

-------------------------------------------------------------------------------
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 1, paragraph 1
>    There are two YANG modules defined in this document.  Module iana-
>    msd-types defines the identities for Maximum SID Depth (MSD) Types as
>    per the IANA the IGP MSD-Types registry [IANA-IGP-MSD-Types].  This
>    document also defines module ietf-mpls-msd augmenting the IETF MPLS
>    YANG model [RFC8960], which itself augments the routing RIB data
>    model [RFC8349], to provide operational state for various
>    MSDs[RFC8662].  The module augments the base MPLS model with a list
>    of various types of node MSDs, as well as various types of MSDs on
>    links.

s/per the IANA the IGP/per the IANA IGP/

Section 2.2, paragraph 2
>    As defined in [RFC8491], MSD is the number of SIDs supported by a
>    node or a link on a node.  The module defines lists of MSDs with
>    different MSD Types for a node and links.  Please note that these are
>    read-only data as per the node's hardware capability.

s/read-only data/read-only nodes/

Section 4.2, paragraph 11
>        leaf msd-value {
>          type uint8;
>          mandatory true;
>          description
>            "MSD value, in the range of 0-255. 0 represents the lack
>             of ability to support a SID statck of any depth.";
>        }

s/statck/stack/

Uncited references: [RFC2119] and [RFC8174].

Section 4.2, paragraph 13
> RFC3688], the following registrations is requested to be made: COMMENT: URI:
>                                       ^^
The verb form "is" does not seem to match the subject "registrations".

Section 5, paragraph 6
> he title of the document added. When a MSD type is added to the "IGP MSD-Typ
>                                      ^
Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
"an article", "an hour".

Thank you to Paul Kyzivat for the GENART review.

Thanks for quickly addressing my previous DISCUSS and most of the COMMENTS at:
https://mailarchive.ietf.org/arch/msg/mpls/WZhopispxwElFmjhaJTlpQwWY6Q/