Subject: [mpls] AD review of draft-ietf-mpls-entropy-label

From: "Adrian Farrel" <[email-masked]>
Date: Thu, 16 Aug 2012 14:19:53 +0100

Hi,

I have conducted my usual review of this draft before it moves on for
IETF last call and IESG evaluation. The purpose of the review is to 
check that the document is ready to be published as an RFC and to try
to dig out any nits that should be resolved before wider review.

I am very impressed with the readability of this document and thank the
authors for their work so far. There are a number of minor nits and 
small technical questions. I considered leaving these to be handled 
during IETF last call, but I think there are enough of them that we
should get them fixed in a new revision before progressing further.

Please feel free to debate any of the points with me.

Can you please make a new revision, and as soon as I see it I will start
the last call.

Thanks,
Adrian

===

Please fix the following idnits

  -- The draft header indicates that this document updates RFC5036, but the
     abstract doesn't seem to mention this, which it should.

  -- The draft header indicates that this document updates RFC3031, but the
     abstract doesn't seem to mention this, which it should.

  == Unused Reference: 'RFC4364' is defined on line 946, but no explicit
     reference was found in the text

  == Unused Reference: 'RFC4761' is defined on line 957, but no explicit
     reference was found in the text

  == Unused Reference: 'RFC4762' is defined on line 961, but no explicit
     reference was found in the text

---

Need to expand a number of acronyms on first use because they show up 
before Section 1.1
LSR

You need to add the following acronyms to Section 1.1
LSP
PW

---

Section 1.2

s/MPLS is very/MPLS is a very/
s/most notably: IP, PWE3/most notably: IP, PWs/
(in fact, in most cases s/PWE3/PW/)

---

Section 3

   The CoS field of an entropy label can be set
   to any value deemed appropriate.

and

   The CoS field can be set to any value deemed appropriate;
   typically, this will be the value in the label above the ELI in the
   label stack.

The field (formerly the EXP bits) is now known as the TC field per
RFC 5462.

---

Section 3

   This is accomplished by REQUIRING that the label

That is not a 2119 word. Try...

   To accomplish this, it is REQUIRED that the label

---

Somewhere in the document you need to say what would happen if a 
legacy node accidentally received an ELI. I think the answer is easy:
there is no label mapping for the received label, so the packet is
dropped per section 3.18 of RFC 3031. It would be helpful to say this
in Section 4.1.

---

Section 4.2

   1.  On an incoming packet, identify the application to which the
       packet belongs, and thereby pick the fields to input to the load
       balancing function; call the output LB.

I think "LB" is load balancing function. Suggest...

   1.  On an incoming packet, identify the application to which the
       packet belongs, and thereby pick the fields to input to the load
       balancing function (LB); call the output LB.

---

Section 4.2

   4.  If, for the chosen tunnel, Y has not indicated that it can
       process ELs, push <TL> onto the packet.  If Y has indicated that
       it can process ELs for the tunnel, push <TL, ELI, EL> onto the
       packet.  X SHOULD put the same TTL and TC fields for the ELI as
       it does for TL.  The TTL for the EL MUST be zero.  The TC for the
       EL may be any value.

Why is this "SHOULD". Under what conditions MAY this be varied?

---

4.4.  Penultimate Hop LSR

   No change is needed at penultimate hop LSRs.

I'm not clear about this.

Consider a simple tunnel carrying IP.
Before using the entropy label, PHP was a pop-and-go function such that
the whole MPLS header (including BoS) was stripped and only a native IP
packet was forwarded onto the final hop of the LSP.

The implication in what you write is that for when the entropy label is
used, the forwarded packet will contain ELI and EL. Surely you want to
strip them as well? Otherwise a router that did not previously handle
MPLS headers will suddenly need to.

I know that signaling (as described) will enable the egress to say
whether it can handle EL, but I am not sure that helps.

So the rule would be that whenever you pop a label that is immediately 
followed by ELI, you must also pop ELI and EL.

---

From: Kireeti Kompella <[email-masked]>
Date: Thu, 16 Aug 2012 15:35:43 -0700

Hi Adrian,

> to dig out any nits that should be resolved before wider review.

I appreciate that, and appreciate as well your thorough review.

> Please feel free to debate any of the points with me.

See inline.

> Can you please make a new revision, and as soon as I see it I will start

I agree with most of what you say, and will produce a new version incorporating them, with the tweaks mentioned below.  I'll post this Monday Aug 20.  If before then I get wording or a reference for behavior for ununderstood reserved labels, I'll include that as well.  If anyone has an alternative to the way I propose dealing with your comments re sect 4.4, please speak up!

> Please fix the following idnits

<snipped>

Will do.

> Section 3

Okay to all of the above.

> dropped per section 3.18 of RFC 3031. It would be helpful to say this

Good point.

Since the ELI is (will be) a reserved label, the handling of that should be well-defined.  However, I don't see it; section 2.1 of 3032 sets some labels apart as reserved, gives meaning to a few of them, and remains mum about the rest.  Is there an RFC that describes the desired/expected behavior on receiving an ununderstood reserved label?  That would be perfect for this purpose.

If not, I will make up some words.

> Section 4.2

Will rewrite for clarity.

> Why is this "SHOULD". Under what conditions MAY this be varied?

This relates to your point below about PHP.  If the tunnel is PHP-ed, the ELI serves as a proxy for the tunnel label, at least insofar as the TTL and CoS^h^h^h TC bits go.  Hence the SHOULD.  Different choices of TTL and TC may lead to unexpected behavior on the egress.  However, if PHP is not used (or if as you suggest the PHP LSR strips ELI+EL), it might not matter, but the lack of warmth and fuzziness distresses me.  I'm open to alternate opinions, though.

> packet was forwarded onto the final hop of the LSP.

Good point.

> MPLS headers will suddenly need to.

So far, ELs affect ingress and egress LSRs, which is nice.  I'll be happy to add this as a MAY in this section.  Also mumble something about load balancing on the last hop.

> whether it can handle EL, but I am not sure that helps.

I'll also remind folks (in section 5) that an LSR signaling EL capability had better be prepared to pop ELI+EL.

Thanks again for the detailed review!

Kireeti.

---

From: Kireeti Kompella <[email-masked]>
Date: Thu, 16 Aug 2012 15:49:53 -0700

>     abstract doesn't seem to mention this, which it should.

While on the subject, should this document state that it updates 5420 (LSP attrs) and labeled BGP (3107)?  (If not, should this document NOT state that it updates 5036?)

If I add mumble about dealing with ELI, can that be construed as updating 3032?

I don't know the accepted practices regarding the "updates <>" attributes, so just say the word.

Kireeti.

---

From: "Adrian Farrel" <[email-masked]>
Date: Fri, 17 Aug 2012 07:28:38 +0100

Well, you would ask, wouldn't you?

The IESG is currently going through a debate about the correct meaning of
"updates".

The currently in force meaning is found in RFC 2223 although there is an
rfc2223bis draft. 
RFC 2223 states
   Updates
      To be used as a reference from a new item that cannot be used
      alone (i.e., one that supplements a previous document), to refer
      to the previous document.  The newer publication is a part that
      will supplement or be added on to the existing document; e.g., an
      addendum, or separate, extra information that is to be added to
      the original document.
rfc2223bis states
         Updates
            Specifies an earlier document whose contents are modified or
            augmented by the new document.  The new document cannot be
            used alone, it can only be used in conjunction with the
            earlier document.

The IESG thinks:
- these definitions suck
- "updates" has not been used consistently in the past
- it doesn't (yet) have consensus on a better meaning.

Several ADs have been trying to pursue a more limited view of "updates" as
follows...
      RFC Y updates RFC X if and only if new implementations of RFC X will
      be expected to also implement RFC Y.
This allows for the following to be classed as updates:
- clarifications and interpretations
- bug fixes
- resolved omissions
- mandatory extensions
- optional extensions
But the last of these sits a little on the fence because it is unclear to say
what it means to support an optional extension by opting to not implement it :-)
However, I think a view of this is to ask, suppose you made a "full function"
implementation of RFC X: would you also implement RFC Y?

Now, to your specific case...
As I understand it, the intention is that entropy label becomes a core feature
of MPLS networks. We hope that going forward, all implementations will support
EL. That means that the signaling protocols will also need to support EL.
So I am comfortable with updates for 3031 and 5036. 
I thought about 3032, but had a look and didn't think it is updated.
Yes, you could include 3209 and 3017.
I don't believe this updates 5420 - it defines a new flag, but makes no change
to LSP Attributes processing.

Clear?
I hope no clearer for you than for me!

Adrian

> -----Original Message-----
> From: Kireeti Kompella [mailto:[email-masked]]
> Sent: 16 August 2012 23:50
> To: [email-masked]
> Cc: Kireeti Kompella; [email-masked];
[email-masked]
> Subject: Re: AD review of draft-ietf-mpls-entropy-label
> 
> While on the subject, should this document state that it updates 5420 (LSP
attrs)
> and labeled BGP (3107)?  (If not, should this document NOT state that it
updates
> 5036?)
> I don't know the accepted practices regarding the "updates <>" attributes, so
just
> say the word.

---

From: "Adrian Farrel" <[email-masked]>
Date: Fri, 17 Aug 2012 07:43:14 +0100

Hi Kireeti,

All looks good. See in snipped-line.

A

> Since the ELI is (will be) a reserved label, the handling of that should be
well-
> defined.  However, I don't see it; section 2.1 of 3032 sets some labels apart
as
> reserved, gives meaning to a few of them, and remains mum about the rest.  Is
> there an RFC that describes the desired/expected behavior on receiving an
> ununderstood reserved label?  That would be perfect for this purpose.
> 

I think I already gave the reference for processing unknown reserved labels. The
processing is the same as for any unknown label. That is in section 3.18 of RFC
3031.

> This relates to your point below about PHP.  If the tunnel is PHP-ed, the ELI
> serves as a proxy for the tunnel label, at least insofar as the TTL and
CoS^h^h^h
> TC bits go.  Hence the SHOULD.  Different choices of TTL and TC may lead to
> unexpected behavior on the egress.  However, if PHP is not used (or if as you
> suggest the PHP LSR strips ELI+EL), it might not matter, but the lack of
warmth
> and fuzziness distresses me.  I'm open to alternate opinions, though.

So...
OLD
   X SHOULD put the same TTL and TC fields for the ELI as
   it does for TL.
NEW
   X SHOULD put the same TTL and TC fields for the ELI as
   it does for TL to protect LSP behavior in cases where PHP is used
   and the ELI and EL are not stripped at the previous hop (see Section
   4.4). If it is known that PHP is not used or that PHP will strip the ELI
   and EL, the TTL and TC of the ELI MAY be set differently.
END

***or***

s/SHOULD/MUST/

Cheers,
Adrian

---

From: Kireeti Kompella <[email-masked]>
Date: Fri, 17 Aug 2012 10:48:48 -0700

Hi Adrian,

Yes, I had to ask.  I'm sorry I did :-)

Don't really want to get into the debate, but has anyone considered:

     RFC Y updates RFC X iff the content of Y will be incorporated into X if
     X is ever republished, e.g., promoted (PS to Draft Standard to Full S).

?  (Don't answer that.  I will NOT be sucked into this debate.)

> to LSP Attributes processing.

Clear enough.

I will add 3107 and 3209, leave out 5420 and 3032.

Thanks for your help!

Kireeti.

---

From: Kireeti Kompella <[email-masked]>
Date: Fri, 17 Aug 2012 11:01:42 -0700

>   and EL, the TTL and TC of the ELI MAY be set differently.

X will likely be blissfully unaware of whether PHP will be used (and whether the PHP LSR will pop ELI+EL), unless this is a very short LSP.

> s/SHOULD/MUST/

I have a pathological fear of MUST.

How about:

NEW
  X SHOULD put the same TTL and TC fields for the ELI as
  it does for TL.  This protects LSP behavior in cases where PHP is used
  and the ELI and EL are not stripped at the penultimate hop (see Section
  4.4).
END

I'd better get out a new version before the list of things to do gets any longer.

Kireeti.

---

From: "Adrian Farrel" <[email-masked]>
Date: Sat, 18 Aug 2012 18:18:26 +0100

Yeah, and it is great that you give a reason for doing it, but by using SHOULD
you have to explain why you might vary. That was what my MAY sentence was trying
to do. If I got the reason wrong then sorry, please add your own reason. But if
your reason is, because there might turn out to be a reason one day in the
future, then use MUST and write a new I-D sometime in the future.

A

> Sent: 17 August 2012 19:02
> To: [email-masked]
[email-masked]
> X will likely be blissfully unaware of whether PHP will be used (and whether
the
> PHP LSR will pop ELI+EL), unless this is a very short LSP.
> 
> > ***or***
> >
> > s/SHOULD/MUST/
> 
> Kireeti.=

---

From: Kireeti Kompella <[email-masked]>
Date: Mon, 20 Aug 2012 11:29:34 -0700

On Aug 18, 2012, at 10:18 , Adrian Farrel wrote:

> to do. If I got the reason wrong then sorry, please add your own reason.

Okay.  Proposed text:

   X MAY choose different values for the TTL and TC fields if it is
   known that the ELI will not be exposed as the top label at any point
   along the LSP.

If this is acceptable, can this wait, or should we spin an -06?

Kireeti.

> But if

---

From: Kireeti Kompella <[email-masked]>
Date: Mon, 20 Aug 2012 11:37:38 -0700

While we're on the subject, I'd like to make another change -- a clarification.

section 4.3:

OLD:

   If a transit LSR recognizes the ELI, it MAY choose to load balance
   solely on the following label (the EL); otherwise, it SHOULD use as
   much of the whole label stack as feasible as keys for the load
   balancing function, with the exception that reserved labels MUST NOT
   be used.

NEW:

   If a transit LSR recognizes the ELI, it MAY choose to load balance
   solely on the following label (the EL); otherwise, it SHOULD use as
   much of the whole label stack as feasible as keys for the load
   balancing function.  In any case, reserved labels MUST NOT be used
   as keys for the load balancing function.

Kireeti.

PS: there, you see, I do (sometimes) use MUST/MUST NOT.

---

From: "Adrian Farrel" <[email-masked]>
Date: Mon, 20 Aug 2012 19:40:21 +0100

Hi,

Take it as an IETF last call comment (since I already started IETF last call).

Thanks,
Adrian

> Sent: 20 August 2012 19:30
> To: [email-masked]
[email-masked]

---

From: "Adrian Farrel" <[email-masked]>
Date: Mon, 20 Aug 2012 19:41:57 +0100

Yes, I like that.

Store it up as another last call comment.

A
(PS, thanks for keeping these comments public to the MPLS WG. That really helps)

> Sent: 20 August 2012 19:38
> To: [email-masked]
[email-masked]

---

From: John E Drake <[email-masked]>
Date: Mon, 20 Aug 2012 11:55:46 -0700

Kireeti,

In the last sentence of your proposed text, I think it would be clearer if we changed "In any case" to "Irregardless", which, according to Webster's Third New International, is a blend of 'irrespective' and 'regardless'.

Thanks,

John

Sent from my iPhone

> From: Kireeti Kompella
> Sent: Monday, August 20, 2012 11:38 AM
> To: [email-masked]

---

From: Kireeti Kompella <[email-masked]>
Date: Mon, 27 Aug 2012 11:36:02 -0700

Hi Group,

In consolidating text on the setting of TTL, TC and BoS, I noticed that the current text says that the TTL of an EL SHOULD be zero.  I think this is a case for MUST (to prevent accidental forwarding based on an EL), and will reflect that in the next revision.

If there are any objections to a MUST instead of SHOULD, please reply!

Thanks,
Kireeti.