Subject: Re: [mpls] AD review of draft-ietf-mpls-ldp-p2mp

From: IJsbrand Wijnands <[email-masked]>
Date: Tue, 14 Jun 2011 15:08:34 +0200

Hi Adrian,

Thanks for the detailed review, see comments inline.

> ---
> 
> As noted in the write-up, RFC 4664 is an unused reference. I think it can simply
> be removed unless it was intended to be added to the final paragraph of Section
> 1.

Ice: ok.

> 
> ---
> 
> Abstract
> 
> Some consistency in hyphenation, please. I think you need to adopt
> "point-to-multipoint" and "multipoint-to-multipoint" as in the title.

Ice: Ok.

> 
> ---
> 
> Acronyms
> Need to expand them on first use in the main text (Abstract doesn't  count). I
> find:
> 
> (LDP is a permitted acronym - no action needed)
> LSP
> LSR

Ice: Ok

> 
> ---
> 
> Please s/draft/document/ in the text so that when it is published as an RFC the
> text is accurate.

Ice: Is this a comment on its own or related to above or below?

> 
> ---
> 
> Section 1
> 
>   Only a single copy of the packet will be sent
>   on any link traversed by the MP LSP (see note at end of
>   Section 2.4.1).
> 
> Is this strictly true in all cases. are there not fringe cases, just as there
> were in P2MP RSVP-TE, where a branch is logically made upstream of a common link
> because of limited branching capabilities at the downstream end of the link?

Ice: With mLDP this would typically not happen, but you do raise a good point. The text is not accurate and there are examples where the same packet may be transmitted over the the same link. For example with running mLDP over Targeted LDP sessions, described in draft-napierala-mpls-targeted-mldp-01. It changed 'link' into 'LDP neighbor', that captures it more accurate.

> 
> ---
> 
> Section 1.2
> 
>   Leaf node:  A Leaf node can be either an Egress or Bud LSR when
>      referred in the context of a P2MP LSP.  In the context of a MP2MP
>      LSP, an LSR is both Ingress and Egress for the same MP2MP LSP and
>      can also be a Bud LSR.
> 
> I think...
> s/an LSR is both Ingress and Egress/a leaf is both Ingress and Egress/

Ice: Ok.

> 
> ---
> 
> Section 2.1
> 
> The figure in this section appears to state that the S-bit must always be set to
> 1. That means that the capability cannot be withdrawn. I have no issue with
> that, but I think that if this is you intention, you must say that the S-bit
> must always be set to 1, the capability cannot be withdrawn, and the processing
> if S=0 is received. OTOH, if you allow withdrawal, you can leave "S" in the
> figure and refer to RFC5561 for the meaning.

Ice: Good catch. There is no intention to make it non withdraw-able. I've changed to S and pointed to RFC5561.

> 
> ---
> 
> Pedantic point I don't propose you address (unless you have more spare than you
> should have)...
> 
> The Opaque Value field is not truly opaque, since we can look into it and find a
> series of opaque elements.

Ice: Ok.

> 
> ---
> 
> Section 2.2
> 
> Can you explain to me why the working group wanted to allow the P2MP FEC to use
> any address family from IANA's "Address Family Numbers" registry to identify the
> root LSR?

Ice: mLDP tree building is based on the reachability of the Root Address. That Root address is very likely to be IPv4 or IPv6, but nothing prevents an implementer to stick in a IPX or CLNS address and route the LSP across the network. As long as the Root address is reachable in each node, it will work. I don't see why we should restrict its use.

> 
> ---
> 
> Section 2.2
> 
>   The combination of (Root Node Address,
>   Opaque Value) uniquely identifies a P2MP LSP within the MPLS network.
> 
> With the current specification as shown in the figure, this is not completely
> accurate since two addresses from different families could have the same root
> node address value.
> 
> So your text should refer to the combination of {Root Node Address type, Root
> Node Address, Opaque Value).

Ice: Good catch, fixed it.

> 
> ---
> 
> 2.3
> 
> OLD
>   Type:  The Type of the LDP MP Opaque Value Element basic type is to
>      be assigned by IANA.
> NEW
>   Type:  The Type of the LDP MP Opaque Value Element. IANA maintains a
>      registry of basic types (see Section 11).

Ice: Ok.

> 
> ---
> 
> 2.3
> 
> We don't generally define empty TLVs and objects.
> 
> Can you convince me of the need for the Extended Type? Are you predicting a
> large number of FCFS opaque values?. You are surely not expecting many standards
> track types. 

Ice: Its always hard to predict these things, since the Type field is only 8 bits, I think it was a good idea to reserve 255 and assigned an extended type to it. Just in case we need it.

> 
> ---
> 
> 2.3
> 
> OLD
>   Extended Type:  The Extended Type of the LDP MP Opaque Value Element
>      extended type is to be assigned by IANA.
> NEW
>   Extended Type:  The Extended Type of the LDP MP Opaque Value Element.
>      IANA maintains a registry of extended types (see Section 11).

Ice: Ok.

> 
> ---
> 
> A point of pedantry, but you repeatedly refer to the "Label Map" message and (of
> course :-) it is really the "Label Mapping" message.

Ice: Agreed, changed it, that reads a lot better.

> 
> ---
> 
> 2.4
> 
>   2.  P2MP Label Map <X, Y, L>: a Label Map message with a FEC TLV with
>       a single P2MP FEC Element <X, Y> and Label TLV with label L.
> 
>       Label L MUST be allocated from the per-platform label space (see
>       [RFC3031] section 3.14) of the LSR sending the Label Map Message.
> 
> While you are at liberty to make this "MUST" requirement with the WGs support,
> it would be good if you gave the reason rather than "sneaking" it in. I assume
> that you want to do this to make FRR easier, but it is not immediately clear why
> P2MP is in any way different from P2P on upstream interfaces. Or maybe it is
> because you want to allow the choice of downstream interface to rest with the
> upstream LSR (per 2.4.1.2).
> 
> Can you add a short clause to say why the per-platform label space is used?

Ice: Correct, the choice of the forwarding interface rests with the upstream LSR and FRR is easier this way. But I would not say that is the reason we choose platform label space, it is mostly that we are just following what is used for unicast. I don't think anything prevents mLDP from working with per interface label space. FRR becomes harder, but that would be the same for unicast.. So I'm sure if we need to say anything more here.

> 
> ---
> 
> 2.4.1
> 
>   The remainder of this section specifies the procedures for
>   originating P2MP Label Map messages and for processing received P2MP
> 
> s/The remainder of this/This/

Ice: Ok.

> 
> ---
> 
> You have two instances (spot the block copy!) of "label map" in lower case.

Ice: Got it.

> 
> ---
> 
> 2.4.1.1
> 
>   A node Z that
>   wants to join a MP LSP <X, Y> determines the LDP peer U which is Z's
>   next-hop on the best path from Z to the root node X. If there is more
>   than one such LDP peer, only one of them is picked.  U is Z's
>   "Upstream LSR" for <X, Y>.      
> 
>   When there are several candidate upstream LSRs, the LSR MAY select
>   one upstream LSR.
> 
> The "MAY" seems to be in contradiction to the previous paragraph that appears to
> say that the LSR MUST select exactly one upstream LSR.

Ice: Agreed, changed to MUST.

> 
> ---
> 
> 2.4.1.1
> 
> CRC32 is not a well-known acronym and would benefit from a reference.

Ice: Done.

> 
> ---
> 
> 2.4.1 and 2.4.1.1
> 
> It seems that it is important to be precise about what is meant by the "opaque
> value" (Y). This is particularly important in 2.4.1.1 because there is an
> interop dependency relying on the input to the hash function. You might mean:
> - the whole opaque value TLV
> - the opaque value field of the opaque value TLV
>  (i.e., the concatenation of the whole opaque value elements)
> - the value field of the opaque value element
> - the concatenation of the value fields from each of the value fields
>  of the opaque value elements
> 
> Please add text to clarify (at least for the CRC32 case).

