bgpd: improve cleanup in bgp_delete()

Signed-off-by: Lou Berger <lberger@labn.net>
(cherry picked from commit 82dd707988b7481e203cab058c92f0b3041dd558)

Conflicts:
	bgpd/bgp_nexthop.h
	bgpd/bgp_route.c
	bgpd/bgp_routemap.c
	bgpd/bgp_zebra.h
	bgpd/bgpd.c
	bgpd/bgpd.h

diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c
index d959076e595626e6fc01d1d489efadf9e3f49a3a..d81c6a80521c82409613b8228bc0a9b97c826b5b 100644 (file)
--- a/bgpd/bgp_nexthop.c
+++ b/bgpd/bgp_nexthop.c
@@ -145,6 +145,14 @@ bgp_address_init (struct bgp *bgp)
                                   bgp_address_hash_cmp);
 }
 
+void
+bgp_address_destroy (struct bgp *bgp)
+{
+  hash_clean(bgp->address_hash, NULL);
+  hash_free(bgp->address_hash);
+  bgp->address_hash = NULL;
+}
+
 static void
 bgp_address_add (struct bgp *bgp, struct prefix *p)
 {

diff --git a/bgpd/bgp_nexthop.h b/bgpd/bgp_nexthop.h
index a2d5ffdcc70efcd81f00dbfc93b02ea0d424931d..861da5740faf3e9bd6794f6b90673a993d678028 100644 (file)
--- a/bgpd/bgp_nexthop.h
+++ b/bgpd/bgp_nexthop.h
@@ -73,7 +73,6 @@ extern void bgp_connected_delete (struct bgp *bgp, struct connected *c);
 extern int bgp_multiaccess_check_v4 (struct in_addr, struct peer *);
 extern int bgp_config_write_scan_time (struct vty *);
 extern int bgp_nexthop_self (struct bgp *, struct attr *);
-extern void bgp_address_init (struct bgp *);
 extern struct bgp_nexthop_cache *bnc_new(void);
 extern void bnc_free(struct bgp_nexthop_cache *bnc);
 extern void bnc_nexthop_free(struct bgp_nexthop_cache *bnc);
@@ -81,5 +80,7 @@ extern char *bnc_str(struct bgp_nexthop_cache *bnc, char *buf, int size);
 extern void bgp_scan_init(struct bgp *bgp);
 extern void bgp_scan_finish(struct bgp *bgp);
 extern void bgp_scan_vty_init(void);
+extern void bgp_address_init (struct bgp *bgp);
+extern void bgp_address_destroy (struct bgp *bgp);
 
 #endif /* _QUAGGA_BGP_NEXTHOP_H */

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index d9dec8772b7c0a39dfe7da8805b3b21aef349ea0..8b39f3a561b4bb9391cddb6b407cd00a4f690d00 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -2997,6 +2997,56 @@ bgp_clear_route_all (struct peer *peer)
       bgp_clear_route (peer, afi, safi);
 }
 
+/*
+ * Finish freeing things when exiting
+ */
+static void
+bgp_drain_workqueue_immediate (struct work_queue *wq)
+{
+  if (!wq)
+    return;
+
+  if (!wq->thread)
+    {
+      /*
+       * no thread implies no queued items
+       */
+      assert(!wq->items->count);
+      return;
+    }
+
+   while (wq->items->count)
+     {
+       if (wq->thread)
+         thread_cancel(wq->thread);
+       work_queue_run(wq->thread);
+     }
+}
+
+/*
+ * Special function to process clear node queue when bgpd is exiting
+ * and the thread scheduler is no longer running.
+ */
+void
+bgp_peer_clear_node_queue_drain_immediate(struct peer *peer)
+{
+  if (!peer)
+    return;
+
+  bgp_drain_workqueue_immediate(peer->clear_node_queue);
+}
+
+/*
+ * The work queues are not specific to a BGP instance, but the
+ * items in them refer to BGP instances, so this should be called
+ * before each BGP instance is deleted.
+ */
+void
+bgp_process_queues_drain_immediate(void)
+{
+  bgp_drain_workqueue_immediate(bm->process_main_queue);
+}
+
 void
 bgp_clear_adj_in (struct peer *peer, afi_t afi, safi_t safi)
 {
@@ -3050,37 +3100,54 @@ bgp_clear_stale_route (struct peer *peer, afi_t afi, safi_t safi)
     }
 }
 
