Subject: Re: [Gen-art] Gen-ART LC review of draft-ietf-mpls-p2mp-lsp-ping-16

From: "Adrian Farrel" <[email-masked]>
Date: Thu, 11 Aug 2011 15:41:04 +0100

Alexey, hello.

Thanks for the review.

In line.

Adrian

> Minor issues:
> 
>  1.1. Design Considerations
> 
>    As is described in [RFC4379], to avoid potential Denial of Service
>    attacks, it is RECOMMENDED to regulate the LSP Ping traffic passed to
>    the control plane.  A rate limiter should be applied to the
>    well-known UDP port defined for use by LSP Ping traffic.
> 
> What is this port? Is mentioning of the port significant?

It is well-known, so you obviously don't need to ask :-)

The port number is in the IANA registry and can be seen in RFC4379 (the base
definition of LSP Ping) sections 4.3, 4.4, 6, and 7.

The port number is 3503

I don't think it needs to be mentioned here. In general, restating definitions
causes potential conflicts. It is far better to have a normative reference to
RFC 4379 for all elements of LSP Ping that are not changed by this document.

> 3.1.2.1. Multicast LDP FEC Stack Sub-TLVs
> 
>    Address Family
> 
>       Two octet quantity containing a value from ADDRESS FAMILY NUMBERS
>       in [IANA-PORT] that encodes the address family for the Root LSR
>       Address.
> 
>    [IANA-PORT] IANA Assigned Port Numbers, http://www.iana.org
> 
> Which IANA registry do you have in mind? Seeing a link would be helpful.

Yes. This is a typo.
We fixed it for you in -17

> 3.2. Limiting the Scope of Responses
> 
>    The P2MP Responder Identifier TLV only has meaning on an echo request
>    message.  If present on an echo response message, it SHOULD be
>    ignored.
> 
> Are there known reasons for violating the SHOULD? I.e. what are the reasons
> for having multiple sub-TLVs in the first place?

Future extensibility.
There is a corner of the world that builds conformance testers and would
interpret a MUST statement here as preventing any extensions that used this TLV
on an echo response as being in violation of this spec. Therefore we would have
to respin this spec if/when we made such an extension.
Furthermore, it is harmless to this protocol to not ignore such a TLV.

Anyway, in response to Dave Harrington's Discuss, we will be re-examining all
uses of "SHOULD" with a view to tightening them or supplying an appropriate
"MAY" clause.

> 3.2.2. Node Address P2MP Responder Identifier Sub-TLVs
> 
>    The address in this Sub-TLV SHOULD be of any transit, branch, bud or
>    egress node for that P2MP LSP.
> 
> Is the use of SHOULD correct here (instead of a MUST)? Are there any choices
> left if the SHOULD is violated?

The MAY clause can be added.

> 3.5. Downstream Detailed Mapping TLV
> 
>   Downstream Detailed Mapping TLV is described in [DDMT].  A transit,
>   branch or bud node can use the Downstream Detailed Mapping TLV to
>   return multiple Return Codes for different downstream paths. This
>   functionality can not be achieved via the Downstream Mapping TLV.
> 
> Are "Downstream Detailed Mapping TLV" and "Downstream Mapping TLV" two
> different things?
> 
>   As
>   per Section 4.3 of [DDMT], the Downstream Mapping TLV as described in
>   [RFC4379] is being deprecated. 

They are different precisely per the referenced text.

But well done! You caught a bug :-)

s/4.3/3.4/

> 4.1.2. Jittered Responses to Echo Requests
> 
>    Echo response jittering SHOULD be used for P2MP LSPs.  If the Echo
>    Jitter TLV is present in an echo request for any other type of LSPs,
>    the responding egress MAY apply the jitter behavior as described
>    here.
> 
> Can you provide a bit more information about how this work?

Obviously we authors think there is nothing more to say (or we would have said
it).

Maybe you can ask a more specific question?

If a jitter TLV shows up on an echo request for (e.g.) a P2P LSP, the egress may
delay sending the echo response in exactly the same way as the leaf of a P2MP
LSP would as described here.

