[quagga-dev 12181] [PATCH 1/2] bgpd: GR Restart-bit should be stateful rather than timer from startup

Paul Jakma paul at opensourcerouting.org
Mon Apr 27 16:36:09 BST 2015


As an RFC mostly, I havn't actually tested this. Current code sets R-bit only 
for a certain amount of time after startup. It seems to me that might not be
right.

Also interesting is that the R-bit in the OPEN is global, but the EoRs are
per-AFI/SAFI - slight conceptual mismatch in the spec maybe.

* The GR restart bit should be set on the OPEN of at least the first
  connection to a remote GR peer since startup.  If a connection comes up
  slowly after a restart for some reason, there's not much point having the
  other side wait for us (if it had restarted too but kept), by not setting
  the R-bit.
  First connection is deemed successful once the peer sends EoR on all
  configured AFI.
* bgpd.{c,h}: Remove the old timer for the GR R-bit. Instead, set
  PEER_STATUS_GR_SEND_R_BIT by default on new peer structures, to be unset
  when connection first is fully up.
* bgp_packet.c: (bgp_update_all_eor_recvd) Tally up whether all EoRs have been
  received.
  (bgp_update_receive) IPv6/Multicast EoR case was missing setting of the
  EoR flag on the peer af_sflags.
  Make format of IPv6/Unicast EoR flag set the same as others.
  Unset the "Send the R-bit" flag when all EoRs are in from the peer.
---
 bgpd/bgp_open.c   |  2 +-
 bgpd/bgp_packet.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
 bgpd/bgpd.c       | 22 +++-------------------
 bgpd/bgpd.h       |  3 +--
 lib/zebra.h       |  1 +
 5 files changed, 47 insertions(+), 29 deletions(-)

diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c
index b9d6e93..7cdd961 100644
--- a/bgpd/bgp_open.c
+++ b/bgpd/bgp_open.c
@@ -1032,7 +1032,7 @@ bgp_open_capability (struct stream *s, struct peer *peer)
   rcapp = stream_get_endp (s);          /* Set Restart Capability Len Pointer */
   stream_putc (s, 0);
   restart_time = peer->bgp->restart_time;
-  if (peer->bgp->t_startup)
+  if (CHECK_FLAG (peer->sflags, PEER_STATUS_GR_SEND_R_BIT))
     {
       SET_FLAG (restart_time, RESTART_R_BIT);
       SET_FLAG (peer->cap, PEER_CAP_RESTART_BIT_ADV);
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 14fd6e5..0a916d0 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -1596,6 +1596,22 @@ bgp_open_receive (struct peer *peer, bgp_size_t size)
   return 0;
 }
 
+static bool
+bgp_update_all_eor_recvd (struct peer *peer)
+{
+  int conf = 0, recvd = 0;
+  for (afi_t afi = AFI_IP; afi < AFI_MAX; afi++)
+    for (safi_t safi = SAFI_UNICAST; safi < SAFI_MAX; safi++)
+      if (peer->afc[afi][safi])
+        {
+          conf++;
+          if (CHECK_FLAG (peer->af_sflags[afi][safi],
+                          PEER_STATUS_EOR_RECEIVED))
+            recvd++;
+        }
+  return (conf == recvd);
+}
+
 /* Parse BGP Update packet and make attribute object. */
 static int
 bgp_update_receive (struct peer *peer, bgp_size_t size)
@@ -1612,7 +1628,8 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
   struct bgp_nlri withdraw;
   struct bgp_nlri mp_update;
   struct bgp_nlri mp_withdraw;
-
+  bool any_eor_recvd = false;
+  
   /* Status must be Established. */
   if (peer->status != Established) 
     {
@@ -1796,7 +1813,9 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
 	  /* End-of-RIB received */
 	  SET_FLAG (peer->af_sflags[AFI_IP][SAFI_UNICAST],
 		    PEER_STATUS_EOR_RECEIVED);
-
+          
+          any_eor_recvd = true;
+          
 	  /* NSF delete stale route */
 	  if (peer->nsf[AFI_IP][SAFI_UNICAST])
 	    bgp_clear_stale_route (peer, AFI_IP, SAFI_UNICAST);
@@ -1826,7 +1845,9 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
 	  /* End-of-RIB received */
 	  SET_FLAG (peer->af_sflags[AFI_IP][SAFI_MULTICAST],
 		    PEER_STATUS_EOR_RECEIVED);
-
+          
+          any_eor_recvd = true;
+          
 	  /* NSF delete stale route */
 	  if (peer->nsf[AFI_IP][SAFI_MULTICAST])
 	    bgp_clear_stale_route (peer, AFI_IP, SAFI_MULTICAST);
@@ -1854,8 +1875,11 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
 	  && mp_withdraw.length == 0)
 	{
 	  /* End-of-RIB received */
-	  SET_FLAG (peer->af_sflags[AFI_IP6][SAFI_UNICAST], PEER_STATUS_EOR_RECEIVED);
-
+	  SET_FLAG (peer->af_sflags[AFI_IP6][SAFI_UNICAST],
+	            PEER_STATUS_EOR_RECEIVED);
+          
+          any_eor_recvd = true;
+          
 	  /* NSF delete stale route */
 	  if (peer->nsf[AFI_IP6][SAFI_UNICAST])
 	    bgp_clear_stale_route (peer, AFI_IP6, SAFI_UNICAST);
@@ -1883,7 +1907,11 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
 	  && mp_withdraw.length == 0)
 	{
 	  /* End-of-RIB received */
-
+	  SET_FLAG (peer->af_sflags[AFI_IP6][SAFI_MULTICAST],
+	            PEER_STATUS_EOR_RECEIVED);
+          
+          any_eor_recvd = true;
+          
 	  /* NSF delete stale route */
 	  if (peer->nsf[AFI_IP6][SAFI_MULTICAST])
 	    bgp_clear_stale_route (peer, AFI_IP6, SAFI_MULTICAST);
@@ -1917,7 +1945,13 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
 		  peer->host);
 	}
     }
