Re: [mpls] MPLS working group last call on draft-ietf-mpls-lsp-ping-relay-reply

["[e-mail]" <[e-mail]>]

Hi George,
For Adrian's below comments related with MTU Exceeded Return Code, in the draft, it says: "The return sub-code MUST be set to the value that otherwise would have been sent". I tend to set the return sub-code to zero also. Since we have changed the return code, then it is meanless to retain the original sub-code. Any comments?



Regards
Lizhong
 
From: [e-mail]
Date: 2015-06-10 00:09
To: adrian; loa; mpls; mpls-chairs; draft-ietf-mpls-lsp-ping-relay-reply; db3546
Subject: RE: [mpls] MPLS working group last call on draft-ietf-mpls-lsp-ping-relay-reply
Hi Adrian,
Thank you for the review. Please see my reply inline below. Correct me if other authors have different opinion.
I will update the draft after the end of last call.



Regards
Lizhong
 
From: Adrian Farrel
Date: 2015-06-06 00:34
To: 'Loa Andersson'; [e-mail]; [e-mail]; [e-mail]; 'BRUNGARD, DEBORAH A'
Subject: RE: [mpls] MPLS working group last call on draft-ietf-mpls-lsp-ping-relay-reply
Hi,
 
Just being a good citizen and reviewing this I-D during WG last call.
I didn't have much time so I only found a number of nits most of which
are probably not significant.
 
Cheers,
Adrian
 
---
 
idnits notes the presence of a pre-RFC5378 disclaimer. Do you really 
need that?
[Lizhong] this may need the answer from chairs and AD. We only follow the rules.
 
---
 
Abstract
 
 
You expand "LSR" correctly, but not on first use.
[Lizhong] Good catch, thanks.
 
---
 
Abstract
 
   a replying LSR may not have the available route to the
   initiator
 
s/the/an/
[Lizhong] accept
 
---
 
Section 1
 
You need to expand "LSR" on first use.
[Lizhong] accept
 
---
 
Section 1
 
   This document describes the extensions to the Label Switched Path
   (LSP) Ping as specified in [RFC4379], by adding a relayed echo reply
   mechanism which could be used to detect data plane failures for the
   inter autonomous system (AS) and inter-area LSPs. 
 
s/the extensions/extensions/
s/detect/report/ ???
s/for the/for/
[Lizhong] accept
 
---
 
Section 1
 
Delete "The extensions are to update the [RFC4379]." as it duplicates
the previous sentence.
[Lizhong] accept
 
---
 
Section 1
 
   may not have the available route
 
s/the/an/
[Lizhong] accept
 
---
 
Section 2
 
   LSP Ping [RFC4379] defines a mechanism to detect the data plane
   failures and localize faults. 
 
s/the data/data/
[Lizhong] accept
 
---
 
Section 2
 
OLD
   using an UDP packet with
   the IPv4/IPv6 address of the originating LSR
NEW
   using a UDP packet with
   the IPv4/IPv6 destination address set to an address of the
   LSR that originated the Echo Request.
 [Lizhong] accept

---
 
Section 2
 
OLD
   IP addresses reachability are allowed
NEW
   IP address reachability is allowed
 [Lizhong] accept
---
 
Section 2
 
OLD
   In fact, it
   is almost uniformly the case that in inter-AS scenarios, it is not
   allowed the distribution or direct routing to the IP addresses of any
   of the nodes other than the ASBR in another AS.
NEW
   In fact, it
   is almost always the case that in inter-AS scenarios the only node in
   one AS to which direct routing is allowed from the other AS is the 
   ASBR, and routing information from within one AS is not distributed 
   into another AS.
 [Lizhong] accept
---
 
Section 2
 
   Figure 1 demonstrates a case where one LSP is set up between PE1 and
 
s/one/an/
[Lizhong] accept
---
 
Section 2
 
You should expand "AN" on first use.
 [Lizhong] accept
---
 
Section 3
 
   A new TLV named Relay Node Address Stack TLV is defined in this
   document, to carry the IP addresses of the possible relay nodes for
   the replying LSR.
 
In what way are they "possible" relay nodes? The description of the
mechanism seems to say that these addresses must be used an in the
supplied order.
[Lizhong] the word "possible" is not correctly used here. I will remove it.
 
---
 
Section 3.1 and onwards
 
It would help IANA if you distinguished your two instances of "TBD" by
using "TBD1", "TBD2", and "TBD3".
 [Lizhong] accept
---
 
Section 3.2
 
