bgpd: speed up "no-hit" withdraws for routeservers

This accelerates handling of incoming Withdraw messages for routes that
don't exist in the table to begin with.  Cisco IOS 12.4(24)T4 has a bug
in this regard - it sends withdraws instead of doing nothing for
prefixes that are filtered.

Pulling up the adj_in removal in Quagga should have no ill effect, but
we can avoid the costly iteration over all rsclients if there was no
adj_in entry.

Performance impact of this change on routeserver with 3 buggy peers,
startup/sync time:

before patch:  143.12 seconds (user cpu)
after patch:     7.01 seconds (user cpu)

Many thanks to Nick Hilliard & INEX for providing real-world test data!

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Acked-by: Paul Jakma <paul@jakma.org>

diff --git a/bgpd/bgp_advertise.c b/bgpd/bgp_advertise.c
index 73b2619c263d44f69dc0a1116e0cd55c2bd91462..de27513a4275d0217f9a44d4e5bcd9918588d374 100644 (file)
--- a/bgpd/bgp_advertise.c
+++ b/bgpd/bgp_advertise.c
@@ -222,7 +222,7 @@ bgp_adj_in_remove (struct bgp_node *rn, struct bgp_adj_in *bai)
   XFREE (MTYPE_BGP_ADJ_IN, bai);
 }
 
-void
+int
 bgp_adj_in_unset (struct bgp_node *rn, struct peer *peer,
                   u_int32_t addpath_id)
 {
@@ -230,6 +230,10 @@ bgp_adj_in_unset (struct bgp_node *rn, struct peer *peer,
   struct bgp_adj_in *adj_next;
 
   adj = rn->adj_in;
+
+  if (!adj)
+    return 0;
+
   while (adj)
     {
       adj_next = adj->next;
@@ -242,6 +246,8 @@ bgp_adj_in_unset (struct bgp_node *rn, struct peer *peer,
 
       adj = adj_next;
     }
+
+  return 1;
 }
 
 void

diff --git a/bgpd/bgp_advertise.h b/bgpd/bgp_advertise.h
index ae6e6bd14770a17674771a2946b0621808d5370d..86c959478d7f35f8af314886738a905fae30d40c 100644 (file)
--- a/bgpd/bgp_advertise.h
+++ b/bgpd/bgp_advertise.h
@@ -172,7 +172,7 @@ struct bgp_synchronize
 /* Prototypes.  */
 extern int bgp_adj_out_lookup (struct peer *, struct bgp_node *, u_int32_t);
 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 int 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 1b17dc36b5bcb5fe9a232f0936c479c8da9daa41..d9dec8772b7c0a39dfe7da8805b3b21aef349ea0 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -2561,10 +2561,26 @@ bgp_withdraw (struct peer *peer, struct prefix *p, u_int32_t addpath_id,
   rn = bgp_afi_node_get (bgp->rib[afi][safi], afi, safi, p, prd);
 
   /* If peer is soft reconfiguration enabled.  Record input packet for
-     further calculation. */
+   * further calculation.
+   *
+   * Cisco IOS 12.4(24)T4 on session establishment sends withdraws for all
+   * routes that are filtered.  This tanks out Quagga RS pretty badly due to
+   * the iteration over all RS clients.
+   * Since we need to remove the entry from adj_in anyway, do that first and
+   * if there was no entry, we don't need to do anything more.
+   */
   if (CHECK_FLAG (peer->af_flags[afi][safi], PEER_FLAG_SOFT_RECONFIG)
       && peer != bgp->peer_self)
-    bgp_adj_in_unset (rn, peer, addpath_id);
+    if (!bgp_adj_in_unset (rn, peer, addpath_id))
+      {
+        if (bgp_debug_update (peer, p, NULL, 1))
+          zlog_debug ("%s withdrawing route %s/%d "
+                     "not in adj-in", peer->host,
+                     inet_ntop(p->family, &p->u.prefix, buf, SU_ADDRSTRLEN),
+                     p->prefixlen);
+        bgp_unlock_node (rn);
+        return 0;
+      }
 
   /* Lookup withdrawn route. */
   for (ri = rn->info; ri; ri = ri->next)