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

[Jonathan Hardwick <[e-mail]>]

Thanks Mach.  Your proposed markups all look good to me.
Cheers
Jon

-----Original Message-----
From: Mach Chen [mailto:[e-mail]] 
Sent: 23 May 2018 11:04
To: Jonathan Hardwick <[e-mail]>; [e-mail]; [e-mail]; [e-mail]
Cc: [e-mail]
Subject: RE: Routing directorate review of draft-ietf-mpls-lsp-ping-lag-multipath-03

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