Ice: Ok, done. Its the Opaque Value as indicated in the FEC element. (this may or may not have multiple Opaque TLV's in it).

> 
> ---
> 
> 2.4.1.4
> 
> Trivially...
> 
>   Assuming its old forwarding state was
>   L'-> {<I1, L1> <I2, L2> ..., <In, Ln>}, its new forwarding state
>   becomes L'-> {<I1, L1> <I2, L2> ..., <In, Ln>, <I, L>}.
> 
> Misses the case that I=Ii for some value of i.

Ice: Don't understand this comment. Can you elaborate?

> 
> ---
> 
> 2.4.2.1
> 
>   If a leaf node Z discovers (by means outside the scope of this
>   document) that it has no downstream neighbors in that LSP, and that
>   it has no need to be an egress LSR for that LSP, then it SHOULD send
> 
> Just some subtle re-wording needed. We *do* know the means by which Z discovers
> it has no downstream neighbors -- that is what this document is about! The
> parentheses apply to the second clause (that it has no need to be an egress).

Ice: Ok.

> 
> ---
> 
> 2.4.2.2.
> 
> In the light of the procedure set out in 2.4.3 and 8., should you recommend that
> propagation of Label Withdraw should be delayed to prevent full LSP teardown
> when there is a small change in the path of the trunk of an LSP? Maybe a fringe
> case we can leave to implementations?

Ice: Delaying the withdraw is only useful if the change is temporarily and likely to recover before the new path is setup. It is hard to detect that. With the procedures in section 8 you sort of get the right behavior for this. But if you delay the withdraw, you delay convergence as well. Nothing in the draft prevents a withdraw to be delayed for certain purposes, I would just leave it up to the implementation to decide and not document in the draft.

> 
> ---
> 
> Section 3
> 
> Many of the nits in section 2 apply here, too.

Ice: Ok, I think I got them.

> 
> ---
> 
> Section 3
> 
> While it is possible that the root of an MP2MP LSP is also a leaf and can act as
> a data source, I think that...
> 
>   An MP2MP LSP is much like a P2MP LSP in that it consists of a single
>   root node, zero or more transit nodes and one or more leaf LSRs
>   acting equally as Ingress or Egress LSR. 
> 
> Should read "two or more leaf LSRs" since the semantic change from P2MP is that
> leaf nodes are ingresses as well as egresses, and it would make no sense to have
> only one.

Ice: Well, you can have a Root node on a stick. This is typically a case where you have a single leaf. 

> 
> ---
> 
> Section 3
> 
> I can't help feeling that some figures would help understanding the path that
> packets are expected to take on an MP2MP LSP since this is key to working out
> what label state is installed.

Ice: It is not that hard. Keep in mind that you build the LSP just like a P2MP to the root. To get the MP2MP behavior you send label mappings starting at the root down the tree following the previous installed LSP branches. You end up with labels in both directions. With that said, the path of the LSP is fixed now.

> 
> ----
> 
> 5.1
> 
> OLD
>   Type:  The type of the LDP MP Status Value Element is to be assigned
>      by IANA.
> NEW
>   Type:  The type of the LDP MP Status Value Element. IANA maintains a
>      registry of status value types (see Section 11).
> 
> 
> And in the figure s/Type(TBD)/Type/

Ice: Done.

> 
> ---
> 
> 5.2.  LDP Messages containing LDP MP Status messages
> 
>   The LDP MP status message may appear either in a label mapping
>   message or a LDP notification message.
> 
> I think s/status message/status TLV/  throughout.

Ice: Ok.

> 
> ---
> 
> 5.2.1
> 
>   An LDP MP status TLV sent in a notification message must be
>   accompanied with a Status TLV.
> 
> This is a statement of fact not a piece of new protocol spec, so you are correct
> to use lower case "must". But you should include a reference at this point.

Ice: Ok.

> 
> ---
> 
> 7.1.  Root node redundancy - procedures for P2MP LSPs
> 
>   Since all leafs have set up P2MP LSPs to all the roots, they are
>   prepared to receive packets on either one of these LSPs.  However,
>   only one of the roots should be forwarding traffic at any given time,
>   for the following reasons: 1) to achieve bandwidth savings in the
>   network and 2) to ensure that the receiving leafs don't receive
>   duplicate packets (since one cannot assume that the receiving leafs     
>   are able to discard duplicates).  How the roots determine which one
>   is the active sender is outside the scope of this document.
> 
> The "should" seems strong since I know of deployed networks where 1+1 style P2MP
> transmission is performed and the application layer is responsible for sorting
> out duplicates that arise on failover. 1+1 is chosen in the face of network cost
> because of the high level of reliability that is wanted.

