bgpd: track correct originator-id in reflected routes

ISSUE:

Suppose route1 and route2 received from route-reflector-client1 and client2
respectively have identical attributes. The current logic of creating the
adj-rib-out for a peer threads the 'adv' structures for both routes against
the same attribute. This results in 'bgp_update_packet()' to pack those
routes in the same UPDATE message with one attr structure formatted. The
originator-id is thus set according to the first route's received router id.
This is incorrect.

PATCH:

Fix bgp_announce_check() function to set the originator-id in the
advertising attr structure. Also, fix the attribute hash function and
compare function to consider originator-id. Otherwise attributes where all
fields except the originator-id are identical get merged into one memory
location.

Signed-off-by: Pradosh Mohapatra <pmohapat at cumulusnetworks.com>
Reviewed-by: Scott Feldman <sfeldma at cumulusnetworks.com>
Reviewed-by: Ken Yin <kyin at cumulusnetworks.com>
[DL: whitespace changes dropped]
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index feb073c662a29c79cdb1bd67a1ad8586467e77ad..a0dfc65dfa651565f413e6c2784fec6304fbee1a 100644 (file)
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -379,6 +379,7 @@ attrhash_key_make (void *p)
       MIX(extra->aggregator_addr.s_addr);
       MIX(extra->weight);
       MIX(extra->mp_nexthop_global_in.s_addr);
+      MIX(extra->originator_id.s_addr);
     }
   
   if (attr->aspath)
@@ -434,7 +435,8 @@ attrhash_cmp (const void *p1, const void *p2)
           && IPV4_ADDR_SAME (&ae1->mp_nexthop_global_in, &ae2->mp_nexthop_global_in)
           && ae1->ecommunity == ae2->ecommunity
           && ae1->cluster == ae2->cluster
-          && ae1->transit == ae2->transit)
+          && ae1->transit == ae2->transit
+          && IPV4_ADDR_SAME (&ae1->originator_id, &ae2->originator_id))
         return 1;
       else if (ae1 || ae2)
         return 0;

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 7f68b8d0aa4eafa876a7d92a6211c760f3c15c5c..0428531f77c79321e60557a182a7345578be756a 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -965,6 +965,17 @@ bgp_announce_check (struct bgp_info *ri, struct peer *peer, struct prefix *p,
       attr->local_pref = bgp->default_local_pref;
     }
 
+  /* If originator-id is not set and the route is to be reflected,
+     set the originator id */
+  if (peer && from && peer->sort == BGP_PEER_IBGP &&
+      from->sort == BGP_PEER_IBGP &&
+      (! (attr->flag & ATTR_FLAG_BIT(BGP_ATTR_ORIGINATOR_ID))))
+    {
+      attr->extra = bgp_attr_extra_get(attr);
+      IPV4_ADDR_COPY(&(attr->extra->originator_id), &(from->remote_id));
+      SET_FLAG(attr->flag, BGP_ATTR_ORIGINATOR_ID);
+    }
+
   /* Remove MED if its an EBGP peer - will get overwritten by route-maps */
   if (peer->sort == BGP_PEER_EBGP
       && attr->flag & ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC))