Subject: [Idr] draft-ietf-idr-error-handling-08

From: "John G. Scudder" <[email-masked]>
Date: Tue, 13 May 2014 17:27:54 -0700

Folks,

The version I've just posted incorporates most of the suggestions received during the WGLC to date, including:

- Added to the rationale given in the introduction.
- Refactored and cleaned up (the old) section 2 ("Revision to Base Specification").
- Further restrictions on where and how NLRI fields can be sent (in summary, don't mix your encodings and don't mix MP_REACH and MP_UNREACH). These are believed to follow current practice.
- Attribute length of zero generally disallowed, except in cases where it actually makes sense (ATOMIC_AGGREGATE and sometimes AS_PATH).
- New "Guidance for Authors of BGP Specifications" section.

And various other bits of polish. Rfcdiff is your friend.

Thanks for all the review so far, it's been very productive. The main open issue I'm currently aware of is Bruno's regarding whether we should stop being draconian with MP_{UN}REACH flags. For my part, I take no strong position on this and am happy to follow whatever the WG consensus is.

--John

---

From: Deniz Bahadir <[email-masked]>
Date: Wed, 14 May 2014 10:27:32 +0200

Hi guys,

I support making this draft an RFC.

However, I would like to see the following points addressed before that.

1.
In section 4.2 there are some wrong attribute-names. MP_REACH should be 
MP_REACH_NLRI and MP_UNREACH should be MP_UNREACH_NLRI.

2.
In section 6.4 there is also a wrong attribute-name. MULTI_EXIT_DESC 
should be MULTI_EXIT_DISC.

3.
Is there a general approach (session-reset, treat-as-withdraw or 
attribute-discard) which should be taken if someone finds an 
error-situation which is neither covered by this document nor any other RFC?
Section 7 mentions treat-as-withdraw as preferred approach. However, 
that section is "only" a guidance for authors of new BGP specifications.
So maybe one should add the preferred approach in another section, e.g. 
in a section 2.1 ("General approach if nothing else applies").

4.
Section 3.d suggests to use the approach "treat-as-withdraw" if any of 
the well-known mandatory attributes are not present. Maybe one should 
add an extra note, that this does not apply for NEXT_HOP in the case of 
MP_REACH_NLRI being present in the UPDATE-message!?
(I know, this is what RFC 4760 already says, but I think the current 
document would be more complete to include this note, too.)

5.
(This point is probably very controversial.)
If there is no known (up-to-date) implementation (or not even an old 
one) which violates the restrictions on MP_REACH_NLRI, MP_UNREACH_NLRI 
mentioned at the beginning of section 4, might it be possible to remove 
the following sentence which relaxes these restrictions again?
(The sentence in question states: "In all these cases, however, an 
implementation MUST still be prepared to receive these fields in any 
position or combination.")

6.
I remember to have read somewhere on the Internet or the 
IETF-mailing-list that it is current best practice to not send 
UPDATE-messages with both, non-empty Withdrawn Routes field and 
non-empty Network Layer Reachability Information field, in it 
(regardless of the presence of MP_(UN)REACH_NLRI).
Maybe this information should also be included in the beginning of 
section 4, which then could become a subsection of its own.

Thanks,
Deniz

Am 14.05.2014 02:27, schrieb John G. Scudder:
> Folks,
>
> The version I've just posted incorporates most of the suggestions
> received during the WGLC to date, including:
>
> - Further restrictions on where and how NLRI fields can be sent (in
> summary, don't mix your encodings and don't mix MP_REACH and
> MP_UNREACH). These are believed to follow current practice.
> - Attribute length of zero generally disallowed, except in cases where
> it actually makes sense (ATOMIC_AGGREGATE and sometimes AS_PATH).
> Thanks for all the review so far, it's been very productive. The main
> open issue I'm currently aware of is Bruno's regarding whether we should
> stop being draconian with MP_{UN}REACH flags. For my part, I take no
> strong position on this and am happy to follow whatever the WG consensus is.
>
> --John
>
>
> _______________________________________________
> Idr mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/idr
>

---

From: Thomas Mangin <[email-masked]>
Date: Wed, 14 May 2014 10:23:56 +0100

Hello John,

Sorry for the late comment, I am so late behind my mail. Could I just ask you why the document recommend :

   o  The MP_REACH_NLRI or MP_UNREACH_NLRI attribute (if present) SHALL
      be encoded as the very first path attribute in an UPDATE.

For the UN_UNREACH_NLRI, I have no question as AFAIU it does not matter where it is.

However to mirror the behaviour of RFC 4271 I would have expected to see MP_REACH_NLRI as LAST attribute. ie:

WITHDRAWN NLRI
MP_UNREACH_NLRI
<OTHER ATTRIBUTES>
MP_REACH_NLRI
ANNOUNCED NLRI

IPv4 only implementations are able to fully parse attributes and then be able to associate them with the NLRI present at the end of the UPDATE. Therefore I would have thought that to make it "simple" (yeah, right ! :p), it would have been more logical to place MP_REACH_NLRI at the end of the attribute set to mirror the non MP behaviour.

I remember swearing to myself ( and cursing too most likely - please excuse my french :p ) when I implemented MP support, as I could not assume that MP_REACH_NLRI would be last.

If I understand you correctly, you are saying that most major implementations do not do this and place MP_REACH_NLRI first. I would be curious to understand why and if the behaviour is indeed seen by every implementation ? Even so, a SHALL can not help with any code optimisation ( please tell me if you beg to differ ) therefore I fail to understand why this was included (not that I am asking to the removal of the text), I am barely trying to understand why it is here and if it is the right thing to recommend.

I wrote this paragraph before seeing the comment from Deniz.

> If there is no known (up-to-date) implementation (or not even an old one) 
> which violates the restrictions on MP_REACH_NLRI, MP_UNREACH_NLRI mentioned at
> the beginning of section 4, might it be possible to remove the following
> sentence which relaxes these restrictions again?
> implementation MUST still be prepared to receive these fields in any position
> or combination.")

ExaBGP orders the attributes by increasing ID value as it is recommended somewhere in 4271 I believe (I did not dig to find out if it is true) and then append MP_UNREACH_NLRI and then MP_REACH_NLRI

yield self._message(prefix(packed_del) + prefix(attr + packed_mp_del + packed_mp_add) + packed_add)

It is trivial to change but it does mean that at lease one implementation do not follow this rule.

>> There are any number of heuristics that may be applied to this
>> problem.  It would be helpful if, in practice, the vast majority of
>> implementations only ever send one set of NLRI per message.  It helps
>> a bit if, in practice, MP_xx_REACH attributes only appear in messages
>> where both the Withdraw Routes and NLRI parts are empty.  Does anybody
>> know if either of these is the case ?

> I (strongly) believe these to be common practice. I'm confident of it for the
> code base I've currently got access to, but can't speak authoritatively for
> others of course. Maybe other folks will chime in. 

I know I never give this a thought. Chris, could you please elaborate on why you seen this behaviour as advisable ?

Thomas

---

From: "Chris Hall" <[email-masked]>
Date: Wed, 14 May 2014 11:00:56 +0100

Deniz Bahadir wrote (on Wed 14-May-2014 at 09:28 +0100):
....
> If there is no known (up-to-date) implementation (or not even 
> an  old one) which violates the restrictions on MP_REACH_NLRI,
> MP_UNREACH_NLRI mentioned at the beginning of section 4, might
> it be possible to remove the following sentence which relaxes
> these restrictions again?

I'm not sure this is true of the first restriction -- that MP_xx_NLRI
must appear as the first attribute.  But happily the second and third
restrictions are sufficient to simplify the NLRI recovery problem.

Assuming it is true, the most helpful thing that the draft could say
is that: because there are (virtually) no known implementations which
violate the second and third restrictions, the receiver MAY (well)
choose to assume the sender conforms, whether or not it conforms to
the rest of the draft/RFC.

Having thus calmed the nerves of the paranoid (who can stop worrying
about lost-NLRI as soon as they have one set in their hands), this
leaves one case, where the receiver has found the message to be
broken, would like to treat-as-withdraw, but has found no NLRI !
Presumably the receiver has no choice and must session-reset.  This
case is, of course, the rationale for the first restriction.

Following the general impetus to not session-reset unless compelled
to, I guess: a message which, putting to one side any MP_UNREACH_NLRI,
contains a completely valid, but not empty, set of attributes, but no
reachable NLRI should be treated-as-withdraw (of an empty set of NLRI,
but logged as treat-as-withdraw).

> IETF-mailing-list that it is current best practice to not
> send UPDATE-messages with both, non-empty Withdrawn Routes
> field and non-empty Network Layer Reachability Information
> field, in it (regardless of the presence of MP_(UN)REACH_NLRI).

It uses less pencil to specify that one UPDATE should contain just one
set of NLRI, and is tidier :-)  But, from the perspective of
recovering the NLRI from a broken UPDATE message, the key is to limit
the number of MP_xx_NLRI attributes.

Chris

---

From: Thomas Mangin <[email-masked]>
Date: Wed, 14 May 2014 11:07:19 +0100

