Re: [mpls] QA Review draft-akiya-mpls-lsp-ping-reply-mode-simple-03

[Lou Berger <[e-mail]>]

On 09/06/2014 02:41 PM, Nobo Akiya (nobo) wrote:
> Hi Lou,
> 
> Many thanks for your time for reviewing and providing constructive and positive comments.
> 
> Please see in-line with [NOBO].
> 

Nobo,
	responses in-line.

>> -----Original Message-----
>> From: Lou Berger [mailto:[e-mail]]
>> Sent: Friday, September 05, 2014 4:21 PM
>> To: [e-mail]
>> Cc: [e-mail]
>> Subject: QA Review draft-akiya-mpls-lsp-ping-reply-mode-simple-03
>>
>>
>> Hello,
>>     I've been asked to do a QA review of this draft. Overall it looks like a solid,
>> well written document,  and more than reasonable -00 WG document.
> 
> [NOBO] :)
> 
>>
>> I have some technical nits/comments/questions that I think can be
>> addressed as part of normal WG process:
>>
>> Section 3.1
>> - "Reverse LSP is in relation to the last FEC  specified in the Target FEC Stack
>> TLV."
>> I think I understand this statement for the egress, but what about at transit
>> nodes?  Perhaps just drop the sentence?
> 
> [NOBO] The text pointed out was added as result of comments received
> on the list, so I believe it is worth keeping some form of this text
> in the document. Although this is something I've been meaning to
> revisit, so thanks for highlighting it! I believe the text is mostly
> correct, even for transit LSRs. When FOO is going over a
> bi-directional LSP, then the transit LSR within the bi-directional
> LSR:
> 
> (1) has a reverse LSP in relation to the first FEC (top label).
> (2) does not have a reverse LSP in relation to the second (last) FEC (bottom label).
> 
> In this case, the transit LSR shouldn't be using the reverse LSP for
> (1) since that may not reach the ingress of the LSP being tested.
> 
> What is missing from this particular text is that the reverse LSP is
> not always in relation to the last FEC described. A label stack
> having special purpose label as bottom of the stack (ex: Explicit
> NULL) will result in the Target FEC Stack to have a Nil FEC as the
> last FEC. So what the text should say is that reverse LSP is in
> relation to the last "transport" FEC specified in the Target FEC
> Stack TLV.
> 

> If you are ok with this, I'd like the change the text to:
> 
> [OLD]
>    Reverse LSP is in relation to the last FEC
>    specified in the Target FEC Stack TLV.
> 
> [NEW]
>    Reverse LSP is in relation to the last "transport"
>    FEC specified in the Target FEC Stack TLV.  A FEC such as Nil FEC
>    (Target FEC Stack TLV Sub-Type 16) is not considered a "transport"
>    FEC.
> 
> What do you think?

Isn't the reverse LSP based on the received LSP? If not, a ping can be
(perhaps miss) delivered on one LSP and have a response on a different
reverse LSP.  What am I missing?

> 
>>
>> - Missing "in the range of" in the last sentence of the section.
> 
> [NOBO] Yes, thanks for catching this.
> 
>>
>> Section 3.2
>> - The phrasing of 4 is a bit stricter than I think you mean with respect to use
>> of Reply Mode field.  One could read it as saying that a value carried in the
>> field can't be used even when it is in the Order TLV, as well as precluding its
>> use described in the subsequent requirement (5).
>> Perhaps just rephrase to say that it is only used after any mode included in
>> the ordered TLV is excluded/invalid/unavailable.
> 
> [NOBO] Good point. Let me change (4):
> 
> [OLD]
>    4.  If a responder node understands the Reply Mode Order TLV and the
>        TLV is valid, then the responder MUST consider Reply Mode values
>        described in the TLV and MUST NOT use the value described in the
>        Reply Mode field of received MPLS echo request.
> 
> [NEW]
>    4.  If a responder node understands the Reply Mode Order TLV and the
>        TLV is valid, then the responder MUST consider Reply Mode values
>        described in the TLV.

sure, although "consider" is a bit ambiguous.   Perhaps ... "MUST
select the first available return path as identified by the ordered list
of Reply Mode values contained in the TLV."

also change the start of 5 to be "If no valid return path can be
identified based on the TLV or if the responder..."

> 
>>
>> -I think the document should be explicit about ordering relationship of the
>> reply modes listed in the Order TLV and to the order of paths included in
>> other TLVs, e.g., sub-TLVs/FECs in the Reply Path TLV.  (Yes it should obvious,
>> but it is a potential interop issue)
> 
> [NOBO] Reply Path TLV is the only one that lists paths for the
> purpose of replies. Section 4.1.1 intends to clarify this. Can you
> take a look and let me know if this is sufficient, or you think more
> texts will be helpful.
> 

How about something along the lines of (if you can make this simpler,
please do so!):
(section 3.2) 9. When the same Reply Mode value appears more than once
in a Reply Mode Order TLV and the indicated reply mode uses information
contained in the MPLS echo request, the ordering of Reply Mode values
will correspond to the order the information appears in the MPLS echo
request.


>>
>> - Section 4.1.1
>>  o The Reply Path TLV carrying {FEC X, FEC Y} Wouldn't it be cleaner,
>> particularly when considering non-draft supporting implementations, to use
>> multiple Reply Path TLVs (one per possible reply path)?
> 
> [NOBO] I believe authors of RFC7110 (which defines the Reply Path
> TLV) can clarify its usage, but here's the way I understand that
> document.

At this point, it's the document that matters most ;-)

> 
> - Number of Reply Path TLVs that can appear in the MPLS Echo
>   Request/Reply messages is not strictly enforced in the document.
> 
If this means that it is under defined/open to interpretation, I agree.

> - The document defines and describes scenarios where one Reply Path
> TLV is in the MPLS Echo Request/Reply (that can carry multiple return
> FECs).
> 
Yeah, but it also mentioned encoding multiple return path(s).

> - The document does not spell out what happens when there are more
> than one Reply Path TLVs in the MPLS Echo Request/Reply messages
> (i.e. which takes precedence).
> 
Fair point. That said, rfc7110 doesn't also doesn't spell out when there
are more than one fecs/paths in the Reply Path TLVs.

> Hence I assumed that author intended that MPLS Echo Request/Reply can
> carry only one Reply Path TLV. And hence, I descried the 4.1.1
> example as one Reply Path TLV carrying {FEC X, FEC Y} instead of
> having multiple Reply Path TLVs. After scanning through RFC7110, I
> still think my understanding of the document is probably what the
> author intended.
> 

Again, once it's what is actually said in the RFC(s) that matter most at
this point.

To me your last point is fairly convincing, although as said above,
there's still ambiguity. Either way, it probably means that (a) the
expected Reply Path TLV processing needs to be spelled out and (b) this
document will be an update to rfc7110.  This is something that can be
worked out by the WG as the document progresses.

> I suppose the last two comments are sort of related. Do let me know
> your thoughts after reading my repose to you.
> 

Yup.

Thanks,
Lou

>>
>> That's it,
>> Lou
> 
> Again, many thanks for thorough review!
> 
> Thanks!
> 
> -Nobo
> 
> 
>