It looks to me that you intend that the address list can include 
addresses of mixed types (otherwise you would not need the address
type in each entry). However, your offset is provided as a counter
and that means that each time this list is processed, the list must
be walked from top down since each entry may be of a different length.
Thus, while the stack is optimized for stripping unwanted addresses 
from the end of the TLV, it is as suboptimal as you could have made it
for parsing because every node that processes the TLV must scan every
entry in the list.
 
Why not replace the count of addresses with a count of octets to enable
quicker access into the list?
[Lizhong] either is OK. George has same preferrence for octets. I will
change to octets number across the document.
 
---
 
Section 3.2
 
   K bit: if the K bit is set to 1, then this sub-TLV MUST be kept in
   Relay Node Address Stack during TLV compress process described in
   section 4.2.
 
The address stack entry is not a sub-TLV.
 
I am inferring from this text that set to 0 the K bit indicates that the
address stack entry can be kept in or left out at will, but actually 4.2
gives some considerably more complex rules for stripping or not 
stripping when k=0. Perhaps you need...
 
   K bit: if the K bit is set to 1, then this address stack entry MUST
   NOT be stripped from the Relay Node Address Stack during processing
   described in Section 4.2. If the K bit is clear, the entry might be
   stripped according to the processing described in Section 4.2.
[Lizhong] accepted, much clear now.
 
---
 
Section 3.3 seems very "sudden". It doesn't seem to be related to this
document in particular. It is true that the presence of a Relay Node 
Address Stack TLV increases the size of the Echo Reply (or Relayed
Echo Reply) message, but presumably the problem with exceeding the
MTU is only exacerbated by, not caused by, this TLV.
 
Furthermore, section 3.3 talks about dropping TLVs (or sending them 
with length zero), but gives no hints about which should or can be 
dropped to get the MTU down to size. For example, dropping the Relay
Node Address Stack TLV would presumably be frowned upon.
[Lizhong] section 4.2 talks about this as below. How about to add a 
reference in section 3.3?
   If the full reply message would exceed the MTU size, the Relay Node
Address Stack TLV SHOULD be included in the Echo Reply message (i.e.,
other optional TLVs are excluded).
 
Lastly, I cannot understand how the new return code is handled when
some other return code is also demanded.
[Lizhong] I will add the following:
In section 4.4 of [RFC4379], step 7 will be updated. 
Before sending Echo Reply, the new packet size should be checked.
If Best-return-code is 3 ("Replying router is an egress for the FEC at stack depth"), 
or 8 ("Label switched at stack-depth"), and if the packet size exceeds MTU size, then
Best-return-code is TBD3 ("One or more TLVs not returned due to MTU size")


Maybe there is text missing from section 4. All I could find was the
last paragraph of 4.2.
 
---
 
Section 4.1
 
   In addition to the procedures described in section 4.3 of [RFC4379],
   a Relay Node Address Stack TLV MUST be carried in the Echo Request
   message to facilitate the relay functionality.
 
This is ambiguous. The update to 4379 is not that the new TLV must be 
included in the echo request in order to facilitate the relay 
functionality. I think you mean that you have defined an optional TLV
that can be included in an echo request message to facilitate the relay
functionality, and that where that functionality is needed in order to
relay echo reply messages, the new TLV must be included in the echo
request message.
[Lizhong] correct. Rephrased as below:
In addition to the procedures described in section 4.3 of [RFC4379],
a Relay Node Address Stack TLV MUST be carried in the Echo Request
message if the relay functionality is required.
 
---
 
Section 4.1
 
   The source UDP port field MUST be set
   to the source UDP port.
 
There is no "source UDP port" field. Perhaps the "Initiator Source Port"
field?
 
But also, this text is quite confusing. The text in 3.2 is much clearer.
[Lizhong] yes, should be: 
The source UDP port field MUST be set to the initiator source port.
 
---
 
Section 4.1 appears to lack guidance about how to construct a Relay Node
Address Stack TLV to include in an Echo Request message. One may cross-
reference to 4.6 and then a lot becomes clear (including the processes 
in 4.2 that otherwise appear convoluted!).
[Lizhong] accept, will add reference to 4.6.
 
I believe that what you have in mind is that before you can do end-to-
end LSP ping, you must perform a traceroute in order to collect and
construct the full Relay Node Address Stack TLV. Shouldn't the document
make this clear?
[Lizhong] traceroute is not mandatory before ping. If operator has knowledge 
of the relay nodes, the initiator could directly send ping with Relay Node 
Address Stack TLV containing the already known relay nodes.
 
---
 
