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

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

John G. Scudder wrote (on Fri 31-Aug-2012 at 17:24 +0100):
> By the way, since this is now in *IETF* last call, if you want your
> comments to be considered you should send them to [email-masked].
> Feel free to cc IDR and GROW if you like.

OK.  Message previously posted to [email-masked] follows:
----------------------------------------------------------------------
--------

OK.  Clearly I am late to the party, but here goes...

In section 2 the draft talks of three "sub-classes" of error:

  1) "...UPDATE message is considered invalid...due to an error
     within a path attribute...[which]...is syntactically valid..."

  2) "...where a valid NLRI attribute can be extracted..." 

  3) "...where an attribute is not able to be parsed..."

and of the "key requirement to maximize the number of cases in which
it is possible to extract NLRI from a BGP message".

The draft is not specific on what is "syntactically valid", or what is
a "completely parsed" a set of attributes.  However, section 2.1.2
enumerates some "Semantic BGP Errors", which imply a desire to accept
a wide range of very badly broken sets of attributes.

In that context, the proposal to place any MP_REACH_NLRI and/or
MP_UNREACH_NLRI at the start of the attributes is insufficient.

If it is possible to parse both MP_REACH_NLRI and MP_UNREACH_NLRI at
the start of the attributes, and at least the first attribute which
follows is completely valid, then it seems highly likely that all NLRI
can be extracted, no matter how badly broken the rest of the attribute
set is.

But what does it mean if that is not the case ?  It is not known
whether the sender will send attributes in the required order.  It is
not known what combination of 'Withdrawn Routes', 'Network Layer
Reachability Information', MP_REACH_NLRI and MP_UNREACH_NLRI may have
been sent.  So, does the fact that MP attributes are not visible mean
they are not there, or that they are obscured by some other broken
attribute ?

The requirements appear to say that almost any random sequence of
octets should be an acceptable set of attributes, provided that the
NLRI can be identified.  But relaxing the rules on what is an
individually valid attribute means that any (apparent) absence of MP
attribute(s) is more than a little ambiguous.  Accepting (say) an
ATOMIC_AGGREGATOR attribute with a length of (say) 421 makes me feel
more than a little queasy.  It does not contain any NLRI, but I would
not care to guess what earlier misbegotten attribute may have lead us
to this odd looking attribute, or what nonsense may follow, or what
the sender really meant to say, or whether an MP attribute is lost in
the noise.

Reliable identification of the NLRI might be achieved:

  1) as suggested in the draft, by sending MP_UNREACH_NLRI
     and MP_REACH_NLRI as the first attributes.

     But I would add the ORIGIN to that, so that the receiver
     sees:

       * no attributes at all -- hence no MP NLRI,

       * or, just an exactly correct ORIGIN -- signalling no
         MP NLRI,

       * or, MP_UNREACH_NLRI all on its own,

       * or, MP_REACH_NLRI followed by an exactly correct
         3 octets of ORIGIN attribute (as a sort of
         terminator)

       * or, all three.

     *PLUS*

       * a Capability that promises to do so, so that there
         is no ambiguity.

  2) or, more directly, by:

       * adding an UPDATEv2 message type

       * and Capability,

     to completely separate NLRI from Attributes.

Now, it is possible that no known BGP implementation out there sends
more than one form of NLRI in an UPDATE message.  So, the presence of
any one implies the absence of all others, and the presence of any
attribute other than MP_UNREACH_NLRI implies the presence of
MP_REACH_NLRI if there are no other NLRI.  If that is the assumption,
then it should be stated.

However, if the intent is to keep going despite (inter alia) problems
caused by malfunctioning software, then completely separating NLRI
from attributes seems to me the cleanest approach.

If the intent is for a "smart new" BGP implementation to be able to
avoid bringing down sessions when talking to a current BGP
implementation, then some more detailed analysis is required.  For
example, in 2.1.2 the draft defines this as a semantic error: 

    o Zero or invalid length errors in path attributes excluding those
      containing NLRI, or where the length of all path attributes
      contained within the UPDATE does not correspond to the total
path
      attributes length.  In this case, the NLRI can be correctly
      extracted, and hence acted upon.

but in the face of such broken-ness, how, exactly, can NLRI be
correctly extracted ?  (Without some guarantee of how NLRI are sent --
a guarantee which may be relied upon when the sender's attribute
handling is busted ?)

But, if it is expected that both ends of a BGP session must be "smart
new" implementations to get (full) improved error handling, why not
make a clean and complete separation of NLRI from attributes ?  One
can then stop struggling quite so hard to extract sense from nonsense
attributes.

Chris

---

From: "Chris Hall" <[email-masked]>
Date: Fri, 31 Aug 2012 17:47:58 +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 had never existed.

     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: "Chris Hall" <[email-masked]>
Date: Fri, 31 Aug 2012 18:06:30 +0100

Jakob Heitz wrote (on Fri 31-Aug-2012 at 17:20 +0100):
> On Aug 31, 2012, at 12:55 AM, "Chris Hall" wrote:

> That does not "completely separate". You can separate with a FIN/SYN
> or put them into different packets: use UDP instead of TCP. With
> TCP, you are always at the mercy of length fields.

Perhaps I got a little over-excited, and overstated the degree of
separation.

But by moving the NLRI up a level, out of the mish-mash of Attributes
(back to the top level of the UPDATE where they were originally), then
at least extracting the NLRI from the message is not tangled up with
trying to decide how to make sense of a broken set of attributes.
Particularly as the whole point of the exercise is to minimise the
impact of broken sets of attributes.

> I mean, if you are going to require both ends of a connection to
> upgrade...

The recommendation is that MP_REACH_NLRI and MP_UNREACH_NLRI are moved
to the front of the attributes, so that they are less vulnerable to
problems in other attributes.  That is an upgrade.  But, as I said, I
don't think that is sufficient.  The receiver needs to know that the
sender is going to do that -- because otherwise the absence of
MP_REACH_NLRI and MP_UNREACH_NLRI is ambiguous.  Which requires a
capability.  That is also an upgrade.  To be fully effective,
improvements to the error handling will involve an upgrade.  In which
case, I suggest, one could go back to the future and disentangle NLRI
from Attributes.

Chris

---

From: Jakob Heitz <[email-masked]>
Date: Fri, 31 Aug 2012 13:38:23 -0400

UPDATEv2 is also inadequate.

Either, you have a fix that does not require the other end to upgrade
or you fix it properly.

--
Jakob Heitz.

> -----Original Message-----
> From: Chris Hall [mailto:[email-masked]]
> Sent: Friday, August 31, 2012 10:07 AM
> To: [email-masked]
> Cc: [email-masked]; Jakob Heitz
> Subject: RE: [Idr] draft-ietf-grow-ops-reqs-for-bgp-error-handling-05
>

---

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

> UPDATEv2 is also inadequate.

If we are agreed that the current proposals are inadequate, then we
are agreed on something.

> Either, you have a fix that does not require the other end to
> upgrade or you fix it properly.

Without an upgrade to improve the ability to extract the NLRI when
Attributes are broken, the fix is incomplete.

I look forward to a proper fix.

Chris