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

[Mach Chen <[e-mail]>]

Hi George,

> From: George Swallow [mailto:[e-mail]]
> Sent: Thursday, May 24, 2018 9:14 PM
> To: Mach Chen <[e-mail]>
> Cc: Jonathan Hardwick <[e-mail]>; draft-ietf-mpls-
> [e-mail]; [e-mail]; [e-mail]; rtg-
> [e-mail]
> Subject: Re: Routing directorate review of draft-ietf-mpls-lsp-ping-lag-
> multipath-03
> 
> 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".

Yes, you are right.

Best regards,
Mach


> 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-multipat
> > h/
> >
> > 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