bgpd: Refine restarter operation - R-bit & F-bit

Introduce BGP-wide flags to denote if BGP has started gracefully and GR is in progress or not. Use this for setting of the R-bit in the GR capability, and not a timer which is set for any new instance creation. Mark graceful restart is complete when the deferred path selection has been done and route sync with zebra as well as deferred EOR advertisement has been initiated. Introduce a function to check on F-bit setting rather than just base it on configuration. Subsequent commits will extend these functionalities. Signed-off-by: Vivek Venkatraman <[email protected]>
Pdoijode · May 28, 2024 · 1960c35 · 1960c35
1 parent 06840a2
commit 1960c35
Show file tree

Hide file tree

Showing 8 changed files with 125 additions and 52 deletions.
diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c
@@ -2046,9 +2046,10 @@ static int bgp_start_deferral_timer(struct bgp *bgp, afi_t afi, safi_t safi,
 	}
 	gr_info->eor_required++;
 	/* Send message to RIB indicating route update pending */
-	if (gr_info->af_enabled[afi][safi] == false) {
-		gr_info->af_enabled[afi][safi] = true;
-		/* Send message to RIB */
+	if (gr_info->af_enabled == false) {
+		gr_info->af_enabled = true;
+		gr_info->route_sync = false;
+		bgp->gr_route_sync_pending = true;
 		bgp_zebra_update(bgp, afi, safi,
 				 ZEBRA_CLIENT_ROUTE_UPDATE_PENDING);
 	}
@@ -2082,7 +2083,7 @@ static int bgp_update_gr_info(struct peer *peer, afi_t afi, safi_t safi)
 	if (BGP_PEER_GRACEFUL_RESTART_CAPABLE(peer)
 	    && BGP_PEER_RESTARTING_MODE(peer)) {
 		/* Check if the forwarding state is preserved */
-		if (CHECK_FLAG(bgp->flags, BGP_FLAG_GR_PRESERVE_FWD)) {
+		if (bgp_gr_is_forwarding_preserved(bgp)) {
 			gr_info = &(bgp->gr_info[afi][safi]);
 			ret = bgp_start_deferral_timer(bgp, afi, safi, gr_info);
 		}
@@ -2199,8 +2200,7 @@ bgp_establish(struct peer_connection *connection)
 			} else {
 				if (BGP_PEER_GRACEFUL_RESTART_CAPABLE(peer) &&
 				    BGP_PEER_RESTARTING_MODE(peer) &&
-				    CHECK_FLAG(peer->bgp->flags,
-					       BGP_FLAG_GR_PRESERVE_FWD))
+				    bgp_gr_is_forwarding_preserved(peer->bgp))
 					peer->bgp->gr_info[afi][safi]
 						.eor_required++;
 			}

diff --git a/bgpd/bgp_main.c b/bgpd/bgp_main.c
@@ -507,13 +507,15 @@ int main(int argc, char **argv)
 
 	/* BGP master init. */
 	bgp_master_init(frr_init(), buffer_size, addresses);
+	bm->startup_time = monotime(NULL);
 	bm->port = bgp_port;
 	if (bgp_port == 0)
 		bgp_option_set(BGP_OPT_NO_LISTEN);
 	if (no_fib_flag || no_zebra_flag)
 		bgp_option_set(BGP_OPT_NO_FIB);
 	if (no_zebra_flag)
 		bgp_option_set(BGP_OPT_NO_ZEBRA);
+	SET_FLAG(bm->flags, BM_FLAG_GRACEFUL_RESTART);
 	bgp_error_init();
 	/* Initializations. */
 	bgp_vrf_init();

diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c
@@ -1587,15 +1587,12 @@ static void bgp_peer_send_gr_capability(struct stream *s, struct peer *peer,
 	uint32_t restart_time;
 	unsigned long capp = 0;
 	unsigned long rcapp = 0;
+	struct bgp *bgp = peer->bgp;
 
 	if (!CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART)
 	    && !CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART_HELPER))
 		return;
 
-	if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
-		zlog_debug("[BGP_GR] Sending helper Capability for Peer :%s :",
-			   peer->host);
-
 	SET_FLAG(peer->cap, PEER_CAP_RESTART_ADV);
 	stream_putc(s, BGP_OPEN_OPT_CAP);
 	capp = stream_get_endp(s); /* Set Capability Len Pointer */