> the rest of the draft/RFC.

Sorry I realise my point about the order is invalid as ExaBGP does not yet implement this draft.
So all implementation may well have the MP first.

Thomas

---

From: Ondrej Zajicek <[email-masked]>
Date: Wed, 14 May 2014 12:53:49 +0200

> position or combination.")

BIRD currently violates the first and the second restriction from
section 4. It combines MP_REACH_NLRI and MP_UNREACH_NLRI in one update,
sending MP_UNREACH_NLRI as first, then regular attributes, then
MP_REACH_NLRI as last.

> section 4, which then could become a subsection of its own.

I agree. I suggest that the second and the third restriction from section
4 should be merged and extended to:

  An UPDATE message MUST NOT contain more than one of the following:
  non-empty Withdrawn Routes field, non-empty Network Layer Reachability
  Information field, MP_REACH_NLRI attribute, and MP_UNREACH_NLRI
  attribute.

-- 
Ondrej 'Santiago' Zajicek

---

From: Ondrej Zajicek <[email-masked]>
Date: Wed, 14 May 2014 13:15:49 +0200

> For the UN_UNREACH_NLRI, I have no question as AFAIU it does not matter where it is.

AFAIU, MP_UNREACH_NLRI should be alone - if there are no 'positive'
paths, then there is no point to add other attributes.

> IPv4 only implementations are able to fully parse attributes and then
> be able to associate them with the NLRI present at the end of the
> UPDATE. Therefore I would have thought that to make it "simple" (yeah,
> right ! :p), it would have been more logical to place MP_REACH_NLRI at
> the end of the attribute set to mirror the non MP behaviour.

Putting MP_REACH_NLRI last also makes sense from sender implementation
point of view - you pack fixed number of your regular attributes first
and then fill remaining available space by MP_REACH_NLRI (if you have
too many paths with the same set of attributes), therefore i would
also expect more implementations to send it last.

---

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

Thomas Mangin wrote (on Wed 14-May-2014 at 10:24 +0100):
....
> >> There are any number of heuristics that may be applied to 
> >> this problem.  It would be helpful if, in practice, the vast
> >> majority of implementations only ever send one set of NLRI
> >> per message.  It helps a bit if, in practice, MP_xx_REACH
> >> attributes only appear in messages where both the Withdraw
> >> Routes and NLRI parts are empty.  Does anybody know if
> >> either of these is the case ?

> > I (strongly) believe these to be common practice. I'm 
> > confident of it for the code base I've currently got access
> > to, but can't speak authoritatively for others of course.
> > Maybe other folks will chime in.

> I know I never give this a thought. Chris, could you please
> elaborate on why you seen this behaviour as advisable ?

OK... you asked, so here goes...

...in the following: "MP_xx_NLRI" means one or both of MP_REACH_NLRI
and MP_UNREACH_NLRI attributes; Body-NLRI refers to one or both of the
Withdrawn Routes and NLRI sections of the UPDATE message -- the NLRI
outside the attributes; a "set of NLRI" refers to any one of the
MP_xx_NLRI and Body-NLRI; and "NLRI" refers to any or all sets of NLRI
in an UPDATE message.

...and this is my understanding of the problem...

...(1) for treat-as-withdraw to be "safe" (or as safe as possible) it
is essential that *all* NLRI in a broken update can be recovered.  For
if not, then treat-as-withdraw is "incomplete", and leaves the
receiver with out-of-date routes in its hands, whose value is (to say
the least) indeterminate.

...(2) the standard(s) allow for an UPDATE to contain between 1 and 4
sets of NLRI.

...(3) depending on how broken the message is, there may be 0, 1 or 2
MP_xx_NLRI attributes buried somewhere in the rubble, and the receiver
cannot tell how much has been lost -- the lost-NLRI.  If there are no
Body-NLRI, then the receiver may assume that there are 1 or 2
lost-NLRI.

...(4) if the sender follows the MP_xx_NLRI first rule, they sit on
top of the rubble and can be recovered.  This is a change in the
standard which one hopes will be implemented and deployed in some
time.  But in any case, the receiver has no way of telling whether the
sender intends to follow the rule (or has succeeded !).  So, if the
receiver does not find MP_xx_NLRI, it may have to assume the worst.

...(5) if lost-NLRI are a problem, or a problem worse than
session-reset, then the receiver appears to be in the unenviable
position of having to balance the risk/cost of an "incomplete"
treat-as-withdraw against the benefit of avoiding (the dreaded)
session-reset.

...(6) if some/many/most/all lost-NLRI are worse than session-reset,
then when can treat-as-withdraw be used ?

At this point I go and hide under my desk.

But, if in practice an UPDATE only ever contains 1 set of NLRI, then
my descent into a paranoid, quivering wreck stops at step 2.  Hurrah !

This is slightly stricter than it needs to be: there is no problem
with both sets of body-NLRI appearing together.

Now, you can argue that the only thing a receiver *can* do is to
treat-as-withdraw on the basis of the NLRI it can see, and not worry
about things it cannot see -- so that's what it *ought* to do.  As a
practical matter, errors are rare... errors which result in lost-NLRI
are rarer still... the result is probably not as bad as
session-reset... and the operator should be got out of bed to deal
with the root-cause pretty sharpish, so any bad effects will be
temporary... and in any case treat-as-withdraw is not a perfect
work-around, just the best we have.  Trying to decide when it is more
or less likely that there are lost-NLRI -- searching through the
rubble for possible survivors -- looks like a tricky problem, and best
avoided.

If, de facto, the receiver can assume that messages contain either
body-NLRI or just one MP_xx_NLRI attribute, then what the receiver
*can* do doesn't change, but what it *ought* to do is less
problematic, and the rationale requires much, much less hand waving.

It's not necessary for absolutely every sender to be absolutely
guaranteed to follow this (post hoc) rule.  For the minority that
don't work this way, the hand waving argument may be deployed.  But
the smaller that minority is, the less vigorous the hand waving has to
be to make the point.

For completeness, note that the MP_xx_NLRI at the front rule is
orthogonal to this.  Senders which follow the rule are less likely to
generate a broken UPDATE from which the NLRI cannot be recovered.  The
rule is not relevant to what the receiver does with the UPDATE.

Chris

---

From: <[email-masked]>
Date: Wed, 14 May 2014 13:21:28 +0000

Hi all,

> ...(5) if lost-NLRI are a problem, or a problem worse than session-reset, then the receiver appears to be in the unenviable position of having to balance the risk/cost of an "incomplete"
treat-as-withdraw against the benefit of avoiding (the dreaded) session-reset.

IINM, we have a new tool to protect/insure against the risk of an incomplete treat-as-withdraw (aka missing NLRI in an erroneous update): http://tools.ietf.org/html/draft-ietf-idr-bgp-enhanced-route-refresh-06
Possibly, draft-ietf-idr-error-handling could add a note about this recovery option. (if so, I would propose to (randomly) delay the sending of the Route Refresh message since the BGP speaker (our BGP peer) is possibly busy handling BGP NOTIFICATIONs from others peers)

Regards,
Bruno

PS: Looks somewhat ironic (to me) that this tool has been defined to recover missing NLRI from well-formed BGP UPDATE, and not from erroneous ones ;-)

