Addpath does not work for soft-reconfiguration

diff --git a/bgpd/bgp_advertise.c b/bgpd/bgp_advertise.c
index bd60ca10a92130eb55fb769026bde1d598f374d3..70d878739ca668593dc898e0ff882e95a1c59e66 100644 (file)
--- a/bgpd/bgp_advertise.c
+++ b/bgpd/bgp_advertise.c
@@ -189,13 +189,14 @@ bgp_adj_out_lookup (struct peer *peer, struct prefix *p,
 
 
 void
-bgp_adj_in_set (struct bgp_node *rn, struct peer *peer, struct attr *attr)
+bgp_adj_in_set (struct bgp_node *rn, struct peer *peer, struct attr *attr,
+                u_int32_t addpath_id)
 {
   struct bgp_adj_in *adj;
 
   for (adj = rn->adj_in; adj; adj = adj->next)
     {
-      if (adj->peer == peer)
+      if (adj->peer == peer && adj->addpath_rx_id == addpath_id)
        {
          if (adj->attr != attr)
            {
@@ -208,6 +209,7 @@ bgp_adj_in_set (struct bgp_node *rn, struct peer *peer, struct attr *attr)
   adj = XCALLOC (MTYPE_BGP_ADJ_IN, sizeof (struct bgp_adj_in));
   adj->peer = peer_lock (peer); /* adj_in peer reference */
   adj->attr = bgp_attr_intern (attr);
+  adj->addpath_rx_id = addpath_id;
   BGP_ADJ_IN_ADD (rn, adj);
   bgp_lock_node (rn);
 }
@@ -222,19 +224,25 @@ bgp_adj_in_remove (struct bgp_node *rn, struct bgp_adj_in *bai)
 }
 
 void
-bgp_adj_in_unset (struct bgp_node *rn, struct peer *peer)
+bgp_adj_in_unset (struct bgp_node *rn, struct peer *peer,
+                  u_int32_t addpath_id)
 {
   struct bgp_adj_in *adj;
+  struct bgp_adj_in *adj_next;
 
-  for (adj = rn->adj_in; adj; adj = adj->next)
-    if (adj->peer == peer)
-      break;
+  adj = rn->adj_in;
+  while (adj)
+    {
+      adj_next = adj->next;
 
-  if (! adj)
-    return;
+      if (adj->peer == peer && adj->addpath_rx_id == addpath_id)
+        {
+          bgp_adj_in_remove (rn, adj);
+          bgp_unlock_node (rn);
+        }
 
-  bgp_adj_in_remove (rn, adj);
-  bgp_unlock_node (rn);
+      adj = adj_next;
+    }
 }
 
 void

diff --git a/bgpd/bgp_advertise.h b/bgpd/bgp_advertise.h
index a1474374704cc6e508c1f869db04d120778397aa..d5b737b9edd4cc5d95eecdc35d500eb82e2dc528 100644 (file)
--- a/bgpd/bgp_advertise.h
+++ b/bgpd/bgp_advertise.h
@@ -101,6 +101,9 @@ struct bgp_adj_in
 
   /* Received attribute.  */
   struct attr *attr;
+
+  /* Addpath identifier */
+  u_int32_t addpath_rx_id;
 };
 
 /* BGP advertisement list.  */
@@ -168,8 +171,8 @@ struct bgp_synchronize
 extern int bgp_adj_out_lookup (struct peer *, struct prefix *, afi_t, safi_t,
                        struct bgp_node *);
 
-extern void bgp_adj_in_set (struct bgp_node *, struct peer *, struct attr *);
-extern void bgp_adj_in_unset (struct bgp_node *, struct peer *);
+extern void bgp_adj_in_set (struct bgp_node *, struct peer *, struct attr *, u_int32_t);
+extern void bgp_adj_in_unset (struct bgp_node *, struct peer *, u_int32_t);
 extern void bgp_adj_in_remove (struct bgp_node *, struct bgp_adj_in *);
 
 extern void bgp_sync_init (struct peer *);

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 7a5f01a3d13be1108cfa0cd3ec5cf3b9e40ce654..2e572fe757837d1c675aea166d11a673f2f0694e 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -2882,7 +2882,7 @@ bgp_update_main (struct peer *peer, struct prefix *p, u_int32_t addpath_id,
      Adj-RIBs-In.  */
   if (! soft_reconfig && CHECK_FLAG (peer->af_flags[afi][safi], PEER_FLAG_SOFT_RECONFIG)
       && peer != bgp->peer_self)
-    bgp_adj_in_set (rn, peer, attr);
+    bgp_adj_in_set (rn, peer, attr, addpath_id);
 
   /* Check previously received route. */
   for (ri = rn->info; ri; ri = ri->next)
@@ -3281,7 +3281,7 @@ bgp_withdraw (struct peer *peer, struct prefix *p, u_int32_t addpath_id,
      further calculation. */
   if (CHECK_FLAG (peer->af_flags[afi][safi], PEER_FLAG_SOFT_RECONFIG)
       && peer != bgp->peer_self)
-    bgp_adj_in_unset (rn, peer);
+    bgp_adj_in_unset (rn, peer, addpath_id);
 
   /* Lookup withdrawn route. */
   for (ri = rn->info; ri; ri = ri->next)
@@ -3485,7 +3485,7 @@ bgp_soft_reconfig_table (struct peer *peer, afi_t afi, safi_t safi,
            struct bgp_info *ri = rn->info;
            u_char *tag = (ri && ri->extra) ? ri->extra->tag : NULL;
 
-           ret = bgp_update (peer, &rn->p, ri->addpath_rx_id, ain->attr,
+           ret = bgp_update (peer, &rn->p, ain->addpath_rx_id, ain->attr,
                               afi, safi, ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL,
                              prd, tag, 1);
 
@@ -3494,7 +3494,6 @@ bgp_soft_reconfig_table (struct peer *peer, afi_t afi, safi_t safi,
                bgp_unlock_node (rn);
                return;
              }
-           continue;
          }
       }
 }
@@ -3543,6 +3542,9 @@ bgp_clear_route_node (struct work_queue *wq, void *data)
   
   assert (rn && peer);
   
+  /* It is possible that we have multiple paths for a prefix from a peer
+   * if that peer is using AddPath.
+   */
   for (ri = rn->info; ri; ri = ri->next)
     if (ri->peer == peer || cnq->purpose == BGP_CLEAR_ROUTE_MY_RSCLIENT)
       {
@@ -3554,7 +3556,6 @@ bgp_clear_route_node (struct work_queue *wq, void *data)
           bgp_info_set_flag (rn, ri, BGP_INFO_STALE);
         else
           bgp_rib_remove (rn, ri, peer, afi, safi);
-        break;
       }
   return WQ_SUCCESS;
 }
@@ -3624,6 +3625,7 @@ bgp_clear_route_table (struct peer *peer, afi_t afi, safi_t safi,
     {
       struct bgp_info *ri;
       struct bgp_adj_in *ain;
+      struct bgp_adj_in *ain_next;
       struct bgp_adj_out *aout;
 
       /* XXX:TODO: This is suboptimal, every non-empty route_node is
@@ -3656,14 +3658,23 @@ bgp_clear_route_table (struct peer *peer, afi_t afi, safi_t safi,
        * Given that our per-peer prefix-counts now should be reliable,
        * this may actually be achievable. It doesn't seem to be a huge
        * problem at this time,
+       *
+       * It is possible that we have multiple paths for a prefix from a peer
+       * if that peer is using AddPath.
        */
-      for (ain = rn->adj_in; ain; ain = ain->next)
-        if (ain->peer == peer || purpose == BGP_CLEAR_ROUTE_MY_RSCLIENT)
-          {
-            bgp_adj_in_remove (rn, ain);
-            bgp_unlock_node (rn);
-            break;
-          }
+      ain = rn->adj_in;
+      while (ain)
+        {
+          ain_next = ain->next;
+
+          if (ain->peer == peer || purpose == BGP_CLEAR_ROUTE_MY_RSCLIENT)
+            {
+              bgp_adj_in_remove (rn, ain);
+              bgp_unlock_node (rn);
+            }
+
+          ain = ain_next;
+        }
 
       /*
        * Can't do this anymore. adj-outs are not maintained per peer.
@@ -3788,17 +3799,30 @@ bgp_clear_adj_in (struct peer *peer, afi_t afi, safi_t safi)
   struct bgp_table *table;
   struct bgp_node *rn;
   struct bgp_adj_in *ain;
+  struct bgp_adj_in *ain_next;
 
   table = peer->bgp->rib[afi][safi];
 
+  /* It is possible that we have multiple paths for a prefix from a peer
+   * if that peer is using AddPath.
+   */
   for (rn = bgp_table_top (table); rn; rn = bgp_route_next (rn))
-    for (ain = rn->adj_in; ain ; ain = ain->next)
-      if (ain->peer == peer)
-       {
-          bgp_adj_in_remove (rn, ain);
-          bgp_unlock_node (rn);
-          break;
-       }
+    {
+      ain = rn->adj_in;
+
+      while (ain)
+        {
+          ain_next = ain->next;
+
+          if (ain->peer == peer)
+            {
+              bgp_adj_in_remove (rn, ain);
+              bgp_unlock_node (rn);
+           }
+
+          ain = ain_next;
+        }
+    }
 }
 
 void