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

[Dhruv Dhody <[e-mail]>]

Thanks for taking my comments into consideration! The new revision looks
good to me!

On Wed, Jun 5, 2024 at 6:55 AM Yingzhen Qu <[e-mail]> wrote:

> 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
>>
>>
>>
>> --
> last-call mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]
>