[RTG-DIR]Re: Rtgdir last call review of draft-ietf-mpls-msd-yang-07

[Yingzhen Qu <[e-mail]>]

Hi Dhruv,

Thanks for the review and comments. I've uploaded version -08 to address
your comments. Please see my detailed answers below.

Thanks,
Yingzhen

On Mon, Jun 3, 2024 at 11:06 AM Dhruv Dhody via Datatracker <
[e-mail]> wrote:

> Reviewer: Dhruv Dhody
> Review result: Has Issues
>
> Hello,
>
> I have been selected as the Routing Directorate reviewer for this draft.
> The
> Routing Directorate seeks to review all routing or routing-related drafts
> as
> they pass through the IETF last call and IESG review, and sometimes on
> special
> request. The purpose of the review is to assist the Routing ADs. For more
> information about the Routing Directorate, please see
> https://wiki.ietf.org/en/group/rtg/RtgDir
>
> Although these comments are primarily for the use of the Routing ADs, it
> would
> be helpful if you could consider them along with any other IETF Last Call
> comments that you receive, and strive to resolve them through discussion
> or by
> updating the draft.
>
> Document: draft-ietf-mpls-msd-yang-07
> Reviewer: Dhruv Dhody
> Review Date: 2024-06-03
> IETF LC End Date: 2024-06-04
> Intended Status: Proposed Standard
>
> ## Summary:
>
> * I have some minor concerns about this document that I think should be
> resolved before publication.
>
> ## Comment:
>
> * This draft defines 2 YANG models one is IANA-maintained to mirror the
> msd-type registry and the other is augmenting base MPLS to include MSD
> values.
>
> ### Major Issues:
>
> - Please remove the BCP14 boilerplate (Section 1.1) as you are not using
> any of
> those keywords. Also, remove from the ietf-mpls-msd YANG model.
>
[Yingzhen]: removed.

>
> - You should explicitly state that this is an initial version of
> "iana-msd-types" YANG model - "This document defines the initial version
> of the
> IANA-maintained 'iana-msd-types' YANG module."
>
> [Yingzhen]: fixed.


> ### Minor Issues:
>
> - Title: Please change to "A YANG Data Model for MPLS Maximum Segment
> Identifier (SID) Depth (MSD)". Also, update the reference in the YANG model
> around RFC XXXX.
>
> [Yingzhen]: changed the title to "YANG Data Model for MPLS Maximum Segment
Identifier (SID) Depth (MSD)" .

- The abstract suggests that only one YANG model is defined in this I-D.
> Consider rephrasing or adding some hints about the IANA model as well.
>
> [Yingzhen]: updated.


> - Section 1, "YANG [RFC7950] is a data definition language.."; I suggest
> changing it to data modeling as that is the term used in the referenced
> RFC.
>
> [Yingzhen]: modified.


> - Section 1, I am unsure about the text "The augmentation defined in this
> document requires support..."; isn't it obvious that one needs to support
> the
> model one is augmenting...
>
> [Yingzhen]: removed this sentence.


> - Section 4, please add this text in the description inside the YANG
> module -
> "This YANG module is maintained by IANA and reflects the 'IGP MSD-Types'
> registry."
>
> [Yingzhen]: Done.


> - identity msd-erld, should also have a reference to RFC9088.
>
> [Yingzhen]: Done.


> - In "ietf-mpls-msd", please remove the reference "RFC XXXX: A YANG Data
> Model
> for MPLS MSD." immediately after the module description. The revision
> statement
> is the correct place to have this reference.
>
> [Yingzhen]: removed.


> - leaf msd-value should also include text for "0 represents the lack of
> ability
> to support a SID stack of any depth".
>

[Yingzhen]: Added.

>
> - I can not parse "A type of Node MSD is the smallest same type link MSD
> supported by the node.";"
>

[Yingzhen]: tried to rephrase.  Hope it reads better now.

>
> - RFC8340 should be normatively referenced.
>
> [Yingzhen]: RFC8340 has been an informational reference in YANG RFCs.

### Nits:
>
> - s/(MSD) Types as the IANA the IGP MSD-Types registry/(MSD) Types as per
> the
> IANA IGP MSD-Types registry/
>
> - s/which itself augments [RFC8349]/which itself augments routing RIB data
> model [RFC8349]/
>
> - s/IANA maintained module/IANA-maintained module/
>
> - s/This module will be maintained by IANA if more MSD types are added to
> the
> registry./This module will be maintained by IANA and updated if and when
> there
> is any change in the registry./
>
> - s/and it is to provide support of different types of MSDs in MPLS data
> plane./and it provides support for different types of MSDs in the MPLS data
> plane./
>
> - s/read-only data decided by/read-only data as per/
>
> - Section 4, expand SID on first use in the YANG model.
>
> [Yingzhen]: nits all fixed.


> Thanks,
> Dhruv
>
>
>
>