]> git.proxmox.com Git - mirror_frr.git/commit
bgpd: Regularise bgp_update_receive, add missing notifies and checks
authorPaul Jakma <paul.jakma@hpe.com>
Thu, 4 Feb 2016 13:27:04 +0000 (13:27 +0000)
committerDonald Sharp <sharpd@cumulusnetworks.com>
Wed, 26 Oct 2016 13:36:08 +0000 (09:36 -0400)
commit96e52474fdee05140e323c800342b23b4d1da03c
tree11b4e8fc19704ba2b5cddb7dbc50b61e16f93cef
parent48a5452b5b1c947d34aedde154cc477e728e8d78
bgpd: Regularise bgp_update_receive, add missing notifies and checks

* bgp_packet.c: (bgp_update_receive) Lots of repeated code, doing same
  thing for each AFI/SAFI.  Except when it doesn't, e.g.  the IPv4/VPN
  case was missing the EoR bgp_clear_stale_route call - the only action
  really needed for EoR.

  Make this function a lot more regular, using common, AFI/SAFI
  independent blocks so far as possible.

  Replace the 4 separate bgp_nlris with an array, indexed by an enum.

  The distinct blocks that handle calling bgp_nlri_parse for each
  different AFI/SAFI can now be replaced with a loop.

  Transmogrify the nlri SAFI from the SAFI_MPLS_LABELED_VPN code-point
  used on the wire, to the SAFI_MPLS_VPN safi_t enum we use internally
  as early as possible.

  The existing code was not necessarily sending a NOTIFY for NLRI
  parsing errors, if they arose via bgp_nlri_sanity_check.  Send the
  correct NOTIFY - INVAL_NETWORK for the classic NLRIs and OPT_ATTR_ERR
  for the MP ones.

  EoR can now be handled in one block.  The existing code seemed broken
  for EoR recognition in a number of ways:

  1.  A v4/unicast EoR should be an empty UPDATE.  However, it seemed
     to be treating an UPDATE with attributes, inc.  MP REACH/UNREACH,
     but no classic NLRIs, as a v4/uni EoR.

  2.  For other AFI/SAFIs, it was treating UPDATEs with no classic
     withraw and with a zero-length MP withdraw as EoRs.  However, that
     would mean an UPDATE packet _with_ update NLRIs and a 0-len MP
     withdraw could be classed as an EoR.

  This seems to be loose coding leading to ambiguous protocol
  situations and likely incorrect behaviour, rather than simply being
  liberal.  Be more strict about checking that an UPDATE really is an
  EoR and definitely is not trying to update any NLRIs.

  This same loose EoR parsing was noted by Chris Hall previously on
  list.

  (bgp_nlri_parse) Front end NLRI parse function, to fan-out to the correct
  parser for the AFI/SAFI.

* bgp_route.c: (bgp_nlri_sanity_check) We try convert NLRI safi to
  internal code-point ASAP, adjust switch for that.  Leave the wire
  code point in for defensive coding.

  (bgp_nlri_parse) rename to bgp_nlri_parse_ip.

* tests/bgp_mp_attr_test.c: Can just use bgp_nlri_parse frontend.
bgpd/bgp_packet.c
bgpd/bgp_packet.h
bgpd/bgp_route.c
bgpd/bgp_route.h
tests/bgp_mp_attr_test.c