[mpls] Mahesh Jethanandani's No Objection on draft-ietf-mpls-msd-yang-10: (with COMMENT)

Mahesh Jethanandani via Datatracker <noreply@ietf.org> Fri, 05 July 2024 00:35 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: mpls@ietf.org
Delivered-To: mpls@ietfa.amsl.com
Received: from [10.244.2.3] (unknown [104.131.183.230]) by ietfa.amsl.com (Postfix) with ESMTP id 5227FC1D4CF8; Thu, 4 Jul 2024 17:35:45 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Mahesh Jethanandani via Datatracker <noreply@ietf.org>
To: The IESG <iesg@ietf.org>
X-Test-IDTracker: no
X-IETF-IDTracker: 12.17.1
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <172013974499.1283404.15319893834947377257@dt-datatracker-5f88556585-g8gwj>
Date: Thu, 04 Jul 2024 17:35:45 -0700
Message-ID-Hash: M4FEX4S3UPOUEYSKDW4QDEBSDB23T65T
X-Message-ID-Hash: M4FEX4S3UPOUEYSKDW4QDEBSDB23T65T
X-MailFrom: noreply@ietf.org
X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-mpls.ietf.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header
CC: draft-ietf-mpls-msd-yang@ietf.org, mpls-chairs@ietf.org, mpls@ietf.org, tsaad@cisco.com
X-Mailman-Version: 3.3.9rc4
Reply-To: Mahesh Jethanandani <mjethanandani@gmail.com>
Subject: [mpls] Mahesh Jethanandani's No Objection on draft-ietf-mpls-msd-yang-10: (with COMMENT)
List-Id: Multi-Protocol Label Switching WG <mpls.ietf.org>
Archived-At: <https://mailarchive.ietf.org/arch/msg/mpls/-2NswHTi_IAT1cGdXDRkoaAAFjU>
List-Archive: <https://mailarchive.ietf.org/arch/browse/mpls>
List-Help: <mailto:mpls-request@ietf.org?subject=help>
List-Owner: <mailto:mpls-owner@ietf.org>
List-Post: <mailto:mpls@ietf.org>
List-Subscribe: <mailto:mpls-join@ietf.org>
List-Unsubscribe: <mailto:mpls-leave@ietf.org>

Mahesh Jethanandani has entered the following ballot position for
draft-ietf-mpls-msd-yang-10: 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-mpls-msd-yang/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------


"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".