bgpd:send EOR during GR only when fib install comeplete for wfi enabled

bgpd/bgp_fsm.c

@@ -84,6 +84,9 @@ static void bgp_connect_timer(struct event *event);
 static void bgp_holdtime_timer(struct event *event);
 static void bgp_delayopen_timer(struct event *event);
 
+/* BGP GR functions. */
+static bool bgp_gr_check_all_eors(struct bgp *bgp, afi_t afi, safi_t safi);
+
 /* Register peer with NHT */
 int bgp_peer_reg_with_nht(struct peer *peer)
 {
@@ -864,6 +867,16 @@ static void bgp_graceful_deferral_timer_expire(struct event *thread)
 	bgp->gr_info[afi][safi].select_defer_over = true;
 	XFREE(MTYPE_TMP, info);
 
+	/* Check if graceful restart deferral completion is needed */
+	if (BGP_SUPPRESS_FIB_ENABLED(bgp) && bgp_gr_check_all_eors(bgp, afi, safi) &&
+	    !bgp->gr_info[afi][safi].gr_deferred && bgp->gr_route_sync_pending) {
+		if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
+			zlog_debug("%s: Triggering GR deferral completion from timer expiry for %s",
+				   bgp->name_pretty, get_afi_safi_str(afi, safi, false));
+		bgp_process_gr_deferral_complete(bgp, afi, safi);
+		return;
+	}
+
 	/* Best path selection */
 	bgp_do_deferred_path_selection(bgp, afi, safi);
 }
@@ -1285,12 +1298,14 @@ void bgp_gr_check_path_select(struct bgp *bgp, afi_t afi, safi_t safi)
 
 	if (bgp_gr_check_all_eors(bgp, afi, safi)) {
 		gr_info = &(bgp->gr_info[afi][safi]);
-		if (gr_info->t_select_deferral) {
-			void *info = EVENT_ARG(gr_info->t_select_deferral);
+		if (!BGP_SUPPRESS_FIB_ENABLED(bgp)) {
+			if (gr_info->t_select_deferral) {
+				void *info = EVENT_ARG(gr_info->t_select_deferral);
 
-			XFREE(MTYPE_TMP, info);
+				XFREE(MTYPE_TMP, info);
+			}
+			event_cancel(&gr_info->t_select_deferral);
 		}
-		event_cancel(&gr_info->t_select_deferral);
 		gr_info->select_defer_over = true;
 		bgp_do_deferred_path_selection(bgp, afi, safi);
 	}

bgpd/bgp_route.c

@@ -3942,6 +3942,7 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
 		 * Ensure that on uninstall that the INSTALL_PENDING
 		 * is no longer set
 		 */
+		bgp_dest_decrement_gr_fib_install_pending_count(dest);
 		UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
 	}
 
@@ -4010,6 +4011,102 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
 	return;
 }
 
