Subject: [mpls] Comments on draft-ietf-mpls-ldp-p2mp

From: Thomas Morin <[email-masked]>
Date: Wed, 15 Sep 2010 14:19:43 +0200

Hello,

Generally, I think that draft-ietf-mpls-ldp-p2mp is in a pretty nice 
shape, and it would be great to now see it progress.

Without waiting for a last call on the draft, I would like to point out 
a few details that would be good to improve.

--------  On make before break procedures --------

1. The make before break procedures could be made a little more explicit 
and indicate that a router moves from state (MBB notif has been 
received) to state 2 (waiting for MBB notification) when the current 
upstream router for the FEC goes down. It could also write it explicitly 
that another case for router X to be in state 3 is when it is the root 
for the FEC.

2. When a router Z receives a Label Map from a router T that happens to 
be Z's upstream, Z must wait before sending an MBB notification to T.

     More precisely Section 2.4.1.4 in the base procedures was modified 
to address this case of 1-hop routing loop:

    [...] a transit node Z receives a P2MP Label Map <X, Y, L> from LSR
    T.  [...] Z determines its upstream LSR U for <X, Y>.  Using this
    Label Map to update the label forwarding table MUST NOT be done as
    long as LSR T is equal to LSR U.

     The MBB procedures need something equivalent.

     I would propose to add the following at the end of section 9.4.4:

    If router Z upstream LSR for <X,Y> happens to be N2, then Z MUST not
    send an MBB notification to N2 at once. Sending the MBB notification
    to N2 must be done only after Z upstream for <X,Y> stops being N2.

3. (if I'm correct) the modification above makes the MBB procedures 
robust to 1-hop transient unicast routing loops (a routing loop between 
two neighbor LSRs is easy to detect, and in this case no MBB 
notification will be sent before the loop is resolved), but it still 
does not offer any kind of make-before-break in cases where a topology 
or metric change causes a more-than-1-hop transient routing loop.  I 
think the draft's should be clear about this and not create expectations 
that it does not actually meet. I would propose to add the following to 
section 9.4, for instance as a short new sub-section:

    MBB and unicast routing loops

    The procedures in this section offer a make-before-break behavior,
    except in cases where a topology or metric change causes a transient
    routing loop involving more than 2 LSRs (also see Section 5 about this).

-------- Upstream LSR change --------

4. Section 2.4.3 currently says "Installing the forwarding state for L' 
MUST NOT be done before the forwarding state L is removed to avoid 
microloops.", but in fact doing so does not "avoid microloops", but just 
limits and reduce their effect. The document should instead say 
something like "to minimize the effects of transient microloops, by 
avoiding new packets to be forwarded into an existing microloop".

-------- Editorial --------

5. "more then" instead of "more than" in 3 places (2 in section 5, one 
in section 7)

------------

Thank you,

-Thomas

---

From: IJsbrand Wijnands <[email-masked]>
Date: Fri, 24 Sep 2010 12:04:02 +0200

Hi Thomas,

> Generally, I think that draft-ietf-mpls-ldp-p2mp is in a pretty nice shape, and it would be great to now see it progress.
> 
> Without waiting for a last call on the draft, I would like to point out a few details that would be good to improve.

Thanks for the comments, see inline.

> 1. The make before break procedures could be made a little more explicit and indicate that a router moves from state (MBB notif has been received) to state 2 (waiting for MBB notification) when the current upstream router for the FEC goes down. It could also write it explicitly that another case for router X to be in state 3 is when it is the root for the FEC.

Ok, I've changed the text to address it.

> 
> 2. When a router Z receives a Label Map from a router T that happens to be Z's upstream, Z must wait before sending an MBB notification to T.
> 
>    More precisely Section 2.4.1.4 in the base procedures was modified to address this case of 1-hop routing loop:
> 

Good point, I've included your text.

> 
> 3. (if I'm correct) the modification above makes the MBB procedures robust to 1-hop transient unicast routing loops (a routing loop between two neighbor LSRs is easy to detect, and in this case no MBB notification will be sent before the loop is resolved), but it still does not offer any kind of make-before-break in cases where a topology or metric change causes a more-than-1-hop transient routing loop.  I think the draft's should be clear about this and not create expectations that it does not actually meet. I would propose to add the following to section 9.4, for instance as a short new sub-section:
> 

In the MBB text I'm not setting any expectations regarding a solution for routing loops. The MBB procedure is a way to influence the timing when you switch from one upstream LSP to an other to avoid traffic loss. After the decision has been made to switch upstream LSP, the normal mLDP procedures apply. So I think section 5 addresses the above good enough. 

> 4. Section 2.4.3 currently says "Installing the forwarding state for L' MUST NOT be done before the forwarding state L is removed to avoid microloops.", but in fact doing so does not "avoid microloops", but just limits and reduce their effect. The document should instead say something like "to minimize the effects of transient microloops, by avoiding new packets to be forwarded into an existing microloop".

That is a good point. Thinking more about this, changing your upstream peer does not contribute to transient micro-loops if the outgoing paths don't change. The order in which you add/delete the local labels will contribute to either traffic loss or duplication. I propose to remove this sentence and leave it up to the implementation to choose the order of programming.

> 5. "more then" instead of "more than" in 3 places (2 in section 5, one in section 7)
> 

Done.

Thanks,

Ice.

---

From: Eric Gray <[email-masked]>
Date: Fri, 24 Sep 2010 11:11:18 -0400

Folks,

	The WG chair has sent out an E-Mail with two MS Word documents
in it.  It is about 250K.

	Because it contains the author's response to WG comments, I did 
not see a choice, and approved the message.

	You will be able to recognize this message by the subject:

		"wg last call reslution in on mpls-tp-oam framework"

	If you have limited bandwidth, you may want to wait to download 
this mail message.

	Apologies...
--
Eric

---

From: Thomas Morin <[email-masked]>
Date: Mon, 27 Sep 2010 17:44:04 +0200

Hi Ice,

IJsbrand Wijnands a écrit :
>>    [...] a transit node Z receives a P2MP Label Map<X, Y, L>  from LSR
>>    T.  [...] Z determines its upstream LSR U for<X, Y>.  Using this
>>    If router Z upstream LSR for<X,Y>  happens to be N2, then Z MUST not
>>    to N2 must be done only after Z upstream for<X,Y>  stops being N2.

Thanks.

> In the MBB text I'm not setting any expectations regarding a solution for routing loops. The MBB procedure is a way to influence the timing when you switch from one upstream LSP to an other to avoi…

I disagree, because the "make before break" term itself carries quite 
high expectations : that the current path will not be broken until the 
new one is ready.   And the procedures don't actually guarantee this 
under some conditions, because if a loop of more than one-hop is formed 
on the new path, then the LSR downstream to this loop will see a service 
disruption because the current upstream will be cut, before the new path 
has actually become fully ready (the case where a loop includes the node 
which is updating its next-hop, resulting in an MBB notification 
travelling from this node to itself, letting him believe that the new 
path is ready).

Given the name "make before break" the reader could be be lead to 
believe this is robust to loops, and that the new path is used only 
after loops on it have been resolved.  So, the cases where the 
procedures don't ensure "make before break" should be documented.

> That is a good point. Thinking more about this, changing your upstream peer does not contribute to transient micro-loops if the outgoing paths don't change. The order in which you add/delete the lo…

I disagree: changing the upstream on an LSR (say LSR X) *can* contribute 
to forming a loop ; this is the case if the new upstream (call it Y) 
happens to be a node whose upstream path, after going through a few 
nodes, actually ends up X (Y is "downstream" X), and thus the order in 
which labels are updated *does* matter a lot:
   * adding the label for the new upstream before removing the label for 
the old upstream, will result in the local node to possibly accept 
traffic simultaneously from two upstream routers : this allows dataplane 
loop with an injection point, thus with a (possibly very fast) increase 
of traffic, which traffic will go to all nodes downstream => harmful
   * removing the label for the old upstream before adding the label for 
the new upstream prevent having a dataplane loop with an injection point 
: a simple loop with a limited number of packets, which will be 
TTL-killed in a reasonable amount of time => reasonably harmless

Thanks,

-Thomas

---

From: IJsbrand Wijnands <[email-masked]>
Date: Tue, 28 Sep 2010 09:08:29 +0200

Hi Thomas,

> I disagree, because the "make before break" term itself carries quite high expectations : that the current path will not be broken until the new one is ready.   And the procedures don't actually guarantee this under some conditions, because if a loop of more than one-hop is formed on the new path, then the LSR downstream to this loop will see a service disruption because the current upstream will be cut, before the new path has actually become fully ready (the case where a loop includes the node which is updating its next-hop, resulting in an MBB notification travelling from this node to itself, letting him believe that the new path is ready).
> 
> Given the name "make before break" the reader could be be lead to believe this is robust to loops, and that the new path is used only after loops on it have been resolved.  So, the cases where the procedures don't ensure "make before break" should be documented.

Make before break guarantees that the control plane state is built for the new tree before the old tree is removed. The primary reason to do this is too avoid packet loss. If the new tree happens to be part of a multi-hop micro loop, then it will forward duplicate traffic down. The MBB procedure still did the right thing, that is that there is no packet loss ;-) If mLDP will not be in a micro-loop, MBB procedure will also not be or contribute to. That is why I think section 5 is good enough. But, if you really think that people will confuse MBB for a micro loop prevention method I can state that in the text.

> I disagree: changing the upstream on an LSR (say LSR X) *can* contribute to forming a loop ; this is the case if the new upstream (call it Y) happens to be a node whose upstream path, after going through a few nodes, actually ends up X (Y is "downstream" X), and thus the order in which labels are updated *does* matter a lot:
>  * adding the label for the new upstream before removing the label for the old upstream, will result in the local node to possibly accept traffic simultaneously from two upstream routers : this allows dataplane loop with an injection point, thus with a (possibly very fast) increase of traffic, which traffic will go to all nodes downstream => harmful
>  * removing the label for the old upstream before adding the label for the new upstream prevent having a dataplane loop with an injection point : a simple loop with a limited number of packets, which will be TTL-killed in a reasonable amount of time => reasonably harmless

I should have chosen my wording more careful. You right that if Y is accepting packets that will come from X a loop is formed. But that is not the point I wanted to make. 

The effect of what you describe above is depending in how quickly a router can swap the local label from X to Y. Suppose that the router HW can switch with only a few packets either being drops or duplicates, there are 2 choices;

1. Deterministically choose for packet loss to first remove the old label and then add the new label. This is done to avoid contribution to a potentially existing Multi-hop micro loop.

2. Deterministically choose for packet duplication to first add the new label before removing the old. This may contribute to an existing Multi-hop routing loop, but you consider the effect to be minimal to the network.

Choosing either 1 or 2 depends on 3 factors:

1. Router HW
2. Network resilience to multi-hop micro loops.
3. Customer preference.

For that reason I don't want to mandate a particular order in the mLDP draft and let the implementor decide.

Thx,

Ice.

---

From: Thomas Morin <[email-masked]>
Date: Tue, 28 Sep 2010 11:35:55 +0200

Hi Ice,

IJsbrand Wijnands :
> Make before break guarantees that the control plane state is built for the new tree before the old tree is removed.

Well, the "make before break" term does imply such a promise, but 
procedures called "make before break" in the document don't offer this 
guarantee, (because there may be a control plane loop).

> The primary reason to do this is too avoid packet loss. If the new tree happens to be part of a multi-hop micro loop, then it will forward duplicate traffic down. The MBB procedure still did the right thing, that is that there is no packet loss ;-)

This is an original definition of packet loss... ;-)
When a loop forms, you would for some time receive duplicated packets 
instead of the expected traffic (this is packet loss), and when all 
packets in the loops are TTL-killed your don't receive anything until 
the loop is resolved (this is packet loss too).

But anyways, the intent behind "make before break" is not only "no 
packet loss", but "nothing breaks", and having looped/duplicated packets 
means something was broken.

>   If mLDP will not be in a micro-loop, MBB procedure will also not be or contribute to. That is why I think section 5 is good enough. But, if you really think that people will confuse MBB for a micro loop prevention method I can state that in the text.

Yes, I think it is important to document that the "make before break" 
effect is limited in the presence of loops.

Something like:

    As documented in section x.y.z mLDP is not exempt from microloop
    issues.  The MBB procedures documented here are no exception, and
    may not offer a make before break in the cases where a topology or
    metric change causes a transient routing loop involving more than 2
    LSRs (also see Section 5 about this).

> 2. Deterministically choose for packet duplication to first add the new label before removing the old. This may contribute to an existing Multi-hop routing loop, but you consider the effect to be m…

Ok, I see your point. You are correct that the current version only 
documents one way of avoiding "issues".
That another path exist is not a reason to stop documenting anything 
though, and we also must keep in mind that not all issues are equal: 
packet loss may only causes harm to one application, while a dataplane 
loop on a P2MP LSP may create a peak amount of traffic on a large amount 
of links an have a wider impact.

> For that reason I don't want to mandate a particular order in the mLDP draft and let the implementor decide.

The issue with "let the implementor decide" is that the implementor may 
not known in advance what the network conditions and customer preference 
are.

Here is what I would propose:

    If during some time, an LSR simultaneously forwards packets from two
    upstream LSRs for a said FEC, the possibility exists that these
    packets increase the amount of traffic in a dataplane loop of more
    than one hop. This side effect is harmful if it occurs for an
    extended amount of time, and even for shorter amount of times, may
    be undesirable. To fully avoid this, it is RECOMMENDED to not
    install the forwarding state for L' before the forwarding state L is
    removed.

    But, symmetrically, if during some time, an LSR does not forwards
    packets nor from the previous upstream LSR or the new upstream LSR,
    then some packets may be lost. If this is undesirable, and it is
    known at deployment time that the effects of transient unicast
    routing loops are minimal, or that the network is designed to not
    have any, then an LSR MAY operate in a mode where the forwarding
    state for L' is installed before the forwarding state L is removed.

-Thomas

---

From: IJsbrand Wijnands <[email-masked]>
Date: Tue, 28 Sep 2010 17:23:53 +0200

Hi Thomas,

> The issue with "let the implementor decide" is that the implementor may not known in advance what the network conditions and customer preference are.
> 

I think both options are equally valid, so I would not make one RECOMMENDED and the other MAY. I actually think that in most deployments it will be the other way around.

Thx,

Ice.

---

From: Thomas Morin <[email-masked]>
Date: Wed, 29 Sep 2010 10:39:41 +0200

Hi Ice,

IJsbrand Wijnands a écrit :
> I think both options are equally valid, so I would not make one RECOMMENDED and the other MAY.
>   I actually think that in most deployments it will be the other way around.

If we don't want to constraint deployment time choices, shouldn't we 
ensure that an implementation will be able to do both ?
With that in mind, I would suggest to at least RECOMMEND that an 
implementation be able to do both.
Then, based on the arguments you put forward, it looks reasonable to not 
recommend anything particular for deployers.

-Thomas