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 <yingzhen.ietf@xxxxxxxxx> 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,YingzhenOn Mon, Jun 3, 2024 at 11:06 AM Dhruv Dhody via Datatracker <noreply@xxxxxxxx> 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 -- last-call@xxxxxxxx
To unsubscribe send an email to last-call-leave@xxxxxxxx
-- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
