From 6b87f736187d1a40a6b4f8d4fe42716bac09e857 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Mon, 13 Apr 2015 09:50:00 +0200 Subject: [PATCH] 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 Acked-by: Paul Jakma --- bgpd/bgp_advertise.c | 8 +++++++- bgpd/bgp_advertise.h | 2 +- bgpd/bgp_route.c | 20 ++++++++++++++++++-- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/bgpd/bgp_advertise.c b/bgpd/bgp_advertise.c index 73b2619c26..de27513a42 100644 --- 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 ae6e6bd147..86c959478d 100644 --- 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 1b17dc36b5..d9dec8772b 100644 --- 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) -- 2.39.5