@@ -1605,54 +1602,58 @@ static void bgp_peer_send_gr_capability(struct stream *s, struct peer *peer,
 	/* Set Restart Capability Len Pointer */
 	rcapp = stream_get_endp(s);
 	stream_putc(s, 0);
-	restart_time = peer->bgp->restart_time;
-	if (peer->bgp->t_startup) {
+	restart_time = bgp->restart_time;
+	if (bgp_in_graceful_restart()) {
 		SET_FLAG(restart_time, GRACEFUL_RESTART_R_BIT);
 		SET_FLAG(peer->cap, PEER_CAP_GRACEFUL_RESTART_R_BIT_ADV);
-		if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
-			zlog_debug("[BGP_GR] Sending R-Bit for peer: %s",
-				   peer->host);
 	}
 
-	if (CHECK_FLAG(peer->bgp->flags, BGP_FLAG_GRACEFUL_NOTIFICATION)) {
+	if (CHECK_FLAG(bgp->flags, BGP_FLAG_GRACEFUL_NOTIFICATION)) {
 		SET_FLAG(restart_time, GRACEFUL_RESTART_N_BIT);
 		SET_FLAG(peer->cap, PEER_CAP_GRACEFUL_RESTART_N_BIT_ADV);
-		if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
-			zlog_debug("[BGP_GR] Sending N-Bit for peer: %s",
-				   peer->host);
 	}
 
 	stream_putw(s, restart_time);
 
+	if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
+		zlog_debug("%s: Sending GR Capability, Restart time %d R-bit %s, N-bit %s",
+			   peer->host, bgp->restart_time,
+			   CHECK_FLAG(peer->cap,
+				      PEER_CAP_GRACEFUL_RESTART_R_BIT_ADV)
+				   ? "SET"
+				   : "NOT-SET",
+			   CHECK_FLAG(peer->cap,
+				      PEER_CAP_GRACEFUL_RESTART_N_BIT_ADV)
+				   ? "SET"
+				   : "NOT-SET");
+
 	/* Send address-family specific graceful-restart capability
 	 * only when GR config is present
 	 */
 	if (CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART)) {
-		if (CHECK_FLAG(peer->bgp->flags, BGP_FLAG_GR_PRESERVE_FWD)
-		    && BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
-			zlog_debug("[BGP_GR] F bit Set");
 
 		FOREACH_AFI_SAFI (afi, safi) {
+			bool f_bit = false;
+
 			if (!peer->afc[afi][safi])
 				continue;
 
-			if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
-				zlog_debug(
-					"[BGP_GR] Sending GR Capability for AFI :%d :, SAFI :%d:",
-					afi, safi);
-
 			/* Convert AFI, SAFI to values for
 			 * packet.
 			 */
 			bgp_map_afi_safi_int2iana(afi, safi, &pkt_afi,
 						  &pkt_safi);
 			stream_putw(s, pkt_afi);
 			stream_putc(s, pkt_safi);
-			if (CHECK_FLAG(peer->bgp->flags,
-				       BGP_FLAG_GR_PRESERVE_FWD))
-				stream_putc(s, GRACEFUL_RESTART_F_BIT);
-			else
-				stream_putc(s, 0);
+
+			f_bit = bgp_gr_is_forwarding_preserved(bgp);
+
+			if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
+				zlog_debug("... F-bit %s for %s",
+					   f_bit ? "SET" : "NOT-SET",
+					   get_afi_safi_str(afi, safi, false));
+
+			stream_putc(s, f_bit ? GRACEFUL_RESTART_F_BIT : 0);
 		}
 	}
 

diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
@@ -1298,7 +1298,7 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
 		stream_putc(s, 0);
 		gr_restart_time = peer->bgp->restart_time;
 
-		if (peer->bgp->t_startup) {
+		if (bgp_in_graceful_restart()) {
 			SET_FLAG(gr_restart_time, GRACEFUL_RESTART_R_BIT);
 			SET_FLAG(peer->cap, PEER_CAP_GRACEFUL_RESTART_R_BIT_ADV);
 		}

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
@@ -3857,6 +3857,7 @@ void bgp_best_path_select_defer(struct bgp *bgp, afi_t afi, safi_t safi)
 	struct bgp_dest *dest;
 	int cnt = 0;
 	struct afi_safi_info *thread_info;
+	bool route_sync_pending = false;
 
 	if (bgp->gr_info[afi][safi].t_route_select) {
 		struct event *t = bgp->gr_info[afi][safi].t_route_select;
@@ -3866,7 +3867,7 @@ void bgp_best_path_select_defer(struct bgp *bgp, afi_t afi, safi_t safi)
 		EVENT_OFF(bgp->gr_info[afi][safi].t_route_select);
 	}
 
-	if (BGP_DEBUG(update, UPDATE_OUT)) {
+	if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) {
 		zlog_debug("%s: processing route for %s : cnt %d", __func__,
 			   get_afi_safi_str(afi, safi, false),
 			   bgp->gr_info[afi][safi].gr_deferred);
@@ -3899,6 +3900,23 @@ void bgp_best_path_select_defer(struct bgp *bgp, afi_t afi, safi_t safi)
 		/* Send route processing complete message to RIB */
 		bgp_zebra_update(bgp, afi, safi,
 				 ZEBRA_CLIENT_ROUTE_UPDATE_COMPLETE);
+		bgp->gr_info[afi][safi].route_sync = true;
+
+		/* If this instance is all done, check for GR completion overall */
+		for (afi = AFI_IP; afi < AFI_MAX; afi++) {
+			for (safi = SAFI_UNICAST; safi <= SAFI_MPLS_VPN;
+			     safi++) {
+				if (bgp->gr_info[afi][safi].af_enabled &&
+				    !bgp->gr_info[afi][safi].route_sync) {
+					route_sync_pending = true;
+					break;
+				}
+			}
+		}
+		if (!route_sync_pending) {
+			bgp->gr_route_sync_pending = false;
+			bgp_update_gr_completion();
+		}
 		return;
 	}
 

diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
@@ -3689,7 +3689,7 @@ DEFUN (bgp_neighbor_graceful_restart_disable_set,
 
 	ret = bgp_neighbor_graceful_restart(peer, PEER_DISABLE_CMD);
 	if (ret == BGP_GR_SUCCESS) {
-		if (peer->bgp->t_startup)
+		if (bgp_in_graceful_restart())
 			bgp_peer_gr_flags_update(peer);
 
 		VTY_BGP_GR_ROUTER_DETECT(bgp, peer, peer->bgp->peer);

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
@@ -3391,14 +3391,6 @@ int peer_group_bind(struct bgp *bgp, union sockunion *su, struct peer *peer,
 	return 0;
 }
 
-static void bgp_startup_timer_expire(struct event *thread)
-{
-	struct bgp *bgp;
-
-	bgp = EVENT_ARG(thread);
-	bgp->t_startup = NULL;
-}
-
 /*
  * On shutdown we call the cleanup function which
  * does a free of the link list nodes,  free up
@@ -3557,9 +3549,6 @@ static struct bgp *bgp_create(as_t *as, const char *name,
 	if (name)
 		bgp->name = XSTRDUP(MTYPE_BGP_NAME, name);
 
-	event_add_timer(bm->master, bgp_startup_timer_expire, bgp,
-			bgp->restart_time, &bgp->t_startup);
-
 	/* printable name we can use in debug messages */
 	if (inst_type == BGP_INSTANCE_TYPE_DEFAULT) {
 		bgp->name_pretty = XSTRDUP(MTYPE_BGP_NAME, "VRF default");
@@ -3958,7 +3947,6 @@ int bgp_delete(struct bgp *bgp)
 		EVENT_OFF(bgp->t_revalidate[afi][safi]);
 
 	EVENT_OFF(bgp->t_condition_check);
-	EVENT_OFF(bgp->t_startup);
 	EVENT_OFF(bgp->t_maxmed_onstartup);
 	EVENT_OFF(bgp->t_update_delay);
 	EVENT_OFF(bgp->t_establish_wait);

diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
@@ -167,6 +167,8 @@ struct bgp_master {
 #define BM_FLAG_GR_RESTARTER             (1 << 3)
 #define BM_FLAG_GR_DISABLED              (1 << 4)
 #define BM_FLAG_GR_PRESERVE_FWD          (1 << 5)
+#define BM_FLAG_GRACEFUL_RESTART	 (1 << 6)
+#define BM_FLAG_GR_COMPLETE		 (1 << 7)
 
 #define BM_FLAG_GR_CONFIGURED \
 	(BM_FLAG_GR_RESTARTER | BM_FLAG_GR_DISABLED)
@@ -177,6 +179,9 @@ struct bgp_master {
 	uint32_t select_defer_time;
 	uint32_t rib_stale_time;
 
+	time_t startup_time;
+	time_t gr_completion_time;
+
 	bool terminating;	/* global flag that sigint terminate seen */
 
 	/* DSCP value for TCP sessions */
@@ -306,9 +311,9 @@ struct graceful_restart_info {
 	/* Best route select */
 	struct event *t_route_select;
 	/* AFI, SAFI enabled */
-	bool af_enabled[AFI_MAX][SAFI_MAX];
+	bool af_enabled;
 	/* Route update completed */
-	bool route_sync[AFI_MAX][SAFI_MAX];
+	bool route_sync;
 };
 
 enum global_mode {
@@ -443,9 +448,6 @@ struct bgp {
 	struct as_confed *confed_peers;
 	int confed_peers_cnt;
 
-	/* start-up timer on only once at the beginning */
-	struct event *t_startup;
-
 	uint32_t v_maxmed_onstartup; /* Duration of max-med on start-up */
 #define BGP_MAXMED_ONSTARTUP_UNCONFIGURED  0 /* 0 means off, its the default */
 	uint32_t maxmed_onstartup_value;     /* Max-med value when active on
@@ -560,6 +562,9 @@ struct bgp {
 	 */
 	enum zebra_gr_mode present_zebra_gr_state;
 
+	/* Is deferred path selection still not complete? */
+	bool gr_route_sync_pending;
+
 	/* BGP Per AF flags */
 	uint16_t af_flags[AFI_MAX][SAFI_MAX];
 #define BGP_CONFIG_DAMPENING				(1 << 0)
@@ -2755,6 +2760,65 @@ static inline bool bgp_in_graceful_shutdown(struct bgp *bgp)
 	        !!CHECK_FLAG(bm->flags, BM_FLAG_GRACEFUL_SHUTDOWN));
 }
 
+static inline bool bgp_in_graceful_restart(void)
+{
+	/* True if BGP has (re)started gracefully (based on flags
+	 * noted at startup) and GR is not complete.
+	 */
+	if (CHECK_FLAG(bm->flags, BM_FLAG_GRACEFUL_RESTART) &&
+	    !CHECK_FLAG(bm->flags, BM_FLAG_GR_COMPLETE))
+		return true;
+	return false;
+}
+
+static inline bool bgp_is_graceful_restart_complete(void)
+{
+	/* True if BGP has (re)started gracefully (based on flags
+	 * noted at startup) and GR is marked as complete.
+	 */
+	if (CHECK_FLAG(bm->flags, BM_FLAG_GRACEFUL_RESTART) &&
+	    CHECK_FLAG(bm->flags, BM_FLAG_GR_COMPLETE))
+		return true;
+	return false;
+}
+
+static inline void bgp_update_gr_completion(void)
+{
+	struct listnode *node, *nnode;
+	struct bgp *bgp;
+
+	/*
+	 * Check and mark GR complete. This is done when deferred
+	 * path selection has been completed for all instances and
+	 * route-advertisement/EOR and route-sync with zebra has
+	 * been invoked.
+	 */
+	if (!CHECK_FLAG(bm->flags, BM_FLAG_GRACEFUL_RESTART) ||
+	    CHECK_FLAG(bm->flags, BM_FLAG_GR_COMPLETE))
+		return;
+
+	for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp)) {
+		if (bgp->gr_route_sync_pending)
+			return;
+	}
+
+	SET_FLAG(bm->flags, BM_FLAG_GR_COMPLETE);
+	bm->gr_completion_time = monotime(NULL);
+}
+
+static inline bool bgp_gr_is_forwarding_preserved(struct bgp *bgp)
+{
+	/*
+	 * Is forwarding state preserved? Based either on config
+	 * or if BGP restarted gracefully.
+	 * TBD: Additional AFI/SAFI based checks etc.
+	 */
+	if (CHECK_FLAG(bm->flags, BM_FLAG_GRACEFUL_RESTART) ||
+	    CHECK_FLAG(bgp->flags, BGP_FLAG_GR_PRESERVE_FWD))
+		return true;
+	return false;
+}
+
 /* For benefit of rfapi */
 extern struct peer *peer_new(struct bgp *bgp);