Browse Source

bgpd: Rejiggle exported nht function names and consolidate some code

* bgp_nht.h: Tweak the API a bit to simplify and make names a bit clearer on
  function. Remove AFI argument, it's implied in both bgp_infos and peers.

  (bgp_find_nexthop) this doesn't so much find a bnc, as check the bnc
  for the given bgp_info is valid. Rename to (bgp_nexthop_check).

  (bgp_find_or_add_nexthop) This ensures a bnc exists, so call it
  (bgp_ensure_nexthop).

  (bgp_unlink_nexthop_by_peer) Remove via peer.

* bgp_nht.c: Adjust to above.
  (bgp_get_nexthop_rn) helper to get the rn.
  (bgp_find_nexthop) further helper to get the bnc for path or peer.
  (bgp_unlink_nexthop_check) helper to check whether a bnc should go.
  (bgp_ensure_nexthop) Use the helpers.

* bgp_{route,fsm}.c: s/bgp_find_or_add_nexthop/bgp_ensure_nexthop/
Paul Jakma 3 years ago
parent
commit
3dda6b3ecc
4 changed files with 132 additions and 77 deletions
  1. 2 3
      bgpd/bgp_fsm.c
  2. 104 55
      bgpd/bgp_nht.c
  3. 19 12
      bgpd/bgp_nht.h
  4. 7 7
      bgpd/bgp_route.c

+ 2 - 3
bgpd/bgp_fsm.c

@@ -507,7 +507,7 @@ bgp_stop (struct peer *peer)
       /* Reset peer synctime */
       peer->synctime = 0;
     }
-
+  
   /* Stop read and write threads when exists. */
   BGP_READ_OFF (peer->t_read);
   BGP_WRITE_OFF (peer->t_write);
@@ -720,8 +720,7 @@ bgp_start (struct peer *peer)
       ! CHECK_FLAG (peer->flags, PEER_FLAG_DISABLE_CONNECTED_CHECK))
     connected = 1;
 
-  bgp_find_or_add_nexthop(family2afi(peer->su.sa.sa_family), NULL, peer,
-			  connected);
+  bgp_ensure_nexthop (NULL, peer, connected);
   status = bgp_connect (peer);
 
   switch (status)

+ 104 - 55
bgpd/bgp_nht.c

@@ -53,13 +53,21 @@ static void path_nh_map(struct bgp_info *path, struct bgp_nexthop_cache *bnc,
 			int keep);
 
 int
-bgp_find_nexthop (struct bgp_info *path, int connected)
+bgp_nexthop_check (struct bgp_info *path, int connected)
 {
   struct bgp_nexthop_cache *bnc = path->nexthop;
 
   if (!bnc)
     return 0;
 
+  if (BGP_DEBUG(nht, NHT))
+    {
+      char buf[INET6_ADDRSTRLEN];
+      zlog_debug("%s: NHT checking %s", 
+                 __FUNCTION__,
+                 bnc_str (bnc, buf, INET6_ADDRSTRLEN));
+    }
+
   if (connected && !(CHECK_FLAG(bnc->flags, BGP_NEXTHOP_CONNECTED)))
     return 0;
 
@@ -67,6 +75,67 @@ bgp_find_nexthop (struct bgp_info *path, int connected)
           CHECK_FLAG(bnc->flags, BGP_NEXTHOP_VALID));
 }
 
+/* Helper to get the rn for the appropriate nexthop for path or peer.
+ * returns the locked rn - caller must bump down the refcnt.
+ *
+ * may return NULL in error cases.
+ */
+static
+struct bgp_node *
+bgp_get_nexthop_rn (struct bgp_info *path, struct peer *peer)
+{
+  struct prefix p;
+  afi_t afi;
+  
+  assert (path || peer);
+  
+  if (!(path || peer))
+    return NULL;
+  
+  if (path)
+    {
+      afi = family2afi (path->net->p.family);
+      if (make_prefix(afi, path, &p) < 0)
+	return NULL;
+    }
+  else
+    {
+      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 NULL;
+    }
+  
+  return bgp_node_get (bgp_nexthop_cache_table[afi], &p);
+}
+
+static
+struct bgp_nexthop_cache *
+bgp_find_nexthop (struct bgp_info *path, struct peer *peer)
+{
+  struct bgp_nexthop_cache *bnc = NULL;
+  struct bgp_node *rn = bgp_get_nexthop_rn (path, peer);
+  
+  if (!rn)
+    return NULL;
+  
+  bnc = rn->info;
+  bgp_unlock_node (rn);
+  
+  return bnc;
+}
+
 static void
 bgp_unlink_nexthop_check (struct bgp_nexthop_cache *bnc)
 {
@@ -76,13 +145,13 @@ bgp_unlink_nexthop_check (struct bgp_nexthop_cache *bnc)
 	{
 	  char buf[INET6_ADDRSTRLEN];
 	  zlog_debug("bgp_unlink_nexthop: freeing bnc %s",
-		     bnc_str(bnc, buf, INET6_ADDRSTRLEN));
-	}
+		     bnc_str (bnc, buf, INET6_ADDRSTRLEN));
+ 	}
       unregister_nexthop(bnc);
       bnc->node->info = NULL;
