From f9164b1d74f6a20d69d7ef10d2e39b4ae7996cbf Mon Sep 17 00:00:00 2001
From: Paul Jakma <paul.jakma@hpe.com>
Date: Tue, 6 Sep 2016 17:23:48 +0100
Subject: [PATCH] bgpd: bgp_nexthop_cache not deleted with peers

* Fix mild leak, bgp_nexthop_caches were not deleted when their peer was.
  Not a huge one, but makes valgrinding for other leaks noisier.

  Credit to Lou Berger <lberger@labn.net> for doing the hard work of
  debugging and pinning down the leak, and supplying an initial fix.
  That one didn't quite get the refcounting right, it seemed, hence
  this version.

  This version also keeps bncs pinned so long as the peer is defined, where
  Lou's tried to delete whenever the peer went through bgp_stop. That causes
  lots of zebra traffic if down peers go Active->Connect->Active, etc., so
  leaving bnc's in place until peer_delete seemed better.

* bgp_nht.c: (bgp_unlink_nexthop_by_peer) similar to bgp_unlink_nexthop, but
  by peer.
* bgp_nht.c: (bgp_unlink_nexthop_check) helper to consolidate checking
  if a bnc should be deleted.
  (bgp_unlink_nexthop_by_peer) ensure the bnc->nht_info peer reference
  is removed, and hence allow bncs to be removed by previous.
* bgpd.c: (peer_delete) cleanup the peer's bnc.
---
 bgpd/bgp_nht.c | 66 +++++++++++++++++++++++++++++++++++++++++---------
 bgpd/bgp_nht.h |  1 +
 bgpd/bgpd.c    |  5 +++-
 3 files changed, 59 insertions(+), 13 deletions(-)

diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c
index 48f48ede4b..9aae3e7b33 100644
--- a/bgpd/bgp_nht.c
+++ b/bgpd/bgp_nht.c
@@ -75,16 +75,9 @@ bgp_find_nexthop (struct bgp_info *path, int connected)
   return (bgp_isvalid_nexthop(bnc));
 }
 
-void
-bgp_unlink_nexthop (struct bgp_info *path)
+static void
+bgp_unlink_nexthop_check (struct bgp_nexthop_cache *bnc)
 {
-  struct bgp_nexthop_cache *bnc = path->nexthop;
-
-  if (!bnc)
-    return;
-
-  path_nh_map(path, NULL, 0);
-
   if (LIST_EMPTY(&(bnc->paths)) && !bnc->nht_info)
     {
       if (BGP_DEBUG(nht, NHT))
@@ -96,10 +89,60 @@ bgp_unlink_nexthop (struct bgp_info *path)
       unregister_zebra_rnh(bnc, CHECK_FLAG(bnc->flags, BGP_STATIC_ROUTE));
       bnc->node->info = NULL;
       bgp_unlock_node(bnc->node);
+      bnc->node = NULL;
       bnc_free(bnc);
     }
 }
 
+void
+bgp_unlink_nexthop (struct bgp_info *path)
+{
+  struct bgp_nexthop_cache *bnc = path->nexthop;
+
+  if (!bnc)
+    return;
+
+  path_nh_map(path, NULL, 0);
+  
+  bgp_unlink_nexthop_check (bnc);
+}
+
+void
+bgp_unlink_nexthop_by_peer (struct peer *peer)
+{
+  struct prefix p;
+  struct bgp_node *rn;
+  struct bgp_nexthop_cache *bnc;
+  afi_t afi = family2afi(peer->su.sa.sa_family);
+
+  if (afi == AFI_IP)
+    {
+      p.family = AF_INET;
+      p.prefixlen = IPV4_MAX_BITLEN;
+      p.u.prefix4 = peer->su.sin.sin_addr;
+    }
+  else if (afi == AFI_IP6)
+    {
+      p.family = AF_INET6;
+      p.prefixlen = IPV6_MAX_BITLEN;
+      p.u.prefix6 = peer->su.sin6.sin6_addr;
+    }
+  else
+    return;
+
+  rn = bgp_node_get (peer->bgp->nexthop_cache_table[afi], &p);
+
+  if (!rn->info)
+    return;
+  
+  bnc = rn->info;
+  
+  /* cleanup the peer reference */
+  bnc->nht_info = NULL;
+  
+  bgp_unlink_nexthop_check (bnc);
+}
+
 int
 bgp_find_or_add_nexthop (struct bgp *bgp, afi_t afi, struct bgp_info *ri,
                          struct peer *peer, int connected)
@@ -231,8 +274,7 @@ bgp_find_or_add_nexthop (struct bgp *bgp, afi_t afi, struct bgp_info *ri,
        */
       bgp_unlink_nexthop (ri);
 
-      /* Link to new nexthop cache. */
-      path_nh_map(ri, bnc, 1);
+      path_nh_map(ri, bnc, 1); /* updates NHT ri list reference */
 
       if (CHECK_FLAG(bnc->flags, BGP_NEXTHOP_VALID) && bnc->metric)
 	(bgp_info_extra_get(ri))->igpmetric = bnc->metric;
@@ -240,7 +282,7 @@ bgp_find_or_add_nexthop (struct bgp *bgp, afi_t afi, struct bgp_info *ri,
 	ri->extra->igpmetric = 0;
     }
   else if (peer)
-    bnc->nht_info = (void *)peer;
+    bnc->nht_info = (void *)peer; /* NHT peer reference */
 
   return (bgp_isvalid_nexthop(bnc));
 }
diff --git a/bgpd/bgp_nht.h b/bgpd/bgp_nht.h
index af64750d75..02a7e5a45c 100644
--- a/bgpd/bgp_nht.h
+++ b/bgpd/bgp_nht.h
@@ -55,6 +55,7 @@ extern int bgp_find_or_add_nexthop(struct bgp *bgp, afi_t a,
  *   p - path structure.
  */
 extern void bgp_unlink_nexthop(struct bgp_info *p);
+void bgp_unlink_nexthop_by_peer (struct peer *);
 
 /**
  * bgp_delete_connected_nexthop() - Reset the 'peer' pointer for a connected
diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 9ad68fbc67..71c2c84e61 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -1971,7 +1971,10 @@ peer_delete (struct peer *peer)
 
   UNSET_FLAG(peer->sflags, PEER_STATUS_ACCEPT_PEER);
   bgp_fsm_change_status (peer, Deleted);
-
+  
+  /* Remove from NHT */
+  bgp_unlink_nexthop_by_peer (peer);
+  
   /* Password configuration */
   if (peer->password)
     {
-- 
2.39.5