Re: [RTG-DIR] Routing directorate review of draft-ietf-mpls-lsp-ping-lag-multipath-03

[George Swallow <[e-mail]>]

Mach -

>>
>> Top of page 13:
>>    The initiator LSR sends two MPLS echo request messages to traverse
>>    the two LAG members at TTL=1:
>> “TTL=1” should be “TTL=n”.
>
>Good catch, fixed.

At this point in the procedure you have already reached the node at ttl=n.
You are now probing a LAG that extends to the node at TTL=n+1.

S0 the text should be "TTL=n+1".

Thanks,

George

On Wed, May 23, 2018 at 6:03 AM, Mach Chen <[e-mail]> wrote:

> Hi Jon,
>
> Thanks for the detailed review and useful comments!
>
> Please see some responses inline...
>
> > From: Jonathan Hardwick [mailto:[e-mail]]
> > Sent: Tuesday, May 22, 2018 3:39 AM
> > To: [e-mail]; [e-mail];
> mpls-
> > [e-mail]
> > Cc: [e-mail]
> > Subject: Routing directorate review of draft-ietf-mpls-lsp-ping-lag-
> multipath-03
> >
> > Hello
> >
> > I have been selected to do a routing directorate “early” review of this
> draft.
> > https://datatracker.ietf.org/doc/draft-ietf-mpls-lsp-ping-lag-multipath/
> >
> > 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
> close
> > to 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
> >
> > Best regards
> > Jon
> >
> >
> > Document: draft-ietf-mpls-lsp-ping-lag-multipath-03.txt
> > Reviewer: Jonathan Hardwick
> > Review Date: 21 May 2018
> > Intended Status: Standards Track
> >
> > Summary
> > This document looks ready for working group last call.  I have a few
> minor
> > issues that I am sure can be resolved during the last call.
> >
> >
> > Section 2
> > First paragraph: the reference to section 3.3 of [RFC8029] looks wrong.
> Should
> > it be a reference to section 4?
>
> It was intended to refer to Section 3.3 RFC4079 (Downstream Mapping).
>
> How about the following text:
> "Reader is expected to be familiar with mechanics of Downstream Mapping
> described in Section 3.3 of RFC8029 and Downstream Detailed Mapping TLV
> (DDMAP) described in Section 3.4 of RFC8029."
>
>
> >
> > Section 3
> > “When the responder LSR receives an MPLS echo reply message” <- you mean
> > “MPLS echo request message”.
>
> Yes.
>
> >
> > Section 5.1
> > This is fine, but I found it a bit cumbersome to read.  How about this
> rewording?
> > NEW
> >   If the downstream LSR does not return Remote Interface Index sub-TLVs
> in
> >   the DDMAP, then the initiator LSR validates LAG member link traversal
> by
> >   traversing all available LAG member links and then using the procedure
> > described
> >   below.  This section provides the mechanism for the initiator LSR to
> obtain
> > additional information from the downstream LSRs and describes the
> additional
> > logic in the initiator LSR to validate the L2 ECMP traversal.
> > END
>
> This looks good to me, thanks for the new text!
>
> >
> > Section 5.1.3
> > For my interest, why are you using “entropy” here?  It sounds like you
> mean
> > “probability”, but I might have misunderstood your meaning.
>
> The "entropy" is used to select specific LAG member link, it has the
> similar concept as "entropy label".
>
> >
> > Top of page 13:
> >    The initiator LSR sends two MPLS echo request messages to traverse
> >    the two LAG members at TTL=1:
> > “TTL=1” should be “TTL=n”.
>
> Good catch, fixed.
>
> >
> > Section 6
> > Typo “in the in the”
>
> Fixed.
>
> >
> > Section 8 and 9
> > This draft only discusses using the new Local & Remote Interface Index
> Sub-
> > TLVs in the context of a DDMAP for a LAG interface, so I was surprised
> to read
> > that it is permissible to set M=0 in these TLVs.  You should describe
> how the
> > TLV is used in that case, if you are going to allow it.
> > Does the M flag need to be set consistently in all Local & Remote
> Interface
> > Index Sub-TLVs  in a given DDMAP TLV?
> > In fact, isn’t the M flag redundant, given that the enclosing DDMAP has
> the
> > "LAG Description Indicator flag"?
>
> Indeed, seems redundant, I will do double check on it.
>
> >
> > Section 10
> > Why do you need the Sub-TLV length field?  It can be inferred from the
> TLV
> > length and the address type.
>
> Indeed, and I personally agree, I will talk to the co-authors, if there is
> no further reasons, will remove the sub-TLV length field.
>
> > Section 10.1.1 – if the LSR received no labels (e.g. PHP case) then
> should it omit
> > this sub-TLV, or include an empty sub-TLV?
>
> The sub-TLV is derived from Label Stack Sub-TLV defined in 8029, it has
> the same usage as Label Stack Sub-TLV. So, for that case, the sub-TLV
> should be included and an Implicit Null label returned.
>
> >
> > Other nits
> > Throughout, English grammar needs to be fine-tuned e.g. there are
> definite
> > and indefinite articles missing.  However, I found the document perfectly
> > readable, so perhaps this can be left for the RFC editor.
>
> Sure, thanks.
>
> Best regards,
> Mach
>