+void bgp_process_gr_deferral_complete(struct bgp *bgp, afi_t afi, safi_t safi)
+{
+	bool route_sync_pending = false;
+
+	bgp_send_delayed_eor(bgp);
+	/* 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 */
+	FOREACH_AFI_SAFI_NSF (afi, 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();
+	}
+}
+
+/* This function increments gr_route_fib_install_pending_cnt if needed based on BGP_NODE_FIB_INSTALL_PENDING flag */
+void bgp_dest_increment_gr_fib_install_pending_count(struct bgp_dest *dest)
+{
+	struct bgp_table *table = NULL;
+	struct bgp *bgp = NULL;
+	afi_t afi = AFI_UNSPEC;
+	safi_t safi = SAFI_UNSPEC;
+
+	table = bgp_dest_table(dest);
+	if (!table)
+		return;
+
+	bgp = table->bgp;
+	afi = table->afi;
+	safi = table->safi;
+
+	if (BGP_SUPPRESS_FIB_ENABLED(bgp) && bgp->gr_route_sync_pending &&
+	    !CHECK_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING)) {
+		bgp->gr_info[afi][safi].gr_route_fib_install_pending_cnt++;
+		if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
+			zlog_debug("%s: GR route FIB install count incremented to %u for %s (prefix: %pBD)",
+				   bgp->name_pretty,
+				   bgp->gr_info[afi][safi].gr_route_fib_install_pending_cnt,
+				   get_afi_safi_str(afi, safi, false), dest);
+	}
+}
+
+/* This function decrements gr_route_fib_install_pending_cnt if needed based on BGP_NODE_FIB_INSTALL_PENDING flag */
+void bgp_dest_decrement_gr_fib_install_pending_count(struct bgp_dest *dest)
+{
+	struct bgp_table *table = NULL;
+	struct bgp *bgp = NULL;
+	afi_t afi = 0;
+	safi_t safi = 0;
+
+	table = bgp_dest_table(dest);
+	if (!table)
+		return;
+
+	bgp = table->bgp;
+	afi = table->afi;
+	safi = table->safi;
+
+	if (BGP_SUPPRESS_FIB_ENABLED(bgp) && bgp->gr_route_sync_pending &&
+	    CHECK_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING) &&
+	    bgp->gr_info[afi][safi].gr_route_fib_install_pending_cnt > 0) {
+		bgp->gr_info[afi][safi].gr_route_fib_install_pending_cnt--;
+		if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
+			zlog_debug("%s: GR route FIB install count decremented to %u for %s (prefix: %pBD)",
+				   bgp->name_pretty,
+				   bgp->gr_info[afi][safi].gr_route_fib_install_pending_cnt,
+				   get_afi_safi_str(afi, safi, false), dest);
+	}
+
+	/* Check if graceful restart deferral completion is needed */
+	if (!bgp->gr_info[afi][safi].gr_deferred &&
+	    !bgp->gr_info[afi][safi].gr_route_fib_install_pending_cnt &&
+	    bgp->gr_route_sync_pending) {
+		struct graceful_restart_info *gr_info = &(bgp->gr_info[afi][safi]);
+
+		if (gr_info->t_select_deferral) {
+			void *info = EVENT_ARG(gr_info->t_select_deferral);
+
+			XFREE(MTYPE_TMP, info);
+		}
+		event_cancel(&gr_info->t_select_deferral);
+		if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
+			zlog_debug("%s: Triggering GR deferral completion from FIB notification for %s",
+				   bgp->name_pretty, get_afi_safi_str(afi, safi, false));
+		bgp_process_gr_deferral_complete(bgp, afi, safi);
+	}
+}
+
 /* Process the routes with the flag BGP_NODE_SELECT_DEFER set */
 void bgp_do_deferred_path_selection(struct bgp *bgp, afi_t afi, safi_t safi)
 {
@@ -4054,29 +4151,11 @@ void bgp_do_deferred_path_selection(struct bgp *bgp, afi_t afi, safi_t safi)
 
 	/* Send EOR message when all routes are processed */
 	if (!bgp->gr_info[afi][safi].gr_deferred) {
-		bool route_sync_pending = false;
-
-		bgp_send_delayed_eor(bgp);
-		/* 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
+		/* t_select_deferral will be NULL when either gr_route_fib_install_pending_cnt is 0
+		 * or deferral timer for fib install expires
 		 */
-		FOREACH_AFI_SAFI (afi, 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();
-		}
+		if (!BGP_SUPPRESS_FIB_ENABLED(bgp) || !bgp->gr_info[afi][safi].t_select_deferral)
+			bgp_process_gr_deferral_complete(bgp, afi, safi);
 		return;
 	}
 

bgpd/bgp_route.h

@@ -980,6 +980,9 @@ extern int bgp_show_table_rd(struct vty *vty, struct bgp *bgp, afi_t afi, safi_t
 			     enum bgp_show_type type, void *output_arg,
 			     uint16_t show_flags);
 extern void bgp_do_deferred_path_selection(struct bgp *bgp, afi_t afi, safi_t safi);
+extern void bgp_dest_increment_gr_fib_install_pending_count(struct bgp_dest *dest);
+extern void bgp_dest_decrement_gr_fib_install_pending_count(struct bgp_dest *dest);
+extern void bgp_process_gr_deferral_complete(struct bgp *bgp, afi_t afi, safi_t safi);
 extern bool bgp_update_martian_nexthop(struct bgp *bgp, afi_t afi, safi_t safi,
 				       uint8_t type, uint8_t stype,
 				       struct attr *attr, struct bgp_dest *dest);

bgpd/bgp_zebra.c

@@ -1917,13 +1917,16 @@ void bgp_zebra_route_install(struct bgp_dest *dest, struct bgp_path_info *info,
 			 * the route to peers since they have a path toward us
 			 * As such let's just let normal mechanisms fly
 			 */
-			if (!CHECK_FLAG(dest->flags, BGP_NODE_FIB_INSTALLED))
+			if (!CHECK_FLAG(dest->flags, BGP_NODE_FIB_INSTALLED)) {
+				bgp_dest_increment_gr_fib_install_pending_count(dest);
 				SET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
+			}
 		}
 
 		if (bgp->main_zebra_update_hold && !is_evpn)
 			return;
 	} else {
+		bgp_dest_decrement_gr_fib_install_pending_count(dest);
 		UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
 	}
 