Section 4.2
 
   An LSR that does not recognize the Relay Node Address Stack TLV,
   SHOULD ignore it as per section 3 of [RFC4379].
 
You are not seeking to change 4379 behavior here, so using 2119 SHOULD
is confusing. Indeed, you cannot change legacy behavior in this 
document. 
 
I think you need...
                                                                
   The Type of the Relay Node Address Stack TLV is chosen from the range
   32768 to 49161 so that (per section 3 of [RFC4379]) an LSR that does
   not recognize the TLV knows that the TLV is optional and can safely
   ignore it.
[Lizhong] accept
 
---
 
I tried to work out how things would pan out if two ASes on the path 
used the same address space within their AS. Would an address appear in
the stack and seem to be routable when it is really an address in the
other AS?
[Lizhong] we have an example in section 5. And address of P1 and P2 could be 
same. In that case, ASBR1 must adds its interface address facing ASBR2 with 
the K bit set. Then relay reply will not be miss-routed.
 
---
 
4.3 has
 
   If the first IP address in the Relay Node Address Stack TLV is not
   the next relay node address
 
There are two questions that follow:
- You have a "SHOULD" to resolve this "if". What are the conditions for
  varying that behavior?
- You don't say what to do if this "if" is not satisfied. Presumably, in
  this case it sends an Echo Reply and you can address my question with
  a pointer to 4.5.
[Lizhong] accept. Will add the following:
   When the next relay node address is the first one in the address 
list, please refer to section 4.5. 
 
Similarly, 4.4 has:
 
   If the next relay node address is not the first one in the address
   list, i.e., another intermediate relay node, the relay node MUST send
   an Relayed Echo Reply message to the determined upstream node with
   the payload unchanged other than the Relay Node Address Stack TLV.
 
If this "if" does not hold, what does the implementation do? Again, a 
pointer to 4.5 may be enough.
[Lizhong] accept, will add the following:
   When the next relay node address is the first one in the address 
list, please refer to section 4.5. 
 
---
 
The third case in 4.5 is when the receiver does not understand the TLV
and ignores it. In this case it will send an Echo Reply without itself
including the TLV.
[Lizhong] the receiver is unable to send the Echo Reply, because it does
know the destination IP address and UDP port number. So if the receiver
could not understand the TLV, then the relay message will be dropped.
 
---
 
Section 6 should note that the new TLV provides a way for Echo Reply
messages to be diverted so that information can be collected. For
example, if a stack entry can be inserted, the Echo Reply messages can
be caused to transit another AS unrelated to the LSP under test. Since
the Echo Reply reveals path information about the LSP, this is a
valuable attack.  Having said that, you can say how this TLV is 
protected in the Echo Reply message.
[Lizhong] Do you mean the new TLV could be used to collect path information
unrelated to the LSP under test? This is not true. Only the node along the LSP 
will add path information into the new TLV. The relay node in the new TLV
will only relay the Echo Reply to the initiator, and will not add information to
the new TLV.


> -----Original Message-----
> From: mpls [mailto:[e-mail]] On Behalf Of Loa Andersson
> Sent: 27 May 2015 10:21
> To: [e-mail]; [e-mail]; draft-ietf-mpls-lsp-ping-relay-
> [e-mail]; BRUNGARD, DEBORAH A
> Subject: [mpls] MPLS working group last call on
draft-ietf-mpls-lsp-ping-relay-
> reply
> 
> Working Group,
> 
> This is to initiate a two week working group last call on
> draft-ietf-mpls-lsp-ping-relay-reply.
> 
> Please send your comments to the mpls wg mailing list ([e-mail]).
> 
> There are two IPR disclosures against this document. All the authors
> and contributors have stated on the working group mailing list that
> they are not aware of any other IPRs that relates to this draft.
> 
> This working group last call ends June 10, 2015.
> 
> A little bit of background. We requested publication for this draft
> earlier, but very late in the process it was discovered that the
> document needed more work - and it was returned to the working group.
> Document has been updated, and you can find a diff:
> 
> https://www.ietf.org/rfcdiff?url1=draft-ietf-mpls-lsp-ping-relay-reply-
> 05&url2=draft-ietf-mpls-lsp-ping-relay-reply-09
> 
> A discussion on the updates can be found at:
> http://www.ietf.org/proceedings/92/slides/slides-92-mpls-8.pdf
> 
> /Loa
> for the MPLS wg chairs
> --
> 
> 
> Loa Andersson                        email: [e-mail]
> Senior MPLS Expert                          [e-mail]
> Huawei Technologies (consultant)     phone: [phone]
> 
> _______________________________________________
> mpls mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/mpls