Ice: In this procedure its mLDP that provides a redundancy where the application layer does not
need to be responsible in filtering the duplicates. There are applications that don't support that behavior.
This redundancy proposal is a generic Network based solution that is hidden to the end customer. For the scenario that you described you probably set up dedicated P2MP LSPs to achieve 1+1. This is normally not a generic service that you apply to all the customer traffic.

> 
> ---
> 
> 8.3
> 
> Same issue with the S-bit apparently always set meaning the capability cannot be
> withdrawn.

Ice: Ok.

> 
> ---
> 
> Section 9
> 
> This is not clear. The figure shows "Type=mLDP". There are two FEC Elements
> defined in the document so I think you need...
> 
> OLD
> 
>       0                   1                   2                   3
>       0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>      | Typed Wcard   | Type = mLDP   |   Len = 2     |      AFI      ~
>      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>      ~               |
>      +-+-+-+-+-+-+-+-+
> 
>   Type Wcard:  As specified in [RFC5918]
> 
> 
>   Type:  mLDP FEC Element Type as documented in this draft.
> 
> NEW
> 
>       0                   1                   2                   3
>       0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>      | Typed Wcard   |     Type      |   Len = 2     |      AFI      ~
>      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>      ~               |
>      +-+-+-+-+-+-+-+-+
> 
>   Type Wcard:  As specified in [RFC5918]
> 
> 
>   Type:  The type of FEC Element Type. Either the P2MP FEC Element 
>        or the MP2MP FEC Element using the values defined for those
>        FEC Elements when carried in the FEC TLV as defined in this
>        document

Ice: Ok.

> 
> ---
> 
> Section 10
> 
> I am a bit disappointed by the Security Section. While it is true that the
> security relationships and techniques between LDP peers are not changed, and the
> imperatives are the same, you have introduced a new type of service that has a
> new attack vector.
> 
> In P2P LSPs, the sender has some control over the receiver, but this is less the
> case in leaf-initiated join to P2MP LSPs. I think this section should at least
> note that there is no way for a root to validate the leafs of the P2MP tree.
> Probably the only security you can offer is that;
> - the leafs all form part of the same trusted network
> - the opaque values could be made unguessably large
> - the opaque values could be distributed in a secure way

Ice: Let me think about this and discuss.

> 
> ---
> 
> Section 11
> 
> Can you please add...
> 
>   The requested code point values listed below have been allocated by
>   IANA through early allocation.
> 
> ...between...
> 
>      The allocation policy for this space is 'Standards Action with
>      Early Allocation'
> 
> ...and...
> 
>   This document requires allocation of three new code points from the
>   IANA managed LDP registry "Forwarding Equivalence Class (FEC) Type
>   Name Space".  The values are:

Ice: Done.

---

From: "Adrian Farrel" <[email-masked]>
Date: Wed, 15 Jun 2011 19:16:23 +0100

Hello Ice,

Thanks for the good progress.

responses in line.

A

> Ice: Is this a comment on its own or related to above or below?

It is a standalone comment.

The text you are writing will (you hope) be published as an RFC. So the text
should not say (e.g.) "this draft will describe" but "this document describes"

