[Last-Call] Re: Genart last call review of draft-ietf-mpls-spring-inter-domain-oam-16

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Peter,

Thanks for the careful review and comments.
Pls see inline for replies.
-17 will address your comments
Rgds
Shraddha


Juniper Business Use Only
-----Original Message-----
From: Peter Yee via Datatracker <noreply@xxxxxxxx>
Sent: Tuesday, May 28, 2024 6:39 AM
To: gen-art@xxxxxxxx
Cc: draft-ietf-mpls-spring-inter-domain-oam.all@xxxxxxxx; last-call@xxxxxxxx; mpls@xxxxxxxx
Subject: Genart last call review of draft-ietf-mpls-spring-inter-domain-oam-16

[External Email. Be cautious of content]


Reviewer: Peter Yee
Review result: Ready with Issues

I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair.  Please treat these comments just like any other last call comments.

For more information, please see the FAQ at

<https://urldefense.com/v3/__https://wiki.ietf.org/en/group/gen/GenArtFAQ__;!!NEt6yMaO-gk!He5RRRLEAN8J3IPaDptanSk3aeoJ5px4W0v6H4LUuWc7yJYKUFnf9-dvOIEbstcMuRsArpR0RUWoBko3$ >.

Document: draft-ietf-mpls-spring-inter-domain-oam-16
Reviewer: Peter Yee
Review Date: 2024-05-27
IETF LC End Date: 2024-05-17
IESG Telechat date: 2024-06-13

Summary: This specification is a Standards Track method for performing MPLS ping and traceroute in segment routing networks. Not exactly an area in which I have a lot of experience, but the document reads reasonably well. I found one small issue and a bevy of nits. [Ready with issues.]

Major issues: None

Minor issues:

Page 21, section 8.2, 2nd paragraph, 2nd sentence: The flags should be numbered
0 to 7, not 0 to 8. While this is more of a nit, I put it in the minor issues category as it’s the only thing I’m submitting that really needs to be corrected especially as it is part of the IANA Considerations.

<SH> Yes. It should be 0-7. Fixed

 I have to ask, for curiosity’s sake, why is the first bit actually used 1 and not 0? It really confused me as to what your preference was for the bit numbering, but based on Figure 6 in section 4.4, I’m left to believe 0-7 is correct and bit 1 is the first one allocated.
<SH> We are trying to align this OAM TLVs/sub-TLVs to the RFC 9256 that defined these type of segments. That is the reason bit 1 is assigned and not bit 0.
Bit 0 is assigned for another purpose in 9256 which is not relevant for the scope of this OAM draft. Later if another OAM draft comes
Which has scope to cover what is assigned to bit 0 then it will be used.
Beginning paragraph of section 4 talks about this motivation but somehow gets missed by the reader.
Multiple people asked the same question.

Nits/editorial comments:

General:

So many missing commas and articles. Okay, you’ll find them enumerated below.
This will make things easier for the RFC Editor. Maybe even for some readers. I hope you’ll find my input useful. I also hope I got it right. ;-)

Capitalization of things like “Segment” and “Node” varies in the document and I don’t know enough about the SR community and its RFCs to be sure of the preferred usage.
<SH> Segment is capitalized only in case of sub-TLV context. Fixed for consistency

Change “implementation-dependent” to “implementation dependent” throughout the document.
<SH> I don't know which is correct usage. Gammarly tool suggested this change. Will leave it to RFC editors.

Decide if the term is “headend” or “head-end” and use it consistently in the document.
<SH> Fixed

In each subsection in section 8, change “Multi-Protocol” to “Multiprotocol” to match the actual names of the IANA registries.
<SH> Fixed

Specific:

Page 1, title: change “end based” to “end-based”. I’m might even consider changing “System/” to “System-/”.
<SH> That doesn't look right to me.

Page 1, Abstract, 2nd sentence: Insert a space before “(ASes)”.
<SH> Fixed

Page 3, section 1, 1st paragraph, 2nd sentence: Insert a space before “(ASes)”.
<SH> fixed

Page 3, Figure 1, AS Boundary Router: append a space after the colon.

Page 4, 1st paragraph, 1st sentence: append a comma after “AS2”.

Page 4, 1st paragraph, 2nd sentence: append a comma after “AS2”.

Page 4, 1st paragraph, 4th sentence: append a comma after “Adjacency-SIDs”.
<SH> Fixed all above

