bgpd: bgpd-restart-bit-fix.patch

ISSUE:

Quagga BGP doesn't send or use the restart-bit via the Graceful-Restart(GR)
capability. GR capability implementation isn't complete as per the RFC.

PATCH:

Patch uses BGP instance creation as the beginning of the startup period,
and 'restart_time' is taken as the startup period. As a result, BGP will
set the restart bit in the GR capability of the OPEN messages during the
startup period.

As an indication of quagga implementation's capability of sending End-Of-RIB,
helping a restarting neighbor, quagga BGP will now send global GR capability
irrespective of the graceful-restart config in BGP and the address-family
specific GR capability will be sent only if the GR config is present.
Forwarding bit is not set assuming its not preserved.

Signed-off-by: Vipin Kumar <vipin@cumulusnetworks.com>
Reviewed-by: Pradosh Mohapatra <pmohapat@cumulusnetworks.com>

diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c
index 7bf350165b670028591465cdfa4713bc4d1db3a0..fe741aa3acdac50acbd3c6563fde5e7554b91f14 100644 (file)
--- a/bgpd/bgp_open.c
+++ b/bgpd/bgp_open.c
@@ -344,7 +344,10 @@ bgp_capability_restart (struct peer *peer, struct capability_header *caphdr)
   SET_FLAG (peer->cap, PEER_CAP_RESTART_RCV);
   restart_flag_time = stream_getw(s);
   if (CHECK_FLAG (restart_flag_time, RESTART_R_BIT))
-    restart_bit = 1;
+    {
+      SET_FLAG (peer->cap, PEER_CAP_RESTART_BIT_RCV);
+      restart_bit = 1;
+    }
   UNSET_FLAG (restart_flag_time, 0xF000);
   peer->v_gr_restart = restart_flag_time;
 
@@ -898,10 +901,11 @@ void
 bgp_open_capability (struct stream *s, struct peer *peer)
 {
   u_char len;
-  unsigned long cp;
+  unsigned long cp, capp, rcapp;
   afi_t afi;
   safi_t safi;
   as_t local_as;
+  u_int32_t restart_time;
 
   /* Remember current pointer for Opt Parm Len. */
   cp = stream_get_endp (s);
@@ -1020,16 +1024,43 @@ bgp_open_capability (struct stream *s, struct peer *peer)
       stream_putc (s, CAPABILITY_CODE_DYNAMIC_LEN);
     }
 
-  /* Graceful restart capability */
+  /* Sending base graceful-restart capability irrespective of the config */
+  SET_FLAG (peer->cap, PEER_CAP_RESTART_ADV);
+  stream_putc (s, BGP_OPEN_OPT_CAP);
+  capp = stream_get_endp (s);           /* Set Capability Len Pointer */
+  stream_putc (s, 0);                   /* Capability Length */
+  stream_putc (s, CAPABILITY_CODE_RESTART);
+  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)
+    {
+      SET_FLAG (restart_time, RESTART_R_BIT);
+      SET_FLAG (peer->cap, PEER_CAP_RESTART_BIT_ADV);
+    }
+  stream_putw (s, restart_time);
+
+  /* Send address-family specific graceful-restart capability only when GR config
+     is present */
   if (bgp_flag_check (peer->bgp, BGP_FLAG_GRACEFUL_RESTART))
     {
-      SET_FLAG (peer->cap, PEER_CAP_RESTART_ADV);
-      stream_putc (s, BGP_OPEN_OPT_CAP);
-      stream_putc (s, CAPABILITY_CODE_RESTART_LEN + 2);
-      stream_putc (s, CAPABILITY_CODE_RESTART);
-      stream_putc (s, CAPABILITY_CODE_RESTART_LEN);
-      stream_putw (s, peer->bgp->restart_time);
-     }
+      for (afi = AFI_IP ; afi < AFI_MAX ; afi++)
+        for (safi = SAFI_UNICAST ; safi < SAFI_MAX ; safi++)
+          if (peer->afc[afi][safi])
+            {
+              stream_putw (s, afi);
+              stream_putc (s, safi);
+              stream_putc (s, 0); //Forwarding is not retained as of now.
+            }
+    }
+
+  /* Total Graceful restart capability Len. */
+  len = stream_get_endp (s) - rcapp - 1;
+  stream_putc_at (s, rcapp, len);
+
+  /* Total Capability Len. */
+  len = stream_get_endp (s) - capp - 1;
+  stream_putc_at (s, capp, len);
 
   /* Total Opt Parm Len. */
   len = stream_get_endp (s) - cp - 1;

diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 65c6cac16ac77afc9152d50a35e9f9582125f37c..62ef86166fc3ba7ea52197a1e136c43fdf5a05a7 100644 (file)
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -614,6 +614,10 @@ bgp_write_packet (struct peer *peer)
              {
                if (CHECK_FLAG (adv->binfo->peer->cap, PEER_CAP_RESTART_RCV)
                    && CHECK_FLAG (adv->binfo->peer->cap, PEER_CAP_RESTART_ADV)
+                   && ! (CHECK_FLAG (adv->binfo->peer->cap,
+                                      PEER_CAP_RESTART_BIT_RCV) &&
+                         CHECK_FLAG (adv->binfo->peer->cap,
+                                      PEER_CAP_RESTART_BIT_ADV))
                    && ! CHECK_FLAG (adv->binfo->flags, BGP_INFO_STALE)
                    && safi != SAFI_MPLS_VPN)
                  {

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 4d374cc19a7607157239bc79f5f67a81d7b95fc7..140cb189cc685e52933d38ab3dbe86f3ec8d20e4 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -1927,6 +1927,18 @@ 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)
@@ -1972,6 +1984,9 @@ 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;
 }
 

diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 322cc7ed61915401f581d060b2b4c6b296e2fadd..73f2849ef564367f16f77dc4b348a83b13bc4730 100644 (file)
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -104,6 +104,8 @@ 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)
@@ -368,6 +370,8 @@ struct peer
 #define PEER_CAP_RESTART_RCV                (1 << 6) /* restart received */
 #define PEER_CAP_AS4_ADV                    (1 << 7) /* as4 advertised */
 #define PEER_CAP_AS4_RCV                    (1 << 8) /* as4 received */
+#define PEER_CAP_RESTART_BIT_ADV            (1 << 9) /* sent restart state */
+#define PEER_CAP_RESTART_BIT_RCV            (1 << 10) /* peer restart state */
 
   /* Capability flags (reset in bgp_stop) */
   u_int16_t af_cap[AFI_MAX][SAFI_MAX];