> > were in P2MP RSVP-TE, where a branch is logically made upstream of a
> > common link because of limited branching capabilities at the
> > downstream end of the link?
> 
> Ice: With mLDP this would typically not happen, but you do raise a good point.
> The text is not accurate and there are examples where the same packet may be
> transmitted over the the same link. For example with running mLDP over
> Targeted LDP sessions, described in draft-napierala-mpls-targeted-mldp-01. It
> changed 'link' into 'LDP neighbor', that captures it more accurate.

I'm not going to push back any more on this. I don't think the case is as strong
as it would be in a traffic engineered situation. I still think it could happen
that packets are duplicated upstream of the physical branch node.

> > large number of FCFS opaque values?. You are surely not expecting many
> > standards track types.
> 
> Ice: Its always hard to predict these things, since the Type field is only 8
bits, I
> think it was a good idea to reserve 255 and assigned an extended type to it.
Just
> in case we need it.

Hmmm. Mutter, mutter. We can leave it and see whether the IESG complains.

> > While you are at liberty to make this "MUST" requirement with the WGs
> > support, it would be good if you gave the reason rather than "sneaking" 
> > it in. I assume that you want to do this to make FRR easier, but it is not
> > immediately clear why P2MP is in any way different from P2P on
> > upstream interfaces. Or maybe it is because you want to allow the
> > choice of downstream interface to rest with the upstream LSR 
> > (per 2.4.1.2).
> >
> Ice: Correct, the choice of the forwarding interface rests with the upstream
LSR
> and FRR is easier this way. But I would not say that is the reason we choose
> platform label space, it is mostly that we are just following what is used for
> unicast. I don't think anything prevents mLDP from working with per interface
> label space. FRR becomes harder, but that would be the same for unicast.. So
I'm
> sure if we need to say anything more here.

Well, you may be right in your description of most implementations. But, 5036 is
quiet on the subject (implying that either label space is suitable) despite
post-dating 4090.

So, if you want to restrict implementations in this case (for example, suppose I
don't want to support FRR, but am implementing this I-D) then you should give
the reason.

Or if, as you say, "I don't think anything prevents mLDP from working with per
interface label space" then you need to remove this "MUST"

> Ice: Don't understand this comment. Can you elaborate?

Ignore me. I'm thinking of something else!

> > Should read "two or more leaf LSRs" since the semantic change from P2MP
> > is that  leaf nodes are ingresses as well as egresses, and it would make no
> > sense to have only one.
> 
> Ice: Well, you can have a Root node on a stick. This is typically a case where
you
> have a single leaf.

Right, so the single leaf sends data to the root. And then the root sends the
data, erm, where?

But, whatever...

> Ice: It is not that hard. Keep in mind that you build the LSP just like a P2MP
to the
> root. To get the MP2MP behavior you send label mappings starting at the root
> down the tree following the previous installed LSP branches. You end up with
> labels in both directions. With that said, the path of the LSP is fixed now.

Yes. I'm interested in how the data does not get back to the sender.

> > security relationships and techniques between LDP peers are not changed, 
> > and the imperatives are the same, you have introduced a new type of
> > service that has a new attack vector.
> >

OK. Let me know.

---

From: IJsbrand Wijnands <[email-masked]>
Date: Thu, 16 Jun 2011 10:03:00 +0200

Adrian,

> interface label space" then you need to remove this "MUST"

Ice: Lets take the approach to claim Interface label space is out of scope.

> Right, so the single leaf sends data to the root. And then the root sends the

Ice: This is a characteristic of MP2MP LSP. A Leaf may inject packets to the root of the tree but has no knowledge if there would be a receiver behind the root. The root will always receive the traffic, and drop it if there are no receivers. Its the same in PIM Bidir trees.

> Yes. I'm interested in how the data does not get back to the sender.

Ice: Please see section 3.3.1.5;

If it does not, it allocates a label
   Lu' and creates a new label swap for Lu' with Label Lu over interface
   Iu.  Interface Iu is determined via the procedures in
   Section 2.4.1.2.  In addition, it also adds the label swap(s) from
   the forwarding state downstream <X, Y>, omitting the swap on
   interface I for node D. The swap on interface I for node D is omitted
   to prevent packet originated by D to be forwarded back to D.

Thx,

Ice.