Subject: Re: [RTG-DIR] Rtgdir early review of draft-ietf-isis-mpls-elc-08

From: Peter Psenak <[email-masked]>
Date: Fri, 4 Oct 2019 13:14:35 +0200

Hi Dhruv,

please see inline (##PP):

On 12/09/2019 12:06, Dhruv Dhody via Datatracker wrote:
> Reviewer: Dhruv Dhody
> Review result: Has Issues
> 
> Subject: RtgDir Early review: draft-ietf-isis-mpls-elc-08
> 
> Hello
> 
> I have been selected to do a routing directorate “early” review of this draft.
> ​https://datatracker.ietf.org/doc/draft-ietf-isis-mpls-elc/
> 
> 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
> ​http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
> 
> Document: draft-ietf-isis-mpls-elc-08
> Reviewer: Dhruv Dhody
> Review Date: 12-09-2019
> Intended Status: Standards Track
> 
> Summary: I have some minor concerns about this document that I think should be
> resolved before it is submitted to the IESG.
> 
> The draft is focused and straightforward, the reader needs to be aware of
> RFC6790 and draft-ietf-mpls-spring-entropy-label beforehand. I have reviewed
> this and the OSPF I-D together and you will find similar comments for both I-Ds.
> 
> Minor
> *****
> (1) Could you mark that the codepoints mentioned in the draft are early
> allocated by IANA? Currently it says the value are desired. I also suggest
> following change in Section 7 (IANA Considerations) -
> 
> OLD:
>     IANA is requested to allocate the E-bit (bit position 3 is desired)
>     from the "Bit Values for Prefix Attribute Flags Sub-TLV" registry.
> 
>     IANA is requested to allocate a MSD type (the type code of 2 is
>     desired) from the "IGP MSD Types" registry for ERLD.
> NEW:
>     IANA is requested to confirm the early allocation of the E-bit (Bit
>     position 3) in the "Bit Values for Prefix Attribute Flags Sub-TLV"
>     registry.
> 
>     IANA is requested to confirm the early allocation of the ERLD (type
>     code of 2) in the "IGP MSD Types" registry.
> END

##PP
I'm not sure above is necessary, given that the above text would change
eventually to simply say which code points have been allocated.

> 
> (2) Section 3 talks about ERLD in Node MSD sub-TLV. But what happens if one
> receives ERLD in the Link MSD sub-TLV? As per my understanding this is not
> allowed, better to add normative text for the case then.

##PP
added a sentence

> 
> Also we have this text in draft-ietf-mpls-spring-entropy-label -
> 
>     In a distributed switching architecture, each linecard may have a
>     different capability in terms of ERLD.  For simplicity, an
>     implementation MAY use the minimum ERLD of all linecards as the ERLD
>     value for the system.
> 
>     There may also be a case where a router has a fast switching path
>     (handled by an ASIC or network processor) and a slow switching path
>     (handled by a CPU) with a different ERLD for each switching path.
>     Again, for simplicity's sake, an implementation MAY use the minimum
>     ERLD as the ERLD value for the system.
> 
>     The drawback of using a single ERLD for a system lower than the
>     capability of one or more specific component is that it may increase
>     the number of ELI/ELs inserted.  This leads to an increase of the
>     label stack size and may have an impact on the capability of the
>     ingress node to push this label stack.
> 
> If we are deviating from this and opting for the node (marked 'MAY' above) as
> the only possibility, we need to handle this properly. Maybe check with
> chairs/AD on this!

##PP
A remote router introducing the EL may not always know the LC/interface 
over which the traffic he sends is received on the remote node. So I 
don't really see much of the value advertising ERLD per link.

Sure, chairs/AD will have their chance during the review :)

> 
> (3) Section 4, can we add some more description on what the 'E' flags means, in
> the similar style of other flags
> [https://tools.ietf.org/html/rfc7794#section-2.1]

##PP
done

> 
> (4) Section 8, suggest to also add one sentence for the impact of advertising
> incorrect ERLD. If there isn't any, that can also be stated.

##PP
done

> 
> Nits
> ****
> (1) Suggested ordering of sections - ..ELC/ERLD/BGP-LS/ACK..  [matching between
> OSPF/ISIS] 

##PP
done

>(2) Section 2, add [I-D.ietf-mpls-spring-entropy-label] for
> terminology reference 

##PP
done

>(3) Section 3, Add reference to
> draft-ietf-mpls-spring-entropy-label for the definition and usage of ERLD 

The Introduction section has:

"This capability, referred to as Entropy Readable Label
    Depth (ERLD) as defined in [I-D.ietf-mpls-spring-entropy-label] "

> (4)
> Section 6,
> 
>     The ERLD MSD-type introduced for IS-IS in Section 3 is advertised
>     using the Node MSD TLV (TLV 266) of the BGP-LS Node NLRI Attribute as
>     defined in section 3 of [I-D.ietf-idr-bgp-ls-segment-routing-ext].
> 
> I think you mean draft-ietf-idr-bgp-ls-segment-routing-msd here!

##PP
right, corrected it.

> 
> Also, maybe change the title "BGP-LS Extension" as there is no 'extension'
> required, ELC/ERLD is BGP-LS would be automatically supported.

##PP
renamed to "Signaling ELC and ERLD in BGP-LS".

> 
> (5) Expand MSD on first use.

##PP
done.

> (6) The first figure is titled Figure 2!

##PP
fixed

> (7) Section 4, mark the figure as "Prefix Attribute Flags"

##PP
done

> (8) All references are marked Normative, please re-check if this is intentional.

##PP
done.

thanks,
Peter
> Thanks!
> Dhruv