> -----Original Message-----
> From: Idr [mailto:[email-masked]] On Behalf Of Chris Hall
> Sent: Wednesday, May 14, 2014 2:15 PM
> To: 'idr@ietf. org'
> Subject: Re: [Idr] draft-ietf-idr-error-handling-08
> 
> > >> implementations only ever send one set of NLRI per message.  It
> > >> helps a bit if, in practice, MP_xx_REACH attributes only appear in
> > >> messages where both the Withdraw Routes and NLRI parts are empty.
> > >> Does anybody know if either of these is the case ?
> 
> > > I (strongly) believe these to be common practice. I'm confident of
> > > it for the code base I've currently got access to, but can't speak
> > > authoritatively for others of course.
> > I know I never give this a thought. Chris, could you please elaborate
> > on why you seen this behaviour as advisable ?
> 
> outside the attributes; a "set of NLRI" refers to any one of the MP_xx_NLRI
> and Body-NLRI; and "NLRI" refers to any or all sets of NLRI in an UPDATE
> message.
> 
> ...(1) for treat-as-withdraw to be "safe" (or as safe as possible) it is essential
> that *all* NLRI in a broken update can be recovered.  For if not, then treat-
> as-withdraw is "incomplete", and leaves the receiver with out-of-date routes
> in its hands, whose value is (to say the least) indeterminate.
> 
> ...(2) the standard(s) allow for an UPDATE to contain between 1 and 4 sets of
> NLRI.
> 
> cannot tell how much has been lost -- the lost-NLRI.  If there are no Body-
> NLRI, then the receiver may assume that there are 1 or 2 lost-NLRI.
> 
> ...(4) if the sender follows the MP_xx_NLRI first rule, they sit on top of the
> rubble and can be recovered.  This is a change in the standard which one
> hopes will be implemented and deployed in some time.  But in any case, the
> receiver has no way of telling whether the sender intends to follow the rule
> (or has succeeded !).  So, if the receiver does not find MP_xx_NLRI, it may
> have to assume the worst.
> 
> ...(5) if lost-NLRI are a problem, or a problem worse than session-reset, then
> the receiver appears to be in the unenviable position of having to balance
> the risk/cost of an "incomplete"
> treat-as-withdraw against the benefit of avoiding (the dreaded) session-
> reset.
> 
> ...(6) if some/many/most/all lost-NLRI are worse than session-reset, then
> when can treat-as-withdraw be used ?
> 
> But, if in practice an UPDATE only ever contains 1 set of NLRI, then my
> descent into a paranoid, quivering wreck stops at step 2.  Hurrah !
> 
> This is slightly stricter than it needs to be: there is no problem with both sets
> of body-NLRI appearing together.
> 
> Now, you can argue that the only thing a receiver *can* do is to treat-as-
> withdraw on the basis of the NLRI it can see, and not worry about things it
> cannot see -- so that's what it *ought* to do.  As a practical matter, errors are
> rare... errors which result in lost-NLRI are rarer still... the result is probably
> not as bad as session-reset... and the operator should be got out of bed to
> deal with the root-cause pretty sharpish, so any bad effects will be
> temporary... and in any case treat-as-withdraw is not a perfect work-around,
> just the best we have.  Trying to decide when it is more or less likely that
> there are lost-NLRI -- searching through the rubble for possible survivors --
> looks like a tricky problem, and best avoided.
> 
> If, de facto, the receiver can assume that messages contain either body-NLRI
> or just one MP_xx_NLRI attribute, then what the receiver
> *can* do doesn't change, but what it *ought* to do is less problematic, and
> the rationale requires much, much less hand waving.
> 
> It's not necessary for absolutely every sender to be absolutely guaranteed to
> follow this (post hoc) rule.  For the minority that don't work this way, the
> hand waving argument may be deployed.  But the smaller that minority is,
> the less vigorous the hand waving has to be to make the point.
> 
> For completeness, note that the MP_xx_NLRI at the front rule is orthogonal
> to this.  Senders which follow the rule are less likely to generate a broken
> UPDATE from which the NLRI cannot be recovered.  The rule is not relevant
> to what the receiver does with the UPDATE.
> 
> Chris
> 
> 
> 

_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

---

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

Ondrej Zajicek wrote (on Wed 14-May-2014 11:54 at +0100):
...
> section 4. It combines MP_REACH_NLRI and MP_UNREACH_NLRI in one
> update, sending MP_UNREACH_NLRI as first, then regular attributes,
> then MP_REACH_NLRI as last.

Current versions of Quagga also send MP_UNREACH_NLRI as the last
attribute.  But it only sends one set of NLRI per message.

>From the point of view of the receiver, the second restriction can be
loosened a little, so that it may assume any MP_UNREACH_NLRI will be
the first or only attribute.

Hence, if the attributes are broken in any way: (a) if the first
attribute is not MP_UNREACH_NLRI, then the receiver assumes there are
no MP_UNREACH_NLRI, and (b) if there are any other attributes, then
there must be an MP_REACH_NLRI somewhere.

The draft can specify that, in future, the sender MUST send just one
set of NLRI and MUST send any MP_xx_NLRI as the first attribute.  And
when senders do that, things are simpler and NLRI less likely to be
lost.

>From the receiver's perspective, what matters is the assumptions it
may reasonably make when deciding whether it can "safely" do
treat-as-withdraw.  In the short run it cannot assume the sender is
following the new rules.  But it looks as if, de facto, there are
simplifying assumptions the receiver can make, now -- whether or not
the sender is following the new rules.  IMO the draft is the place to
document those de facto assumptions.

Chris

---

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

A receiver can not assume that there will only ever be one "set of NLRI" and that it will appear first (unless it is the NLRI section), unless we add a capability.

--
Jakob Heitz.

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

---

From: "Chris Hall" <[email-masked]>
Date: Wed, 14 May 2014 20:20:45 +0100

Jakob Heitz wrote (on Wed 14-May-2014 at 17:25 +0100):
>
> A receiver can not assume that there will only ever be one "set of
> NLRI" and that it will appear first (unless it is the NLRI section),
> unless we add a capability.

I think that perhaps we are using "assume" to mean slightly different
things here.

In the absence of a capability, the sender is entitled to follow
existing standards, and the receiver is required to cope.  On receipt
of such a capability the receiver would be entitled to expect the new
behaviour.  That much I think is uncontroversial.

However, the object of the whole exercise is to handle errors more
gracefully, so whatever the receiver feels entitled to expect is
largely moot.

When trying to cope with broken attributes the receiver cannot *know*
what was intended and at some point has to *assume* something.  This
is in this sense of "assume" I have in mind.

The choice of assumptions that the receiver can make is a large part
of the draft, and each assumption is designed to balance simplicity,
routeing safety and a desire to avoid the (dreaded) session-reset.

For example, the current draft says that: faced with what appears to
be a LOCAL_PREF attribute of length 2000, the receiver MUST assume
that this is indeed an oversize LOCAL_PREF attribute -- although it
could be that the previous attribute was, in fact, broken and the
receiver is currently attempting to parse complete nonsense. 

It appears that many/most/nearly-all BGP implementations only do a
subset of the things allowed by the current (de jure) standard.  From
what I've heard recently, in current implementations:

  * MP_xx_NLRI and Body-NLRI do not appear together in an
    UPDATE message.
 
  * MP_UNREACH_NLRI is always the first (or only) attribute.

Happily, the proposed de jure standard in the current draft is
compatible with this, but a smidge more restrictive.

If this de facto "standard" turns out to be followed by sufficient
implementations, then for the purposes of dealing with broken
attributes, and in the absence of anything else to go on, I suggest
the receiver not only can, but should, assume these things.

Chris

---

From: "John G. Scudder" <[email-masked]>
Date: Wed, 14 May 2014 14:03:12 -0700

Deniz,

On May 14, 2014, at 1:27 AM, Deniz Bahadir <[email-masked]> wrote:
> In section 4.2 there are some wrong attribute-names. MP_REACH should be MP_REACH_NLRI and MP_UNREACH should be MP_UNREACH_NLRI.
> 
> 2.
> In section 6.4 there is also a wrong attribute-name. MULTI_EXIT_DESC should be MULTI_EXIT_DISC.

Fixed both the above for forthcoming -09. Thanks.

> 3.
> Is there a general approach (session-reset, treat-as-withdraw or attribute-discard) which should be taken if someone finds an error-situation which is neither covered by this document nor any other RFC?
> Section 7 mentions treat-as-withdraw as preferred approach. However, that section is "only" a guidance for authors of new BGP specifications.
> So maybe one should add the preferred approach in another section, e.g. in a section 2.1 ("General approach if nothing else applies").

I hesitate to try to be more prescriptive than the draft already is, for cases which are by definition undefined. One hopes that the implementor will Do The Right thing, we have pretty strongly indicated what we think the right thing usually is, but without knowledge of the specific error case it's hard to say that t-a-w is *definitely* right.

> 4.
> Section 3.d suggests to use the approach "treat-as-withdraw" if any of the well-known mandatory attributes are not present. Maybe one should add an extra note, that this does not apply for NEXT_HOP in the case of MP_REACH_NLRI being present in the UPDATE-message!?
> (I know, this is what RFC 4760 already says, but I think the current document would be more complete to include this note, too.)

Will be incorporated in -09, thanks.

> If there is no known (up-to-date) implementation (or not even an old one) which violates the restrictions on MP_REACH_NLRI, MP_UNREACH_NLRI mentioned at the beginning of section 4, might it be possible to remove the following sentence which relaxes these restrictions again?
> (The sentence in question states: "In all these cases, however, an implementation MUST still be prepared to receive these fields in any position or combination.")

We've had several indications that these assumptions are not universally true (exaBGP, BIRD, I believe) so we can't do this.

> 6.
> I remember to have read somewhere on the Internet or the IETF-mailing-list that it is current best practice to not send UPDATE-messages with both, non-empty Withdrawn Routes field and non-empty Network Layer Reachability Information field, in it (regardless of the presence of MP_(UN)REACH_NLRI).
> Maybe this information should also be included in the beginning of section 4, which then could become a subsection of its own.

Seems like by prescribing this behavior we've got this covered?

--John

---

From: "John G. Scudder" <[email-masked]>
Date: Wed, 14 May 2014 14:04:40 -0700

Hi Thomas,

On May 14, 2014, at 2:23 AM, Thomas Mangin <[email-masked]> wrote:

> Sorry for the late comment, I am so late behind my mail.

Not at all, your comment is very timely.

> Could I just ask you why the document recommend :
> 

I see that Chris has answered this exhaustively. I agree with his analysis.

Thanks,

--John

---

From: Ondrej Zajicek <[email-masked]>
Date: Thu, 15 May 2014 01:32:48 +0200

> - New "Guidance for Authors of BGP Specifications" section.

Hello

I have another two minor comments:

  3. c.  Attribute Flag error handling:

