Hi Loa, thank you for your help in improving this document; much appreciated.
Regards, Greg On Mon, Feb 3, 2025 at 7:08 AM Loa Andersson <[email protected]> wrote: > Greg, > > With the caveatg that there are a lot of things happening to the draft > just now, as far as I can see you have correctly captured and addressed > my comments. Thank you! > > /Loa > > Den 02/02/2025 kl. 00:43, skrev Greg Mirsky: > > Hi Loa, > > thank you for your thorough review and constructive comments; greatly > > appreciated. My apologies for the belated response. Please find my notes > > below tagged GIM>>. Attached is the new working version of the draft and > > the diff to highlight updates noted below and those that are intended to > > address the Alvaro's WGLC comments. > > > > Regards, > > Greg > > > > On Fri, Dec 20, 2024 at 3:46 AM Loa Andersson <[email protected] > > <mailto:[email protected]>> wrote: > > > > All, > > > > Sorry I forgot to clean up the template at one point, the Summary > > should > > say: > > > > Summary: > > > > I have some minor concerns about this document that I think should be > > resolved before publication. > > > > Overview: > > > > The document describes the use of > BFD for monitoring individual > > segment lists of candidate paths of an SR Policy. It documents the > use > > of various BFD modes and features such as BFD Demand mode, Seamless > > BFD, > > and BFD Echo function with the BFD Control packet payload in an > SR-MPLS > > domain. The document also defines the use of LSP Ping for Segmen > > Routing > > networks over the MPLS data plane [RFC8287] to bootstrap and control > > path of a BFD session from the egress LER to the ingress LER using > > Segment Routing segment list with MPLS data plane (SR-MPLS). > > > > Then it should be followed by "Minor Issues" as below. > > > > /Loa > > > > > > Den 20/12/2024 kl. 12:31, skrev Loa Andersson via Datatracker: > > > Reviewer: Loa Andersson > > > Review result: Has Issues > > > > > > RTG-DIR LC review of draft-ietf-spring-bfd > > > RTG-DIR Last Call review of draft-ietf-spring-bfd-12 > > > > > > Routing Directorate Last Call Review Template > > > To: > > > > > > [email protected] <mailto:[email protected]> > > > Cc: > > > > > > [email protected] <mailto:[email protected]>; > > > [email protected] <mailto:draft-ietf-spring- > > [email protected]>; > > > [email protected] <mailto:[email protected]>; > > > [email protected] <mailto:[email protected]>; > > > Subject: > > > > > > RtgDir Last Call review: draft-ietf-spring-bfd-12 > > > 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 IETF last call and IESG review, and sometimes > > on special > > > request. The purpose of the review is to provide assistance to > > the Routing ADs. > > > For more information about the Routing Directorate, please see > > > https://wiki.ietf.org/en/group/rtg/RtgDir <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-spring-bfd-12 > > > Reviewer: Loa Andersson > > > Review Date: date > > > IETF LC End Date: 2024-12-13 > > > Intended Status: Experimental > > > > > > Summary: > > > Choose from this list... > > > > > > No issues found. This document is ready for publication. > > > This document is basically ready for publication but has nits > > that should be > > > considered prior to publication. I have some minor concerns about > > this document > > > that I think should be resolved before publication. I have > > significant concerns > > > about this document and recommend that the Routing ADs discuss > > these issues > > > further with the authors. Comments: Please supply an overview of > > the draft > > > quality and readability. Include anything else that you think > > will be helpful > > > toward understanding your review. Overview The document describes > > the use of > > > BFD for monitoring individual segment lists of candidate paths of > > an SR Policy. > > > It documents the use of various BFD modes and features such as > > BFD Demand mode, > > > Seamless BFD, and BFD Echo function with the BFD Control packet > > payload. in the > > > SR-MPLS domain. The document also defines the use of LSP Ping for > > Segment > > > Routing networks over the MPLS data plane [RFC8287] to bootstrap > > and control > > > path of a BFD session from the egress LER to the ingress LER > > using Segment > > > Routing segment list with MPLS data plane (SR-MPLS). > > > > > > Mailing list discussion > > > The SPRING mailining list has been actvie discussing this WGLC. I > > had not read > > > all the mails when I started my review, but has done so prior to > > finish it. > > > There are some overlaps, and some differences between my comments > > and thise on > > > the mailing list, for example ALvaro and I both have a comment on > > the use of > > > the "expected" in the Abstract, I suggest a change and Alvaro to > > drop the enire > > > sentence. While I prefer what I proposed, I have no problem > > living with what > > > Alvaro proposes. This is true for almost all overlapping > > comments, I have made > > > a note if I strongly prefer something I suggested. > > > > > > > > > Minor issues > > > > > > IANA Considerations > > > > > > I have a some concerns about the IANA > > > Considerations. Ketan had almost the same concerns in a mail to > > the list, but > > > the document has changed, so I go through. > > > > > > It is OK to allocate the new "Non-FEC Path TLV" the > > > way you do. > > > > > > However you assign it from a range that require a response > > > if the the TLV is not recognized, which is fine if that is > > > what you want. If that is the case this need to be > > > described in section 3.1. > > > > GIM>> You are correct. The expectation (and the running experiment) is > > that the addressee that does not recognize Non-FEC Path TLV will respond > > with the "One or more of the TLVs was not understood " Return Code. > > Added the following text in Section 3.1: > > NEW TEXT: > > If the receiver of the > > echo request doesn't recognize Non-FEC Path TLV, it MUST set the > > Return Code in an echo reply to 2 ("One or more of the TLVs was not > > understood"). > > > > > > > > If youy think that it can be silently dropped if not > > > recognized you should use range 49162-64507. Again you > > > need to describe this in section 3.1. > > > Table 1 should have a column listing if there are sub-TLV > > registries or not. > > > > > > +=======+==================+===============+============+ > > > | Value | TLV Name | Reference | Sub-TLV | > > > | | | |registry | > > > +=======+==================+===============+============| > > > | TBD1 | Non-FEC Path TLV | This document | Non-FEC | > > > | | | |Path sub-TLV| > > > +-------+------------------+---------------+------------| > > > > > > Table 1: New Non-FEC Path TLV > > > Since you assign a sub-TLV I prefer that you list it. > > > > GIM>> Please let me know if I correctly followed you suggestion: > > > > +=======+==================+===============+==================+ > > | Value | TLV Name | Reference | Sub-TLV Registry | > > +=======+==================+===============+==================+ > > | TBD1 | Non-FEC Path TLV | This document | Path sub-TLV | > > +-------+------------------+---------------+------------------+ > > > > Table 1: New Non-FEC Path TLV > > > > > > > > You create the Non-FEC Path sub-TLV registry almost correctly, > > but I think > > > Table 2 should look like this: > > > > > > +=============+==============+===========================+ > > > | 0-16383 | Standards | This range is for sub-TLVs| > > > | | Action | that require an error | > > > | | | message if notrecognized. | > > > | | | [RFC9041, Section 3.1 | > > > +=============+==============+===========================+ > > > | 16384-31739 | RFC | This range is for sub-TLVs| > > > | | Required | that require an error | > > > | | | message if notrecognized. | | > > | > > > | [RFC9041, Section 3.1 | > > > +=============+==============+===========================+ | > > 31740-31743 | > > > Experimental | Reserved, not to be | | | Use > > | > > > assigned. | | | | This > > range is for > > > sub-TLVs| | | | that require an error > > | | > > > | | message if notrecognized. | | > | > > > | [RFC9041, Section 3.1 | > > > +=============+==============+===========================+ | > > 31744-32767 | > > > First Come | This range is for sub-TLVs| | | Use > > | that > > > require an error | | | | message if > > notrecognized. > > > | | | | [RFC9041, Section 3.1 | > > > +=============+==============+===========================+ | > > 32768-49161 | > > > Standards | This range is for sub-TLVs| | | > > Action | that > > > that can be silently | | | | dropped if > > notrecognized. > > > | +=============+==============+===========================+ | > > 49162-64507 | > > > RFC | This range is for sub-TLVs| | | > > Required | that > > > that can be silently | | | | dropped if > > notrecognized. > > > | +=============+==============+===========================+ > > | 64508-64511 > > > | Experimental | Reserved, not to be | | | Use > > | > > > assigned. | | | | This > > range is for > > > sub-TLVs| | | | that that can be > > silently | | > > > | | dropped if notrecognized. | > > > +=============+==============+===========================+ | > > 64512-65535 | > > > First Come | This range is for sub-TLVs| | | Use > > | that > > > that can be silently | | | | dropped if > > notrecognized. > > > | +=============+==============+===========================+ I.e. > > do it exactly > > > as for other sub-TLV registries. If not you'll have to motivate > this. > > > > GIM>> Please let me know if updates of Table 2 capture your suggestions. > > I noticed that RFC 8126 <https://datatracker.ietf.org/doc/rfc8126/ > > > among the allocation policies lists "First Come First Served" , and I > > used that. also, I noticed that for the Experimental Use you suggest a > > Reserved, not to be assigned. Is that because Section 4.2 of RFC 8126 > > notes that IANA does not tracks the use of values from the experimental > > range? Should I reference Section 4.2 in the Notes? > > > > > > > > Some questions: > > > > > > Experimental RFC needed > > > In the "Note column" of the "Non-FEC Path sub-TLV registry" you > > "Specification > > > Required", that registration policy is the widest we have, almost > > anything that > > > we can imagine to be a "specification" is allowed. All documents > > from any SDO > > > is liokely to pass that var. You scribble something on a napkin, > > take a photo > > > of it and store somewhere where it is publicly retrievable, and > > you can make a > > > case for "specification". Then we go to the "Note" field and > > there you say > > > "Experimental RFC needed", so you limit our widest category down > > to a single > > > type of document. Please not that "Experimental RFC needed" is > not a > > > registrtation policy. If you want it you have to describe it, > > > > > > Why is that? Isn't RFC do it like this? Isn't RFC Required" > > sufficicient? > > > > GIM>> As noted above, Table 2 was updated according to your > > recommendation, including the for Experimental Use ranges. I couldn't > > figure out whether "Reserved, Not to be assigned" belongs - Registration > > Procedures column of Note. > > > > > > > > Populate new registries > > > When you create a new registry you can populate if, the "TBDs" > > are not needed, > > > there are no conflicts. IANA will review and do what you say. We > > let INA pick > > > the values when there are a risk for conflict. Add a value > > instead of "TBD2". > > > > GIM>> Thank you for the suggestion. Done. > > > > > > > > First Come, First Served > > > Why did you remove FCFS? We had a long discussion on including it > > when we wrote > > > RFC 9041. > > > > GIM>> Thank you for pointing that out to me. The new Non-FEC Path sub- > > TLV registry now uses FCFS policy. > > > > > > > > Assignement conflicts > > > For "Non-FEC Path sub-TLV registry" you first say that we have a > > small problem. > > > You want to Reserve to values "0" and "65535". The glitch is that > > you first say > > > that for "0" the registration policy is "Standard Tracks", and > > then yo try to > > > "Reserve" it from and Experimental RFC. It shold not work. > > > > > > For "65535" you first say it "First Come, First Served" and > > then"Reserve", it > > > can't be both. I think you can reserve "0" and "65535" and make > > the first > > > Standard track range 1-16383, and the Private Use range > > 64512-65534. But I > > > think yuo should get an opinion from IANA before you commit. > > > > > > Alternatively you could do as all other sub-TLV registries does, > > skip reserving > > > 65535. > > > > GIM>> I agree with the partition of the values you suggested for > > the Non-FEC Path sub-TLV registry. > > > > > > > > Assigment from the Return Code registry > > > You are requesting that a Standard Track code point from the > > "Return Codes" > > > registry. You can't do that from an Experimental RFC. I suggest > > that you ask > > > for a value from the RFC Required range instead. > > > > GIM>> Updated according to your recommendation. > > > > > > > > Nits: > > > > > > Nits are editorial or layout items. They are things that would > > ideally be > > > resolved before publication to make the document more readable > > and may be > > > raised now to save the RFC Editor's work. Usually a reviewer will > > not be > > > looking for this type of issue but may find some in the course of > > their review. > > > Please try to avoid raising esoteric questions about English > > usage. The RFC > > > Editor will spot these, and it is not a wise use of time to > > discuss these > > > things. If you find no nits, please leave this section out. > > Abstract SR-MPLS is > > > not a wellknow abbreviation, so it need to expanded at first use. > > If it is used > > > both in the Abstract and the document text I think it should be > > expanded twice. > > > The Abstract is treated as something stand-alone. > > > > > > Terminology > > > Consistency > > > This is is inconsistence, sometimes there is a ":" between the > > abbreviation and > > > expansion, sometime not. I prefer with the ":". > > > > > > SR-MPLS > > > In the terminology section you expand SR-MPLS as: > > > > > > SR-MPLS Segment Routing with MPLS data plane > > > The working group have told the RFC Editor to use: > > > > > > SR-MPLS Segment Routing over MPLS > > > in the Abbreviation list. We should follow the abbreviation list. > > > > GIM>> Updated per your suggestion. > > > > > > > > The terminology section kisses: > > > > > > LSR Label Switching Router > > > which is used in setion 11. > > > > GIM>> Changed to the expanded form. > > > > > > > > Section 2 > > > You correctly have a reference to RFC 8660, but the forwarding > > plane operation > > > "NEXT" shows up a bit abrupt and if I undedrstand correctly it is > > explained in > > > RFC 8402, could please add some text on what "NEXT" is and where > > to find the > > > definition. > > > > > > > > In section 2 you have this paragraph: > > > > > > When LSP Ping is used to bootstrapping a BFD session for > > > SR-MPLS segment list the FEC corresponding to the last > > > segment to be associated with the BFD session MUST be > > > as the very last sub-TLV in the Target FEC TLV. > > > First, a "Target FEC" TLV does not exist. There is a "Target FEC > > Stack" TLV > > > (TLV # 1)is that the TLV you mean? > > > > > > Second, the "Target FEC Stack" TLV shares sub-TLVs with the > > "Reverse-path > > > Target FEC Stack" TLV and the "Reply Path" TLV, together there > > are in the order > > > of 50 sub-TLVs defined. Then you say "last must be last", is that > > among all > > > sub-TLVs, or just among those defined in this document. > > > > GIM>> As a result of addressing WG LC Chair's comments <https:// > > mailarchive.ietf.org/arch/browse/spring/? > > gbt=1&index=J1bVdJsKfxTRhHOSau02xnNe-wQ>, Section 2 changed. I hope that > > you agree with the updates and that they address your concerns. > > > > > > > > You also have this paragraph: > > > > > > Encapsulation of a BFD Control packet in Segment Routing > > > network with MPLS data plane MUST follow Section 7 > > > [RFC5884] when the IP/UDP header used and MUST follow > > > Section 3.4 [RFC6428] without IP/UDP header being used. > > > I think this would be better: > > > > > > Encapsulation of a BFD Control packet in Segment Routing > > > networks over a MPLS data plane MUST follow Section 7 of > > > [RFC5884] when the IP/UDP is header used. The > > > encapsulation MUST follow Section 3.4 of [RFC6428] when > > > an IP/UDP header is not used. > > > > GIM>> Thank you for the recommendation that improves the readability. > > Applied in the working version. > > > > > Section 3 > > > I found the text hard to read, I think it could be improved. If > > you want I can > > > do an attempt off-line. > > > > GIM>> Thank you for your kind offer. Section 3 was updated to address > > Alavor's comments. I greatly appreciate your help in improving the new > > version in the attached copy of the working version of the draft. > > > > > > > > Section 3.1 > > > RFC 8029 gives the names of the LÖSP Ping messages as: > > > > > > MPLS echo request; and > > > MPLS echo reply > > > In section 3.1 you use "echo request" and "Echo Request", please > > align with RFC > > > 8029. > > > > GIM>> Thank you. Done throughout the document. > > > > > > > > Section 3.2 > > > Section 3.2 is unclear. You say: > > > > > > "...BFD Reverse Path TLV MAY use Target FEC sub-TLVs > > > defined in [RFC8287]." > > > Do you mean that it can use ALL the Target FEC Stack TLVs, or > > just the three > > > sub-TLVs defined in RFC 8287? If the latter I think we should say: > > > > > > "...the BFD Reverse Path TLV MAY use the three Target FEC > > > sub-TLVs defined in [RFC8287]." > > > I also wonder about the "MAY", is there an alternative. maybe the > > is "may"? > > > > GIM>> As the result of addressing WG Chair's comments Section 3.2 now > > reads as: > > Target FEC sub-TLVs defined in [RFC8287] are applicable in SR domains > > that are in the scope of [RFC8287]. > > > > Does the new text resolve your concern? > > > > > > > > Section 4 > > > s/can be following in SR-MPLS/can be followed in SR-MPLS > > > > GIM>> Done. > > > > > > > > You say: > > > > > > "an ingress SR node bootstraps BFD session over SR-MPLS in Async > > BFD mode" > > > > > > RFC 5880 does not use "Async BFD mode", but uses "Asynchronous > > mode", I think > > > we should follow RFC 5880. > > > > GIM>> Fixed in two remaining occurences. > > > > > > > > You say: > > > > > > "it sends its BFD Control packet to the ingress SR node over the > > IP network > > > with a Poll sequence" > > > > > > You might want to add a reference to RFC 5880 section 6.5. > > > > GIM>> Thank you for the suggestion. Done. > > > > > > > > Section 5 > > > In your text is p2mp and multicast synoumous? I have a tendency > > to think of > > > multicast as a application offered to "users" that they can join > > or leave, > > > while p2mp is a service that a lower layer supply. I'm not > > dogmnatic about just > > > wanna know how to understand your text. > > > > GIM>> Thank you for another great question, Loa. I agree with your view > > of p2mp vs. multicast. The text refers to multicat in connection with a > > service and a distribution tree. In that section, p2mp is used as a > > synonym of Multipoint, and in P2MP SR Policy. Would you suggest an > > editorial update or a clarification? > > > > > > > > Section 6 > > > In section 6 you use "Echo-BFD", "Echo BFD". and "Echo BFD > > packets", RFC 5880 > > > talks about "BFD Echo packets", please align. > > > > GIM>> Done. > > > > > > > > Section 10 > > > Security considerations are normally outside the scope of my > > competence, I'll > > > be waiting for the SEC-DIR review of the document. > > > > > > I know from experience that the SEC-DIR does not like "no > > additional security > > > risks". > > > > > > Grammar concerns > > > I have a couple of comments that are grammatical in nature. > > Please take care > > > with these comments. English is not my mother tongue, but I'm > > happy if you read > > > and consider (even if you decide not to take what I suggest). > > > > > > MPLS Data Plane > > > In the Abstract you talk about the MPLS Data Plane and say: > > > > > > Inm the 2nd sentence "It can be realized in the Multiprotocol > > Label Switching > > > (MPLS) network without changing the data plane." > > > > > > I have the feeling that "without changing the data plane" can be > > understood > > > that the entire data plane is swapped, maybe: > > > > > > s/without changing the data plane/without any changes to the data > > plane/ > > > > GIM>> I agree and updated accordingly. > > > > > > > > "Expected to" > > > The second sentence in the Abstract says: > > > > > > "Bidirectional Forwarding Detection (BFD) is expected to monitor > > a segment > > > list..." > > > > > > I have the feeling that "expectations" leave quite a bit of > > > uncertainty. What about: > > > s/(BFD) is expected to/(BFD) is used to/ > > > > GIM>> Abstract got significant re-work, and that wording now reads as: > > This > > document describes using Bidirectional Forwarding Detection (BFD) for > > monitoring individual segment lists of candidate paths of an SR > > Policy. > > Please let me know if the readability improved. > > > > > > > > Language concerns > > > I think there is a need to clean up the language in this > > document, but for > > > easily understandle reasons I'm not the person to do this. > > > > > > I can point at some problems, for example I find that articles > > (the, a and an) > > > are often missing. However, my English is not good enough. > > > > > > > > > > > > > -- > > Loa Andersson > > Senior MPLS Expert > > Bronze Dragon Consulting > > [email protected] <mailto:[email protected]> > > [email protected] <mailto:[email protected]> > > > > > > -- > Loa Andersson > Senior MPLS Expert > Bronze Dragon Consulting > [email protected] > [email protected] > >
_______________________________________________ spring mailing list -- [email protected] To unsubscribe send an email to [email protected]
