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 <loa@xxxxx> 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 <loa@xxxxx
> <mailto:loa@xxxxx>> 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:
> >
> > rtg-ads@xxxxxxxx <mailto:rtg-ads@xxxxxxxx>
> > Cc:
> >
> > rtg-dir@xxxxxxxx <mailto:rtg-dir@xxxxxxxx>;
> > draft-ietf-spring-bfd.all@xxxxxxxx <mailto:draft-ietf-spring-
> bfd.all@xxxxxxxx>;
> > spring@xxxxxxxx <mailto:spring@xxxxxxxx>;
> > last-call@xxxxxxxx <mailto:last-call@xxxxxxxx>;
> > 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
> loa@xxxxx <mailto:loa@xxxxx>
> loa.pi.nu.@xxxxxxxxx <mailto:loa.pi.nu.@xxxxxxxxx>
>
>
--
Loa Andersson
Senior MPLS Expert
Bronze Dragon Consulting
loa@xxxxx
loa.pi.nu.@xxxxxxxxx
-- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx