Subject: [Idr] I-D Action: draft-ietf-idr-error-handling-09.txt

From: [email-masked]
Date: Mon, 19 May 2014 12:33:51 -0700

A New Internet-Draft is available from the on-line Internet-Drafts directories.
 This draft is a work item of the Inter-Domain Routing Working Group of the IETF.

        Title           : Revised Error Handling for BGP UPDATE Messages
        Authors         : Enke Chen
                          Juniper Networks
                          Pradosh Mohapatra
                          Keyur Patel
	Filename        : draft-ietf-idr-error-handling-09.txt
	Pages           : 14
	Date            : 2014-05-19

Abstract:
   According to the base BGP specification, a BGP speaker that receives
   an UPDATE message containing a malformed attribute is required to
   reset the session over which the offending attribute was received.
   This behavior is undesirable as a session reset would impact not only
   routes with the offending attribute, but also other valid routes
   exchanged over the session.  This document partially revises the
   error handling for UPDATE messages, and provides guidelines for the
   authors of documents defining new attributes.  Finally, it revises
   the error handling procedures for a number of existing attributes.

   This document updates error handling for RFCs 1997, 4271, 4360, 4456,
   4760 and 5701.

The IETF datatracker status page for this draft is:
https://datatracker.ietf.org/doc/draft-ietf-idr-error-handling/

There's also a htmlized version available at:
http://tools.ietf.org/html/draft-ietf-idr-error-handling-09

A diff from the previous version is available at:
http://www.ietf.org/rfcdiff?url2=draft-ietf-idr-error-handling-09

Please note that it may take a couple of minutes from the time of submission
until the htmlized version and diff are available at tools.ietf.org.

Internet-Drafts are also available by anonymous FTP at:
ftp://ftp.ietf.org/internet-drafts/

---

From: "John G. Scudder" <[email-masked]>
Date: Mon, 19 May 2014 15:51:39 -0400

There are just a few relatively small changes in this version:

- Note that when MP-BGP is in use, NEXT_HOP is discretionary.
- Better text for NLRI section (thanks to Ondrej).
- Fix typos.

With luck, we are spiraling in to a final version.

Thanks, all. 

--John

---

From: Deniz Bahadir <[email-masked]>
Date: Tue, 20 May 2014 09:19:11 +0200

I have nothing to complain or to add.
Therefore, I support this draft being the final version and becoming an RFC.

Thanks for the good work,
Deniz

---

From: "Chris Hall" <[email-masked]>
Date: Tue, 20 May 2014 19:39:28 +0100

John G. Scudder wrote (on Mon 19-May-2014 at 20:52 +0100):
...
> With luck, we are spiraling in to a final version.

The requirement to put MP_xxx_NLRI as the first attribute appears to
have gone walk-about ?

So... treat-as-withdraw is to be applied to whatever (valid) NLRI the
receiver manages to find.  If there are lost-NLRI, tant pis, go ahead
and do an "incomplete" treat-as-withdraw.

If no NLRI whatsoever can be found, then the receiver SHALL
treat-as-withdraw an empty set of NLRI.  Yes ?

I accept that the receiver cannot do anything about lost-NLRI, not
least because (in the general case) it cannot tell whether there are
any !

However, unless somebody knows better, it appears that existing
implementations currently:

  * send at most 2 sets of NLRI in an UPDATE

  * never mix MP_xx_NLRI attributes with non-empty
    Withdrawn Routes or NLRI fields.

  * send an MP_UNREACH_NLRI as the first (and
    possibly only) attribute.

This does not change what the receiver can do...

... but given that the likelihood of lost-NLRI is (in practice)
effectively zero, one can ignore the impact of "incomplete"
treat-as-withdraw (whatever that might be).

This seems to me worth covering in the draft... unless the impact of
"incomplete" treat-as-withdraw is deemed negligible -- in which case
it seems to me *that* would be worth covering too.

Chris

---

From: John Scudder <[email-masked]>
Date: Tue, 20 May 2014 20:42:07 +0000

> The requirement to put MP_xxx_NLRI as the first attribute appears to

Oops! Thanks, will fix. 

> it seems to me *that* would be worth covering too.

Let me ask you much the same question I asked Deniz. That is: what, if anything, would you expect an implementor to do differently as a result of the proposed text? 

(That's not to say "no", mind you.)

--John

---

From: "Chris Hall" <[email-masked]>
Date: Wed, 21 May 2014 13:36:38 +0100

John Scudder wrote (on Tue 20-May-2014 at 21:42 +0100):
> On May 20, 2014, at 2:39 PM, Chris Hall wrote:
....

...
> Let me ask you much the same question I asked Deniz. That is: what,
> if anything, would you expect an implementor to do differently as a
> result of the proposed text?

Not quite answering the question directly... let me put the case ('A')
of an UPDATE message in which the receiver finds:

  Withdrawn Routes field -- empty
  Attributes: ORIGIN    -- some valid origin
              ....      )  other valid stuff... 
              ....      )  ...but NOT MP_xx_NLRI
              <invalid> -- attributes overrun
  NLRI field -- empty

...ie. the receiver can find no NLRI, but there is every reason to
believe some were sent.  For obvious reasons, many implementations
place MP_REACH_NLRI after all other attributes, so this is a pretty
obvious case.  AFAICS this is not explicitly covered in the draft, so
I ask:

  (1) what should the receiver do ?

        (a) "treat-as-withdraw" ?
     or (b) "session-reset" ?

  (2) what is the rationale for the choice ?

Not wishing to sway the jury here, but if the receiver had found an
MP_REACH_NLRI which was not "perfectly-formed", then that would be
"session-reset".

And then there is the similar case ('B') of:

  Withdrawn Routes field -- empty
  Attributes: MP_UNREACH_NLRI -- "perfectly formed"
              ORIGIN    -- some valid origin
              ....      )  other valid stuff... 
              ....      )  ...but NOT MP_REACH_NLRI
              <invalid> -- attributes overrun
  NLRI field -- empty

What should the receiver do now, and why ?

Chris

---

From: Jakob Heitz <[email-masked]>
Date: Wed, 21 May 2014 14:24:57 +0000

Good one.

What SHOULD a receiver do with this?

 Withdrawn Routes field -- empty
 Attributes: MP_UNREACH_NLRI -- "perfectly formed"
             ....      )  valid attributes ... 
             ....      )  ...but NOT MP_REACH_NLRI
 NLRI field -- empty

--
Jakob Heitz.

> On May 21, 2014, at 5:37 AM, "Chris Hall" <[email-masked]> wrote:
>

---

From: Deniz Bahadir <[email-masked]>
Date: Wed, 28 May 2014 13:13:25 +0200

Hi there,

while trying to implement this draft I stumbled across a point which 
made me think if I really understood it correctly.

The sentence I stumbled across is in section 3.g. of the draft:

"[...] If any other attribute appears more than once in an UPDATE 
message, then all the occurrences of the attribute other than the first 
one SHALL be discarded and the UPDATE message continue to be processed."

This pretty much says the same as the corresponding sentence from 
section 6.3 of RFC 4271 (except for the action taken):

"If any attribute appears more than once in the UPDATE message, then the 
Error Subcode MUST be set to Malformed Attribute List."

What I am asking myself (and now you) is:
Does this really address *any* attribute which appears more than once, 
or only *recognized* attributes?

Of course, my implementation could realize by the attribute's type-code 
that an unrecognized (most likely optional) attribute appeared more than 
once in an UPDATE-message. But maybe, that could be valid for that 
unrecognized path-attribute?

So, if there is any chance of a (new) path-attribute to be allowed to 
appear more than once in an UPDATE-message, I would recommend to change 
the sentence in question to the following in the next version of this draft:

"[...] If any other recognized attribute appears more than once in an 
UPDATE message, then all the occurrences of the attribute other than the 
first one SHALL be discarded and the UPDATE message continue to be 
processed. Unrecognized attributes are still allowed to appear more than 
once."

If this it not what is intended, then I would recommend to explicitly 
state that this sentence is meant for recognized as well as unrecognized 
attributes, so that nobody might misinterpret it (as I might have done):

"[...] If any other attribute (no matter if recognized or not) appears 
more than once in an UPDATE message, then all the occurrences of the 
attribute other than the first one SHALL be discarded and the UPDATE 
message continue to be processed. (Multiple appearances of even 
unrecognized attributes can be be realized by the attributes' type-codes.)"

(The sentence in parentheses is just for clarification and could be 
removed if it is too obvious.)

Deniz

Am 20.05.2014 09:19, schrieb Deniz Bahadir:
> _______________________________________________
> Idr mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/idr

---

From: Deniz Bahadir <[email-masked]>
Date: Wed, 28 May 2014 13:22:17 +0200

My English seems not to be the best. I should probably replace 
"realized" with "identified" in my proposed-sentence.
I replaced it inline further down.

Deniz

> unrecognized attributes can be be realized by the attributes' type-codes.)"

NEW:
unrecognized attributes can be be identified by the attributes'
type-codes.)"

---

From: "Jakob Heitz (jheitz)" <[email-masked]>
Date: Wed, 28 May 2014 13:57:43 +0000

We know better than to ever invent an attribute type that can appear more than once in an UPDATE. An UPDATE with multiple attributes of a single type would never survive a trip through a legacy router. It is not a serious restriction. For example, we can fit many communities into a community attribute.

--
Jakob Heitz.

> On May 28, 2014, at 4:22 AM, "Deniz Bahadir" <[email-masked]> wrote:
> 
> My English seems not to be the best. I should probably replace "realized" with "identified" in my proposed-sentence.

---

From: Deniz Bahadir <[email-masked]>
Date: Wed, 28 May 2014 16:26:35 +0200

> We know better than to ever invent an attribute type that can appear more than once in an UPDATE. An UPDATE with multiple attributes of a single type would never survive a trip through a legacy rou…

OK, than it seems correct as written in the draft.
But still, does anything argue against being a little bit more explicit 
and change
   "If any other attribute appears..."
to
   "If any other attribute (no matter if recognized or not) appears..."
so that superfluous misinterpretation (as I did) can be prevented?

Deniz

---

From: "Chris Hall" <[email-masked]>
Date: Wed, 28 May 2014 15:41:09 +0100

Deniz Bahadir wrote (on Wed 28-May-2014 at 12:13 +0100):
....
> message, then all the occurrences of the attribute other than the
> first one SHALL be discarded and the UPDATE message continue to
> be processed."
> 
> "If any attribute appears more than once in the UPDATE message, then
> the Error Subcode MUST be set to Malformed Attribute List."

Strictly between ourselves: I've wrestled with the requirement to
discard second and subsequent instances of the same attribute.  I
confess, for the longest time I was tempted to think that
treat-as-withdraw would be the better way.  But I have seen the light,
and I regret my previous heresy -- I repent and recant, and I am
saved.  The scales have dropped from my eyes: it is clear to me now
that the first AS_PATH attribute in a message is blessed and anointed,
but any second or subsequent one is an abomination, and should be cast
into oblivion, and never spoken of again.  I can see that now.

> What I am asking myself (and now you) is: Does this really 
> address *any* attribute which appears more than once,

You are right, there is a difference between casting-out repeated
attributes you do recognise and casting-out ones you don't.  The
difference is that you are saving some down-stream BGP speaker, who
does understand the attribute, from having to even look at any
repeats, which (as above) are manifestly unclean.  If their
error-handling is not up to it, the down-stream would have
session-reset, so you have done them a favour and saved them from
themselves.  They will thank you for it and bless you and your
children too, so they will.

Chris

---

From: "John G. Scudder" <[email-masked]>
Date: Wed, 28 May 2014 17:34:35 -0400

On May 28, 2014, at 10:41 AM, Chris Hall <[email-masked]> wrote:
...
> into oblivion, and never spoken of again.  I can see that now.

IMO, any error for which “attribute discard” is specified MAY be handled using treat-as-withdraw instead, at the implementor’s option. This is basically because treat-as-withdraw is black-box identical to a routing policy that says “reject routes that match conditions foo”, and as we know (all together now!) policy is a local matter.