-      bgp_unlock_node(bnc->node);
+      bgp_unlock_node (bnc->node);
       bnc->node = NULL;
-      bnc_free(bnc);
+      bnc_free (bnc);
     }
 }
 
@@ -94,6 +163,13 @@ bgp_unlink_nexthop (struct bgp_info *path)
   if (!bnc)
     return;
 
+  if (BGP_DEBUG(nht, NHT))
+    {
+      char buf[INET6_ADDRSTRLEN];
+      zlog_debug("%s: NHT unlinking %s", 
+                 __FUNCTION__, bnc_str (bnc, buf, INET6_ADDRSTRLEN));
+    }
+  
   path_nh_map(path, NULL, 0);
   
   bgp_unlink_nexthop_check (bnc);
@@ -102,70 +178,36 @@ bgp_unlink_nexthop (struct bgp_info *path)
 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
+  struct bgp_nexthop_cache *bnc = bgp_find_nexthop (NULL, peer);
+     
+  if (!bnc)
     return;
 
-  rn = bgp_node_get (bgp_nexthop_cache_table[afi], &p);
-
-  if (!rn->info)
-    return;
-  
-  bnc = rn->info;
+  if (BGP_DEBUG(nht, NHT))
+    zlog_debug("%s: NHT unlinking %s", 
+                __FUNCTION__, peer->host);
   
-  /* cleanup the peer reference */
   bnc->nht_info = NULL;
   
   bgp_unlink_nexthop_check (bnc);
 }
 
 int
-bgp_find_or_add_nexthop (afi_t afi, struct bgp_info *ri, struct peer *peer,
-			 int connected)
+bgp_ensure_nexthop (struct bgp_info *ri, struct peer *peer,
+                    int connected)
 {
   struct bgp_node *rn;
   struct bgp_nexthop_cache *bnc;
-  struct prefix p;
-
-  if (ri)
-    {
-      if (make_prefix(afi, ri, &p) < 0)
-	return 1;
-    }
-  else if (peer)
+  
+  rn = bgp_get_nexthop_rn (ri, peer);
+  
+  if (!rn)
     {
-      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;
-	}
+      zlog_debug("%s: NHT could not ensure, failed to get rn!", 
+                 __FUNCTION__);
+      return 0;
     }
-
-  rn = bgp_node_get (bgp_nexthop_cache_table[afi], &p);
-
+  
   if (!rn->info)
     {
       bnc = bnc_new();
@@ -194,6 +236,13 @@ bgp_find_or_add_nexthop (afi_t afi, struct bgp_info *ri, struct peer *peer,
   else if (peer)
     bnc->nht_info = (void *)peer; /* NHT peer reference */
 
+  if (BGP_DEBUG(nht, NHT))
+    {
+      char buf[INET6_ADDRSTRLEN];
+      zlog_debug("%s: NHT ensured %s", 
+                 __FUNCTION__, bnc_str (bnc, buf, INET6_ADDRSTRLEN));
+    }
+  
   return (bgp_zebra_num_connects() == 0 ||
           CHECK_FLAG(bnc->flags, BGP_NEXTHOP_VALID));
 }

+ 19 - 12
bgpd/bgp_nht.h

@@ -25,33 +25,40 @@
 /**
  * bgp_parse_nexthop_update() - parse a nexthop update message from Zebra.
  */
-extern void bgp_parse_nexthop_update(void);
+void bgp_parse_nexthop_update (void);
 
 /**
- * bgp_find_nexthop() - lookup the nexthop cache table for the bnc object
+ * bgp_nexthop_check() - check if the bnc object is valid.
  * ARGUMENTS:
  *   p - path for which the nexthop object is being looked up
  *   connected - True if NH MUST be a connected route
  */
-extern int bgp_find_nexthop(struct bgp_info *p, int connected);
+int bgp_nexthop_check (struct bgp_info *, int connected);
 
 /**
- * bgp_find_or_add_nexthop() - lookup the nexthop cache table for the bnc
- *  object. If not found, create a new object and register with ZEBRA for
- *  nexthop notification.
+ * bgp_ensure_nexthop() - Ensure a bgp_nexthop_cache object exists for 
+ *  the given prefix or peer.  If an existing one is not found,
+ *  create a new object and register with ZEBRA for nexthop
+ *  notification.
  * ARGUMENTS:
- *   a - afi: AFI_IP or AF_IP6
- *   p - path for which the nexthop object is being looked up
- *   peer - The BGP peer associated with this NHT
+ *   afi: AFI_IP or AF_IP6
+ *     struct bgp_info *: path for which the nexthop object is 
+ *                        being looked up
+ *   OR
+ *     struct peer The BGP peer associated with this NHT
  *   connected - True if NH MUST be a connected route
  */
-extern int bgp_find_or_add_nexthop(afi_t a, struct bgp_info *p,
-				   struct peer *peer, int connected);
+int bgp_ensure_nexthop (struct bgp_info *, struct peer *, int connected);
 
 /**
  * bgp_unlink_nexthop() - Unlink the nexthop object from the path structure.
  * ARGUMENTS:
- *   p - path structure.
+ *   struct bgp_info *: path structure.
+ */
+void bgp_unlink_nexthop (struct bgp_info *);
+
+/**
+ * bgp_unlink_nexthop() - Unlink the nexthop object for the given peer.
  */
 extern void bgp_unlink_nexthop(struct bgp_info *p);
 void bgp_unlink_nexthop_by_peer (struct peer *);

+ 7 - 7
bgpd/bgp_route.c

@@ -134,7 +134,7 @@ bgp_info_free (struct bgp_info *binfo)
   if (binfo->attr)
     bgp_attr_unintern (&binfo->attr);
 
-  bgp_unlink_nexthop(binfo);
+  bgp_unlink_nexthop (binfo);
   bgp_info_extra_free (&binfo->extra);
   bgp_info_mpath_free (&binfo->mpath);
 
@@ -2345,7 +2345,7 @@ bgp_update_main (struct peer *peer, struct prefix *p, struct attr *attr,
 	  else
 	    connected = 0;
 
-	  if (bgp_find_or_add_nexthop (afi, ri, NULL, connected))
+	  if (bgp_ensure_nexthop (ri, NULL, connected))
 	    bgp_info_set_flag (rn, ri, BGP_INFO_VALID);
 	  else
 	    {
@@ -2397,7 +2397,7 @@ bgp_update_main (struct peer *peer, struct prefix *p, struct attr *attr,
       else
 	connected = 0;
 
-      if (bgp_find_or_add_nexthop (afi, new, NULL, connected))
+      if (bgp_ensure_nexthop (new, NULL, connected))
 	bgp_info_set_flag (rn, new, BGP_INFO_VALID);
       else
 	{
@@ -3543,7 +3543,7 @@ bgp_static_update_rsclient (struct peer *rsclient, struct prefix *p,
 	  /* Nexthop reachability check. */
 	  if (bgp_flag_check (bgp, BGP_FLAG_IMPORT_CHECK))
 	    {
-	      if (bgp_find_or_add_nexthop (afi, ri, NULL, 0))
+	      if (bgp_ensure_nexthop (ri, NULL, 0))
 		bgp_info_set_flag (rn, ri, BGP_INFO_VALID);
 	      else
 		{
@@ -3572,7 +3572,7 @@ bgp_static_update_rsclient (struct peer *rsclient, struct prefix *p,
   /* Nexthop reachability check. */
   if (bgp_flag_check (bgp, BGP_FLAG_IMPORT_CHECK))
     {
-      if (bgp_find_or_add_nexthop (afi, new, NULL, 0))
+      if (bgp_ensure_nexthop (new, NULL, 0))
 	bgp_info_set_flag (rn, new, BGP_INFO_VALID);
       else
 	{
@@ -3692,7 +3692,7 @@ bgp_static_update_main (struct bgp *bgp, struct prefix *p,
 	  /* Nexthop reachability check. */
 	  if (bgp_flag_check (bgp, BGP_FLAG_IMPORT_CHECK))
 	    {
-	      if (bgp_find_or_add_nexthop (afi, ri, NULL, 0))
+	      if (bgp_ensure_nexthop (ri, NULL, 0))
 		bgp_info_set_flag (rn, ri, BGP_INFO_VALID);
 	      else
 		{
@@ -3722,7 +3722,7 @@ bgp_static_update_main (struct bgp *bgp, struct prefix *p,
   /* Nexthop reachability check. */
   if (bgp_flag_check (bgp, BGP_FLAG_IMPORT_CHECK))
     {
-      if (bgp_find_or_add_nexthop (afi, new, NULL, 0))
+      if (bgp_ensure_nexthop (new, NULL, 0))
 	bgp_info_set_flag (rn, new, BGP_INFO_VALID);
       else
 	{