Subject: Re: [GROW] [Idr] draft-ietf-grow-ops-reqs-for-bgp-error-handling-05

From: "Chris Hall" <[email-masked]>
Date: Fri, 31 Aug 2012 09:02:28 +0100

In trying to classify Errors with BGP-4 UPDATE Messages I think it
would be useful to distinguish between the form of an error and the
severity of that error and how BGP should respond.

It seems to me that there are four severities/responses:

  1) "Critical Error" -> drop/restart session or AFI/SAFI

     So the overall response then depends on how gracefully the
     session drop and/or restart can be handled.

  2) "Serious Error" -> do something with NLRI, short of
     dropping the session.

     The "treat-as-withdraw" mechanism is mentioned.

     The requirements obviously do not wish to specify mechanisms.

     But I think that the requirements should address what outcome
     is expected if errors in an individual UPDATE message are to
     be limited to that message.  I think what that means is:

       * it must be possible to identify all NLRI that the message
         could be carrying.

       * whatever is done with those NLRI must reflect the fact
         that the recipient has an incomplete, possibly empty,
         set of attributes for those NLRI.

  3) "Ignorable Error" -> process the UPDATE message as if the
     ignored attributes

     Some errors in some trivial attributes may be ignorable.
     The requirements could cover the criteria for being deemed
     trivial.

     Some errors in Optional Transitive may be dealt with by
     ignoring the attribute altogether.  The requirements
     mention this, but do not specify criteria for being
     ignorable.

  4) "Recoverable Error" -> process the UPDATE message which has
     had errors "patched up".

     The draft-ieft-idr-error-handling, for example, suggests
     that invalid Attribute Flags may simply be overwritten
     by the expected value.

I would then divide the forms of error into (1) "framing" and (2)
"content" (or "semantic").

A BGP UPDATE message has three levels of framing:

  * Level 1 -- the 16 octet "Marker" + Message Length
                                     + Withdrawn Routes Length
                                     + Total Path Attributes Length

    If the Message Length is broken, it is extremely likely that the
    "Marker" on the next message will be invalid.

  * Level 2(a) -- the Withdrawn Routes

    Each prefix must have a valid prefix length, and the last
    must run exactly to the end of this part of the message.

  * Level 2(b) -- the Attributes

    Each attribute must be correctly framed, and at the end of the
    attributes the last one must run to exactly the end of the
    attribute part of the message.

  * Level 2(c) -- the Network Layer Reachability Information.

    Same as 2(a).

  * Level 3 -- various Attributes

    Some attributes have internal framing.

So far, so obvious.  To judge if an individual attribute is properly
framed, we need to consider the red-tape:

  * the Flags octet has a limited set of valid values, depending
    on the Type.

  * the Type may be more or less anything, but repeats are not
    valid.
 
  * the Length is constrained for some Types

There is some redundancy here, more for known types than unknown ones,
which helps.  The Total Path Attributes Length is, effectively, a
checksum for all the Lengths of all the Attributes.  It would be
possible to specify that a set of attributes should be deemed
correctly framed solely on the basis of passing that test.  However,
my feeling is that all the available redundancy (such as it is) should
be used to minimise the possibility of accepting a broken attributes
-- *particularly* where an error is going to be treated as Ignorable.

Once attributes are correctly framed, then one can consider their
content.  Wherever the line between framing and content is drawn, I
think it helps to be clear about the distinction between them --
"framing" errors affect the attribute and the attributes around it,
"content" errors affect only the attribute.

The framing of an Optional Transitive is a special case.  If the
parser recognises an Optional Transitive, but its Length is not valid,
what should the receiver do ?  If the sender did not understand the
Attribute, then the broken Length is a "content" issue.  If the sender
did understand it, then the broken Length is a "framing" issue.  (It
is a serious disappointment to me that the Partial bit does not help
here.  But even if it did, what if the sender made a mess of
setting/clearing it !?)

In section 2.1.2 the draft specifies a number of "Semantic BGP
Errors", which includes many things which I would class as "framing"
errors.

This is all pretty low level stuff.  I can hear an argument that the
requirements document is not the place for this level of detail.
However, without a more precise understanding of how broken attributes
may be parsed, requirements for how to deal with them are hard to
specify and to interpret.

If NLRI were explicitly separate from the attributes, then if a set of
attributes fails a strict "framing" check, then "treat-as-withdraw"
(or equivalent) could be applied, reliably.  This seems to me to be as
safe as possible, short of dropping the session (which has its own
safety issues).

