From: Pradosh Mohapatra <pmohapat@cumulusnetworks.com>
Date: Sat, 7 Sep 2013 07:13:37 +0000 (+0000)
Subject: bgpd: track correct originator-id in reflected routes
X-Git-Tag: frr-2.0-rc1~1598
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=689bb66c6a92d238bed1a8b0920438c5a2271966;p=mirror%2Ffrr.git

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 feb073c662..a0dfc65dfa 100644
--- 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 7f68b8d0aa..0428531f77 100644
--- 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))