From 228da42898c4f7bd72d9c1ee4135108e8d40d860 Mon Sep 17 00:00:00 2001 From: Chris Caputo Date: Sat, 18 Jul 2009 05:44:03 +0000 Subject: [bgpd] Stability fixes including bugs 397, 492 I've spent the last several weeks working on stability fixes to bgpd. These patches fix all of the numerous crashes, assertion failures, memory leaks and memory stomping I could find. Valgrind was used extensively. Added new function bgp_exit() to help catch problems. If "debug bgp" is configured and bgpd exits with status of 0, statistics on remaining lib/memory.c allocations are printed to stderr. It is my hope that other developers will use this to stay on top of memory issues. Example questionable exit: bgpd: memstats: Current memory utilization in module LIB: bgpd: memstats: Link List : 6 bgpd: memstats: Link Node : 5 bgpd: memstats: Hash : 8 bgpd: memstats: Hash Bucket : 2 bgpd: memstats: Hash Index : 8 bgpd: memstats: Work queue : 3 bgpd: memstats: Work queue item : 2 bgpd: memstats: Work queue name string : 3 bgpd: memstats: Current memory utilization in module BGP: bgpd: memstats: BGP instance : 1 bgpd: memstats: BGP peer : 1 bgpd: memstats: BGP peer hostname : 1 bgpd: memstats: BGP attribute : 1 bgpd: memstats: BGP extra attributes : 1 bgpd: memstats: BGP aspath : 1 bgpd: memstats: BGP aspath str : 1 bgpd: memstats: BGP table : 24 bgpd: memstats: BGP node : 1 bgpd: memstats: BGP route : 1 bgpd: memstats: BGP synchronise : 8 bgpd: memstats: BGP Process queue : 1 bgpd: memstats: BGP node clear queue : 1 bgpd: memstats: NOTE: If configuration exists, utilization may be expected. Example clean exit: bgpd: memstats: No remaining tracked memory utilization. This patch fixes bug #397: "Invalid free in bgp_announce_check()". This patch fixes bug #492: "SIGBUS in bgpd/bgp_route.c: bgp_clear_route_node()". My apologies for not separating out these changes into individual patches. The complexity of doing so boggled what is left of my brain. I hope this is all still useful to the community. This code has been production tested, in non-route-server-client mode, on a linux 32-bit box and a 64-bit box. Release/reset functions, used by bgp_exit(), added to: bgpd/bgp_attr.c,h bgpd/bgp_community.c,h bgpd/bgp_dump.c,h bgpd/bgp_ecommunity.c,h bgpd/bgp_filter.c,h bgpd/bgp_nexthop.c,h bgpd/bgp_route.c,h lib/routemap.c,h File by file analysis: * bgpd/bgp_aspath.c: Prevent re-use of ashash after it is released. * bgpd/bgp_attr.c: #if removed uncalled cluster_dup(). * bgpd/bgp_clist.c,h: Allow community_list_terminate() to be called from bgp_exit(). * bgpd/bgp_filter.c: Fix aslist->name use without allocation check, and also fix memory leak. * bgpd/bgp_main.c: Created bgp_exit() exit routine. This function frees allocations made as part of bgpd initialization and, to some extent, configuration. If "debug bgp" is configured, memory stats are printed as described above. * bgpd/bgp_nexthop.c: zclient_new() already allocates stream for ibuf/obuf, so bgp_scan_init() shouldn't do it too. Also, made it so zlookup is global so bgp_exit() can use it. * bgpd/bgp_packet.c: bgp_capability_msg_parse() call to bgp_clear_route() adjusted to use new BGP_CLEAR_ROUTE_NORMAL flag. * bgpd/bgp_route.h: Correct reference counter "lock" to be signed. bgp_clear_route() now accepts a bgp_clear_route_type of either BGP_CLEAR_ROUTE_NORMAL or BGP_CLEAR_ROUTE_MY_RSCLIENT. * bgpd/bgp_route.c: - bgp_process_rsclient(): attr was being zero'ed and then bgp_attr_extra_free() was being called with it, even though it was never filled with valid data. - bgp_process_rsclient(): Make sure rsclient->group is not NULL before use. - bgp_processq_del(): Add call to bgp_table_unlock(). - bgp_process(): Add call to bgp_table_lock(). - bgp_update_rsclient(): memset clearing of new_attr not needed since declarationw with "= { 0 }" does it. memset was already commented out. - bgp_update_rsclient(): Fix screwed up misleading indentation. - bgp_withdraw_rsclient(): Fix screwed up misleading indentation. - bgp_clear_route_node(): Support BGP_CLEAR_ROUTE_MY_RSCLIENT. - bgp_clear_node_queue_del(): Add call to bgp_table_unlock() and also free struct bgp_clear_node_queue used for work item. - bgp_clear_node_complete(): Do peer_unlock() after BGP_EVENT_ADD() in case peer is released by peer_unlock() call. - bgp_clear_route_table(): Support BGP_CLEAR_ROUTE_MY_RSCLIENT. Use struct bgp_clear_node_queue to supply data to worker. Add call to bgp_table_lock(). - bgp_clear_route(): Add support for BGP_CLEAR_ROUTE_NORMAL or BGP_CLEAR_ROUTE_MY_RSCLIENT. - bgp_clear_route_all(): Use BGP_CLEAR_ROUTE_NORMAL. Bug 397 fixes: - bgp_default_originate() - bgp_announce_table() * bgpd/bgp_table.h: - struct bgp_table: Added reference count. Changed type of owner to be "struct peer *" rather than "void *". - struct bgp_node: Correct reference counter "lock" to be signed. * bgpd/bgp_table.c: - Added bgp_table reference counting. - bgp_table_free(): Fixed cleanup code. Call peer_unlock() on owner if set. - bgp_unlock_node(): Added assertion. - bgp_node_get(): Added call to bgp_lock_node() to code path that it was missing from. * bgpd/bgp_vty.c: - peer_rsclient_set_vty(): Call peer_lock() as part of peer assignment to owner. Handle failure gracefully. - peer_rsclient_unset_vty(): Add call to bgp_clear_route() with BGP_CLEAR_ROUTE_MY_RSCLIENT purpose. * bgpd/bgp_zebra.c: Made it so zclient is global so bgp_exit() can use it. * bgpd/bgpd.c: - peer_lock(): Allow to be called when status is "Deleted". - peer_deactivate(): Supply BGP_CLEAR_ROUTE_NORMAL purpose to bgp_clear_route() call. - peer_delete(): Common variable listnode pn. Fix bug in which rsclient was only dealt with if not part of a peer group. Call bgp_clear_route() for rsclient, if appropriate, and do so with BGP_CLEAR_ROUTE_MY_RSCLIENT purpose. - peer_group_get(): Use XSTRDUP() instead of strdup() for conf->host. - peer_group_bind(): Call bgp_clear_route() for rsclient, and do so with BGP_CLEAR_ROUTE_MY_RSCLIENT purpose. - bgp_create(): Use XSTRDUP() instead of strdup() for peer_self->host. - bgp_delete(): Delete peers before groups, rather than after. And then rather than deleting rsclients, verify that there are none at this point. - bgp_unlock(): Add assertion. - bgp_free(): Call bgp_table_finish() rather than doing XFREE() itself. * lib/command.c,h: Compiler warning fixes. Add cmd_terminate(). Fixed massive leak in install_element() in which cmd_make_descvec() was being called more than once for the same cmd->strvec/string/doc. * lib/log.c: Make closezlog() check fp before calling fclose(). * lib/memory.c: Catch when alloc count goes negative by using signed counts. Correct #endif comment. Add log_memstats_stderr(). * lib/memory.h: Add log_memstats_stderr(). * lib/thread.c: thread->funcname was being accessed in thread_call() after it had been freed. Rearranged things so that thread_call() frees funcname. Also made it so thread_master_free() cleans up cpu_record. * lib/vty.c,h: Use global command_cr. Add vty_terminate(). * lib/zclient.c,h: Re-enable zclient_free(). --- bgpd/bgp_aspath.c | 1 + bgpd/bgp_attr.c | 34 +++++++++++ bgpd/bgp_attr.h | 1 + bgpd/bgp_clist.c | 2 +- bgpd/bgp_clist.h | 1 + bgpd/bgp_community.c | 7 +++ bgpd/bgp_community.h | 1 + bgpd/bgp_dump.c | 7 +++ bgpd/bgp_dump.h | 1 + bgpd/bgp_ecommunity.c | 7 +++ bgpd/bgp_ecommunity.h | 1 + bgpd/bgp_filter.c | 31 ++++++++++ bgpd/bgp_filter.h | 1 + bgpd/bgp_main.c | 109 ++++++++++++++++++++++++++++++++- bgpd/bgp_nexthop.c | 28 ++++++++- bgpd/bgp_nexthop.h | 1 + bgpd/bgp_packet.c | 2 +- bgpd/bgp_route.c | 164 +++++++++++++++++++++++++++++++------------------- bgpd/bgp_route.h | 12 +++- bgpd/bgp_table.c | 44 ++++++++++++-- bgpd/bgp_table.h | 8 ++- bgpd/bgp_vty.c | 18 +++++- bgpd/bgp_zebra.c | 2 +- bgpd/bgpd.c | 62 ++++++++++--------- 24 files changed, 435 insertions(+), 110 deletions(-) (limited to 'bgpd') diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c index 002fff9f..13f32b86 100644 --- a/bgpd/bgp_aspath.c +++ b/bgpd/bgp_aspath.c @@ -1799,6 +1799,7 @@ void aspath_finish (void) { hash_free (ashash); + ashash = NULL; if (snmp_stream) stream_free (snmp_stream); diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 82d907e2..94168372 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -148,6 +148,7 @@ cluster_free (struct cluster_list *cluster) XFREE (MTYPE_CLUSTER, cluster); } +#if 0 static struct cluster_list * cluster_dup (struct cluster_list *cluster) { @@ -166,6 +167,7 @@ cluster_dup (struct cluster_list *cluster) return new; } +#endif static struct cluster_list * cluster_intern (struct cluster_list *cluster) @@ -198,6 +200,13 @@ cluster_init (void) { cluster_hash = hash_create (cluster_hash_key_make, cluster_hash_cmp); } + +static void +cluster_finish (void) +{ + hash_free (cluster_hash); + cluster_hash = NULL; +} /* Unknown transit attribute. */ static struct hash *transit_hash; @@ -278,6 +287,13 @@ transit_init (void) { transit_hash = hash_create (transit_hash_key_make, transit_hash_cmp); } + +static void +transit_finish (void) +{ + hash_free (transit_hash); + transit_hash = NULL; +} /* Attribute hash routines. */ static struct hash *attrhash; @@ -435,6 +451,13 @@ attrhash_init (void) attrhash = hash_create (attrhash_key_make, attrhash_cmp); } +static void +attrhash_finish (void) +{ + hash_free (attrhash); + attrhash = NULL; +} + static void attr_show_all_iterator (struct hash_backet *backet, struct vty *vty) { @@ -2302,6 +2325,17 @@ bgp_attr_init (void) transit_init (); } +void +bgp_attr_finish (void) +{ + aspath_finish (); + attrhash_finish (); + community_finish (); + ecommunity_finish (); + cluster_finish (); + transit_finish (); +} + /* Make attribute packet. */ void bgp_dump_routes_attr (struct stream *s, struct attr *attr, diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index 12149a17..ed8753bd 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -140,6 +140,7 @@ struct transit /* Prototypes. */ extern void bgp_attr_init (void); +extern void bgp_attr_finish (void); extern int bgp_attr_parse (struct peer *, struct attr *, bgp_size_t, struct bgp_nlri *, struct bgp_nlri *); extern int bgp_attr_check (struct peer *, struct attr *); diff --git a/bgpd/bgp_clist.c b/bgpd/bgp_clist.c index 8d8c90c4..d6601674 100644 --- a/bgpd/bgp_clist.c +++ b/bgpd/bgp_clist.c @@ -829,7 +829,7 @@ community_list_init (void) } /* Terminate community-list. */ -static void +void community_list_terminate (struct community_list_handler *ch) { struct community_list_master *cm; diff --git a/bgpd/bgp_clist.h b/bgpd/bgp_clist.h index 6d7e363e..5dcb3b4c 100644 --- a/bgpd/bgp_clist.h +++ b/bgpd/bgp_clist.h @@ -125,6 +125,7 @@ extern struct community_list_handler *bgp_clist; /* Prototypes. */ extern struct community_list_handler *community_list_init (void); +extern void community_list_terminate (struct community_list_handler *); extern int community_list_set (struct community_list_handler *ch, const char *name, const char *str, int direct, diff --git a/bgpd/bgp_community.c b/bgpd/bgp_community.c index d40d69a2..ae1d7a15 100644 --- a/bgpd/bgp_community.c +++ b/bgpd/bgp_community.c @@ -636,3 +636,10 @@ community_init (void) comhash = hash_create ((unsigned int (*) (void *))community_hash_make, (int (*) (const void *, const void *))community_cmp); } + +void +community_finish (void) +{ + hash_free (comhash); + comhash = NULL; +} diff --git a/bgpd/bgp_community.h b/bgpd/bgp_community.h index aed7f330..bc1e56ef 100644 --- a/bgpd/bgp_community.h +++ b/bgpd/bgp_community.h @@ -52,6 +52,7 @@ struct community /* Prototypes of communities attribute functions. */ extern void community_init (void); +extern void community_finish (void); extern void community_free (struct community *); extern struct community *community_uniq_sort (struct community *); extern struct community *community_parse (u_int32_t *, u_short); diff --git a/bgpd/bgp_dump.c b/bgpd/bgp_dump.c index 53dea805..8087a403 100644 --- a/bgpd/bgp_dump.c +++ b/bgpd/bgp_dump.c @@ -865,3 +865,10 @@ bgp_dump_init (void) install_element (CONFIG_NODE, &dump_bgp_routes_interval_cmd); install_element (CONFIG_NODE, &no_dump_bgp_routes_cmd); } + +void +bgp_dump_finish (void) +{ + stream_free (bgp_dump_obuf); + bgp_dump_obuf = NULL; +} diff --git a/bgpd/bgp_dump.h b/bgpd/bgp_dump.h index 6bb1197b..e097c784 100644 --- a/bgpd/bgp_dump.h +++ b/bgpd/bgp_dump.h @@ -48,6 +48,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA #define TABLE_DUMP_V2_PEER_INDEX_TABLE_AS4 2 extern void bgp_dump_init (void); +extern void bgp_dump_finish (void); extern void bgp_dump_state (struct peer *, int, int); extern void bgp_dump_packet (struct peer *, int, struct stream *); diff --git a/bgpd/bgp_ecommunity.c b/bgpd/bgp_ecommunity.c index 6152a1db..8d5fa741 100644 --- a/bgpd/bgp_ecommunity.c +++ b/bgpd/bgp_ecommunity.c @@ -262,6 +262,13 @@ ecommunity_init (void) { ecomhash = hash_create (ecommunity_hash_make, ecommunity_cmp); } + +void +ecommunity_finish (void) +{ + hash_free (ecomhash); + ecomhash = NULL; +} /* Extended Communities token enum. */ enum ecommunity_token diff --git a/bgpd/bgp_ecommunity.h b/bgpd/bgp_ecommunity.h index 5c8deb56..942fdc73 100644 --- a/bgpd/bgp_ecommunity.h +++ b/bgpd/bgp_ecommunity.h @@ -66,6 +66,7 @@ struct ecommunity_val #define ecom_length(X) ((X)->size * ECOMMUNITY_SIZE) extern void ecommunity_init (void); +extern void ecommunity_finish (void); extern void ecommunity_free (struct ecommunity *); extern struct ecommunity *ecommunity_parse (u_int8_t *, u_short); extern struct ecommunity *ecommunity_dup (struct ecommunity *); diff --git a/bgpd/bgp_filter.c b/bgpd/bgp_filter.c index bdb756cb..8ee62b01 100644 --- a/bgpd/bgp_filter.c +++ b/bgpd/bgp_filter.c @@ -181,6 +181,11 @@ as_list_new (void) static void as_list_free (struct as_list *aslist) { + if (aslist->name) + { + free (aslist->name); + aslist->name = NULL; + } XFREE (MTYPE_AS_LIST, aslist); } @@ -198,6 +203,7 @@ as_list_insert (const char *name) /* Allocate new access_list and copy given name. */ aslist = as_list_new (); aslist->name = strdup (name); + assert (aslist->name); /* If name is made by all digit character. We treat it as number. */ @@ -693,3 +699,28 @@ bgp_filter_init (void) install_element (ENABLE_NODE, &show_ip_as_path_access_list_cmd); install_element (ENABLE_NODE, &show_ip_as_path_access_list_all_cmd); } + +void +bgp_filter_reset (void) +{ + struct as_list *aslist; + struct as_list *next; + + for (aslist = as_list_master.num.head; aslist; aslist = next) + { + next = aslist->next; + as_list_delete (aslist); + } + + for (aslist = as_list_master.str.head; aslist; aslist = next) + { + next = aslist->next; + as_list_delete (aslist); + } + + assert (as_list_master.num.head == NULL); + assert (as_list_master.num.tail == NULL); + + assert (as_list_master.str.head == NULL); + assert (as_list_master.str.tail == NULL); +} diff --git a/bgpd/bgp_filter.h b/bgpd/bgp_filter.h index d389f165..8c27a930 100644 --- a/bgpd/bgp_filter.h +++ b/bgpd/bgp_filter.h @@ -28,6 +28,7 @@ enum as_filter_type }; extern void bgp_filter_init (void); +extern void bgp_filter_reset (void); extern enum as_filter_type as_list_apply (struct as_list *, void *); diff --git a/bgpd/bgp_main.c b/bgpd/bgp_main.c index 620ca128..9d14683c 100644 --- a/bgpd/bgp_main.c +++ b/bgpd/bgp_main.c @@ -31,10 +31,22 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA #include "log.h" #include "privs.h" #include "sigevent.h" +#include "zclient.h" +#include "routemap.h" +#include "filter.h" +#include "plist.h" #include "bgpd/bgpd.h" #include "bgpd/bgp_attr.h" #include "bgpd/bgp_mplsvpn.h" +#include "bgpd/bgp_aspath.h" +#include "bgpd/bgp_dump.h" +#include "bgpd/bgp_route.h" +#include "bgpd/bgp_nexthop.h" +#include "bgpd/bgp_regex.h" +#include "bgpd/bgp_clist.h" +#include "bgpd/bgp_debug.h" +#include "bgpd/bgp_filter.h" /* bgpd options, we use GNU getopt library. */ static const struct option longopts[] = @@ -61,6 +73,8 @@ void sighup (void); void sigint (void); void sigusr1 (void); +static void bgp_exit (int); + static struct quagga_signal_t bgp_signals[] = { { @@ -182,7 +196,7 @@ sigint (void) if (! retain_mode) bgp_terminate (); - exit (0); + bgp_exit (0); } /* SIGUSR1 handler. */ @@ -191,6 +205,99 @@ sigusr1 (void) { zlog_rotate (NULL); } + +/* + Try to free up allocations we know about so that diagnostic tools such as + valgrind are able to better illuminate leaks. + + Zebra route removal and protocol teardown are not meant to be done here. + For example, "retain_mode" may be set. +*/ +static void +bgp_exit (int status) +{ + struct bgp *bgp; + struct listnode *node, *nnode; + int *socket; + struct interface *ifp; + extern struct zclient *zclient; + extern struct zclient *zlookup; + + /* it only makes sense for this to be called on a clean exit */ + assert (status == 0); + + /* reverse bgp_master_init */ + for (ALL_LIST_ELEMENTS (bm->bgp, node, nnode, bgp)) + bgp_delete (bgp); + list_free (bm->bgp); + + /* reverse bgp_master_init */ + for (ALL_LIST_ELEMENTS_RO(bm->listen_sockets, node, socket)) + { + if (close ((int)(long)socket) == -1) + zlog_err ("close (%d): %s", (int)(long)socket, safe_strerror (errno)); + } + list_delete (bm->listen_sockets); + + /* reverse bgp_zebra_init/if_init */ + if (retain_mode) + if_add_hook (IF_DELETE_HOOK, NULL); + for (ALL_LIST_ELEMENTS (iflist, node, nnode, ifp)) + if_delete (ifp); + list_free (iflist); + + /* reverse bgp_attr_init */ + bgp_attr_finish (); + + /* reverse bgp_dump_init */ + bgp_dump_finish (); + + /* reverse bgp_route_init */ + bgp_route_finish (); + + /* reverse bgp_route_map_init/route_map_init */ + route_map_finish (); + + /* reverse bgp_scan_init */ + bgp_scan_finish (); + + /* reverse access_list_init */ + access_list_add_hook (NULL); + access_list_delete_hook (NULL); + access_list_reset (); + + /* reverse bgp_filter_init */ + as_list_add_hook (NULL); + as_list_delete_hook (NULL); + bgp_filter_reset (); + + /* reverse prefix_list_init */ + prefix_list_add_hook (NULL); + prefix_list_delete_hook (NULL); + prefix_list_reset (); + + /* reverse community_list_init */ + community_list_terminate (bgp_clist); + + cmd_terminate (); + vty_terminate (); + if (zclient) + zclient_free (zclient); + if (zlookup) + zclient_free (zlookup); + + /* reverse bgp_master_init */ + if (master) + thread_master_free (master); + + if (zlog_default) + closezlog (zlog_default); + + if (CONF_BGP_DEBUG (normal, NORMAL)) + log_memstats_stderr ("bgpd"); + + exit (status); +} /* Main routine of bgpd. Treatment of argument and start bgp finite state machine is handled at here. */ diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c index 67a49f7a..0cde665e 100644 --- a/bgpd/bgp_nexthop.c +++ b/bgpd/bgp_nexthop.c @@ -65,7 +65,7 @@ static struct bgp_table *cache2_table[AFI_MAX]; static struct bgp_table *bgp_connected_table[AFI_MAX]; /* BGP nexthop lookup query client. */ -static struct zclient *zlookup = NULL; +struct zclient *zlookup = NULL; /* Add nexthop to the end of the list. */ static void @@ -1281,8 +1281,6 @@ bgp_scan_init (void) { zlookup = zclient_new (); zlookup->sock = -1; - zlookup->ibuf = stream_new (ZEBRA_MAX_PACKET_SIZ); - zlookup->obuf = stream_new (ZEBRA_MAX_PACKET_SIZ); zlookup->t_connect = thread_add_event (master, zlookup_connect, zlookup, 0); bgp_scan_interval = BGP_SCAN_INTERVAL_DEFAULT; @@ -1314,3 +1312,27 @@ bgp_scan_init (void) install_element (RESTRICTED_NODE, &show_ip_bgp_scan_cmd); install_element (ENABLE_NODE, &show_ip_bgp_scan_cmd); } + +void +bgp_scan_finish (void) +{ + bgp_table_unlock (cache1_table[AFI_IP]); + cache1_table[AFI_IP] = NULL; + + bgp_table_unlock (cache2_table[AFI_IP]); + cache2_table[AFI_IP] = NULL; + + bgp_table_unlock (bgp_connected_table[AFI_IP]); + bgp_connected_table[AFI_IP] = NULL; + +#ifdef HAVE_IPV6 + bgp_table_unlock (cache1_table[AFI_IP6]); + cache1_table[AFI_IP6] = NULL; + + bgp_table_unlock (cache2_table[AFI_IP6]); + cache2_table[AFI_IP6] = NULL; + + bgp_table_unlock (bgp_connected_table[AFI_IP6]); + bgp_connected_table[AFI_IP6] = NULL; +#endif /* HAVE_IPV6 */ +} diff --git a/bgpd/bgp_nexthop.h b/bgpd/bgp_nexthop.h index a8b92df6..2dad742f 100644 --- a/bgpd/bgp_nexthop.h +++ b/bgpd/bgp_nexthop.h @@ -47,6 +47,7 @@ struct bgp_nexthop_cache }; extern void bgp_scan_init (void); +extern void bgp_scan_finish (void); extern int bgp_nexthop_lookup (afi_t, struct peer *peer, struct bgp_info *, int *, int *); extern void bgp_connected_add (struct connected *c); diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index d98b689a..1c9a3c91 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -2193,7 +2193,7 @@ bgp_capability_msg_parse (struct peer *peer, u_char *pnt, bgp_size_t length) peer->afc_nego[afi][safi] = 0; if (peer_active_nego (peer)) - bgp_clear_route (peer, afi, safi); + bgp_clear_route (peer, afi, safi, BGP_CLEAR_ROUTE_NORMAL); else BGP_EVENT_ADD (peer, BGP_Stop); } diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 87fe7f5c..8dafd181 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -1464,11 +1464,9 @@ bgp_process_rsclient (struct work_queue *wq, void *data) struct bgp_info *new_select; struct bgp_info *old_select; struct bgp_info_pair old_and_new; - struct attr attr; struct listnode *node, *nnode; struct peer *rsclient = rn->table->owner; - memset (&attr, 0, sizeof (struct attr)); /* Best path selection. */ bgp_best_selection (bgp, rn, &old_and_new); new_select = old_and_new.new; @@ -1476,23 +1474,25 @@ bgp_process_rsclient (struct work_queue *wq, void *data) if (CHECK_FLAG (rsclient->sflags, PEER_STATUS_GROUP)) { - for (ALL_LIST_ELEMENTS (rsclient->group->peer, node, nnode, rsclient)) - { - /* Nothing to do. */ - if (old_select && old_select == new_select) - if (!CHECK_FLAG (old_select->flags, BGP_INFO_ATTR_CHANGED)) - continue; - - if (old_select) - bgp_info_unset_flag (rn, old_select, BGP_INFO_SELECTED); - if (new_select) - { - bgp_info_set_flag (rn, new_select, BGP_INFO_SELECTED); - bgp_info_unset_flag (rn, new_select, BGP_INFO_ATTR_CHANGED); - } - - bgp_process_announce_selected (rsclient, new_select, rn, afi, safi); - } + if (rsclient->group) + for (ALL_LIST_ELEMENTS (rsclient->group->peer, node, nnode, rsclient)) + { + /* Nothing to do. */ + if (old_select && old_select == new_select) + if (!CHECK_FLAG (old_select->flags, BGP_INFO_ATTR_CHANGED)) + continue; + + if (old_select) + bgp_info_unset_flag (rn, old_select, BGP_INFO_SELECTED); + if (new_select) + { + bgp_info_set_flag (rn, new_select, BGP_INFO_SELECTED); + bgp_info_unset_flag (rn, new_select, BGP_INFO_ATTR_CHANGED); + } + + bgp_process_announce_selected (rsclient, new_select, rn, + afi, safi); + } } else { @@ -1509,8 +1509,6 @@ bgp_process_rsclient (struct work_queue *wq, void *data) if (old_select && CHECK_FLAG (old_select->flags, BGP_INFO_REMOVED)) bgp_info_reap (rn, old_select); - bgp_attr_extra_free (&attr); - UNSET_FLAG (rn->flags, BGP_NODE_PROCESS_SCHEDULED); return WQ_SUCCESS; } @@ -1593,9 +1591,11 @@ static void bgp_processq_del (struct work_queue *wq, void *data) { struct bgp_process_queue *pq = data; + struct bgp_table *table = pq->rn->table; - bgp_unlock(pq->bgp); + bgp_unlock (pq->bgp); bgp_unlock_node (pq->rn); + bgp_table_unlock (table); XFREE (MTYPE_BGP_PROCESS_QUEUE, pq); } @@ -1641,10 +1641,12 @@ bgp_process (struct bgp *bgp, struct bgp_node *rn, afi_t afi, safi_t safi) sizeof (struct bgp_process_queue)); if (!pqnode) return; - - pqnode->rn = bgp_lock_node (rn); /* unlocked by bgp_processq_del */ + + /* all unlocked in bgp_processq_del */ + bgp_table_lock (rn->table); + pqnode->rn = bgp_lock_node (rn); pqnode->bgp = bgp; - bgp_lock(bgp); + bgp_lock (bgp); pqnode->afi = afi; pqnode->safi = safi; @@ -1805,8 +1807,6 @@ bgp_update_rsclient (struct peer *rsclient, afi_t afi, safi_t safi, const char *reason; char buf[SU_ADDRSTRLEN]; - //memset (new_attr, 0, sizeof (struct attr)); - /* Do not insert announces from a rsclient into its own 'bgp_table'. */ if (peer == rsclient) return; @@ -1894,10 +1894,10 @@ bgp_update_rsclient (struct peer *rsclient, afi_t afi, safi_t safi, inet_ntop(p->family, &p->u.prefix, buf, SU_ADDRSTRLEN), p->prefixlen, rsclient->host); - bgp_unlock_node (rn); - bgp_attr_unintern (attr_new); + bgp_unlock_node (rn); + bgp_attr_unintern (attr_new); - return; + return; } /* Withdraw/Announce before we fully processed the withdraw */ @@ -1992,13 +1992,13 @@ static void bgp_withdraw_rsclient (struct peer *rsclient, afi_t afi, safi_t safi, struct peer *peer, struct prefix *p, int type, int sub_type, struct prefix_rd *prd, u_char *tag) - { +{ struct bgp_node *rn; struct bgp_info *ri; char buf[SU_ADDRSTRLEN]; if (rsclient == peer) - return; + return; rn = bgp_afi_node_get (rsclient->rib[afi][safi], afi, safi, p, prd); @@ -2017,8 +2017,8 @@ bgp_withdraw_rsclient (struct peer *rsclient, afi_t afi, safi_t safi, p->prefixlen); /* Unlock bgp_node_get() lock. */ - bgp_unlock_node (rn); - } + bgp_unlock_node (rn); +} static int bgp_update_main (struct peer *peer, struct prefix *p, struct attr *attr, @@ -2432,7 +2432,7 @@ void bgp_default_originate (struct peer *peer, afi_t afi, safi_t safi, int withdraw) { struct bgp *bgp; - struct attr attr; + struct attr attr = { 0 }; struct aspath *aspath = { 0 }; struct prefix p; struct bgp_info binfo; @@ -2521,9 +2521,7 @@ bgp_announce_table (struct peer *peer, afi_t afi, safi_t safi, { struct bgp_node *rn; struct bgp_info *ri; - struct attr attr; - - memset (&attr, 0, sizeof (struct attr)); + struct attr attr = { 0 }; if (! table) table = (rsclient) ? peer->rib[afi][safi] : peer->bgp->rib[afi][safi]; @@ -2667,10 +2665,18 @@ bgp_soft_reconfig_in (struct peer *peer, afi_t afi, safi_t safi) bgp_soft_reconfig_table (peer, afi, safi, table); } + +struct bgp_clear_node_queue +{ + struct bgp_node *rn; + enum bgp_clear_route_type purpose; +}; + static wq_item_status bgp_clear_route_node (struct work_queue *wq, void *data) { - struct bgp_node *rn = data; + struct bgp_clear_node_queue *cnq = data; + struct bgp_node *rn = cnq->rn; struct peer *peer = wq->spec.data; struct bgp_info *ri; afi_t afi = rn->table->afi; @@ -2679,7 +2685,7 @@ bgp_clear_route_node (struct work_queue *wq, void *data) assert (rn && peer); for (ri = rn->info; ri; ri = ri->next) - if (ri->peer == peer) + if (ri->peer == peer || cnq->purpose == BGP_CLEAR_ROUTE_MY_RSCLIENT) { /* graceful restart STALE flag set. */ if (CHECK_FLAG (peer->sflags, PEER_STATUS_NSF_WAIT) @@ -2697,9 +2703,13 @@ bgp_clear_route_node (struct work_queue *wq, void *data) static void bgp_clear_node_queue_del (struct work_queue *wq, void *data) { - struct bgp_node *rn = data; + struct bgp_clear_node_queue *cnq = data; + struct bgp_node *rn = cnq->rn; + struct bgp_table *table = rn->table; bgp_unlock_node (rn); + bgp_table_unlock (table); + XFREE (MTYPE_BGP_CLEAR_NODE_QUEUE, cnq); } static void @@ -2707,10 +2717,10 @@ bgp_clear_node_complete (struct work_queue *wq) { struct peer *peer = wq->spec.data; - peer_unlock (peer); /* bgp_clear_node_complete */ - /* Tickle FSM to start moving again */ BGP_EVENT_ADD (peer, Clearing_Completed); + + peer_unlock (peer); /* bgp_clear_route */ } static void @@ -2739,7 +2749,8 @@ bgp_clear_node_queue_init (struct peer *peer) static void bgp_clear_route_table (struct peer *peer, afi_t afi, safi_t safi, - struct bgp_table *table, struct peer *rsclient) + struct bgp_table *table, struct peer *rsclient, + enum bgp_clear_route_type purpose) { struct bgp_node *rn; @@ -2792,21 +2803,30 @@ bgp_clear_route_table (struct peer *peer, afi_t afi, safi_t safi, * problem at this time, */ for (ri = rn->info; ri; ri = ri->next) - if (ri->peer == peer) + if (ri->peer == peer || purpose == BGP_CLEAR_ROUTE_MY_RSCLIENT) { - bgp_lock_node (rn); /* unlocked: bgp_clear_node_queue_del */ - work_queue_add (peer->clear_node_queue, rn); + struct bgp_clear_node_queue *cnq; + + /* both unlocked in bgp_clear_node_queue_del */ + bgp_table_lock (rn->table); + bgp_lock_node (rn); + cnq = XCALLOC (MTYPE_BGP_CLEAR_NODE_QUEUE, + sizeof (struct bgp_clear_node_queue)); + cnq->rn = rn; + cnq->purpose = purpose; + work_queue_add (peer->clear_node_queue, cnq); + break; } for (ain = rn->adj_in; ain; ain = ain->next) - if (ain->peer == peer) + if (ain->peer == peer || purpose == BGP_CLEAR_ROUTE_MY_RSCLIENT) { bgp_adj_in_remove (rn, ain); bgp_unlock_node (rn); break; } for (aout = rn->adj_out; aout; aout = aout->next) - if (aout->peer == peer) + if (aout->peer == peer || purpose == BGP_CLEAR_ROUTE_MY_RSCLIENT) { bgp_adj_out_remove (rn, aout, peer, afi, safi); bgp_unlock_node (rn); @@ -2817,7 +2837,8 @@ bgp_clear_route_table (struct peer *peer, afi_t afi, safi_t safi, } void -bgp_clear_route (struct peer *peer, afi_t afi, safi_t safi) +bgp_clear_route (struct peer *peer, afi_t afi, safi_t safi, + enum bgp_clear_route_type purpose) { struct bgp_node *rn; struct bgp_table *table; @@ -2841,19 +2862,31 @@ bgp_clear_route (struct peer *peer, afi_t afi, safi_t safi) */ if (!peer->clear_node_queue->thread) peer_lock (peer); /* bgp_clear_node_complete */ - - if (safi != SAFI_MPLS_VPN) - bgp_clear_route_table (peer, afi, safi, NULL, NULL); - else - for (rn = bgp_table_top (peer->bgp->rib[afi][safi]); rn; - rn = bgp_route_next (rn)) - if ((table = rn->info) != NULL) - bgp_clear_route_table (peer, afi, safi, table, NULL); - for (ALL_LIST_ELEMENTS (peer->bgp->rsclient, node, nnode, rsclient)) + switch (purpose) { - if (CHECK_FLAG(rsclient->af_flags[afi][safi], PEER_FLAG_RSERVER_CLIENT)) - bgp_clear_route_table (peer, afi, safi, NULL, rsclient); + case BGP_CLEAR_ROUTE_NORMAL: + if (safi != SAFI_MPLS_VPN) + bgp_clear_route_table (peer, afi, safi, NULL, NULL, purpose); + else + for (rn = bgp_table_top (peer->bgp->rib[afi][safi]); rn; + rn = bgp_route_next (rn)) + if ((table = rn->info) != NULL) + bgp_clear_route_table (peer, afi, safi, table, NULL, purpose); + + for (ALL_LIST_ELEMENTS (peer->bgp->rsclient, node, nnode, rsclient)) + if (CHECK_FLAG(rsclient->af_flags[afi][safi], + PEER_FLAG_RSERVER_CLIENT)) + bgp_clear_route_table (peer, afi, safi, NULL, rsclient, purpose); + break; + + case BGP_CLEAR_ROUTE_MY_RSCLIENT: + bgp_clear_route_table (peer, afi, safi, NULL, peer, purpose); + break; + + default: + assert (0); + break; } /* If no routes were cleared, nothing was added to workqueue, the @@ -2887,7 +2920,7 @@ bgp_clear_route_all (struct peer *peer) for (afi = AFI_IP; afi < AFI_MAX; afi++) for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++) - bgp_clear_route (peer, afi, safi); + bgp_clear_route (peer, afi, safi, BGP_CLEAR_ROUTE_NORMAL); } void @@ -12276,3 +12309,10 @@ bgp_route_init (void) install_element (BGP_IPV4_NODE, &bgp_damp_unset_cmd); install_element (BGP_IPV4_NODE, &bgp_damp_unset2_cmd); } + +void +bgp_route_finish (void) +{ + bgp_table_unlock (bgp_distance_table); + bgp_distance_table = NULL; +} diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index e5987972..5eed3486 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -61,7 +61,7 @@ struct bgp_info time_t uptime; /* reference count */ - unsigned int lock; + int lock; /* BGP information status. */ u_int16_t flags; @@ -160,8 +160,15 @@ struct bgp_static #define UNSUPPRESS_MAP_NAME(F) ((F)->usmap.name) #define UNSUPPRESS_MAP(F) ((F)->usmap.map) +enum bgp_clear_route_type +{ + BGP_CLEAR_ROUTE_NORMAL, + BGP_CLEAR_ROUTE_MY_RSCLIENT +}; + /* Prototypes. */ extern void bgp_route_init (void); +extern void bgp_route_finish (void); extern void bgp_cleanup_routes (void); extern void bgp_announce_route (struct peer *, afi_t, safi_t); extern void bgp_announce_route_all (struct peer *); @@ -169,7 +176,8 @@ extern void bgp_default_originate (struct peer *, afi_t, safi_t, int); extern void bgp_soft_reconfig_in (struct peer *, afi_t, safi_t); extern void bgp_soft_reconfig_rsclient (struct peer *, afi_t, safi_t); extern void bgp_check_local_routes_rsclient (struct peer *rsclient, afi_t afi, safi_t safi); -extern void bgp_clear_route (struct peer *, afi_t, safi_t); +extern void bgp_clear_route (struct peer *, afi_t, safi_t, + enum bgp_clear_route_type); extern void bgp_clear_route_all (struct peer *); extern void bgp_clear_adj_in (struct peer *, afi_t, safi_t); extern void bgp_clear_stale_route (struct peer *, afi_t, safi_t); diff --git a/bgpd/bgp_table.c b/bgpd/bgp_table.c index 66332567..5b8c6a49 100644 --- a/bgpd/bgp_table.c +++ b/bgpd/bgp_table.c @@ -38,6 +38,7 @@ bgp_table_init (afi_t afi, safi_t safi) rt = XCALLOC (MTYPE_BGP_TABLE, sizeof (struct bgp_table)); + bgp_table_lock(rt); rt->type = BGP_TABLE_MAIN; rt->afi = afi; rt->safi = safi; @@ -45,11 +46,30 @@ bgp_table_init (afi_t afi, safi_t safi) return rt; } +void +bgp_table_lock (struct bgp_table *rt) +{ + rt->lock++; +} + +void +bgp_table_unlock (struct bgp_table *rt) +{ + assert (rt->lock > 0); + rt->lock--; + + if (rt->lock == 0) + bgp_table_free (rt); +} + void bgp_table_finish (struct bgp_table **rt) { - bgp_table_free (*rt); - *rt = NULL; + if (*rt != NULL) + { + bgp_table_unlock(*rt); + *rt = NULL; + } } static struct bgp_node * @@ -91,6 +111,9 @@ bgp_table_free (struct bgp_table *rt) node = rt->top; + /* Bulk deletion of nodes remaining in this table. This function is not + called until workers have completed their dependency on this table. + A final bgp_unlock_node() will not be called for these nodes. */ while (node) { if (node->l_left) @@ -108,22 +131,31 @@ bgp_table_free (struct bgp_table *rt) tmp_node = node; node = node->parent; + tmp_node->table->count--; + tmp_node->lock = 0; /* to cause assert if unlocked after this */ + bgp_node_free (tmp_node); + if (node != NULL) { if (node->l_left == tmp_node) node->l_left = NULL; else node->l_right = NULL; - - bgp_node_free (tmp_node); } else { - bgp_node_free (tmp_node); break; } } + assert (rt->count == 0); + + if (rt->owner) + { + peer_unlock (rt->owner); + rt->owner = NULL; + } + XFREE (MTYPE_BGP_TABLE, rt); return; } @@ -217,6 +249,7 @@ bgp_lock_node (struct bgp_node *node) void bgp_unlock_node (struct bgp_node *node) { + assert (node->lock > 0); node->lock--; if (node->lock == 0) @@ -344,6 +377,7 @@ bgp_node_get (struct bgp_table *const table, struct prefix *p) if (new->p.prefixlen != p->prefixlen) { match = new; + bgp_lock_node (match); new = bgp_node_set (table, p); set_link (match, new); table->count++; diff --git a/bgpd/bgp_table.h b/bgpd/bgp_table.h index dfa7e1f3..53df0bc6 100644 --- a/bgpd/bgp_table.h +++ b/bgpd/bgp_table.h @@ -35,8 +35,10 @@ struct bgp_table afi_t afi; safi_t safi; + int lock; + /* The owner of this 'bgp_table' structure. */ - void *owner; + struct peer *owner; struct bgp_node *top; @@ -61,13 +63,15 @@ struct bgp_node struct bgp_node *prn; - unsigned int lock; + int lock; u_char flags; #define BGP_NODE_PROCESS_SCHEDULED (1 << 0) }; extern struct bgp_table *bgp_table_init (afi_t, safi_t); +extern void bgp_table_lock (struct bgp_table *); +extern void bgp_table_unlock (struct bgp_table *); extern void bgp_table_finish (struct bgp_table **); extern void bgp_unlock_node (struct bgp_node *node); extern struct bgp_node *bgp_table_top (const struct bgp_table *const); diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index e97b4c97..13c37b57 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -2074,6 +2074,7 @@ peer_rsclient_set_vty (struct vty *vty, const char *peer_str, struct listnode *node, *nnode; struct bgp_filter *pfilter; struct bgp_filter *gfilter; + int locked_and_added = 0; bgp = vty->index; @@ -2089,15 +2090,25 @@ peer_rsclient_set_vty (struct vty *vty, const char *peer_str, { peer = peer_lock (peer); /* rsclient peer list reference */ listnode_add_sort (bgp->rsclient, peer); + locked_and_added = 1; } ret = peer_af_flag_set (peer, afi, safi, PEER_FLAG_RSERVER_CLIENT); if (ret < 0) - return bgp_vty_return (vty, ret); + { + if (locked_and_added) + { + listnode_delete (bgp->rsclient, peer); + peer_unlock (peer); /* rsclient peer list reference */ + } + + return bgp_vty_return (vty, ret); + } peer->rib[afi][safi] = bgp_table_init (afi, safi); peer->rib[afi][safi]->type = BGP_TABLE_RSCLIENT; - peer->rib[afi][safi]->owner = peer; + /* RIB peer reference. Released when table is free'd in bgp_table_free. */ + peer->rib[afi][safi]->owner = peer_lock (peer); /* Check for existing 'network' and 'redistribute' routes. */ bgp_check_local_routes_rsclient (peer, afi, safi); @@ -2190,8 +2201,9 @@ peer_rsclient_unset_vty (struct vty *vty, const char *peer_str, if ( ! peer_rsclient_active (peer) ) { - peer_unlock (peer); /* peer bgp rsclient reference */ + bgp_clear_route (peer, afi, safi, BGP_CLEAR_ROUTE_MY_RSCLIENT); listnode_delete (bgp->rsclient, peer); + peer_unlock (peer); /* peer bgp rsclient reference */ } bgp_table_finish (&peer->rib[bgp_node_afi(vty)][bgp_node_safi(vty)]); diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 0b6ab45a..49380cc3 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -39,7 +39,7 @@ Boston, MA 02111-1307, USA. */ #include "bgpd/bgp_debug.h" /* All information about zebra. */ -static struct zclient *zclient = NULL; +struct zclient *zclient = NULL; struct in_addr router_id_zebra; /* Router-id update message from zebra. */ diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 86bf60ec..60722d27 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -727,7 +727,6 @@ struct peer * peer_lock (struct peer *peer) { assert (peer && (peer->lock >= 0)); - assert (peer->status != Deleted); peer->lock++; @@ -1109,7 +1108,7 @@ peer_deactivate (struct peer *peer, afi_t afi, safi_t safi) bgp_capability_send (peer, afi, safi, CAPABILITY_CODE_MP, CAPABILITY_ACTION_UNSET); - bgp_clear_route (peer, afi, safi); + bgp_clear_route (peer, afi, safi, BGP_CLEAR_ROUTE_NORMAL); peer->pcount[afi][safi] = 0; } else @@ -1177,6 +1176,7 @@ peer_delete (struct peer *peer) safi_t safi; struct bgp *bgp; struct bgp_filter *filter; + struct listnode *pn; assert (peer->status != Deleted); @@ -1185,12 +1185,10 @@ peer_delete (struct peer *peer) if (CHECK_FLAG (peer->sflags, PEER_STATUS_NSF_WAIT)) peer_nsf_stop (peer); - /* If this peer belongs to peer group. Clearn up the + /* If this peer belongs to peer group, clear up the relationship. */ if (peer->group) { - struct listnode *pn; - if ((pn = listnode_lookup (peer->group->peer, peer))) { peer = peer_unlock (peer); /* group->peer list reference */ @@ -1220,22 +1218,25 @@ peer_delete (struct peer *peer) bgp_timer_set (peer); /* stops all timers for Deleted */ /* Delete from all peer list. */ - if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP)) + if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP) + && (pn = listnode_lookup (bgp->peer, peer))) { - struct listnode *pn; - - if ((pn = listnode_lookup (bgp->peer, peer))) - { - peer_unlock (peer); /* bgp peer list reference */ - list_delete_node (bgp->peer, pn); - } + peer_unlock (peer); /* bgp peer list reference */ + list_delete_node (bgp->peer, pn); + } - if (peer_rsclient_active (peer) - && (pn = listnode_lookup (bgp->rsclient, peer))) - { - peer_unlock (peer); /* rsclient list reference */ - list_delete_node (bgp->rsclient, pn); - } + if (peer_rsclient_active (peer) + && (pn = listnode_lookup (bgp->rsclient, peer))) + { + peer_unlock (peer); /* rsclient list reference */ + list_delete_node (bgp->rsclient, pn); + + /* Clear our own rsclient ribs. */ + for (afi = AFI_IP; afi < AFI_MAX; afi++) + for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++) + if (CHECK_FLAG(peer->af_flags[afi][safi], + PEER_FLAG_RSERVER_CLIENT)) + bgp_clear_route (peer, afi, safi, BGP_CLEAR_ROUTE_MY_RSCLIENT); } /* Free RIB for any family in which peer is RSERVER_CLIENT, and is not @@ -1366,7 +1367,7 @@ peer_group_get (struct bgp *bgp, const char *name) group->conf = peer_new (bgp); if (! bgp_flag_check (bgp, BGP_FLAG_NO_DEFAULT_IPV4)) group->conf->afc[AFI_IP][SAFI_UNICAST] = 1; - group->conf->host = strdup (name); + group->conf->host = XSTRDUP (MTYPE_BGP_PEER_HOST, name); group->conf->group = group; group->conf->as = 0; group->conf->ttl = 1; @@ -1822,6 +1823,9 @@ peer_group_bind (struct bgp *bgp, union sockunion *su, { peer_unlock (peer); /* peer rsclient reference */ list_delete_node (bgp->rsclient, pn); + + /* Clear our own rsclient rib for this afi/safi. */ + bgp_clear_route (peer, afi, safi, BGP_CLEAR_ROUTE_MY_RSCLIENT); } bgp_table_finish (&peer->rib[afi][safi]); @@ -1914,7 +1918,7 @@ bgp_create (as_t *as, const char *name) bgp_lock (bgp); bgp->peer_self = peer_new (bgp); - bgp->peer_self->host = strdup ("Static announcement"); + bgp->peer_self->host = XSTRDUP (MTYPE_BGP_PEER_HOST, "Static announcement"); bgp->peer = list_new (); bgp->peer->cmp = (int (*)(void *, void *)) peer_cmp; @@ -2060,14 +2064,13 @@ bgp_delete (struct bgp *bgp) if (i != ZEBRA_ROUTE_BGP) bgp_redistribute_unset (bgp, afi, i); - for (ALL_LIST_ELEMENTS (bgp->group, node, next, group)) - peer_group_delete (group); - for (ALL_LIST_ELEMENTS (bgp->peer, node, next, peer)) peer_delete (peer); - for (ALL_LIST_ELEMENTS (bgp->rsclient, node, next, peer)) - peer_delete (peer); + for (ALL_LIST_ELEMENTS (bgp->group, node, next, group)) + peer_group_delete (group); + + assert (listcount (bgp->rsclient) == 0); if (bgp->peer_self) { peer_delete(bgp->peer_self); @@ -2095,6 +2098,7 @@ bgp_lock (struct bgp *bgp) void bgp_unlock(struct bgp *bgp) { + assert(bgp->lock > 0); if (--bgp->lock == 0) bgp_free (bgp); } @@ -2116,11 +2120,11 @@ bgp_free (struct bgp *bgp) for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++) { if (bgp->route[afi][safi]) - XFREE (MTYPE_ROUTE_TABLE, bgp->route[afi][safi]); + bgp_table_finish (&bgp->route[afi][safi]); if (bgp->aggregate[afi][safi]) - XFREE (MTYPE_ROUTE_TABLE,bgp->aggregate[afi][safi]) ; + bgp_table_finish (&bgp->aggregate[afi][safi]) ; if (bgp->rib[afi][safi]) - XFREE (MTYPE_ROUTE_TABLE,bgp->rib[afi][safi]); + bgp_table_finish (&bgp->rib[afi][safi]); } XFREE (MTYPE_BGP, bgp); } -- cgit v1.2.1