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

[Mach Chen <[e-mail]>]

Hi John,

Thanks for your prompt response!

Some replies inline...

> -----Original Message-----
> From: John Scudder <[e-mail]>
> Sent: Monday, December 11, 2023 11:37 PM
> To: Mach Chen <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail]
> Subject: Re: Rtgdir early review of draft-ietf-idr-entropy-label-13
> 
> 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-iet
> >
> f-idr-entropy-label-13/__;!!NEt6yMaO-gk!DMNkDh4StpythPVuKp4Eg4LQNXd
> vQG
> > izTNqeW9Xe90RMIRYWbwT74NjgdURdBrnKstK5dV0S2DOA$
> >
> > 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!DMNkDh4StpythPVuKp4Eg4LQNXdvQGizTNqeW9Xe90RMI
> RYWbwT74N
> > jgdURdBrnKstK5dSVfUNlu$
> >
> > 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.
I am thinking about that if the NHC can also apply to MP_REAH_NLRI, then is the "match" checking process based on the NEXT_HOP attribute or the Network Address of Next Hop of MP_REACH_NLRI?

> 
> > 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.

Given there were some discussion on this, I am OK with the MUST.

> 
> > 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”.

OK.

> 
> > 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.

OK, thanks for the clarification.

> 
> > 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.

OK.

> 
> > 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”.

But what does it mean that an LSP supports entropy labels? Maybe we could change the way of expressing, for example:
"When a BGP speaker receives a labeled route that includes the ELCv3, it indicates that it can safely insert an entropy label into the label stack of the associated LSP.

Thanks,
Mach

> 
> Thanks,
> 
> —John