Page 4, 4th paragraph, 1st sentence: change “relay based” to “relay-based”.
<SH> That doesn't look right to me

Page 5, 1st full paragraph, 2nd sentence: append a comma after “well”.

Page 7, section 4, 2nd paragraph, 1st sentence: change “Below” to “The below”.
<SH> Fixed above comments

Change the word “apply” to “applied” or “applicable” depending on what think makes the most sense.
<SH> The below types of Segment sub-TLVs apply to the
Reply Path TLV

Page 7, section 4, 2nd paragraph, 2nd sentence: append a comma after “16”.
<SH> Fixed

Page 8, “RESERVED”: delete an extraneous period at the end of the field entry.

Page 8, “TC”: add a period to the end of the field entry.

Page 8, “S”: add a period to the end of the field entry.

Page 8, 3rd paragraph after the field entries: insert a space before “(TC)”.

Page 8, section 4.2, 1st sentence: append a comma after “Algorithm”.

Page 10, “SR Algorithm” entry, 1st sentence: append a space after the end of the sentence.

Page 10, “RESERVED” entry, 1st sentence: change “octet” to “octets”.

Page 11, 2nd paragraph, 1st sentence: append a comma after “S”. Append a space at the end of the sentence.

Page 11, section 4.4, 1st paragraph: delete the extraneous space before “)”.
Append a colon after “)”.
<SH> Fixed all

Page 11, section 5, 2nd sentence: insert “the” before “echo reply”.

Page 11, section 5, 3rd sentence: change “The” to “the”.

Page 12, section 5.1, 2nd sentence: Insert “The” at the beginning of the sentence.

Page 12, section 5.3, 7th sentence: insert “the” before “appropriate”.

Page 13, 1st partial paragraph, 1st partial sentence: change “IPV4” to “IPv4”.

Page 13, 1st partial paragraph, 2nd full sentence: delete the space before “/”.
<SH> fixed

Page 13, section 5.4, 3rd sentence: delete “ (defined in this document)” as you’ve just said that very thing in the previous sentence: append “information”
or something similar after “log”.
<SH> Fixed

Page 13, section 5.4, 4th sentence: delete the extraneous space after “TBA2”.
Insert a space after the immediately following comma. Delete the space between ‘building”’ and the following comma. Insert “a” before “dynamic return path”.

Page 13, section 5.4, 5th sentence: append “information” or something similar after “Log”.

Page 14, section 5.5, 3rd sentence: insert “the” before “Reply Path TLV”.

Page 14, section 5.5.1, 2nd paragraph, 6th sentence: insert “the” before “ABR’s own SRGB”.

Page 14, section 5.5.1, 3rd paragraph, 1st sentence: insert “the” before the second “ASBR” and the before the third “ASBR”.
<SH> Fixed

Page 15, 1st partial paragraph, 1st full sentence: insert “The” at the beginning of the sentence.

Page 15, 1st partial paragraph, 2nd full sentence: insert “the” before “remote”.

Page 15, 1st partial paragraph, 6th sentence: insert “the” before “Reply”.

Page 15, 1st full paragraph, 1st sentence: change “responder of echo” to “responder to echo”.

Page 15, 2nd full paragraph, 1st sentence: delete the comma.
<sH>Fixed the ones I could find.

Page 15, 3rd full paragraph, 2nd sentence: insert “the” before “previously”.
Change “next” to “subsequent”.
<SH> Fixed

Page 15, 4th full paragraph, 3rd sentence: insert “an” before “ASBR”.

Page 16, section 6, 1st paragraph, 2nd sentence: insert “the” before “topology”.

Page 16, section 6, 1st paragraph, 3rd sentence: append a comma after “ASBR1”.

Page 16, section 6, 1st paragraph, 4th sentence: append a comma after “Similarly”. Append a comma after “P4”.

Page 16, section 6, 2nd paragraph, 3rd sentence: append a comma after “ASBR3”.

Page 16, section 6, 2nd paragraph, 4th sentence: change “Topology” to “The topology”.

Page 16, section 6, 3rd paragraph, insert a space before “(SIDs)”.

Page 16, “Node SIDs” entry: should this be “Node-SIDs” as used elsewhere in the document? Append a comm after “N-ASBR1”.

Page 16, “Adjacency SIDs” entry: should this be “Adjacency-SIDs” as used elsewhere in the document? Append a comma after “Adj-P1-P2”.

Page 16, “EPE-SIDs”: append a comma after “EPE-ASBR3-ASBR2”.

