]> git.proxmox.com Git - mirror_frr.git/commit - bgpd/bgp_attr.h
bgpd: Implement revised error handling for partial optional/trans. attributes
authorPaul Jakma <paul@quagga.net>
Tue, 23 Nov 2010 16:35:42 +0000 (16:35 +0000)
committerPaul Jakma <paul@quagga.net>
Mon, 21 Mar 2011 13:51:14 +0000 (13:51 +0000)
commitb881c7074bb698aeb1b099175b325734fc6e44d2
tree70b4816a083166bbf00c1f85f19a67df0c0a5948
parentc112af27ed8f158ecece0d73ce2016c166076c00
bgpd: Implement revised error handling for partial optional/trans. attributes

* BGP error handling generally boils down to "reset session". This was fine
  when all BGP speakers pretty much understood all BGP messages. However
  the increasing deployment of new attribute types has shown this approach
  to cause problems, in particular where a new attribute type is "tunneled"
  over some speakers which do not understand it, and then arrives at a speaker
  which does but considers it malformed (e.g. corruption along the way, or
  because of early implementation bugs/interop issues).

  To mitigate this drafts before the IDR (likely to be adopted) propose to
  treat errors in partial (i.e.  not understood by neighbour), optional
  transitive attributes, when received from eBGP peers, as withdrawing only
  the NLRIs in the affected UPDATE, rather than causing the entire session
  to be reset.  See:

   http://tools.ietf.org/html/draft-scudder-idr-optional-transitive

* bgp_aspath.c: (assegments_parse) Replace the "NULL means valid, 0-length
  OR an error" return value with an error code - instead taking
  pointer to result structure as arg.
  (aspath_parse) adjust to suit previous change, but here NULL really
  does mean error in the external interface.
* bgp_attr.h (bgp_attr_parse) use an explictly typed and enumerated
  value to indicate return result.
  (bgp_attr_unintern_sub) cleans up just the members of an attr, but not the
  attr itself, for benefit of those who use a stack-local attr.
* bgp_attr.c: (bgp_attr_unintern_sub) split out from bgp_attr_unintern
  (bgp_attr_unintern) as previous.
  (bgp_attr_malformed) helper function to centralise decisions on how to
  handle errors in attributes.
  (bgp_attr_{aspathlimit,origin,etc..}) Use bgp_attr_malformed.
  (bgp_attr_aspathlimit) Subcode for error specifc to this attr should be
  BGP_NOTIFY_UPDATE_OPT_ATTR_ERR.
  (bgp_attr_as4_path) be more rigorous about checks, ala bgp_attr_as_path.
  (bgp_attr_parse) Adjust to deal with the additional error level that
  bgp_attr_ parsers can raise, and also similarly return appropriate
  error back up to (bgp_update_receive). Try to avoid leaking as4_path.
* bgp_packet.c: (bgp_update_receive) Adjust to deal with BGP_ATTR_PARSE_WITHDRAW
  error level from bgp_attr_parse, which should lead to a withdraw, by
  making the attribute parameter in call to (bgp_nlri_parse) conditional
  on the error, so the update case morphs also into a withdraw.
  Use bgp_attr_unintern_sub from above, instead of doing this itself.
  Fix error case returns which were not calling bgp_attr_unintern_sub
  and probably leaking memory.
* tests/aspath_test.c: Fix to work for null return with bad segments
bgpd/bgp_aspath.c
bgpd/bgp_attr.c
bgpd/bgp_attr.h
bgpd/bgp_packet.c
tests/aspath_test.c