> 4.2.1.1. Responses from Transit and Branch Nodes
> 
>    The presence of a Downstream Detailed Mapping TLV will influence the
>    choice of Return Code.  As per [DDMT], the Return Code in the echo
>    response header MAY be set to value TBD ('See DDM TLV for Return Code
> 
> Am I correct that the value TBD is specified in [DDMT]?

You are correct as in the very next line that you cut off with your question.

> If not, it is missing in the IANA Considerations section.
> 
>    and Return SubCode') as defined in [DDMT].  The Return Code for each
>    Downstream Detailed Mapping TLV will depend on the downstream path as
>    described in [DDMT].
> 
> 6. OAM and Management Considerations
> 
>       - A MIB module is required for the control and management of LSP
>         Ping operations, and to enable the reported information to be
>         inspected.
> 
> I think it would be better to be explicit that this document doesn't
> define such a MIB.

This section rewritten in -17

> 7.2. New TLVs
> 
>      P2MP Responder Identifier TLV (see Section 3.2) is a mandatory
> 
> What does "mandatory" means in this section? Mandatory for IANA?

No. There are two TLV types. Mandatory TLVs and, erm, not mandatory ones. The
registry is split accordingly.

>      TLV.  Suggested value 11.
>      Four sub-TLVs are defined.
>        - Type 1: IPv4 Egress Address P2MP Responder Identifier
>        - Type 2: IPv6 Egress Address P2MP Responder Identifier
>        - Type 3: IPv4 Node Address P2MP Responder Identifier
>        - Type 4: IPv6 Node Address P2MP Responder Identifier
> 
>      Echo Jitter TLV (see Section 3.3) is a mandatory TLV.  Suggested
> 
> As above.

Also as above.

>      value 12.
> 
> 
> Nits/editorial comments:
> 
> 4.3.1. End of Processing for Traceroutes
> 
>    For P2MP TE LSP, the initiating LSR has a priori knowledge about
>    number of egress nodes and their addresses.  Hence it possible to
> 
> Missing "is" after "it".

yes

>    continue processing till a valid response has been received from each
>    end-point, provided the responses can be matched correctly to the
>    egress nodes.

---

From: Alexey Melnikov <[email-masked]>
Date: Thu, 11 Aug 2011 15:51:29 +0100

>RFC 4379 for all elements of LSP Ping that are not changed by this document.
Ok.

>We fixed it for you in -17
Yes, I've noticed that.

>Furthermore, it is harmless to this protocol to not ignore such a TLV.
Ok. Being more explicit about this in the draft would be better, of course.

>The MAY clause can be added.
I think you've missed my point. If the list of node types is complete, 
then there is no need to use a SHOULD (because it is a MUST and that 
MUST doesn't need to be stated). Are there other types of nodes?

>LSP would as described here.
Ok.

>You are correct as in the very next line that you cut off with your question.
I think it would be a good idea to insert a note for RFC Editor here 
asking to fix TBD when [DDMT] is published. That wasn't very clear to me.
 [...]

>registry is split accordingly.
Ah, Ok.

---

From: "Adrian Farrel" <[email-masked]>
Date: Thu, 11 Aug 2011 16:08:44 +0100

[snippety-snack]

> MUST doesn't need to be stated). Are there other types of nodes? 

Oh, my! Yes, I missed your point.

Yes, indeed, there are other types of nodes.

If you take just a small step back you'll see that there are more nodes in the
Internet than are present as participating nodes in just one P2MP LSP :-)

There might be more than three use cases, but, for example:
- ingress node (self-pinging leads to blindness?)
- node that you want to check is really not on the path
- node you know is not on the path because you want to check
   everyone else is functioning correctly (i.e., not responding)

So perhaps we need to add the MAY clause.

> asking to fix TBD when [DDMT] is published. That wasn't very clear to me.

I think the RFC Editor searches for "TBD". The two documents are lock-stepped in
the process and so on. But we can ask the AD to add an RFC Editor note.

Thanks,
Adrian