Fix BGP's use of restart bit.
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.
Incorporated feedback from David Lamparter via the quagga-dev mailing list.
Signed-off-by: Vipin Kumar <vipin@cumulusnetworks.com>
Reviewed-by: Pradosh Mohapatra <pmohapat@cumulusnetworks.com>
Reviewed-by: Paul Jakma <paul@opensourcerouting.org>
diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c
index 7bf3501..fe741aa 100644
--- a/bgpd/bgp_open.c
+++ b/bgpd/bgp_open.c
@@ -344,7 +344,10 @@
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 @@
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 @@
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 35a22c1..14fd6e5 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -613,6 +613,10 @@
{
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 3a9bc48..79bcaaf 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -1927,6 +1927,18 @@
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 @@
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 eae803d..40c381c 100644
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -104,6 +104,8 @@
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)
@@ -366,6 +368,8 @@
#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];