With NLRI mixed up in the attributes, either one plays safe and treats
all attribute errors as Critical, or a much more detailed analysis of
attribute parsing is required.  What is the cost of missing some NLRI
which were sent, but were obscured by some other broken attribute ?
What is the risk ?  What degree of broken-ness of an attribute can be
deemed not to invalidate the parsing of the attributes before and/or
after it ?  Is that different for different attributes ?

In order to contemplate classifying some attribute errors as
"Ignorable" or "Recoverable", a more detailed analysis of attribute
parsing is also required.  An ATOMIC_AGGREGATE attribute is arguably
trivial and Ignorable.  But is an ATOMIC_AGGREGATE attribute with a
length of 421 (say) likely to be a momentary lapse of concentration at
the sender end, or more likely to be a symptom of a badly broken set
of attributes ? 

Chris

---

From: Rob Shakir <[email-masked]>
Date: Fri, 31 Aug 2012 18:59:35 +0100

Hi Chris,

Thanks for this detailed analysis. It is akin to something that Alton Lo and I worked through whilst defining the critical and semantic error types (and suggested inclusions).

If you'll forgive me for responding to some particular points, I feel this might aid the discussion and positioning. I have added comments in-line marked [rjs].

> specify and to interpret.

[rjs]: What this draft intends to do is provide expectations, requirements and context for error handling in BGP-4, based on current deployments (and operator's experience). It also puts forwards requirements for how each type of error is reacted to in a broad sense. Essentially, where it came from is defining why amending the error handling behaviour is required, and providing a framework against which we can hang the different developments that are being discussed in IDR, such that they meet the operational challenges that come from amending this behaviour and form a complete set of solutions to meet the problem space.

[rjs]: I think the error handling solutions draft (draft-ietf-idr-error-handling) should take the work that we have done within IDR and GROW in this draft and build the next level of detail, which I think that you've made a great start to. I would like to try and keep the requirements draft such that it can be referred to by both existing attributes, and future ones.

> after it ?  Is that different for different attributes ?

[rjs]: Please note that the requirements draft does not present distinctions such as recoverable and ignorable. We went around this loop previously. I think that in some cases, some specific errors may be handled by 'patching' or 'ignoring' specific errors. But generically, these are exceptions - the requirements try and define broader categories, if a particular attribute needs something else (e.g., AS4_PATH may have information it can recover from other attributes) then this can be handled in error handling solution considerations of these attributes or as it is defined going forward.

[rjs]: It came to my attention whilst reviewing this the existing "ignore" wording in the draft is potentially erroneous based on rfc4271:

   All errors detected while processing the UPDATE message MUST be
   indicated by sending the NOTIFICATION message with the Error Code
   UPDATE Message Error.  The error subcode elaborates on the specific
   nature of the error.

[rjs]: Section 3's requirements discuss a need to avoid sending NOTIFICATION, and consider the proposed solution that exists. The only recommendation made is that treat-as-withdraw looks to be a safe behaviour for ensuring that one is not trusting UPDATEs that had some erroneous information in them. 

[rjs]: The requirement the document makes is explicitly that not all errors are defined as critical (if they were, the requirement specified by section 3 would not be met, and we would stick with the behaviour we have right now). The reason for a distinction between critical and semantic is that there are certain errors that mean that cannot be localised to certain NLRI.
 
[rjs]: I hope you do not see these comments as dismissive of what you have put together - I think that this is where operational and implementation views diverge. My view is that I need to understand what the impact to a service, the device and the network is during these error conditions (and balance the risk of incorrectness against the correctness of the protocol). From an implementation perspective, clearly, one needs to understand exactly which circumstances one can extract the NLRI, and the particulars of how this is achieved. I would encourage discussion that falls into the latter category such that we define the solutions draft to have the relevant guidance where required. Comments on the former should absolutely live in the requirements draft

[rjs]: I would of course encourage further feedback on this subject from the WG.

Kind regards,
r.

---

From: John Leslie <[email-masked]>
Date: Fri, 31 Aug 2012 10:54:28 -0400

> severity of that error and how BGP should respond.

   I welcome such a discussion!

>      session drop and/or restart can be handled.

   BGP was designed for a naive case where all your peers were either
dependable or broken. Moving beyond that has proven difficult. :^(

   But we should recognize the "fully broken" case, which amounts to
"drop the session and be in no hurry to try that peer again".

   Arguably, this should even require human intervention before trying
another session, but I have no horse in that race...

   It is indeed interesting to consider that one AFI/SAFI may be
"fully broken" while others seem dependable: perhaps we should consider
dropping only the part of a session concerning that AFI/SAFI...

>   2) "Serious Error" -> do something with NLRI, short of

   This is a bit vague...

>      The "treat-as-withdraw" mechanism is mentioned.

   This is appropriate when we have reason to believe the peer is
dependable, but is passing us NLRI routing information we're not
willing to trust.  IMHO, "not willing to trust" is in the eye of the
beholder...

   The default should be: we won't send such blocks to that peer
because we don't trust what they'd do with it. However, we might
indeed want to send it to that peer if we have no other route to
that destinatin. Thus, it seems it should actually be a disincentive
(possibly a binary disincentive, but presumably usually closer to
a nasty localpref).

   In brief, I'm not sure "treat-as-withdraw" is the right name...

>        * it must be possible to identify all NLRI that the message

   I read this to mean that all length+prefix must be readable
(but not necessarily considered valid), whether or not the path
attributes make sense.

   There is an inevitable grey area when the "total path attribute
length" looks dubious but not downright illegal. IMHO, we cannot
usefully standardize "dubious" here -- if we try, we'll find that
implementations differ anyway.

   So this statement sounds fine to me...

>          set of attributes for those NLRI.

   I take this to mean there's something about the attributes we
don't choose to trust: thus our picture of the "usable" attributes
differs from that of our peer. (Indeed, we may choose to trust
none of them.)

>   3) "Ignorable Error" -> process the UPDATE message as if the

   Some words seem to have gotten lost in tranport...

>      The requirements could cover the criteria for being deemed

   "trivial" may be an unfortunate word...

>      mention this, but do not specify criteria for being

   "Optional Transitive" is well-defined. Perhaps some advertised
capabilities are less well-defined...

>   4) "Recoverable Error" -> process the UPDATE message which has

   This sounds dangerous.

   The path attributes are expected to reflect how our peer came
upon this route. It used to be the case that regardless of the
attributes, we could be sure the peer would _use_ this route; but
we've been chipping away at that assurance.

   I'm very uncomfortable about "patching up" UPDATE information
and passing it to other peers without being fully clear about
whether there's a sufficiently long path that actually _uses_
the route.

>      that invalid Attribute Flags may simply be overwritten

   I confess to being less than comfortable with idr-error-handling.
So I'll let others discuss how wise this is...

>     "Marker" on the next message will be invalid.

   This sort of error clearly qualifies as "broken peer".

>     must run exactly to the end of this part of the message.

   This would appear to qualify as "broken peer"

>     attribute part of the message.

   To the extent that type-length-value triplets should match total
path attribute length, this failure would indicate "broken peer".

>     Some attributes have internal framing.

   Indeed!

>   * the Flags octet has a limited set of valid values, depending

   True, but probably not all BGP speakers check all of these...

>   * the Type may be more or less anything, but repeats are not

   Again, not all BGP speakers necessarily check this...

>   * the Length is constrained for some Types

   I suspect such checks are rather spotty...

> correctly framed solely on the basis of passing that test.

   This feels like the wrong question. Failing that test does indicate
a broken peer, but passing it doesn't prove the attributes safe to
send to a peer.

> However, my feeling is that all the available redundancy (such as
> it is) should be used to minimise the possibility of accepting a
> broken attributes

   Yes, I am uncomfortable here.

   It's probably a good practice to check such minutiae (even for
attributes you know you don't care about), but it's foolish to hope
we can get such checking to be uniform.

   And it's not clear what actual harm would come from inconsistent
checking of such minutiae...

> "content" errors affect only the attribute.

   I like that distinction (though my primary concern is distinguishing
a broken peer from a careless peer).

> what should the receiver do ?

   It depends on what the meaning of "not valid" is...

> If the sender did not understand the

   As I've said for years, "Error-correction is the most error-prone
operation in computing." :^(

> Errors", which includes many things which I would class as "framing"

   Indeed, I prefer to think in terms of "framing".

> specify and to interpret.

   Especially if we try to "correct" errors!

> after it ?  Is that different for different attributes ?

   This deserves discussion...

> the sender end, or more likely to be a symptom of a badly broken set

   That's something I'd guess most peers can't be bothered to check.

--
John Leslie <[email-masked]>

---

From: "Chris Hall" <[email-masked]>
Date: Fri, 31 Aug 2012 17:38:30 +0100

> Chris Hall <[email-masked]> wrote:
...

