This patch corrects bgpd memory not being properly cleaned up on exit.
Errors were identified and verified as part of an effort to use valgrind
to ID any memory leaks.
The patch checked against commit ca8ec20b017393dbe91ff9e5ae2b7ff12872f869
Lou
diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
index 0aec3ef..ed476a2 100644
--- a/bgpd/bgp_aspath.c
+++ b/bgpd/bgp_aspath.c
@@ -98,6 +98,12 @@ assegment_data_new (int num)
return (XMALLOC (MTYPE_AS_SEG_DATA, ASSEGMENT_DATA_SIZE (num, 1)));
}
+static void
+assegment_data_free (as_t *asdata)
+{
+ XFREE (MTYPE_AS_SEG_DATA,asdata);
+}
+
/* Get a new segment. Note that 0 is an allowed length,
* and will result in a segment with no allocated data segment.
* the caller should immediately assign data to the segment, as the segment
@@ -126,7 +132,7 @@ assegment_free (struct assegment *seg)
return;
if (seg->as)
- XFREE (MTYPE_AS_SEG_DATA, seg->as);
+ assegment_data_free (seg->as);
memset (seg, 0xfe, sizeof(struct assegment));
XFREE (MTYPE_AS_SEG, seg);
@@ -195,6 +201,19 @@ assegment_prepend_asns (struct assegment *seg, as_t
asnum, int num)
return seg; /* we don't do huge prepends */
newas = assegment_data_new (seg->length + num);
+
+ if (newas)
+ {
+ int i;
+ for (i = 0; i < num; i++)
+ newas[i] = asnum;
+
+ memcpy (newas + num, seg->as, ASSEGMENT_DATA_SIZE (seg->length, 1));
+ assegment_data_free (seg->as);
+ seg->as = newas;
+ seg->length += num;
+ return seg;
+ }
for (i = 0; i < num; i++)
newas[i] = asnum;
@@ -1879,6 +1898,7 @@ aspath_init (void)
void
aspath_finish (void)
{
+ hash_clean (ashash, (void (*)(void *))aspath_free);
hash_free (ashash);
ashash = NULL;
diff --git a/bgpd/bgp_main.c b/bgpd/bgp_main.c
index 7c2988c..9b41f45 100644
--- a/bgpd/bgp_main.c
+++ b/bgpd/bgp_main.c
@@ -37,6 +37,7 @@ Software Foundation, Inc., 59 Temple Place - Suite
330, Boston, MA
#include "plist.h"
#include "stream.h"
#include "vrf.h"
+#include "workqueue.h"
#include "bgpd/bgpd.h"
#include "bgpd/bgp_attr.h"
@@ -199,10 +200,10 @@ sigint (void)
{
zlog_notice ("Terminating on signal");
- if (! retain_mode)
+ if (! retain_mode) {
bgp_terminate ();
-
- zprivs_terminate (&bgpd_privs);
+ zprivs_terminate (&bgpd_privs);
+ }
bgp_exit (0);
}
@@ -237,6 +238,26 @@ bgp_exit (int status)
for (ALL_LIST_ELEMENTS (bm->bgp, node, nnode, bgp))
bgp_delete (bgp);
list_free (bm->bgp);
+ bm->bgp = NULL;
+
+ /*
+ * bgp_delete can re-allocate the process queues after they were
+ * deleted in bgp_terminate. delete them again.
+ *
+ * It might be better to ensure the RIBs (including static routes)
+ * are cleared by bgp_terminate() during its call to
bgp_cleanup_routes(),
+ * which currently only deletes the kernel routes.
+ */
+ if (bm->process_main_queue)
+ {
+ work_queue_free (bm->process_main_queue);
+ bm->process_main_queue = NULL;
+ }
+ if (bm->process_rsclient_queue)
+ {
+ work_queue_free (bm->process_rsclient_queue);
+ bm->process_rsclient_queue = NULL;
+ }
/* reverse bgp_master_init */
for (ALL_LIST_ELEMENTS_RO(bm->listen_sockets, node, socket))
@@ -307,10 +328,10 @@ bgp_exit (int status)
if (zlog_default)
closezlog (zlog_default);
-
+#if 0
if (CONF_BGP_DEBUG (normal, NORMAL))
+#endif
log_memstats_stderr ("bgpd");
-
exit (status);
}
@@ -433,7 +454,7 @@ main (int argc, char **argv)
/* Parse config file. */
vty_read_config (config_file, config_default);
-
+
/* Start execution only if not in dry-run mode */
if(dryrun)
return(0);
@@ -453,7 +474,8 @@ main (int argc, char **argv)
vty_serv_sock (vty_addr, vty_port, BGP_VTYSH_PATH);
/* Print banner. */
- zlog_notice ("BGPd %s starting: vty@%d, bgp@%s:%d", QUAGGA_VERSION,
+ zlog_notice ("BGPd %s starting: pid %d, vty@%d, bgp@%s:%d",
QUAGGA_VERSION,
+ getpid(),
vty_port,
(bm->address ? bm->address : "<all>"),
bm->port);
diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c
index 20302e3..87e7d88 100644
--- a/bgpd/bgp_nexthop.c
+++ b/bgpd/bgp_nexthop.c
@@ -589,6 +589,14 @@ bgp_address_init (void)
bgp_address_hash_cmp);
}
+void
+bgp_address_destroy (void)
+{
+ hash_clean(bgp_address_hash, NULL);
+ hash_free(bgp_address_hash);
+ bgp_address_hash = NULL;
+}
+
static void
bgp_address_add (struct prefix *p)
{
@@ -1441,29 +1449,52 @@ bgp_scan_init (void)
void
bgp_scan_finish (void)
{
+#if 0 /* results in double free */
/* Only the current one needs to be reset. */
bgp_nexthop_cache_reset (bgp_nexthop_cache_table[AFI_IP]);
+#endif
- bgp_table_unlock (cache1_table[AFI_IP]);
+ if (cache1_table[AFI_IP])
+ bgp_table_unlock (cache1_table[AFI_IP]);
cache1_table[AFI_IP] = NULL;
- bgp_table_unlock (cache2_table[AFI_IP]);
+ if(cache2_table[AFI_IP])
+ bgp_table_unlock (cache2_table[AFI_IP]);
cache2_table[AFI_IP] = NULL;
- bgp_table_unlock (bgp_connected_table[AFI_IP]);
+ if(bgp_connected_table[AFI_IP])
+ bgp_table_unlock (bgp_connected_table[AFI_IP]);
bgp_connected_table[AFI_IP] = NULL;
#ifdef HAVE_IPV6
+#if 0
/* Only the current one needs to be reset. */
bgp_nexthop_cache_reset (bgp_nexthop_cache_table[AFI_IP6]);
+#endif
- bgp_table_unlock (cache1_table[AFI_IP6]);
+ if(cache1_table[AFI_IP6])
+ bgp_table_unlock (cache1_table[AFI_IP6]);
cache1_table[AFI_IP6] = NULL;
- bgp_table_unlock (cache2_table[AFI_IP6]);
+ if(cache2_table[AFI_IP6])
+ bgp_table_unlock (cache2_table[AFI_IP6]);
cache2_table[AFI_IP6] = NULL;
- bgp_table_unlock (bgp_connected_table[AFI_IP6]);
+ if(bgp_connected_table[AFI_IP6])
+ bgp_table_unlock (bgp_connected_table[AFI_IP6]);
bgp_connected_table[AFI_IP6] = NULL;
#endif /* HAVE_IPV6 */
}
+
+void
+bgp_scan_destroy (void)
+{
+ if (zlookup == NULL)
+ return;
+ THREAD_OFF(bgp_import_thread);
+ THREAD_OFF(bgp_scan_thread);
+ THREAD_OFF(zlookup->t_connect);
+ bgp_scan_finish();
+ zclient_free (zlookup);
+ zlookup = NULL;
+}
diff --git a/bgpd/bgp_nexthop.h b/bgpd/bgp_nexthop.h
index 6e5350e..4ae772e 100644
--- a/bgpd/bgp_nexthop.h
+++ b/bgpd/bgp_nexthop.h
@@ -57,5 +57,7 @@ extern int bgp_config_write_scan_time (struct vty *);
extern int bgp_nexthop_onlink (afi_t, struct attr *);
extern int bgp_nexthop_self (struct attr *);
extern void bgp_address_init (void);
+extern void bgp_address_destroy (void);
+void bgp_scan_destroy (void);
#endif /* _QUAGGA_BGP_NEXTHOP_H */
diff --git a/lib/routemap.c b/lib/routemap.c
index 1e1510e..3593ee3 100644
--- a/lib/routemap.c
+++ b/lib/routemap.c
@@ -897,6 +897,9 @@ route_map_finish (void)
route_match_vec = NULL;
vector_free (route_set_vec);
route_set_vec = NULL;
+ /* cleanup route_map */
+ while (route_map_master.head)
+ route_map_delete (route_map_master.head);
}
/* VTY related functions. */
_______________________________________________
Quagga-dev mailing list
[email protected]
https://lists.quagga.net/mailman/listinfo/quagga-dev
