[Last-Call] Re: [RTG-DIR]Rtgdir last call review of draft-ietf-spring-bfd-12

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

 



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

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

  Powered by Linux