From 228da42898c4f7bd72d9c1ee4135108e8d40d860 Mon Sep 17 00:00:00 2001 From: Chris Caputo Date: Sat, 18 Jul 2009 05:44:03 +0000 Subject: [PATCH] [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 ++++++++-------- lib/command.c | 101 +++++++++++++++++++++++--- lib/command.h | 8 ++- lib/log.c | 4 +- lib/memory.c | 47 +++++++++++- lib/memory.h | 1 + lib/routemap.c | 9 +++ lib/routemap.h | 1 + lib/thread.c | 22 +++++- lib/vty.c | 16 ++++- lib/vty.h | 1 + lib/zclient.c | 7 +- lib/zclient.h | 1 + 36 files changed, 630 insertions(+), 133 deletions(-) diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c index 002fff9f5..13f32b867 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 82d907e27..941683728 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 12149a177..ed8753bd9 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 8d8c90c4a..d66016746 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 6d7e363e9..5dcb3b4c1 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 d40d69a2f..ae1d7a155 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 aed7f3303..bc1e56eff 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 53dea805c..8087a403b 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 6bb1197bb..e097c7840 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 6152a1db2..8d5fa741a 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 5c8deb561..942fdc733 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 bdb756cb0..8ee62b013 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 d389f165f..8c27a9302 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 620ca128a..9d14683ca 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 67a49f7ad..0cde665eb 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 a8b92df6a..2dad742ff 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 d98b689a9..1c9a3c911 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 87fe7f5cf..8dafd181e 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 e5987972a..5eed3486d 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 663325678..5b8c6a490 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 dfa7e1f36..53df0bc6c 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 e97b4c972..13c37b578 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 0b6ab45a7..49380cc37 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 86bf60ec1..60722d27a 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); } diff --git a/lib/command.c b/lib/command.c index 0bbd99e5b..31c067a36 100644 --- a/lib/command.c +++ b/lib/command.c @@ -37,6 +37,9 @@ Boston, MA 02111-1307, USA. */ each daemon maintains each own cmdvec. */ vector cmdvec = NULL; +struct desc desc_cr; +char *command_cr = NULL; + /* Host information structure. */ struct host host; @@ -199,8 +202,8 @@ install_node (struct cmd_node *node, static int cmp_node (const void *p, const void *q) { - const struct cmd_element *a = *(struct cmd_element **)p; - const struct cmd_element *b = *(struct cmd_element **)q; + const struct cmd_element *a = *(struct cmd_element * const *)p; + const struct cmd_element *b = *(struct cmd_element * const *)q; return strcmp (a->string, b->string); } @@ -208,8 +211,8 @@ cmp_node (const void *p, const void *q) static int cmp_desc (const void *p, const void *q) { - const struct desc *a = *(struct desc **)p; - const struct desc *b = *(struct desc **)q; + const struct desc *a = *(struct desc * const *)p; + const struct desc *b = *(struct desc * const *)q; return strcmp (a->cmd, b->cmd); } @@ -223,7 +226,7 @@ sort_node () vector descvec; struct cmd_element *cmd_element; - for (i = 0; i < vector_active (cmdvec); i++) + for (i = 0; i < vector_active (cmdvec); i++) if ((cnode = vector_slot (cmdvec, i)) != NULL) { vector cmd_vector = cnode->cmd_vector; @@ -497,7 +500,9 @@ install_element (enum node_type ntype, struct cmd_element *cmd) vector_set (cnode->cmd_vector, cmd); - cmd->strvec = cmd_make_descvec (cmd->string, cmd->doc); + if (cmd->strvec == NULL) + cmd->strvec = cmd_make_descvec (cmd->string, cmd->doc); + cmd->cmdsize = cmd_cmdsize (cmd->strvec); } @@ -1588,7 +1593,6 @@ cmd_describe_command_real (vector vline, struct vty *vty, int *status) int ret; enum match_type match; char *command; - static struct desc desc_cr = { "", "" }; /* Set index. */ if (vector_active (vline) == 0) @@ -1665,7 +1669,6 @@ cmd_describe_command_real (vector vline, struct vty *vty, int *status) for (i = 0; i < vector_active (cmd_vector); i++) if ((cmd_element = vector_slot (cmd_vector, i)) != NULL) { - const char *string = NULL; vector strvec = cmd_element->strvec; /* if command is NULL, index may be equal to vector_active */ @@ -1676,8 +1679,7 @@ cmd_describe_command_real (vector vline, struct vty *vty, int *status) /* Check if command is completed. */ if (command == NULL && index == vector_active (strvec)) { - string = ""; - if (!desc_unique_string (matchvec, string)) + if (!desc_unique_string (matchvec, command_cr)) vector_set (matchvec, &desc_cr); } else @@ -1689,6 +1691,8 @@ cmd_describe_command_real (vector vline, struct vty *vty, int *status) for (j = 0; j < vector_active (descvec); j++) if ((desc = vector_slot (descvec, j))) { + const char *string; + string = cmd_entry_function_desc (command, desc->cmd); if (string) { @@ -3506,6 +3510,8 @@ DEFUN (no_banner_motd, void host_config_set (char *filename) { + if (host.config) + XFREE (MTYPE_HOST, host.config); host.config = XSTRDUP (MTYPE_HOST, filename); } @@ -3529,6 +3535,10 @@ install_default (enum node_type node) void cmd_init (int terminal) { + command_cr = XSTRDUP(MTYPE_STRVEC, ""); + desc_cr.cmd = command_cr; + desc_cr.str = XSTRDUP(MTYPE_STRVEC, ""); + /* Allocate initial top vector of commands. */ cmdvec = vector_init (VECTOR_MIN_SIZE); @@ -3645,3 +3655,74 @@ cmd_init (int terminal) } srand(time(NULL)); } + +void +cmd_terminate () +{ + unsigned int i, j, k, l; + struct cmd_node *cmd_node; + struct cmd_element *cmd_element; + struct desc *desc; + vector cmd_node_v, cmd_element_v, desc_v; + + if (cmdvec) + { + for (i = 0; i < vector_active (cmdvec); i++) + if ((cmd_node = vector_slot (cmdvec, i)) != NULL) + { + cmd_node_v = cmd_node->cmd_vector; + + for (j = 0; j < vector_active (cmd_node_v); j++) + if ((cmd_element = vector_slot (cmd_node_v, j)) != NULL && + cmd_element->strvec != NULL) + { + cmd_element_v = cmd_element->strvec; + + for (k = 0; k < vector_active (cmd_element_v); k++) + if ((desc_v = vector_slot (cmd_element_v, k)) != NULL) + { + for (l = 0; l < vector_active (desc_v); l++) + if ((desc = vector_slot (desc_v, l)) != NULL) + { + if (desc->cmd) + XFREE (MTYPE_STRVEC, desc->cmd); + if (desc->str) + XFREE (MTYPE_STRVEC, desc->str); + + XFREE (MTYPE_DESC, desc); + } + vector_free (desc_v); + } + + cmd_element->strvec = NULL; + vector_free (cmd_element_v); + } + + vector_free (cmd_node_v); + } + + vector_free (cmdvec); + cmdvec = NULL; + } + + if (command_cr) + XFREE(MTYPE_STRVEC, command_cr); + if (desc_cr.str) + XFREE(MTYPE_STRVEC, desc_cr.str); + if (host.name) + XFREE (MTYPE_HOST, host.name); + if (host.password) + XFREE (MTYPE_HOST, host.password); + if (host.password_encrypt) + XFREE (MTYPE_HOST, host.password_encrypt); + if (host.enable) + XFREE (MTYPE_HOST, host.enable); + if (host.enable_encrypt) + XFREE (MTYPE_HOST, host.enable_encrypt); + if (host.logfile) + XFREE (MTYPE_HOST, host.logfile); + if (host.motdfile) + XFREE (MTYPE_HOST, host.motdfile); + if (host.config) + XFREE (MTYPE_HOST, host.config); +} diff --git a/lib/command.h b/lib/command.h index d093df3cb..1275efee2 100644 --- a/lib/command.h +++ b/lib/command.h @@ -147,8 +147,8 @@ struct cmd_element /* Command description structure. */ struct desc { - const char *cmd; /* Command string. */ - const char *str; /* Command's description. */ + char *cmd; /* Command string. */ + char *str; /* Command's description. */ }; /* Return value of the commands. */ @@ -347,6 +347,7 @@ extern int cmd_execute_command (vector, struct vty *, struct cmd_element **, int extern int cmd_execute_command_strict (vector, struct vty *, struct cmd_element **); extern void config_replace_string (struct cmd_element *, char *, ...); extern void cmd_init (int); +extern void cmd_terminate (void); /* Export typical functions. */ extern struct cmd_element config_end_cmd; @@ -361,4 +362,7 @@ extern void print_version (const char *); /* struct host global, ick */ extern struct host host; + +/* "" global */ +extern char *command_cr; #endif /* _ZEBRA_COMMAND_H */ diff --git a/lib/log.c b/lib/log.c index 8c3e2ddce..0c2f655bc 100644 --- a/lib/log.c +++ b/lib/log.c @@ -649,7 +649,9 @@ void closezlog (struct zlog *zl) { closelog(); - fclose (zl->fp); + + if (zl->fp != NULL) + fclose (zl->fp); XFREE (MTYPE_ZLOG, zl); } diff --git a/lib/memory.c b/lib/memory.c index f5d0cba6b..dc09d8a6c 100644 --- a/lib/memory.c +++ b/lib/memory.c @@ -127,7 +127,7 @@ zstrdup (int type, const char *str) static struct { const char *name; - unsigned long alloc; + long alloc; unsigned long t_malloc; unsigned long c_malloc; unsigned long t_calloc; @@ -214,9 +214,9 @@ mtype_zstrdup (const char *file, int line, int type, const char *str) static struct { char *name; - unsigned long alloc; + long alloc; } mstat [MTYPE_MAX]; -#endif /* MTPYE_LOG */ +#endif /* MEMORY_LOG */ /* Increment allocation counter. */ static void @@ -253,6 +253,47 @@ log_memstats(int pri) } } +void +log_memstats_stderr (const char *prefix) +{ + struct mlist *ml; + struct memory_list *m; + int i; + int j = 0; + + for (ml = mlists; ml->list; ml++) + { + i = 0; + + for (m = ml->list; m->index >= 0; m++) + if (m->index && mstat[m->index].alloc) + { + if (!i) + fprintf (stderr, + "%s: memstats: Current memory utilization in module %s:\n", + prefix, + ml->name); + fprintf (stderr, + "%s: memstats: %-30s: %10ld%s\n", + prefix, + m->format, + mstat[m->index].alloc, + mstat[m->index].alloc < 0 ? " (REPORT THIS BUG!)" : ""); + i = j = 1; + } + } + + if (j) + fprintf (stderr, + "%s: memstats: NOTE: If configuration exists, utilization may be " + "expected.\n", + prefix); + else + fprintf (stderr, + "%s: memstats: No remaining tracked memory utilization.\n", + prefix); +} + static void show_separator(struct vty *vty) { diff --git a/lib/memory.h b/lib/memory.h index a23c27875..42eb5caec 100644 --- a/lib/memory.h +++ b/lib/memory.h @@ -81,6 +81,7 @@ extern void mtype_zfree (const char *file, int line, int type, extern char *mtype_zstrdup (const char *file, int line, int type, const char *str); extern void memory_init (void); +extern void log_memstats_stderr (const char *); /* return number of allocations outstanding for the type */ extern unsigned long mtype_stats_alloc (int); diff --git a/lib/routemap.c b/lib/routemap.c index 5f7a31820..4f4e6d620 100644 --- a/lib/routemap.c +++ b/lib/routemap.c @@ -889,6 +889,15 @@ route_map_init (void) route_match_vec = vector_init (1); route_set_vec = vector_init (1); } + +void +route_map_finish (void) +{ + vector_free (route_match_vec); + route_match_vec = NULL; + vector_free (route_set_vec); + route_set_vec = NULL; +} /* VTY related functions. */ DEFUN (route_map, diff --git a/lib/routemap.h b/lib/routemap.h index 321e1927a..1402f5c84 100644 --- a/lib/routemap.h +++ b/lib/routemap.h @@ -153,6 +153,7 @@ struct route_map /* Prototypes. */ extern void route_map_init (void); extern void route_map_init_vty (void); +extern void route_map_finish (void); /* Add match statement to route map. */ extern int route_map_add_match (struct route_map_index *index, diff --git a/lib/thread.c b/lib/thread.c index 47a9dc438..e89af541c 100644 --- a/lib/thread.c +++ b/lib/thread.c @@ -239,6 +239,15 @@ cpu_record_hash_alloc (struct cpu_thread_history *a) return new; } +static void +cpu_record_hash_free (void *a) +{ + struct cpu_thread_history *hist = a; + + XFREE (MTYPE_THREAD_FUNCNAME, hist->funcname); + XFREE (MTYPE_THREAD_STATS, hist); +} + static inline void vty_out_cpu_thread_history(struct vty* vty, struct cpu_thread_history *a) @@ -485,7 +494,8 @@ thread_list_free (struct thread_master *m, struct thread_list *list) for (t = list->head; t; t = next) { next = t->next; - XFREE (MTYPE_THREAD_FUNCNAME, t->funcname); + if (t->funcname) + XFREE (MTYPE_THREAD_FUNCNAME, t->funcname); XFREE (MTYPE_THREAD, t); list->count--; m->alloc--; @@ -505,6 +515,13 @@ thread_master_free (struct thread_master *m) thread_list_free (m, &m->background); XFREE (MTYPE_THREAD_MASTER, m); + + if (cpu_record) + { + hash_clean (cpu_record, cpu_record_hash_free); + hash_free (cpu_record); + cpu_record = NULL; + } } /* Thread list is empty or not. */ @@ -836,6 +853,7 @@ thread_run (struct thread_master *m, struct thread *thread, { *fetch = *thread; thread->type = THREAD_UNUSED; + thread->funcname = NULL; /* thread_call will free fetch's copied pointer */ thread_add_unuse (m, thread); return fetch; } @@ -1079,6 +1097,8 @@ thread_call (struct thread *thread) realtime/1000, cputime/1000); } #endif /* CONSUMED_TIME_CHECK */ + + XFREE (MTYPE_THREAD_FUNCNAME, thread->funcname); } /* Execute thread */ diff --git a/lib/vty.c b/lib/vty.c index 14a36c162..30a94e112 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -1034,7 +1034,7 @@ vty_describe_command (struct vty *vty) if (desc->cmd[0] == '\0') continue; - if (strcmp (desc->cmd, "") == 0) + if (strcmp (desc->cmd, command_cr) == 0) { desc_cr = desc; continue; @@ -2988,3 +2988,17 @@ vty_init (struct thread_master *master_thread) install_element (VTY_NODE, &no_vty_ipv6_access_class_cmd); #endif /* HAVE_IPV6 */ } + +void +vty_terminate (void) +{ + if (vty_cwd) + XFREE (MTYPE_TMP, vty_cwd); + + if (vtyvec && Vvty_serv_thread) + { + vty_reset (); + vector_free (vtyvec); + vector_free (Vvty_serv_thread); + } +} diff --git a/lib/vty.h b/lib/vty.h index 65ae6201c..7df04b5fb 100644 --- a/lib/vty.h +++ b/lib/vty.h @@ -203,6 +203,7 @@ extern char integrate_default[]; /* Prototypes. */ extern void vty_init (struct thread_master *); extern void vty_init_vtysh (void); +extern void vty_terminate (void); extern void vty_reset (void); extern struct vty *vty_new (void); extern int vty_out (struct vty *, const char *, ...) PRINTF_ATTRIBUTE(2, 3); diff --git a/lib/zclient.c b/lib/zclient.c index 4a716a660..d3d532274 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -58,13 +58,11 @@ zclient_new () return zclient; } -#if 0 -/* This function is never used. And it must not be used, because +/* This function is only called when exiting, because many parts of the code do not check for I/O errors, so they could reference an invalid pointer if the structure was ever freed. -*/ -/* Free zclient structure. */ + Free zclient structure. */ void zclient_free (struct zclient *zclient) { @@ -77,7 +75,6 @@ zclient_free (struct zclient *zclient) XFREE (MTYPE_ZCLIENT, zclient); } -#endif /* Initialize zebra client. Argument redist_default is unwanted redistribute route type. */ diff --git a/lib/zclient.h b/lib/zclient.h index 69ada144b..21786ab87 100644 --- a/lib/zclient.h +++ b/lib/zclient.h @@ -125,6 +125,7 @@ extern void zclient_init (struct zclient *, int); extern int zclient_start (struct zclient *); extern void zclient_stop (struct zclient *); extern void zclient_reset (struct zclient *); +extern void zclient_free (struct zclient *); /* Get TCP socket connection to zebra daemon at loopback address. */ extern int zclient_socket (void); -- 2.39.5