+static void
+bgp_cleanup_table(struct bgp_table *table, safi_t safi)
+{
+  struct bgp_node *rn;
+  struct bgp_info *ri;
+  struct bgp_info *next;
+
+  for (rn = bgp_table_top (table); rn; rn = bgp_route_next (rn))
+    for (ri = rn->info; ri; ri = next)
+      {
+        next = ri->next;
+        if (CHECK_FLAG (ri->flags, BGP_INFO_SELECTED)
+            && ri->type == ZEBRA_ROUTE_BGP
+            && (ri->sub_type == BGP_ROUTE_NORMAL ||
+                ri->sub_type == BGP_ROUTE_AGGREGATE))
+          bgp_zebra_withdraw (&rn->p, ri, safi);
+      }
+}
+
 /* Delete all kernel routes. */
 void
 bgp_cleanup_routes (void)
 {
   struct bgp *bgp;
   struct listnode *node, *nnode;
-  struct bgp_node *rn;
-  struct bgp_table *table;
-  struct bgp_info *ri;
+  afi_t afi;
 
   for (ALL_LIST_ELEMENTS (bm->bgp, node, nnode, bgp))
     {
-      table = bgp->rib[AFI_IP][SAFI_UNICAST];
-
-      for (rn = bgp_table_top (table); rn; rn = bgp_route_next (rn))
-       for (ri = rn->info; ri; ri = ri->next)
-         if (CHECK_FLAG (ri->flags, BGP_INFO_SELECTED)
-             && ri->type == ZEBRA_ROUTE_BGP 
-             && (ri->sub_type == BGP_ROUTE_NORMAL ||
-                 ri->sub_type == BGP_ROUTE_AGGREGATE))
-           bgp_zebra_withdraw (&rn->p, ri,SAFI_UNICAST);
+      for (afi = AFI_IP; afi < AFI_MAX; ++afi)
+       {
+         struct bgp_node *rn;
 
-      table = bgp->rib[AFI_IP6][SAFI_UNICAST];
+         bgp_cleanup_table(bgp->rib[afi][SAFI_UNICAST], SAFI_UNICAST);
 
-      for (rn = bgp_table_top (table); rn; rn = bgp_route_next (rn))
-       for (ri = rn->info; ri; ri = ri->next)
-         if (CHECK_FLAG (ri->flags, BGP_INFO_SELECTED)
-             && ri->type == ZEBRA_ROUTE_BGP 
-             && (ri->sub_type == BGP_ROUTE_NORMAL ||
-                 ri->sub_type == BGP_ROUTE_AGGREGATE))
-           bgp_zebra_withdraw (&rn->p, ri,SAFI_UNICAST);
+         /*
+          * VPN and ENCAP tables are two-level (RD is top level)
+          */
+         for (rn = bgp_table_top(bgp->rib[afi][SAFI_MPLS_VPN]); rn;
+               rn = bgp_route_next (rn))
+           if (rn->info)
+                {
+                   bgp_cleanup_table((struct bgp_table *)(rn->info), SAFI_MPLS_VPN);
+                   bgp_table_finish ((struct bgp_table **)&(rn->info));
+                  rn->info = NULL;
+                  bgp_unlock_node(rn);
+                }
+       }
     }
 }
 

diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h
index 8d1eb21010ebb151fb38196848642ebdc2c2cfab..60c406775d1ccef8ffe25eccd98249cfc57ef5e3 100644 (file)
--- a/bgpd/bgp_route.h
+++ b/bgpd/bgp_route.h
@@ -306,4 +306,7 @@ extern int subgroup_announce_check(struct bgp_info *ri,
                                   struct update_subgroup *subgrp,
                                   struct prefix *p, struct attr *attr);
 
+extern void bgp_peer_clear_node_queue_drain_immediate (struct peer *peer);
+extern void bgp_process_queues_drain_immediate (void);
+
 #endif /* _QUAGGA_BGP_ROUTE_H */

diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 17588eb3a4bd7006f0ac0fed3bb17316b09a5207..b966b27c2b490dc39158ae487169322e860775b8 100644 (file)
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -14801,9 +14801,9 @@ bgp_vty_init (void)
   install_element (BGP_IPV6M_NODE, &neighbor_unsuppress_map_cmd);
   install_element (BGP_IPV6M_NODE, &no_neighbor_unsuppress_map_cmd);
   install_element (BGP_VPNV4_NODE, &neighbor_unsuppress_map_cmd);
-  install_element (BGP_VPNV4_NODE, &no_neighbor_unsuppress_map_cmd);  
+  install_element (BGP_VPNV4_NODE, &no_neighbor_unsuppress_map_cmd);
   install_element (BGP_VPNV6_NODE, &neighbor_unsuppress_map_cmd);
-  install_element (BGP_VPNV6_NODE, &no_neighbor_unsuppress_map_cmd);  
+  install_element (BGP_VPNV6_NODE, &no_neighbor_unsuppress_map_cmd);
 
   /* "neighbor maximum-prefix" commands. */
   install_element (BGP_NODE, &neighbor_maximum_prefix_cmd);

diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
index 0241be380fdc2cd2e91263114f01a3733ed21970..06106d87f671b28506794cfdb59083d7d9d2bfe2 100644 (file)
--- a/bgpd/bgp_zebra.c
+++ b/bgpd/bgp_zebra.c
@@ -2094,3 +2094,13 @@ bgp_zebra_init (struct thread_master *master)
   bgp_nexthop_buf = stream_new(BGP_NEXTHOP_BUF_SIZE);
   bgp_ifindices_buf = stream_new(BGP_IFINDICES_BUF_SIZE);
 }
+
+void
+bgp_zebra_destroy(void)
+{
+  if (zclient == NULL)
+    return;
+  zclient_stop(zclient);
+  zclient_free(zclient);
+  zclient = NULL;
+}

diff --git a/bgpd/bgp_zebra.h b/bgpd/bgp_zebra.h
index 1770a2c0d5a453c26629a02cdb4029e5010016b9..1f845bd9184c512eb9bba54fcb32a001e26e4c8c 100644 (file)
--- a/bgpd/bgp_zebra.h
+++ b/bgpd/bgp_zebra.h
@@ -27,7 +27,8 @@ Boston, MA 02111-1307, USA.  */
 extern struct stream *bgp_nexthop_buf;
 extern struct stream *bgp_ifindices_buf;
 
-extern void bgp_zebra_init(struct thread_master *master);
+extern void bgp_zebra_init (struct thread_master *master);
+extern void bgp_zebra_destroy (void);
 extern int bgp_if_update_all (void);
 extern int bgp_config_write_maxpaths (struct vty *, struct bgp *, afi_t,
                                      safi_t, int *);

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 06956b0ad6850ba431e3e0ad8b231d1711a0c19a..eda43d2b600767158026e48c2d9a1079290fa4ec 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -2034,6 +2034,9 @@ peer_delete (struct peer *peer)
       XFREE(MTYPE_HOST, peer->domainname);
       peer->domainname = NULL;
     }
+  
+  if (CHECK_FLAG(bgp->flags, BGP_FLAG_DELETING))
+    bgp_peer_clear_node_queue_drain_immediate(peer);
 
   peer_unlock (peer); /* initial reference */
 
@@ -3044,6 +3047,8 @@ bgp_delete (struct bgp *bgp)
   afi_t afi;
   int i;
 
+  SET_FLAG(bgp->flags, BGP_FLAG_DELETING);
+
   THREAD_OFF (bgp->t_startup);
 
   if (BGP_DEBUG (zebra, ZEBRA))
@@ -3095,6 +3100,15 @@ bgp_delete (struct bgp *bgp)
 
   /* TODO - Other memory may need to be freed - e.g., NHT */
 
+  /*
+   * Free pending deleted routes. Unfortunately, it also has to process
+   * all the pending activity for other instances of struct bgp.
+   *
+   * This call was added to achieve clean memory allocation at exit,
+   * for the sake of valgrind.
+   */
+  bgp_process_queues_drain_immediate();
+
   /* Remove visibility via the master list - there may however still be
    * routes to be processed still referencing the struct bgp.
    */

diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index b3e508634e7f3aad23586f3a5577df6d0c78b989..06a50e228d881ab4c54cd2192aa6566466b13736 100644 (file)
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -281,6 +281,7 @@ struct bgp
 #define BGP_FLAG_MULTIPATH_RELAX_AS_SET   (1 << 17)
 #define BGP_FLAG_FORCE_STATIC_PROCESS     (1 << 18)
 #define BGP_FLAG_SHOW_HOSTNAME            (1 << 19)
+#define BGP_FLAG_DELETING                 (1 << 20)
 
   /* BGP Per AF flags */
   u_int16_t af_flags[AFI_MAX][SAFI_MAX];

diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c
index 75ce5848189a8a84963616b4ef2de55705d99cd9..4d862ed19d60697b55ec9b7c39a7d292b2a6162c 100644 (file)
--- a/vtysh/vtysh.c
+++ b/vtysh/vtysh.c
@@ -361,7 +361,7 @@ vtysh_execute_func (const char *line, int pager)
    * to move into node in the vtysh where it succeeded. */
   if (ret == CMD_SUCCESS || ret == CMD_SUCCESS_DAEMON || ret == CMD_WARNING)
     {
-      if ((saved_node == BGP_VPNV4_NODE || saved_node == BGP_VPNV6_NODE 
+      if ((saved_node == BGP_VPNV4_NODE || saved_node == BGP_VPNV6_NODE
            || saved_node == BGP_IPV4_NODE
           || saved_node == BGP_IPV6_NODE || saved_node == BGP_IPV4M_NODE
           || saved_node == BGP_IPV6M_NODE)