Page 16, section 6.1, 2nd paragraph, 1st sentence: change “end-point” to “endpoint”. That would seem to match the spelling in the title of RFC 8604.

Page 16, section 6.1, 2nd paragraph, 3rd sentence: insert “the” before “Reply”.

Page 17, 1st paragraph, 2nd sentence: append a comma after “off”.

Page 17, 2nd paragraph, 1st sentence: insert “an” before “MPLS”. Append a comma after “request”.

Page 17, 2nd paragraph, 2nd sentence: insert “the” before “Reply”.

Page 17, 2nd paragraph, 4th sentence: insert “the” before “reply”.

Page 17, section 6.2.1, 1st paragraph, 1st sentence: insert “obtaining” or something similar before “echo replies”.

Page 17, section 6.2.1, 1st paragraph, 3rd sentence: Change “Headend” to “The headend” but also consider the general nit about whether headend is hyphenated.

Page 17, section 6.2.1, 3rd paragraph, 1st sentence: change “Inter” to “inter”.
Change “inter-as” to “inter-AS”.

Page 17, section 6.2.1, 3rd paragraph, 3rd sentence: delete the extraneous “section” before “Section” in both places where it occurs in the sentence.

Page 17, section 6.2.1, 5th paragraph, 2nd sentence: change “a SR-MPLS” to “an SR-MPLS”.

Page 17, section 6.2.1, 5th paragraph, 4th sentence: insert “a” before “Reply”.
Insert “a” before “Type-A”. Insert “a” before “label”.

Page 18, 1st paragraph, 1st partial sentence: insert “set by” or “determined by” before “local policy”.

Page 18, 1st partial paragraph, 3rd full sentence: insert “the” before “traceroute”.

Page 18, 1st full paragraph, 1st sentence: insert “The” at the beginning of the sentence.

Page 18, 1st full paragraph, 2nd sentence: insert “a” before “single”.

Page 18, section 6.2.2, 2nd sentence: change “SRGB.” to “SRGBs.”.

Page 18, section 6.2.2, 3rd sentence: insert “the” before “case”.

Page 18, section 6.2.2, 6th sentence: append a comma after “P4”. Append a period after “etc”.

Page 18, section 6.2.2, 7th sentence: append a comma after “ASBR4”.

Page 19, 1st paragraph, 1st sentence: I think this would read better if you merely deleted “multiple ASes consisting of”.

Page 19, 1st paragraph, 2nd sentence: append a comma after “AS2”. Change “ASBR8,PE5” to “ASBR8, PE5”, i.e., insert a space between these elements in the list.

Page 19, 1st paragraph, 6th sentence: change “similarly” to “ Similarly”. Note that there’s a space before “Similarly” in the replacement to maintain consistency with two spaces between sentences. Insert “the” before “Reply”.
That said, I'd actually reword the sentence as "Similarly, while visiting ASBR8, the EPE-SID from ASBR8 to ASBR6 is added to the Reply Path TLV.”

Page 19, 1st paragraph, 7th sentence: change “AS3 Node-SId" to “AS3, the Node-SID”. Append a comma after “added”.
<SH> fixed

Page 19, section 6.3, 1st paragraph, 2nd sentence: change “a label” to “the label”. Delete “as below.”.

Page 19, section 6.3, 1st paragraph, 3rd sentence: insert “the” before “TTL”.

Page 19, section 6.3, 2nd paragraph, 1st sentence: insert “the” before “case”.
Insert “it” before “converts”.

Page 19, section 6.3, 2nd paragraph, 2nd sentence: change “Node SID” to “Node-SID”.

Page 20, 2nd full paragraph, 8th sentence: change “Echo request” to “The echo request”. Insert “the” before “echo reply”.

Page 20, 2nd full paragraph, 9th sentence: Insert “The” at the beginning of the sentence.

Page 20, 2nd full paragraph, 10th sentence: insert “a” before “uniform SRGB”.

Page 20, section 7, 1st paragraph: change “cooperating administration” to “cooperating administrations”.

Page 22, section 9: Remove the numbering for the contributors and single space their entries, as is done with the authors.

Page 23, section 11, 3rd sentence: change “to use” to “using the”.
<SH> Fixed all the above.



-- 
last-call mailing list -- last-call@xxxxxxxx
To unsubscribe send an email to last-call-leave@xxxxxxxx




[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Mhonarc]     [Fedora Users]

  Powered by Linux