From: Lou Berger Date: Tue, 12 Jan 2016 18:41:57 +0000 (-0500) Subject: bgpd: improve cleanup in bgp_delete() X-Git-Tag: frr-2.0-rc1~555 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=bb86c6017eccd03ea1554c7bdef5949cf676f208;p=mirror%2Ffrr.git bgpd: improve cleanup in bgp_delete() Signed-off-by: Lou Berger (cherry picked from commit 82dd707988b7481e203cab058c92f0b3041dd558) Conflicts: bgpd/bgp_nexthop.h bgpd/bgp_route.c bgpd/bgp_routemap.c bgpd/bgp_zebra.h bgpd/bgpd.c bgpd/bgpd.h --- diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c index d959076e59..d81c6a8052 100644 --- a/bgpd/bgp_nexthop.c +++ b/bgpd/bgp_nexthop.c @@ -145,6 +145,14 @@ bgp_address_init (struct bgp *bgp) bgp_address_hash_cmp); } +void +bgp_address_destroy (struct bgp *bgp) +{ + hash_clean(bgp->address_hash, NULL); + hash_free(bgp->address_hash); + bgp->address_hash = NULL; +} + static void bgp_address_add (struct bgp *bgp, struct prefix *p) { diff --git a/bgpd/bgp_nexthop.h b/bgpd/bgp_nexthop.h index a2d5ffdcc7..861da5740f 100644 --- a/bgpd/bgp_nexthop.h +++ b/bgpd/bgp_nexthop.h @@ -73,7 +73,6 @@ extern void bgp_connected_delete (struct bgp *bgp, struct connected *c); extern int bgp_multiaccess_check_v4 (struct in_addr, struct peer *); extern int bgp_config_write_scan_time (struct vty *); extern int bgp_nexthop_self (struct bgp *, struct attr *); -extern void bgp_address_init (struct bgp *); extern struct bgp_nexthop_cache *bnc_new(void); extern void bnc_free(struct bgp_nexthop_cache *bnc); extern void bnc_nexthop_free(struct bgp_nexthop_cache *bnc); @@ -81,5 +80,7 @@ extern char *bnc_str(struct bgp_nexthop_cache *bnc, char *buf, int size); extern void bgp_scan_init(struct bgp *bgp); extern void bgp_scan_finish(struct bgp *bgp); extern void bgp_scan_vty_init(void); +extern void bgp_address_init (struct bgp *bgp); +extern void bgp_address_destroy (struct bgp *bgp); #endif /* _QUAGGA_BGP_NEXTHOP_H */ diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index d9dec8772b..8b39f3a561 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -2997,6 +2997,56 @@ bgp_clear_route_all (struct peer *peer) bgp_clear_route (peer, afi, safi); } +/* + * Finish freeing things when exiting + */ +static void +bgp_drain_workqueue_immediate (struct work_queue *wq) +{ + if (!wq) + return; + + if (!wq->thread) + { + /* + * no thread implies no queued items + */ + assert(!wq->items->count); + return; + } + + while (wq->items->count) + { + if (wq->thread) + thread_cancel(wq->thread); + work_queue_run(wq->thread); + } +} + +/* + * Special function to process clear node queue when bgpd is exiting + * and the thread scheduler is no longer running. + */ +void +bgp_peer_clear_node_queue_drain_immediate(struct peer *peer) +{ + if (!peer) + return; + + bgp_drain_workqueue_immediate(peer->clear_node_queue); +} + +/* + * The work queues are not specific to a BGP instance, but the + * items in them refer to BGP instances, so this should be called + * before each BGP instance is deleted. + */ +void +bgp_process_queues_drain_immediate(void) +{ + bgp_drain_workqueue_immediate(bm->process_main_queue); +} + void bgp_clear_adj_in (struct peer *peer, afi_t afi, safi_t safi) { @@ -3050,37 +3100,54 @@ bgp_clear_stale_route (struct peer *peer, afi_t afi, safi_t safi) } } +static void +bgp_cleanup_table(struct bgp_table *table, safi_t safi) +{ + struct bgp_node *rn; + struct bgp_info *ri; + struct bgp_info *next; + + for (rn = bgp_table_top (table); rn; rn = bgp_route_next (rn)) + for (ri = rn->info; ri; ri = next) + { + next = ri->next; + if (CHECK_FLAG (ri->flags, BGP_INFO_SELECTED) + && ri->type == ZEBRA_ROUTE_BGP + && (ri->sub_type == BGP_ROUTE_NORMAL || + ri->sub_type == BGP_ROUTE_AGGREGATE)) + bgp_zebra_withdraw (&rn->p, ri, safi); + } +} + /* Delete all kernel routes. */ void bgp_cleanup_routes (void) { struct bgp *bgp; struct listnode *node, *nnode; - struct bgp_node *rn; - struct bgp_table *table; - struct bgp_info *ri; + afi_t afi; for (ALL_LIST_ELEMENTS (bm->bgp, node, nnode, bgp)) { - table = bgp->rib[AFI_IP][SAFI_UNICAST]; - - for (rn = bgp_table_top (table); rn; rn = bgp_route_next (rn)) - for (ri = rn->info; ri; ri = ri->next) - if (CHECK_FLAG (ri->flags, BGP_INFO_SELECTED) - && ri->type == ZEBRA_ROUTE_BGP - && (ri->sub_type == BGP_ROUTE_NORMAL || - ri->sub_type == BGP_ROUTE_AGGREGATE)) - bgp_zebra_withdraw (&rn->p, ri,SAFI_UNICAST); + for (afi = AFI_IP; afi < AFI_MAX; ++afi) + { + struct bgp_node *rn; - table = bgp->rib[AFI_IP6][SAFI_UNICAST]; + bgp_cleanup_table(bgp->rib[afi][SAFI_UNICAST], SAFI_UNICAST); - for (rn = bgp_table_top (table); rn; rn = bgp_route_next (rn)) - for (ri = rn->info; ri; ri = ri->next) - if (CHECK_FLAG (ri->flags, BGP_INFO_SELECTED) - && ri->type == ZEBRA_ROUTE_BGP - && (ri->sub_type == BGP_ROUTE_NORMAL || - ri->sub_type == BGP_ROUTE_AGGREGATE)) - bgp_zebra_withdraw (&rn->p, ri,SAFI_UNICAST); + /* + * VPN and ENCAP tables are two-level (RD is top level) + */ + for (rn = bgp_table_top(bgp->rib[afi][SAFI_MPLS_VPN]); rn; + rn = bgp_route_next (rn)) + if (rn->info) + { + bgp_cleanup_table((struct bgp_table *)(rn->info), SAFI_MPLS_VPN); + bgp_table_finish ((struct bgp_table **)&(rn->info)); + rn->info = NULL; + bgp_unlock_node(rn); + } + } } } diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index 8d1eb21010..60c406775d 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -306,4 +306,7 @@ extern int subgroup_announce_check(struct bgp_info *ri, struct update_subgroup *subgrp, struct prefix *p, struct attr *attr); +extern void bgp_peer_clear_node_queue_drain_immediate (struct peer *peer); +extern void bgp_process_queues_drain_immediate (void); + #endif /* _QUAGGA_BGP_ROUTE_H */ diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 17588eb3a4..b966b27c2b 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -14801,9 +14801,9 @@ bgp_vty_init (void) install_element (BGP_IPV6M_NODE, &neighbor_unsuppress_map_cmd); install_element (BGP_IPV6M_NODE, &no_neighbor_unsuppress_map_cmd); install_element (BGP_VPNV4_NODE, &neighbor_unsuppress_map_cmd); - install_element (BGP_VPNV4_NODE, &no_neighbor_unsuppress_map_cmd); + install_element (BGP_VPNV4_NODE, &no_neighbor_unsuppress_map_cmd); install_element (BGP_VPNV6_NODE, &neighbor_unsuppress_map_cmd); - install_element (BGP_VPNV6_NODE, &no_neighbor_unsuppress_map_cmd); + install_element (BGP_VPNV6_NODE, &no_neighbor_unsuppress_map_cmd); /* "neighbor maximum-prefix" commands. */ install_element (BGP_NODE, &neighbor_maximum_prefix_cmd); diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 0241be380f..06106d87f6 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -2094,3 +2094,13 @@ bgp_zebra_init (struct thread_master *master) bgp_nexthop_buf = stream_new(BGP_NEXTHOP_BUF_SIZE); bgp_ifindices_buf = stream_new(BGP_IFINDICES_BUF_SIZE); } + +void +bgp_zebra_destroy(void) +{ + if (zclient == NULL) + return; + zclient_stop(zclient); + zclient_free(zclient); + zclient = NULL; +} diff --git a/bgpd/bgp_zebra.h b/bgpd/bgp_zebra.h index 1770a2c0d5..1f845bd918 100644 --- a/bgpd/bgp_zebra.h +++ b/bgpd/bgp_zebra.h @@ -27,7 +27,8 @@ Boston, MA 02111-1307, USA. */ extern struct stream *bgp_nexthop_buf; extern struct stream *bgp_ifindices_buf; -extern void bgp_zebra_init(struct thread_master *master); +extern void bgp_zebra_init (struct thread_master *master); +extern void bgp_zebra_destroy (void); extern int bgp_if_update_all (void); extern int bgp_config_write_maxpaths (struct vty *, struct bgp *, afi_t, safi_t, int *); diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 06956b0ad6..eda43d2b60 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -2034,6 +2034,9 @@ peer_delete (struct peer *peer) XFREE(MTYPE_HOST, peer->domainname); peer->domainname = NULL; } + + if (CHECK_FLAG(bgp->flags, BGP_FLAG_DELETING)) + bgp_peer_clear_node_queue_drain_immediate(peer); peer_unlock (peer); /* initial reference */ @@ -3044,6 +3047,8 @@ bgp_delete (struct bgp *bgp) afi_t afi; int i; + SET_FLAG(bgp->flags, BGP_FLAG_DELETING); + THREAD_OFF (bgp->t_startup); if (BGP_DEBUG (zebra, ZEBRA)) @@ -3095,6 +3100,15 @@ bgp_delete (struct bgp *bgp) /* TODO - Other memory may need to be freed - e.g., NHT */ + /* + * Free pending deleted routes. Unfortunately, it also has to process + * all the pending activity for other instances of struct bgp. + * + * This call was added to achieve clean memory allocation at exit, + * for the sake of valgrind. + */ + bgp_process_queues_drain_immediate(); + /* Remove visibility via the master list - there may however still be * routes to be processed still referencing the struct bgp. */ diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index b3e508634e..06a50e228d 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -281,6 +281,7 @@ struct bgp #define BGP_FLAG_MULTIPATH_RELAX_AS_SET (1 << 17) #define BGP_FLAG_FORCE_STATIC_PROCESS (1 << 18) #define BGP_FLAG_SHOW_HOSTNAME (1 << 19) +#define BGP_FLAG_DELETING (1 << 20) /* BGP Per AF flags */ u_int16_t af_flags[AFI_MAX][SAFI_MAX]; diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index 75ce584818..4d862ed19d 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -361,7 +361,7 @@ vtysh_execute_func (const char *line, int pager) * to move into node in the vtysh where it succeeded. */ if (ret == CMD_SUCCESS || ret == CMD_SUCCESS_DAEMON || ret == CMD_WARNING) { - if ((saved_node == BGP_VPNV4_NODE || saved_node == BGP_VPNV6_NODE + if ((saved_node == BGP_VPNV4_NODE || saved_node == BGP_VPNV6_NODE || saved_node == BGP_IPV4_NODE || saved_node == BGP_IPV6_NODE || saved_node == BGP_IPV4M_NODE || saved_node == BGP_IPV6M_NODE)