> BGP was designed for a naive case where all your peers were
> either dependable or broken. Moving beyond that has proven
> difficult. :^(

Certainly I have found it tricky when trying to do some of the things
in draft-ietf-idr-error-handling in Quagga !

> "drop the session and be in no hurry to try that peer again".

One of my concerns with improving error handling is that however
inconvenient dropping an entire session may be, and how disruptive
repeatedly dropping and restarting sessions is, at least it draws
attention to a problem.  If the error handling becomes too smooth, on
the other hand may hide problems.

>    Arguably, this should even require human intervention before
> trying another session, but I have no horse in that race...
> 
> "fully broken" while others seem dependable: perhaps we should
> consider dropping only the part of a session concerning that
> AFI/SAFI...

So, I guess by "Critical" what one really means is some error which is
so severe that all NLRI (possibly only for a subset of AFI/SAFI)
learned during the session are suspect.  Suspect because the current
UPDATE message refers to some NLRI (possibly in a subset of AFI/SAFI
for the session) are affected by the broken UPDATE, but it's not
possible to be sure which ones.

>    This is a bit vague...

So, I think the distinction between Critical and Serious is that for a
Serious error it is possible to identify the NLRI which the broken
UPDATE refers to, but not possible to identify enough of the
Attributes to be sure of the new state of those NLRI.  So, whatever
the problem is, it is limited to these NLRI, and all other NLRI may be
assumed to be unchanged.

What degree of proof is required to be sure (or sure enough) of being
able to identify the NLRI is the crux of the matter when it comes to
discussing how the parsing of attributes should proceed in the face of
errors.

> willing to trust.  IMHO, "not willing to trust" is in the eye of
> the beholder...

It would clearly be possible to treat all errors as prima facie
evidence that the peer has left the reservation and is unlikely to
return to their senses any time soon, so that the only sensible
response is to forget all their routes.

So, yes, I think you are correctly observing that there are a number
of possible responses to each of the different severities of error.
The distinction is that with a Serious error the response *can* be
limited to the NLRI in the UPDATE, but for a Critical error it cannot.

...

> (but not necessarily considered valid), whether or not the path

I think that this requirement is best met by separating NLRI from
Attributes.  Then, if all the NLRI have valid lengths, and those add
up to the total length of the NLRI section(s), then you have
identified the NLRI.  For me, anything short of that should be deemed
a Critical error.

...

> differs from that of our peer. (Indeed, we may choose to trust

Yes.

>    Some words seem to have gotten lost in tranport...

:-(  ...as if the ignored attributes were not there.

...

> capabilities are less well-defined...

What I was trying to get at is this: clearly a BGP implementation
which does not understand a given Optional Transitive attribute is
able to use the route(s) without it.  Perhaps this means that if an
implementation which does understand the attribute, but finds that it
is not valid in some way, could simply pretend not to understand it ?
Or, does some greater responsibility come with greater knowledge ?

>    This sounds dangerous.

Amen to that.

...
> > So far, so obvious.  To judge if an individual attribute is
> > properly framed, we need to consider the red-tape:
> >

>    I suspect such checks are rather spotty...

Hmmm.  I can only speak for the Quagga stuff I am working on, which
works through the attributes it knows about with a fine tooth comb.

...
> checking of such minutiae...

As I see it the problem is this: in order to be able to treat errors
in an UPDATE as Serious, but not Critical, it is essential that the
receiver be able to at least identify the NLRI with some (high?) level
of confidence.  While NLRI are mixed up with the Attributes (a bona
fide, first class, copper bottomed KLUDGE, IMNSHO) achieving that
requires some careful analysis of how Attributes are parsed when they
are broken.

Grinding out the detail is sort of an argument for separating NLRI
from Attributes.  The same detail is relevant to efforts to loosen
checks on Attributes, as suggested in draft-ietf-idr-error-handling.

The actual harm which could come from inadequate parsing of
Attributes, is that some NLRI in a broken UPDATE message could be
missed -- that is, some errors in an UPDATE could be treated as
Serious, where they should be treated as Critical.  It's possible that
the risk of such an outcome is acceptable... but I don't think the
requirements cover it.

...
> > "Ignorable" or "Recoverable", a more detailed analysis of
> > attribute parsing is also required.  An ATOMIC_AGGREGATE
> > attribute is arguably trivial and Ignorable.  But is an
> > ATOMIC_AGGREGATE attribute with length of 421 (say) likely 
> > to be a momentary lapse of concentration at the sender end,
> > or more likely to be a symptom of a badly broken set
> > of attributes ?

> That's something I'd guess most peers can't be bothered to check.

Goodness.  I do hope that's not the case :-(

Chris