The draft could be made to say this if the WG thinks it’s helpful. I haven’t added it as yet, because it’s not clear to me that it is.

—John

---

From: "Chris Hall" <[email-masked]>
Date: Thu, 29 May 2014 22:35:45 +0100

John G. Scudder wrote (on Wed 28-May-2014 at 22:35 +0100)"
...
> IMO, any error for which “attribute discard” is specified MAY be
> handled using treat-as-withdraw instead, at the implementor’s
> option. This is basically because treat-as-withdraw is black-box
> identical to a routing policy that says “reject routes that match
> conditions foo”, and as we know (all together now!) policy is a
> local matter.

Well... I guess... one could implement an "I think the RFC is bonkers"
policy option.  

But I think there's a deeper problem with dropping duplicate
optional-transitives.  The default local policy would be to follow the
RFC and do this, but for optional-transitives that I don't understand,
this is (effectively) inflicting my local policy on third parties.

Mind you... I think the draft has a case of the conflicting MUSTs.
Second and subsequent AS_PATH attributes MUST be attribute-discarded,
but attribute-discard "MUST NOT be used except in the case of an
attribute that has no effect on route selection or installation".
Now, if the second rule wins... I am content.  Since I don't know
whether an unrecognised "optional transitive" has "no effect"... then
I am yet more content.  But this is probably wishful thinking on my
part !

Chris

---

From: "John G. Scudder" <[email-masked]>
Date: Fri, 30 May 2014 08:52:06 -0400

> Well... I guess... one could implement an "I think the RFC is bonkers"

Wouldn’t be the first time. :-/

> this is (effectively) inflicting my local policy on third parties.

I think Jakob has it right when he says:
> We know better than to ever invent an attribute type that can appear more than once in an UPDATE. An UPDATE with multiple attributes of a single type would never survive a trip through a legacy rou…

This is doubly-true if we move to change the “no duplicates” rule from common practice to actually codifying it in a standard. 

In any case, any time you configure a routing policy, you’re inflicting it on third parties. 

> I am yet more content.  But this is probably wishful thinking on my

Yes, I think so. I take your point that it’s not possible to say with absolute certainty that a second AS_PATH won’t have an effect on route selection and installation somewhere, so a close reading puts the two in conflict. On the other hand, it’s also not possible to say with absolute certainty that the sun will rise tomorrow morning, but I still have plans for tomorrow afternoon. The draft is in that sense a morass of contradictions, and represents a compromise between the contributors' sometimes individually differing sense of where the practical balance is to be struck between rigor and laxness. The extreme pole of rigor is RFC 4271. The extreme pole of laxness is to just quietly accept anything and have the holdtime or a CEASE be the only things that can ever bring a session down. We’re trying to chart a course down the broad space between these.

Returning from philosophizing to the two MUSTs you quote, I think a good rule of thumb to apply is that a more-specific (MUST be attribute-discarded) takes precedence over a less-specific (MUST NOT be used except in…). To be honest, the latter's a guideline rather than something that can really be considered a hard and fast rule, as our conversation demonstrates, so I’m going to change it in the next version from “MUST NOT” to “must not”. 

Thanks a lot for all the valuable review and comments, and please keep it coming as long as you’ve more to say!

—John

---

From: "Jakob Heitz (jheitz)" <[email-masked]>
Date: Fri, 30 May 2014 17:45:22 +0000

http://tools.ietf.org/html/rfc4271 bottom of page 24:

   The same attribute (attribute with the same type) cannot appear more
   than once within the Path Attributes field of a particular UPDATE
   message.

--Jakob

-----Original Message-----
From: Idr [mailto:[email-masked]] On Behalf Of John G. Scudder
Sent: Friday, May 30, 2014 5:52 AM

This is doubly-true if we move to change the "no duplicates" rule from common practice to actually codifying it in a standard.

---

From: "John G. Scudder" <[email-masked]>
Date: Fri, 30 May 2014 13:59:07 -0400

There you have it. Thanks, Jakob.

—John