Formulation "If any recognized attribute has Attribute Flags that
conflict with the Attribute Type Code" is IMHO a bit vague about which
flags could be in conflict with the attribute code. Optional (O) and
Transitive (T) bits clearly could, undefined bits cleary couldn't,
but i am not sure about Partial and and Extended Legth.

(e.g. if there is 300B long AGGREGATOR with Extended Length flag, should
it be just discarded according to 6.7. based on its length, or should it
be treat-as-withdraw according by 3. c. because AGGREGATOR should not
ever have Extended Length flag set?)

Should the zeroness of Partial (P) bit for other than optional transitive
atributes be checked? For all or just for recognized attributes?

And what if i receive unrecognized attribute without optional flag?
That is cleary broken but not covered by the formulation from the
draft (or from the RFC 4271).

IMHO, O bit and consistency of O-T-P bits (i.e. no partial unless
optional transitive) should be checked for all atributes, additionally
T bit should be checked for recognized attributes. EL bit should
not be checked (should be considered intependently on attribute type).

  4.2 Syntactic Correctness of NLRI Fields

What about the Next Hop field in MP_REACH_NLRI? Although specifying
syntactic correctness for all possible AFIs is out of scope of the
draft, the draft could say in 4.2 how errors of the Next hop field
should be generally handled. Although the draft handles errors in
MP_REACH_NLRI by session-reset, i would suggest that errors in the Next
Hop field of MP_REACH_NLRI should be handled by treat-as-withdraw
(as long as NLRI part of MP_REACH_NLRI is parsed without errors).

> The main open issue I'm currently aware of is Bruno's regarding
> whether we should stop being draconian with MP_{UN}REACH flags. For my
> part, I take no strong position on this and am happy to follow whatever
> the WG consensus is.

I tend to agree with Bruno. Special handling of MP_{UN}REACH flags seems
unnecessary.

---

From: Deniz Bahadir <[email-masked]>
Date: Mon, 19 May 2014 17:04:28 +0200

> I hesitate to try to be more prescriptive than the draft already is, for cases which are by definition undefined. One hopes that the implementor will Do The Right thing, we have pretty strongly ind…

That's fine with me. However, having RFC 4271 in mind, my first reflex 
is still to send a NOTIFICATION in such a situation. (I probably need to 
retrain my reflexes. ;-)

> We've had several indications that these assumptions are not universally true (exaBGP, BIRD, I believe) so we can't do this.

OK, I see. But how about being at least strict with the third point? As 
far as I understand, there seems to be no implementation which mixes 
both, Multi-Protocol NLRI information (MP_REACH_NLRI and/or 
MP_UNREACH_NLRI) with traditional NLRI information (Withdrawn Routes 
field and/or Network Layer Reachability Information field).

> Seems like by prescribing this behavior we've got this covered?

Sorry, I am not entirely sure how I have to interpret this answer?
Sadly, I also could not find the source of my claim for the existence of 
said "current best practice". (Maybe it is no best practice at all?) 
However, if it does not break anything I would prefer to see it merged 
with the three points at the beginning of section 4.
Ondrej's merge-example sounds good to me.
For the sake of completeness I repeat it here:

Deniz

---

From: "John G. Scudder" <[email-masked]>
Date: Mon, 19 May 2014 13:44:43 -0400

On May 19, 2014, at 11:04 AM, Deniz Bahadir <[email-masked]> wrote:

> OK, I see. But how about being at least strict with the third point? As far as I understand, there seems to be no implementation which mixes both, Multi-Protocol NLRI information (MP_REACH_NLRI and/or MP_UNREACH_NLRI) with traditional NLRI information (Withdrawn Routes field and/or Network Layer Reachability Information field).

I take your point. However, since (prior to this draft) the combination of MP and legacy NLRI encodings was not forbidden, it still seems prudent to consider that they MIGHT be mixed even if nobody has admitted to it on-list. 

But suppose the text were written differently, such that the final case was "less strict". What would the implementation do differently? 

My thinking in writing that sentence was basically to caution implementors, "just because the error-handling spec says 'don't do these things' doesn't mean it would be a good idea for your implementation to break if they happen anyway". This is basically the process of deprecating an old behavior -- first you forbid it but make sure it continues to be accepted on input anyway. Much later, you can consider making it a protocol error.

If you have text you'd like to propose, please do.

Thanks,

--John

---

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

> Sorry, I am not entirely sure how I have to interpret this answer?

I meant, since error-handling mandates that you can't combine NLRI fields, I don't see a benefit to saying that not combining them was already best practice? (Notwithstanding the fact neither you nor I can document that best practice claim.)

> Sadly, I also could not find the source of my claim for the existence of said "current best practice". (Maybe it is no best practice at all?) However, if it does not break anything I would prefer to see it merged with the three points at the beginning of section 4.

I don't think making this claim would break anything, but I don't see what it would help either, so I would prefer to leave it out rather than make the document longer. Maybe you could explain how it would help, keeping in mind the primary purpose of the document is to be a protocol spec, i.e. to tell the implementor what to do.

>  Information field, MP_REACH_NLRI attribute, and MP_UNREACH_NLRI

I will do that (or similar) for -09.

Thanks,

--John

---

From: Deniz Bahadir <[email-masked]>
Date: Mon, 19 May 2014 20:10:32 +0200

Hi John,

>>>> (This point is probably very controversial.)
[snip]
> But suppose the text were written differently, such that the final case was "less strict". What would the implementation do differently?

Makes sense. I just thought that being less vague would make it much 
easier for implementers (like me ;-). But I understand, that it might 
lead to practical problems. (That's possibly the down-side of allowing 
too much interpretation in the first-place.)

> My thinking in writing that sentence was basically to caution implementors, "just because the error-handling spec says 'don't do these things' doesn't mean it would be a good idea for your implemen…

Agreed. So I count this as the beginning of the deprecation-process.
I guess, complete removal could then take place in an entirely new 
version of the BGP base-RFC. (Maybe, BGP5? This seems far enough in the 
future. ;-)

> If you have text you'd like to propose, please do.

I am sorry, currently I do not have any text to propose. But I will 
consider that for the future.

Deniz

---

From: Deniz Bahadir <[email-masked]>
Date: Mon, 19 May 2014 20:45:22 +0200

[snip]
> I meant, since error-handling mandates that you can't combine NLRI fields, I don't see a benefit to saying that not combining them was already best practice? (Notwithstanding the fact neither you n…

OK, that's right. I just thought that if someone knew the source of this 
best-practice claim one could reference and repeat it in this document 
as it seems to be addressed, too.
But because no-one knows if and where this claim was made before (if at 
all), it is probably better to not mention it.

> I don't think making this claim would break anything, but I don't see what it would help either, so I would prefer to leave it out rather than make the document longer. Maybe you could explain how…

The idea behind it was that as the current document already lists (new) 
requirements for the four different NLRI fields one could keep the 
direct interaction of them to a minimum.
The current version 8 of the draft says that MP_REACH_NLRI must be alone 
and that MP_UNREACH_NLRI must be alone. But then why not force the 
traditional NLRI and traditional Withdrawn Routes to be alone, too?

It was just for having similar requirements for similar things and 
therefore also reducing the parts in the spec which allow too much 
interpretation for the implementer.

As a side-note: For me, as an implementer of such specs, if I stumble 
across a spec which lets too much room for interpretation, it always 
means either extra implementation-work (or less 
optimization-possibilities) to cover all possible 
(interpretation-)cases, or being afraid that my implementation might 
break under some special conditions.

[snip]
 > I will do that (or similar) for -09.

I am looking forward to -09 and would be glad to comment on it (and 
thereby hopefully not annoy anyone).

Deniz

---

From: Vitkovský Adam <[email-masked]>
Date: Thu, 22 May 2014 20:42:47 +0000

I agree with Chris on this one,
It essentially boils down to whether lost-NRLI (as defined by Chris) is worse than session reset.
My opinion is that session reset is worse.

adam

---

From: Shyam Sethuram <[email-masked]>
Date: Thu, 22 May 2014 15:54:40 -0700

I'd say 'lost-NLRI' is worse than session reset. You end up
with stale routes (forwarding to invalid nexthop, importing to the wrong
VRF, etc).
IMO if you can't successfully parse a full set of NLRIs, then session reset
would be the better.

shyam

On Thu, May 22, 2014 at 1:42 PM, Vitkovský Adam <[email-masked]>wrote:

> MP_xx_NLRI
> sets of
> Body-
> session-reset, then
> treat-as-
> work-around,
> body-NLRI
> guaranteed to
> orthogonal

---

From: Jeffrey Haas <[email-masked]>
Date: Thu, 22 May 2014 19:36:01 -0400

> IMO if you can't successfully parse a full set of NLRIs, then session reset

Speaking as from a vendor who has this particular sin to live down, this
somewhat argues a vendor-wide need of the ability to delete via CLI (and not
policy and reconfig) a received BGP route.  (Whack, as Steve Kent is known
to say.)

-- Jeff

---

From: Robert Raszuk <[email-masked]>
Date: Fri, 23 May 2014 02:27:12 +0200

Jeff,

But when you delete route via cli you hopefully withdraw it if it was
advertised right ?

However of you can not parse nlri you do not have such option.

R.

---

From: Jeffrey Haas <[email-masked]>
Date: Fri, 23 May 2014 09:49:57 -0400

> But when you delete route via cli you hopefully withdraw it if it was

If we want to wrapper my statements in all the nice formalisms of BGP,
consider it "manually removing a route from the Adj-Rib-In".  One would
expect the rest of the BGP machinery to Do The Right Thing.

-- Jeff

---

From: Vitkovský Adam <[email-masked]>
Date: Fri, 23 May 2014 14:20:53 +0000

Ok so the reasoning behind this, if I understand it correctly, is that if the NLRI can't be parsed than BGP doesn't know for which NLRI to send a withdraw for so it rather sends a withdraw for all the routes from the neighbor (by means of resetting the session with the neighbor). 
But the neighbor might be perfectly fine with the routes we have sent to him so why to reset the session and sending withdraws on both TX and RX path from the incident.
Can't the withdraw be sent just on the RX path and on the TX side the BGP could just request a retransmit of all the routes from the neighbor.
Well and only after it receives the similar garbage again than it can assume there's something seriously wrong with the neighbor and that maybe a session reset can solve things or draw operator's attention.

adam
> From: Jeffrey Haas [mailto:[email-masked]]
> Sent: Friday, May 23, 2014 1:36 AM
> To: Shyam Sethuram
> Cc: Vitkovský Adam; idr@ietf. org
> > I'd say 'lost-NLRI' is worse than session reset. You end up with stale
> > routes (forwarding to invalid nexthop, importing to the wrong VRF,
> > etc).
> > IMO if you can't successfully parse a full set of NLRIs, then session
> > reset would be the better.
> 
> policy and reconfig) a received BGP route.  (Whack, as Steve Kent is known to
> say.)
> 
> -- Jeff

---

From: "Smith, Donald" <[email-masked]>
Date: Fri, 23 May 2014 17:08:37 +0000

I would totally disagree. Despite it's dynamic nature peering routes don't change (completely) nearly as often as people seem to think. Thus the whole concept of GR. Add to that the fact that ipv4 is "exhausted" churn isn't as constant as people seem to believe.  We need bgp to "heal" the network when one link goes down but if it does NOT the worse that happens is it doesn't self heal for the portion that is broken. A reset kills ALL the routes between bgp speakers not just the broken link.

 You can't amputate the leg to save the foot. You can amputate the foot to save the leg.

(coffee != sleep) & (!coffee == sleep)
 [email-masked]

From: Idr [[email-masked]] on behalf of Shyam Sethuram [[email-masked]]
Sent: Thursday, May 22, 2014 4:54 PM
To: Vitkovský Adam
Cc: idr@ietf. org
Subject: Re: [Idr] draft-ietf-idr-error-handling-08

I'd say 'lost-NLRI' is worse than session reset. You end up 
with stale routes (forwarding to invalid nexthop, importing to the wrong VRF, etc).
IMO if you can't successfully parse a full set of NLRIs, then session reset
would be the better.

shyam

On Thu, May 22, 2014 at 1:42 PM, Vitkovský Adam <[email-masked]> wrote:

adam

---

From: Vitkovský Adam <[email-masked]>
Date: Fri, 23 May 2014 21:27:02 +0000

I think the default behavior of session reset and its effects that scale from unpleasant to disastrous, are one of the main reasons why we are having this discussion in a first place, so while we are on it we might as well try to avoid it by all means possible. 
 
adam
> From: Smith, Donald [mailto:[email-masked]]
> Sent: Friday, May 23, 2014 7:09 PM
> To: Shyam Sethuram; Vitkovský Adam
> Cc: idr@ietf. org
> Subject: RE: [Idr] draft-ietf-idr-error-handling-08
> 
> I would totally disagree. Despite it's dynamic nature peering routes don't
> change (completely) nearly as often as people seem to think. Thus the whole
> concept of GR. Add to that the fact that ipv4 is "exhausted" churn isn't as
> constant as people seem to believe.  We need bgp to "heal" the network
> when one link goes down but if it does NOT the worse that happens is it
> doesn't self heal for the portion that is broken. A reset kills ALL the routes
> between bgp speakers not just the broken link.
> 
> I'd say 'lost-NLRI' is worse than session reset. You end up with stale routes
> (forwarding to invalid nexthop, importing to the wrong VRF, etc).

---

From: Jeffrey Haas <[email-masked]>
Date: Fri, 23 May 2014 18:18:31 -0400

> I think the default behavior of session reset and its effects that scale from unpleasant to disastrous, are one of the main reasons why we are having this discussion in a first place, so while we a…

The level of disaster is a matter of perspective.  If you're the ISP in
question, perhaps you think keeping the session up is the right thing to do.

If you're a SP that's multiple AS hops away, and stale state creeps into the
routing system because we stopped resetting sessions when known garbage has
hit routing, then your customers may suffer from effectively persistent
blackholes until you figure out what AS has the toxic route and do an AS-map
to depref routes that go through them.

Today, our evils are either local reset which impact SPs multiple hops away
but at least take potentially bad stuff out of the system or our new
proposal which is to hope we did the right thing and flag it
adminsitratively.

Only history will show whether this was a good idea or not.  Meanwhile, it
makes the customers happier.

-- Jeff

---

From: Robert Raszuk <[email-masked]>
Date: Sat, 24 May 2014 01:43:17 +0200

> question, perhaps you think keeping the session up is the right thing to do.

Really ?

Let's not forget about history which brought us here.

It all started when there were a few big SPs who's L3VPN customers
suffered domain wide outages due to some vendor bugs modulo weak BGP
design of the network.

Indeed in L2/L3VPNs especially when your site is single homed some
junk injected by another VPN site may go through PE and impact the
entire service (often via RRs).

In those cases it is clear that resetting the session is disaster and
work has started what can we do about it.

However the proposed solution spreaded IMHO well too far, by becoming
SAFI agnostic.

Correctly engineered IP BGP ISP network never has single path for
Internet destination either at all BGP speakers or at RRs. Moreover it
is very good practice to have RRs running different BGP
implementations. So session reset when applied with GR (for data plane
protection) is a good way to keep IPv4 or IPv6 routing healthy.

It heals via selection of new BGP path and control plane and
forwarding state gets updated accordingly locally to a given AS.

If anything more work should be prioritized on helping troubleshooting
the reason for session reset by BGP Diagnostic messages.

Now because L3VPN users are carried on single engine planes and when
that one goes down they all fall - we are proposing to allow Internet
users which fly on multi-engine machines to continue to fly when some
parts of the plane brake. Sure if this is optional transitive
attribute we could perhaps compare it to failure of entertainment
system on board, but going further and ignoring number of much more
serious red lights in the cockpit I guess is a bit too far.

Cheers,
R.

---

From: Jeffrey Haas <[email-masked]>
Date: Sat, 24 May 2014 10:29:59 -0400

> suffered domain wide outages due to some vendor bugs modulo weak BGP

Yeah, we're all intimately aware of the fun from attr-128.  That specific
incident is worth its own cautionary I-D, I think.

> the reason for session reset by BGP Diagnostic messages.

Issues found locally are obviously something of concern.  My bigger concern
is that our "treat as withdraw" feature relies on correctly parsing NLRI
among the thing with problems.  We already have problems with BGP's stateful
nature not being executed correctly leaving stale routes in the system even
without this particular implementation.

Were it a viable feature, "BGP traceroute" is probably the better diagnostic
feature for some such issues.  SPs I've discussed this idea with are
generally not receptive to it.

-- Jeff

---

From: Robert Raszuk <[email-masked]>
Date: Sat, 24 May 2014 16:45:11 +0200

> My bigger concern
> among the thing with problems.

True. However let me restate my bigger concern with providing an example.

The error handling as defined today is SAFI independent. Moreover it
is implemented (at least in some BGP code basis) as AF-independent
enhancement.

For SAFI 128 in the SP core I guess this may be a brilliant solution.
However for PE-CE link running BGP SAFI 1 to exchange few routes it
should not be used at all in order to detect any trash when it tries
to enter the BGP control plane as early as possible resulting in such
specific VPN site to be brought down till NOC figures out the problem
(modulo auto restart knob we all have).

Same for ISP Internet v4/v6 where one has good BGP peerings design
which result in multiple paths to any BGP destination.

Unfortunately such option AFAIK is not possible to be enabled on a per
AF or per peer basis in the new error handling spec.

Thx,
R.

---

From: <[email-masked]>
Date: Mon, 26 May 2014 08:18:10 +0000

> From: Idr [mailto:[email-masked]] On Behalf Of Robert Raszuk > Sent: Saturday, May 24, 2014 4:45 PM
> 
> > My bigger concern
> > is that our "treat as withdraw" feature relies on correctly parsing
> > NLRI among the thing with problems.
> 
> The error handling as defined today is SAFI independent. Moreover it is
> implemented (at least in some BGP code basis) as AF-independent
> enhancement.
> 
> However for PE-CE link running BGP SAFI 1 to exchange few routes it should
> not be used at all in order to detect any trash when it tries to enter the BGP
> control plane as early as possible resulting in such specific VPN site to be
> brought down till NOC figures out the problem (modulo auto restart knob we
> all have).

I tend to agree with Robert that the requirement in term of level of reaction may be depend upon many parameters, such as the type of session (eBGP, iBGP client, iBGP), the number of (usufull/best) routes learnt over that session, the recent history of failures (from "I don't remember any serious one" to "we've have two majors failures in 2 months")... 
However adding such distinction increase the amount of complexity and bug, while we need a robust error handling mechanism.
IMO draft-ietf-idr-error-handling-08 is a good start that needs to be shipped. We probably need more experience with it, to better handle the tradeoffs.

> Same for ISP Internet v4/v6 where one has good BGP peerings design which
> result in multiple paths to any BGP destination.

My concern is not with the failure of a single BGP session, which indeed, if important, will be redundant.
My concern is that a single error can generate multiple failures and kill all redundancy. In multiple major events, the error was triggered by an attribute set by the originator/destination. Hence this attribute is attached to all routes to that destination. There is no redundant path not having this (perceived) error.

[...]

<kidding > 

> So session reset when applied with GR (for data plane protection) is a good way to keep IPv4 or IPv6 routing healthy.
I guess you mean :s/GR/persistence  ;-) because with GR, the restarted session will typically hit the exact same error.

</kidding > 

> Unfortunately such option AFAIK is not possible to be enabled on a per AF or
> per peer basis in the new error handling spec.
> 
> Thx,
> R.
>

---

From: Robert Raszuk <[email-masked]>
Date: Mon, 26 May 2014 12:18:50 +0200

Hi Bruno,

> I tend to agree with Robert that the requirement in term of level of
> reaction may be depend upon many parameters, such as the type
> of session (eBGP, iBGP client, iBGP), the number of (usufull/best)
> routes learnt over that session, the recent history of failures (from
> "I don't remember any serious one" to "we've have two majors
> failures in 2 months")...
> However adding such distinction increase the amount of
> complexity and bug, while we need a robust error handling
> mechanism.

Therefor I specifically listed IMO the most important one - per
neighbor SAFI based .

> if important, will be redundant.

But current draft does not have a way to allow for such reset upon
even a single error.

> My concern is that a single error can generate multiple failures and
> kill all redundancy. In multiple major events, the error was triggered
> by an attribute set by the originator/destination. Hence this attribute
> is attached to all routes to that destination. There is no redundant
> path not having this (perceived) error.

If malformed attribute made it to some hops away that means that BGP
implementations on the way did not consider it as fatal error to reset
the session.

So the fact that your BGP speaker(s) crashed is a sign of bad design
of the network element when traditionally a router/switch has only a
single control plane (single BGP/IGP implementation). That model as
proven fragile hopefully with new architectures will be changed to
allow multiple redundant control planes driving forwarding of any
network element. To some extend good networks already have vendor
redundancy today, but that is rather expensive solution.

Alternatively services like IPv4 lookup could be processed in DCs
allowing for much easier and cheaper redundancy.

> I guess you mean :s/GR/persistence  ;-) because with GR, the restarted session will typically hit the exact same error.

Nope. I meant only GR. And GR is not to wait for restart ... GR would
be to continue forwarding till new best paths are computed/received.

Best,
R.

---

From: <[email-masked]>
Date: Mon, 26 May 2014 16:20:46 +0000

Hi Robert,

> From: [email-masked] [mailto:[email-masked]] On Behalf Of Robert
> Raszuk > Sent: Monday, May 26, 2014 12:19 PM
> 
> Hi Bruno,
> 
> > reaction may be depend upon many parameters, such as the type of
> > session (eBGP, iBGP client, iBGP), the number of (usufull/best) routes
> > learnt over that session, the recent history of failures (from "I
> > don't remember any serious one" to "we've have two majors failures in
> > 2 months")...
> > However adding such distinction increase the amount of complexity and
> > bug, while we need a robust error handling mechanism.
> 
> Therefor I specifically listed IMO the most important one - per neighbor SAFI
> based .

AFI/SAFI is indeed a valid criteria but on my side, if we had multiple level of error handling (which I'm not asking for) I'd rather leave it to local configuration for flexibility (and because I would not expect rough consensus). 
 
> But current draft does not have a way to allow for such reset upon even a
> single error.

I guess the new behavior could be enabled/disabled through a local knob. However IMO the new behavior is a needed improvement, so I don't see why it should not be enabled (assuming logs are now monitored for errors at the update level, and not only at the session level)

> > by an attribute set by the originator/destination. Hence this
> > attribute is attached to all routes to that destination. There is no
> > redundant path not having this (perceived) error.
> 
> implementations on the way did not consider it as fatal error to reset the
> session.

Yes. Eventually there are the one which are wrong. And we don't need many hops to have a significant multiplier effect.

> So the fact that your BGP speaker(s) crashed is a sign of bad design of the
> network element when traditionally a router/switch has only a single control
> plane (single BGP/IGP implementation). That model as proven fragile
> hopefully with new architectures will be changed to allow multiple redundant
> control planes driving forwarding of any network element. To some extend
> good networks already have vendor redundancy today, but that is rather
> expensive solution.

That is indeed a possible medium term evolution. But in addition to multiple implementations to handle bugs, you may need multiple protocols to handle error in spec. And it's not a local only consideration as it may be your peer which have inserted the error (e.g. extend length issue) so you need parallel redundant sessions between BGP speakers.

Regardless, draft-ietf-idr-error-handling is required in the short term to increase BGP session robustness.
 
> Alternatively services like IPv4 lookup could be processed in DCs allowing for
> much easier and cheaper redundancy.
> 
> >> So session reset when applied with GR (for data plane protection) is a
> good way to keep IPv4 or IPv6 routing healthy.
> > I guess you mean :s/GR/persistence  ;-) because with GR, the restarted
> session will typically hit the exact same error.
> 
> Nope. I meant only GR. And GR is not to wait for restart ... GR would be to
> continue forwarding till new best paths are computed/received.
> 
> Best,
> R.

---

From: Robert Raszuk <[email-masked]>
Date: Mon, 26 May 2014 18:29:39 +0200

> AFI/SAFI is indeed a valid criteria but on my side, if we had
> multiple level of error handling (which I'm not asking for) I'd
> rather leave it to local configuration for flexibility (and because
> I would not expect rough consensus).

But that is precisely what I am after too.  Not #define that this SAFI
is subject or not, but a text in the draft that the error handling
should be possible to be disabled/enabled on a per AFI/SAFI per peer
basis.

Cheers,
R.

---

From: "Chris Hall" <[email-masked]>
Date: Mon, 26 May 2014 19:43:45 +0100

Robert Raszuk wrote (on Mon 26-May-2014 at 17:30 +0100):
> Bruno Decraene wrote:
> > rather leave it to local configuration for flexibility (and
> > because I would not expect rough consensus).

> But that is precisely what I am after too.  Not #define that this
> SAFI is subject or not, but a text in the draft that the error 
> handling should be possible to be disabled/enabled on a per 
> AFI/SAFI per peer basis.

Which would be possible when it's clear what the AFI/SAFI was.  In the
(previously noted) case of:

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

it's (ever so) slightly tricky to tell what the AFI/SAFI was :-(
Unless the session carries only one AFI/SAFI, of course.  

Chris

---

From: Robert Raszuk <[email-masked]>
Date: Mon, 26 May 2014 20:54:44 +0200

Hi Chris,

Isn't your example subject to reset of the session regardless what
AFI/SAFI it is ? The way I see attribute overrun + no parsable NLRI
present is that it does not fall under current error handling draft
exceptions.

r.

---

From: Vitkovský Adam <[email-masked]>
Date: Mon, 26 May 2014 21:02:51 +0000

> From: Idr [mailto:[email-masked]] On Behalf Of Robert Raszuk
> Sent: Monday, May 26, 2014 8:55 PM
> 
> Isn't your example subject to reset of the session regardless what AFI/SAFI it
> is ? The way I see attribute overrun + no parsable NLRI present is that it does
> not fall under current error handling draft exceptions.

Hello Robert,

This is the case where I'd like to see more effort before giving up and resetting the session. 
I don't like the idea of both upstream and downstream systems suffering from one misbehaving update. 
I guess maybe after receiving such an update BGP can request a resent of all the NLRIs from all the AFIs/SAFIs. 
And if it receives some meaningful information along with the garbage it can compare that to the last known received update. 
And afterwards it could only send withdraw for the damaged delta downstream. 

adam

---

From: "Chris Hall" <[email-masked]>
Date: Tue, 27 May 2014 11:36:26 +0100

Robert Raszuk wrote (on Mon 26-May-2014 at 19:55 +0100):
....
> present is that it does not fall under current error handling draft

I don't know :-(  My reading suggests that the default response is to be "treat-as-withdraw".  If no NLRI at all can be found, what the draft specifies seems to hinge on the interpretation of "successfully parsed": is parsing nothing at all trivially successful or trivially a failure ?  Some folk appear to want the session to stay up no matter what (more or less), so it might be helpful if this was less a matter of interpretation.

Assuming that "not finding any NLRI at all" is defined/deemed to require session-reset: what about the cases where the receiver can find *some* NLRI, but knows it has not found *all* NLRI ?  Suppose:

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

In this case it is pretty clear that there is an MP_REACH_NLRI buried in the rubble.

For this case, and other more ambiguous ones, we need to interpret: "the entire NLRI field and/or the MP_REACH_NLRI and MP_UNREACH_NLRI attributes need to be successfully parsed".

Chris

---

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

Hi guys,

I wanted to bring back to mind Ondrej's comments concerning section 3.c 
("Attribute Flag error handling") and his comments concerning handling 
of errors in the Next Hop field of MP_REACH_NLRI.

> not be checked (should be considered intependently on attribute type).

I agree with that interpretation of how attribute flags should be 
checked for validity. (Especially, considering the "extended length" 
flag as independent of the other flags and not checking that.)

Still, I would like to know how current implementations handle that and 
if we could find a (rough) consensus about how to formulate that point 
so that no misinterpretation could occur.

> (as long as NLRI part of MP_REACH_NLRI is parsed without errors).

Ondrej's comment concerning errors in the next-hop-field of 
MP_REACH_NLRI made me think about "similar actions for similar things" 
again.

The draft suggests to use treat-as-withdraw for any kind of error in a 
NEXT_HOP attribute:
  3.c. attribute flags error
  3.d. missing well-know attribute (if not used MP_REACH_NLRI)
  3.e. other errors originally resulting in session-reset

To me it looks logical that errors in MP_REACH_NLRI which (only) concern 
the Next Hop field should be treated uniformly and therefore result in 
treat-as-withdraw. One could actually see this as part of clause 3.d. 
because the Next Hop field in MP_REACH_NLRI takes the role of the 
well-known attribute NEXT_HOP.

Because of that I would support Ondrej's suggestions.

However, that brings me to a "bonus"-question:

Section 7 ("Error Handling") of RFC 4760 ("Multiprotocol Extension") 
states that "if the speaker determines that the attribute is incorrect, 
the speaker MUST delete all the BGP routes received from that neighbor 
whose AFI/SAFI is the same as the one carried in the incorrect 
MP_REACH_NLRI or MP_UNREACH_NLRI attribute."
When talking about "treat-as-withdraw" in the draft we are still meaning 
to *only* withdraw the NLRI information enclosed in the current 
UPDATE-message and not the ones received from earlier UPDATE-messages, 
correct?

Therefore section 7 of RFC 4760 does not apply here --even with the 
above described different handling of erroneous Next Hop field in 
MP_REACH_NLRI-- and MAY only be used if parsing the NLRI from 
MP_{UN}REACH_NLRI could not be parsed successfully (as section 3.j. 
mentions)!?

Looking forward to your comments.
Deniz

PS: It seems, I was a little fast in my earlier email with stating to 
not have any more comments for draft 09, when I still had some left for 
08. :-)

---

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

Deniz Bahadir wrote (on Wed 28-May-2014 at 15:14 +0100):
....
> I wanted to bring back to mind Ondrej's comments concerning section
> 3.c ("Attribute Flag error handling") and his comments concerning
> handling of errors in the Next Hop field of MP_REACH_NLRI.
> 
....
> > Should the zeroness of Partial (P) bit for other than optional
> > transitive atributes be checked? For all or just for recognized
> > attributes?
> >
> > And what if i receive unrecognized attribute without optional
> > flag?  That is cleary broken but not covered by the formulation
> > from the draft (or from the RFC 4271).
> >
> > optional transitive) should be checked for all atributes,
> > additionally T bit should be checked for recognized attributes.
> > EL bit should not be checked (should be considered intependently
> > on attribute type).

> Still, I would like to know how current implementations handle that
> and if we could find a (rough) consensus about how to formulate
> that point so that no misinterpretation could occur.

My understanding of RFC4271 is that the "well-known" attributes are
those listed at the end of the introduction to Section 5, on p25.  And
that all other attributes are "optional".

Section 4.3 on p16-17 specifies for the Optional (O), Transitive (T)
and Partial (P) bits:

  * for "well-known" O=0 T=1 P=0

  * for "optional"   O=1 T=0 P=0
                  or O=1 T=1 P=X (0 or 1)

>From this, I understand: 
 
  (a) you MUST recognise the "well-known" attributes (O=0);

      Hence: any attributes you do not recognise MUST be 
             "optional" (O=1); 

  (b) many "optional" (O=1) attributes may be very common, and 
      recognised by 99.9% of implementations, but that does not
      make them "well-known", within the meaning of the RFC; 

  (c) the state of T and P is independent of whether you 
      recognise the attribute or not; 

  (d) O=0 T=1 does *not* mean "required" and "transitive", it
      means "well-known".

      Whether the attribute is required inbound and/or outbound
      is part of its "well-known"-ness, and the attribute is
      said to be "mandatory"/"discretionary".

      T=1 notwithstanding, "transitive" is not a property of
      "well-known" attributes.

[You might wonder what the sender is doing if they send an O=1 T=0
attribute which you don't recognise, but the standard requires you
throw it away, anyway.  Go figure.]

[I agree that the Extended Length bit is independent of the Type.  I
have not found a requirement that the length be encoded in the minimum
form for any attribute, well-known or otherwise.]

The key, I think, is to realise that "well-known" has a very
restricted meaning here -- it means just what RFC4271 chooses it to
mean —- neither more nor less.  [Please correct me if I have
misunderstood this... I regret it took me a little while to untangle
what RFC4271 seemed to be saying... but it would not surprise me to
learn that the RFC is vastly more subtle than I have so far been able
to discern.]

...
> To me it looks logical that errors in MP_REACH_NLRI which (only)
> concern the Next Hop field should be treated uniformly and 
> therefore result in treat-as-withdraw. One could actually see 
> this as part of clause 3.d.  because the Next Hop field in 
> MP_REACH_NLRI takes the role of the well-known attribute
> NEXT_HOP.
> 

So the draft says "the MP_REACH_NLRI ... need to be successfully
parsed", otherwise "the 'session reset' approach SHOULD be followed".
(I keep banging on about this, but it does seem fundamental.  Note
carefully: SHOULD, not MUST... so your implementation can do something
clever if it wants to !  Though there are MUSTs elsewhere which limit
the wiggle room.)

Now, your MP_REACH_NLRI attribute looks like this:

        +---------------------------------------------------------+
        | Address Family Identifier (2 octets)                    |
        +---------------------------------------------------------+
        | Subsequent Address Family Identifier (1 octet)          |
        +---------------------------------------------------------+
        | Length of Next Hop Network Address (1 octet)            |
        +---------------------------------------------------------+
        | Network Address of Next Hop (variable)                  |
        +---------------------------------------------------------+
        | Reserved (1 octet)                                      |
        +---------------------------------------------------------+
        | Network Layer Reachability Information (variable)       |
        +---------------------------------------------------------+

For many AFI/SAFI the next-hop-length is expected to be one of a small
number of lengths.  The next-hop-length here specifies the start of
the NLRI.  So if the next-hop-length is not valid, it is possible that
the number of octets of next hop sent is, nevertheless, correct.  In
this case, we might have to be deeply unlucky to find an apparently
valid set of NLRI... but if we did it would be incomplete or contain
rubbish.

The length of the NEXT_HOP attribute, on the other hand, does not
specify the start of the NLRI.

This is not to say that an invalid/unexpected next-hop-length should
mean that the MP_REACH_NLRI is "syntactically incorrect".  But, I
don't think that you can treat the Next-Hop here in exactly the same
way as the NEXT-HOP attribute, just because they both specify a
next-hop :-(

....
> MP_{UN}REACH_NLRI could not be parsed successfully (as section 3.j.

The draft does say that it revises RFC4760, but unlike the other RFCs
it revises, the effect on RFC4760 is less directly specified.

Section 3j of the draft offers the "disabling a particular AFI/SAFI",
as specified in RFC4270, as an alternative to "session-reset", if
treat-as-withdraw is not possible.

Were you to suggest that "disable AFI/SAFI" should be covered in
Section 2 of the draft, then I would say amen.  Were you to suggest
that Section 6.13 could directly reference RFC4760 and summarise the
effect of the draft on it, then I would say amen, again.

Chris

---

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

> clever if it wants to !  Though there are MUSTs elsewhere which limit

You are actually reading more wiggle room than I’d intended. The quote with a bit more context:

       this entails is discussed in more detail in Section 4.  If this
       is not possible, the procedures of [RFC4271] continue to apply,
       meaning that the "session reset" approach SHOULD be followed.
       Alternatively the error handling procedures specified in
       [RFC4760] for disabling a particular AFI/SAFI MAY be followed.

My intention was that either you do a reset, or you disable that AFI/SAFI per RFC4760. I think given the demonstrated ambiguity of the formulation above, this should be changed to something like

       this entails is discussed in more detail in Section 4.  If this
       is not possible, the procedures of [RFC4271] continue to apply,
       meaning that the "session reset" approach
       (or the error handling procedures specified in
       [RFC4760] for disabling a particular AFI/SAFI) MUST be followed.

That is, unless the WG decides that something else should be done of course. 

—John

---

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

On May 28, 2014, at 3:28 PM, Chris Hall <[email-masked]> wrote:

> Section 2 of the draft, then I would say amen.  

Good idea. Will do.

> Were you to suggest

The problem with summarizing is usually that if you say the same thing more than once you run the risk of inconsistency, which hurts in a spec. There’s also the problem that i don’t have a summary in mind to put in that section. It sounds as though you do, though — please feel free to send it to the list for possible inclusion.

Thanks,

—John

---

From: "Chris Hall" <[email-masked]>
Date: Thu, 29 May 2014 21:30:46 +0100

John G. Scudder wrote (on Wed 28-May-2014 at 22:25 +0100):
> On May 28, 2014, at 3:28 PM, Chris Hall wrote:
....
> > Were you to suggest that Section 6.13 could directly 
> > reference RFC4760 and summarise the effect of the draft
> > on it, then I would say amen, again.

> The problem with summarizing is usually that if you say the same
> thing more than once you run the risk of inconsistency, which hurts
> in a spec. There’s also the problem that i don’t have a summary in
> mind to put in that section. It sounds as though you do, though —
> please feel free to send it to the list for possible inclusion.

Sure... it's no biggie, but for completeness the following references
the RFC (in the same way as other subsections of section 6) and
attempts to "close the loop", so that a reader of RFC4760 can see
where it and the draft interact.

--------------------------------------------------------
6.13.  MP_REACH_NLRI and MP_UNREACH_NLRI

   The handling of these attributes is discussed in Section 3 and
   Section 4.

     The error handling of [RFC4760] (Section 7) is revised as above,
   which, broadly speaking, replaces:

     "and if the speaker determines that the attribute is 
      incorrect" (in the words of RFC4760)

   by:

     "if it is not possible to 'successful parse' the attribute"
     (where 'successful parse' is defined above)

   Where an MP_REACH_NLRI or MP_UNREACH_NLRI can be successfully
   parsed, but is nevertheless "incorrect" then "treat-as-withdraw"
   MUST be used.
--------------------------------------------------------

However, having written this I cannot say I find it pretty.

Further, as others have observed, the handling of the Next-Hop part of
the MP_REACH_NLRI attribute does not seem to be covered in the draft.

Also, Section 6 covers attribute specific error handling, so why not
treat MP_xx_NLRI in the same way.

Thinking on... that may be done by (1) adjusting 4.2 from:

  Similarly, the MP_REACH_NLRI or MP_UNREACH_NLRI attribute of
  an update SHALL be considered to be incorrect if any of the
  following are true:

to:

  Similarly, the NLRI parts of MP_REACH_NLRI and MP_UNREACH_NLRI 
  attributes SHALL be considered to be incorrect if any of the
  following are true:

And (2) moving the last two bullets to a longer section 6.13, as
follows...

--------------------------------------------------------
6.13.  MP_REACH_NLRI and MP_UNREACH_NLRI

   The error handling of [RFC4760] (Section 7) is replaced by
   this specification.  The error handling of [RFC4760] includes
   the "disable-AFI/SAFI" approach (described in Section 2, 
   above) as an option.  That option is allowed in some cases
   in this specification, as an alternative to session-reset.

   These attributes SHALL be considered "syntactically incorrect"
   if any of the following are true:

   a. The attribute flags of the attribute are inconsistent with
      those specified in [RFC4760].

   b. The length of the MP_UNREACH_NLRI attribute is less than 3,
      or the length of the MP_REACH_NLRI attribute is less than 5.

   c. The "Length of Next Hop Network Address" overruns the end
      of the attribute, or does not leave room for the
      "Reserved" byte.

   d. The "Network Layer Reachability Information" in an
      MP_REACH_NLRI attribute or the "Withdrawn Routes" in an
      MP_UNREACH_NLRI attribute are "syntactically incorrect"
      as defined in section 4.2.

   In all these cases "session-reset" MUST be used, except for
   cases (c) and (d), where "disable-AFI/SAFI" MAY be used 
   instead.

   An MP_REACH_NLRI attribute SHALL be considered invalid if
   the "Length of Next Hop Network Address" is not valid for
   the given AFI/SAFI, or the "Network Address of Next Hop"
   is not valid, and "treat-as-withdraw" MUST be used.
-------------------------------------------------------- 

If the 'Length of Next Hop' is valid, but there are no NLRI then I
guess that's fine.

This retains the "strict" approach to the attribute flags, but is
"loose" in its attitude to invalid "Length of Next Hop Network
Address".

Where the MP_xx_NLRI attribute is the first attribute, then being
"strict" about the flags means that if there is something wrong
downstream of this point, at least it isn't wrong from the get go.

I'm not a big fan of taking an invalid "Length of Next Hop Network
Address" at face value... but if the NLRI part of the attribute checks
out (is not "syntactically incorrect"), then perhaps that is
sufficient.

And (3) recast 3i to be consistent with the above:

  i. [RFC4271] Section 6.3 specifies that:

        The NLRI field in the UPDATE message is checked for 
        syntactic validity. If the field is syntactically
        incorrect, then the Error Subcode MUST be set to
        Invalid Network Field.

     This continues to apply and is extended to the Withdrawn
     Routes field, so that where either field is not empty
     and is "syntactically incorrect" as specified in 
     Section 4.2, below, then session-reset MUST be used,
     but "disable-AFI/SAFI" MAY be used instead.

This version of 3i and the second 6.13 above cover all the cases where
broken NLRI trigger session-reset or disable-AFI/SAFI are mandated.
(OK: this makes "disable-AFI/SAFI" available where previously it
wasn't... but this seems symmetrical.)

And, finally, (4) that leaves 3j to say:

  j. Finally, we observe that in order to use the approach of 
     "treat-as-withdraw", the entire NLRI field and/or the 
     MP_REACH_NLRI and MP_UNREACH_NLRI attributes need to be
     successfully parsed.  If this is not possible, then
     session-reset MUST be used, but "disable-AFI/SAFI" MAY 
     be used instead, provided all AFI/SAFI in the message
     can be identified.

Frankly, I think this brings into sharp relief the ambiguity, given
the full generality of what RFC4271 and RFC4670 allow... that is,
between 1 and 4 sets of NLRI.

If, as previously discussed, it appears that we can de facto state
that "entire" here may (safely) be *deemed* to be satisfied if any one
of:

  o  the Withdrawn Routes and/or NLRI fields are not empty.

  o  there is a single attribute, and that is MP_UNREACH_NLRI.

  o  an MP_REACH_NLRI has been found.

then the ambiguity melts away.

Sadly, because current implementations place MP_REACH_NLRI at the end
of the attributes, it is vulnerable -- but there is nothing one can do
about that, except to observe that it limits the benefit of the new
error handling until it is widely adopted.

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

BTW... I note this second paragraph of section 4.1

   For all path attributes other than those specified as having an
   attribute length that may be zero it SHALL be considered a syntax
   error for the attribute to have a length of zero.  (Of the path
   attributes considered in this specification, only AS_PATH and
   ATOMIC_AGGREGATE may validly have an attribute length of zero.)

I have a feeling this is a hang-over from some previous text in which
a "syntax error" had particular meaning ?  I think this is superseded
by Section 6, and the general requirement that all documentation for
all attributes MUST specify how any and all errors are to be handled.

While I'm picking nits, Section 4 is titled "Parsing of NLRI Fields",
but section 4.1 is "Attribute Length Fields" in general.

Chris

---

From: "John G. Scudder" <[email-masked]>
Date: Thu, 29 May 2014 16:42:09 -0400

On May 28, 2014, at 10:14 AM, Deniz Bahadir <[email-masked]> wrote:

> Section 7 ("Error Handling") of RFC 4760 ("Multiprotocol Extension") states that "if the speaker determines that the attribute is incorrect, the speaker MUST delete all the BGP routes received from that neighbor whose AFI/SAFI is the same as the one carried in the incorrect MP_REACH_NLRI or MP_UNREACH_NLRI attribute."
> When talking about "treat-as-withdraw" in the draft we are still meaning to *only* withdraw the NLRI information enclosed in the current UPDATE-message and not the ones received from earlier UPDATE-messages, correct?
> 
> Therefore section 7 of RFC 4760 does not apply here --even with the above described different handling of erroneous Next Hop field in MP_REACH_NLRI-- and MAY only be used if parsing the NLRI from MP_{UN}REACH_NLRI could not be parsed successfully (as section 3.j. mentions)!?

Actually I think RFC 4760 S. 7 continues in force. Paraphrasing, it says “if an MP_ attribute is broken, either drop all the routes or do a session reset”. It only talks about the MP_ attributes.

draft-ietf-idr-error-handling says "MP_REACH_NLRI and MP_UNREACH_NLRI attributes need to be successfully parsed ... If this is not possible … the "session reset" approach (or the error handling procedures specified in [RFC4760] for disabling a particular AFI/SAFI) MUST be followed.”

These seem consistent to me. 

Thanks,

—John