Re: [Lsr] RtgDir Review: draft-ietf-isis-segment-routing-msd-15

[Julien Meuric <[e-mail]>]

Hi Les,

Thanks for you feedback. Please find some responses, marked [JM] below.
You may consider the trimmed ones as resolved. For others, please keep
in mind that the directorate's purpose is to improve documents'
readability before publication, as opposed to store clarification in
mailing list archives.

Julien


Sep. 24, 2018 - [e-mail]:
> Julien -
> 
> Thanx for your detailed review - and your patience in waiting for a response (I returned from vacation only a few days ago).
> 
> I have published V16 of the draft which addresses your comments except as noted inline below.
> 
> 
>> -----Original Message-----
>> From: Julien Meuric <[e-mail]>
>> Sent: Monday, September 10, 2018 8:28 AM
>>
<snip>
>>
>> - The abstract uses the acronym "SID", however:
>>     * It should be expanded at first use,
>>     * It should be defined, e.g. by adding a (normative) reference to RFC 8402
>> (at least in the introduction),
>>     * The SR context is missing and should be explicitly mentioned (adding a
>> phrase such as "in a Segment Routing-enabled network" would be enough).
> 
> [Les:] SID has been added to the terminology section and a reference to RFC 8402 added.
> However, I don’t think "SR context" is missing. The first sentence of the Introduction starts with
> 
> " When Segment Routing (SR) paths are computed..."
> 
[JM] I fully agree about the introduction, but my comment was about the
abstract where the SR context needs to be guessed from the use of the
"SID" terminology: an explicit phrase would be welcome to a rookie
reading the abstract.

>>
>> - OLD
>>    Path Computation Element Protocol (PCEP) SR extensions draft
>>    [I-D.ietf-pce-segment-routing] signals MSD in SR Path Computation
>>    Element (PCE) Capability TLV and METRIC Object.
>>   NEW
>>    The Path Computation Element communication Protocol (PCEP) SR
>>    extension document [I-D.ietf-pce-segment-routing] defines how to
>>    signal MSD using the SR-PCE-CAPABILITY sub-TLV (per node) and
>>    the METRIC object (per request).
>>
> [Les:] I have updated this and included a reference to RFC 5440 where METRIC object was first defined.
> 
[JM] Even better about METRIC. Consistently, "SR Path Computation
Element (PCE) Capability TLV" still remains a loose phrase, the
technical name from [I-D.ietf-pce-segment-routing] is
"SR-PCE-CAPABILITY", which avoids ambiguity.

>  
>> - The introduction says that the solution to complement BGP-LS lies in IS-IS
>> (the OSPF draft claiming the counterpart on its side). This is a bit rough, the
>> sentence 2 paragraph below already does the necessary scoping job: "This
>> document defines an extension to <your favorite IGP
>> here>". I suggest to temper the early sentence by rephrasing the
>> beginning of page 3 into: "MSD capabilities should be advertised by every
>> router in the network involved in the IGP."
>>
> [Les:] The draft says 
> 
> " MSD capabilities should be
>    advertised by every Intermediate System to Intermediate System(IS-IS)
>    router in the network."
> 
> which I believe meets your suggestion.
> 
[JM] My comment was exactly starting from there. Let me rephrase:
- This I-D says "should be advertised by every IS-IS router";
- draft-ietf-ospf-segment-routing-msd says "should be advertised by
every OSPF router".
Now that we have a single LSR WG, I suggest to drop these competing
wordings and consider a more consensual "should be advertised in the
IGP". Both documents already include a sentence "This document defines
an extension to IS-IS/OSPF" at the end of their introduction, which is
enough to define the scope.

>> - The wording of the following sentence is not clear: "Note that in a non-SR
>> MPLS network, label depth is what is defined by the MSD advertisements." Is
>> it intended to say: "Note that MSD advertisements are applicable beyond SR-
>> enabled networks and may refer to label depth in MPLS networks."?
>>
> [Les:] The draft says:
> 
> " Although MSD advertisements are associated with Segment Routing, the
>    advertisements MAY be present even if Segment Routing itself is not
>    enabled."
> 
> I have made this sentence and the following sentence (which you quoted) a separate paragraph - which hopefully makes this a bit clearer.
> 
[JM] Indeed, a separate paragraph is an improvement but doesn't fully
address my comment. The 2nd sentence I previously quoted combines the
passive voice on top of a subordinate clause starting with "what":
though grammatically correct, don't you disagree that the current
wording barely succeeds in achieving its purpose of building a loud and
clear message to be conveyed by an easy-to-parse sentence? Or do you?
;-) (Should you decide to leave it as is, be prepared to RFC Editor's
review.)

<snip>
>>
>> - The same ASCII art is used for the 2 figures (sections 2 and 3). It would be
>> nice to specialize each one a little bit by explicitly adding their respective
>> codepoint in the type field (23 and 15).
>>
> [Les:] The figure is intended to define the class of information in each field - not the actual values. The actual values are specified in the text which follows the ASCII art.
> 
[JM] This is acceptable to me, but twice the exact same figure makes the
reader wonder why not just two contexts with pointers to a single
definition.

<snip>
> 
>> - In section 5, BMI-MSD is defined as "the total number of MPLS  labels which
>> can be imposed" (which is OK when the incoming packet is unlabled). When
>> the incoming packet is labeled (e.g. use of segment routing binding SID), if
>> the incoming label is to be "replaced" by N outgoing labels, what processing
>> model is assumed when advertising the MSD:
>>  * one swap and one imposition of N-1 labels?
>>  * one pop and one imposition of N labels?
>>
> [Les:] What is being defined is the maximum number of SIDs/labels which can be "imposed". If popping or swapping affects the MSD which can be supported this needs to be accounted for in the advertisement. BMI-MSD is not being used to advertise a value specific to labeled or unlabeled packets nor a value which is "swap specific" or "pop specific". 
> 
[JM] We seem to agree on an architecture-agnostic use of BMI-MSD. "If
popping or swapping affects the MSD [...] this needs to be accounted
for" is a great introduction to the issue and deserves to be included in
the I-D. My comment now binds to: in order to have interoperable
implementations, I believe the document should be more prescriptive on
how we account that for.

>> - For the BMI-MSD use case, the draft seems to clearly target a value
>> attached to the outgoing link. However, this capability on a router is highly
>> hardware-dependent: some implementations may push a stack on the
>> egress linecard while some may perform it on the ingress linecard. The I-D
>> seems to make some implicit hardware assumption and the current link MSD
>> advertisement is not suited to describe these possible combinations of
>> hardware implementations. This needs to be addressed somehow.
>>
> [Les:] If a platform does imposition on ingress, then it should advertise only a Node MSD - as there would be no way to advertise a value which is specific to the ingress-egress interface combination. So, link MSDs for BMI are associated with the use of that interface as the outgoing link.
>[JM] OK. Could you please include that clarification in the I-D itself?

<snip>
>> - s/path doesn't exceed/path does not exceed/
> 
> [Les:] Done. You don't like contractions? :-)
> 
[JM] I don't dislike 'em, but I prefer a slightly more formal language
when writing standard text. :-)

>> - s/and associated attributes and capabilities/as well as associated attributes
>> and capabilities/
> 
> [Les:] I prefer the existing wording. 
> [JM] Acceptable to me, but may trigger a two-cycle reading on some
parser implementations.

Regards,

Julien