-
+    
+  /* The Restart bit is global, but EoR is per-AFI/SAFI. Unsetting the
+   * R-bit when all EoRs are in is best we can do given the protocol.
+   */
+  if (any_eor_recvd && bgp_update_all_eor_recvd (peer))
+    UNSET_FLAG (peer->sflags, PEER_STATUS_GR_SEND_R_BIT);
+  
   /* Everything is done.  We unintern temporary structures which
      interned in bgp_attr_parse(). */
   bgp_attr_unintern_sub (&attr);
diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index d72708e..b006597 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -827,7 +827,8 @@ peer_new (struct bgp *bgp)
 	peer->orf_plist[afi][safi] = NULL;
       }
   SET_FLAG (peer->sflags, PEER_STATUS_CAPABILITY_OPEN);
-
+  SET_FLAG (peer->sflags, PEER_STATUS_GR_SEND_R_BIT);
+   
   /* Create buffers.  */
   peer->ibuf = stream_new (BGP_MAX_PACKET_SIZE);
   peer->obuf = stream_fifo_new ();
@@ -1927,18 +1928,6 @@ peer_group_unbind (struct bgp *bgp, struct peer *peer,
   return 0;
 }
 
-
-static int
-bgp_startup_timer_expire (struct thread *thread)
-{
-  struct bgp *bgp;
-
-  bgp = THREAD_ARG (thread);
-  bgp->t_startup = NULL;
-
-  return 0;
-}
-
 /* BGP instance creation by `router bgp' commands. */
 static struct bgp *
 bgp_create (as_t *as, const char *name)
@@ -1983,10 +1972,7 @@ bgp_create (as_t *as, const char *name)
 
   if (name)
     bgp->name = strdup (name);
-
-  THREAD_TIMER_ON (master, bgp->t_startup, bgp_startup_timer_expire,
-                   bgp, bgp->restart_time);
-
+    
   return bgp;
 }
 
@@ -2103,8 +2089,6 @@ bgp_delete (struct bgp *bgp)
   afi_t afi;
   int i;
 
-  THREAD_OFF (bgp->t_startup);
-
   /* Delete static route. */
   bgp_static_delete (bgp);
 
diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 40c381c..8cfb81b 100644
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -104,8 +104,6 @@ struct bgp
   as_t *confed_peers;
   int confed_peers_cnt;
 
-  struct thread *t_startup;
-
   /* BGP flags. */
   u_int16_t flags;
 #define BGP_FLAG_ALWAYS_COMPARE_MED       (1 << 0)
@@ -437,6 +435,7 @@ struct peer
 #define PEER_STATUS_GROUP             (1 << 4) /* peer-group conf */
 #define PEER_STATUS_NSF_MODE          (1 << 5) /* NSF aware peer */
 #define PEER_STATUS_NSF_WAIT          (1 << 6) /* wait comeback peer */
+#define PEER_STATUS_GR_SEND_R_BIT     (1 << 7) /* 1st connection, send R-bit */
 
   /* Peer status af flags (reset in bgp_stop) */
   u_int16_t af_sflags[AFI_MAX][SAFI_MAX];
diff --git a/lib/zebra.h b/lib/zebra.h
index ad22696..1fc4a4d 100644
--- a/lib/zebra.h
+++ b/lib/zebra.h
@@ -41,6 +41,7 @@ typedef int socklen_t;
 #include <stdio.h>
 #include <stdlib.h>
 #include <stddef.h>
+#include <stdbool.h>
 #include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
-- 
2.1.0





More information about the Quagga-dev mailing list