Re: [RTG-DIR] Rtgdir early review of draft-ietf-idr-entropy-label-13

[John Scudder <[e-mail]>]

Hi Mach,

Thanks for your review. Some replies are inline below. Things I have marked as "done" are in my local copy, and will be published when we push version 14.

> On Dec 11, 2023, at 2:14 AM, Mach Chen via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Mach Chen
> Review result: Has Nits
> 
> Hello
> 
> I have been selected to do a routing directorate “early” review of this draft.
> https://urldefense.com/v3/__https://datatracker.ietf.org/doc/draft-ietf-idr-entropy-label-13/__;!!NEt6yMaO-gk!DMNkDh4StpythPVuKp4Eg4LQNXdvQGizTNqeW9Xe90RMIRYWbwT74NjgdURdBrnKstK5dV0S2DOA$
> 
> The routing directorate will, on request from the working group chair, perform
> an “early” review of a draft before it is submitted for publication to the
> IESG. The early review can be performed at any time during the draft’s lifetime
> as a working group document. The purpose of the early review depends on the
> stage that the document has reached. As this document is in working group last
> call, my focus for the review was to determine whether the document is ready to
> be published. Please consider my comments along with the other working group
> last call comments.
> 
> For more information about the Routing Directorate, please see
> https://urldefense.com/v3/__https://wiki.ietf.org/en/group/rtg/RtgDir__;!!NEt6yMaO-gk!DMNkDh4StpythPVuKp4Eg4LQNXdvQGizTNqeW9Xe90RMIRYWbwT74NjgdURdBrnKstK5dSVfUNlu$
> 
> Document: draft-ietf-idr-entropy-label-13
> Reviewer: Mach Chen
> Review Date: 10 Dec 2023
> Intended Status: Standards Track
> 
> Summary
> I think that this document is basically ready for publication, but there are a
> few points below that I would like to discuss for clarification. I also found a
> few nits that should be fixed at some point before publication.
> 
> Comments and Questions
> In Section 1
> It says:
> "An NHC carried in a given BGP UPDATE message conveys information that relates
> to all Network Layer Reachability Information (NLRI)..." Does the NLRI include
> the MP_REACH_NLRI case? If so, how? If not, it's better to clarify this in the
> document.

I don't understand this point. Surely, MP_REACH_NLRI is the dominant way of advertising reachable NLRI in BGP updates, the only other way is the legacy encoding for AFI/SAFI 1/1. So, I'm not sure what to clarify here.

> Please expand ELCv3 when first use.

Done.

> In Section 2.1, the last paragraph
> Given the a BGP speaker "MUST" elect to be prepared to consume capabilities in
> any order, I am not sure it's necessary to require that the sender "MUST" place
> the Capablity TLVs in increasing oder. Maybe it's better to use some other
> relax requirement.  For example, "It recommends that..."

I think an earlier reviewer pointed out that there's no reason to confuse implementers by suggesting they use anything other than canonical order under any circumstances whatsoever. So, I prefer to keep the MUST. If we were going to soften anything, I guess it would be the one about receiving out-of-order TLVs, but I don't see a need for that either. 

> In Section 2.2,
> 
> s/some other BGP speaker/some other BGP speakers

This is correct as written. The singular is intended.

> s/S need/S needs to

This is also correct as written, but I acknowledge the usage is less common than some other ways of expressing the same thought. So for clarity, I rewrote “S need take no special action" as "S does not need to take any special action”.

> In Section 2.3
> OLD:
> When a BGP speaker receives a BGP route that includes the NHC, it MUST compare
> the address given in the header portion of the NHC and illustrated in Figure 1
> to the next hop of the BGP route. NEW: When a BGP speaker receives a BGP route
> that includes a NHC, it MUST compare the next hop address given in the header
> portion of the NHC illustrated in Figure 1 to the next hop of the BGP route.

I think the sentence as written is OK, and strictly speaking the address in the NHC header isn’t a next hop, it's just an identifier that happens to correlate with a next hop. So, I prefer not to describe it as "next hop address" unless necessary, which I don't think it is — on the contrary, I think it would impede clarity.

> s/some intermediate BGP speaker/some intermediate BGP speakers

Again, the singular is intended.

> s/both does not/both do not

And again. I thought of removing the "both", but it serves to make clear that the two facts separated by the “and” that follows are independent. So I think it's better with.

> In Section 3.2, it says:
> "When a BGP speaker S has a route R it wishes to advertise with next hop N to
> its peer, it SHOULD include the ELCv3 capability if it knows that the egress of
> the associated LSP L is EL-capable, otherwise it MUST NOT include the ELCv3
> capability." Is it appropriate to use "SHOULD" here, IMHO, it's better to use
> "MAY".

Done.

> In Section 3.3,
> OLD:
> "When a BGP speaker receives a labeled route that includes the ELCv3, it
> indicates the associated LSP supports entropy labels." NEW: "When a BGP speaker
> receives a labeled route that includes the ELCv3, it indicates the egress LSR
> of associated LSP supports entropy labels."

I think this isn't quite right, because of the midpoint case covered in "Is re-advertising a BGP route it received with a valid ELCv3 capability, and is changing the next hop that it has received to N, and knows (for example, through configuration) that the new next hop (normally itself) even if not EL-capable will simply swap labels without popping the BGP-advertised label stack and processing the label below, as with a transit LSR”. 

Thanks,

—John