@@ -2908,6 +2911,7 @@ static int bgp_zebra_route_notify_owner(int command, struct zclient *zclient,
 	case ZAPI_ROUTE_INSTALLED:
 		new_select = NULL;
 		/* Clear the flags so that route can be processed */
+		bgp_dest_decrement_gr_fib_install_pending_count(dest);
 		UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
 		SET_FLAG(dest->flags, BGP_NODE_FIB_INSTALLED);
 		if (BGP_DEBUG(zebra, ZEBRA))
@@ -2944,6 +2948,7 @@ static int bgp_zebra_route_notify_owner(int command, struct zclient *zclient,
 		if (BGP_DEBUG(zebra, ZEBRA))
 			zlog_debug("route: %pBD Failed to Install into Fib",
 				   dest);
+		bgp_dest_decrement_gr_fib_install_pending_count(dest);
 		UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
 		UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALLED);
 		for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) {
@@ -2959,6 +2964,7 @@ static int bgp_zebra_route_notify_owner(int command, struct zclient *zclient,
 			zlog_debug("route: %pBD removed due to better admin won",
 				   dest);
 		new_select = NULL;
+		bgp_dest_decrement_gr_fib_install_pending_count(dest);
 		UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
 		UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALLED);
 		for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) {

bgpd/bgpd.h

@@ -322,7 +322,8 @@ enum bgp_instance_type {
 
 #define BGP_SEND_EOR(bgp, afi, safi)                                                              \
 	(!CHECK_FLAG(bgp->flags, BGP_FLAG_GR_DISABLE_EOR) &&                                      \
-	 (!bgp_in_graceful_restart() || bgp->gr_info[afi][safi].select_defer_over))
+	 (!bgp_in_graceful_restart() || bgp->gr_info[afi][safi].select_defer_over) &&             \
+	 (!BGP_SUPPRESS_FIB_ENABLED(bgp) || !bgp->gr_info[afi][safi].t_select_deferral))
 
 /* BGP GR Global ds */
 
@@ -335,6 +336,8 @@ struct graceful_restart_info {
 	struct event *t_select_deferral;
 	/* Routes Deferred */
 	uint32_t gr_deferred;
+	/* Routes waiting for FIB install */
+	uint32_t gr_route_fib_install_pending_cnt;
 	/* Best route select */
 	struct event *t_route_select;
 	/* AFI, SAFI enabled */

tests/topotests/bgp_gr_fib_suppress/__init__.py

@@ -0,0 +1 @@
+# BGP Graceful Restart FIB Suppress Test Module

tests/topotests/bgp_gr_fib_suppress/r1/frr.conf

@@ -0,0 +1,49 @@
+hostname r1
+!
+interface r1-eth0
+ ip address 192.168.12.1/24
+ ipv6 address fd00:12::1/64
+!
+interface r1-eth1
+ ip address 192.168.13.1/24
+ ipv6 address fd00:13::1/64
+!
+interface lo
+ ip address 192.168.1.1/32
+ ipv6 address 2001:DB8:F::1/128
+!
+ip route 192.168.11.1/32 Null0
+ip route 192.168.11.2/32 Null0
+ipv6 route 2001:db8:1::1/128 Null0
+ipv6 route 2001:db8:1::2/128 Null0
+!
+router bgp 100
+ no bgp ebgp-requires-policy
+ bgp router-id 192.168.1.1
+ neighbor 192.168.12.2 remote-as 100
+ neighbor 192.168.12.2 timers 3 10
+ neighbor 192.168.12.2 timers connect 5
+ neighbor 192.168.13.3 remote-as 100
+ neighbor 192.168.13.3 timers 3 10
+ neighbor 192.168.13.3 timers connect 5
+ neighbor fd00:12::2 remote-as 100
+ neighbor fd00:12::2 timers 3 10
+ neighbor fd00:12::2 timers connect 5
+ neighbor fd00:13::3 remote-as 100
+ neighbor fd00:13::3 timers 3 10
+ neighbor fd00:13::3 timers connect 5
+ !
+ address-family ipv4 unicast
+  redistribute static
+  neighbor 192.168.12.2 activate
+  neighbor 192.168.13.3 activate
+ exit-address-family
+ !
+ address-family ipv6 unicast
+  redistribute static
+  neighbor fd00:12::2 activate
+  neighbor fd00:13::3 activate
+ exit-address-family
+!
+line vty
+!

tests/topotests/bgp_gr_fib_suppress/r2/frr.conf

@@ -0,0 +1,52 @@
+hostname r2
+!
+interface r2-eth0
+ ip address 192.168.12.2/24
+ ipv6 address fd00:12::2/64
+!
+interface r2-eth1
+ ip address 192.168.23.2/24
+ ipv6 address fd00:23::2/64
+!
+interface lo
+ ip address 192.168.2.2/32
+ ipv6 address 2001:DB8:F::2/128
+!
+ip route 192.168.22.1/32 Null0
+ip route 192.168.22.2/32 Null0
+ipv6 route 2001:db8:2::1/128 Null0
+ipv6 route 2001:db8:2::2/128 Null0
+!
+router bgp 100
+ no bgp ebgp-requires-policy
+ bgp router-id 192.168.2.2
+ bgp graceful-restart
+ bgp graceful-restart preserve-fw-state
+ bgp suppress-fib-pending
+ neighbor 192.168.12.1 remote-as 100
+ neighbor 192.168.12.1 timers 3 10
+ neighbor 192.168.12.1 timers connect 5
+ neighbor 192.168.23.3 remote-as 100
+ neighbor 192.168.23.3 timers 3 10
+ neighbor 192.168.23.3 timers connect 5
+ neighbor fd00:12::1 remote-as 100
+ neighbor fd00:12::1 timers 3 10
+ neighbor fd00:12::1 timers connect 5
+ neighbor fd00:23::3 remote-as 100
+ neighbor fd00:23::3 timers 3 10
+ neighbor fd00:23::3 timers connect 5
+ !
+ address-family ipv4 unicast
+  redistribute static
+  neighbor 192.168.12.1 activate
+  neighbor 192.168.23.3 activate
+ exit-address-family
+ !
+ address-family ipv6 unicast
+  redistribute static
+  neighbor fd00:12::1 activate
+  neighbor fd00:23::3 activate
+ exit-address-family
+!
+line vty
+!