Merge pull request #15524 from raja-rajasekar/rajasekarr/backpressure_bgp_zebra_client

backpressure bgp zebra client

bgpd/bgp_main.c

@@ -207,6 +207,8 @@ static __attribute__((__noreturn__)) void bgp_exit(int status)
 	bgp_evpn_mh_finish();
 	bgp_nhg_finish();
 
+	zebra_announce_fini(&bm->zebra_announce_head);
+
 	/* reverse bgp_dump_init */
 	bgp_dump_finish();
 

bgpd/bgp_route.c

@@ -3403,8 +3403,8 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
 					|| new_select->sub_type
 						   == BGP_ROUTE_IMPORTED))
 
-					bgp_zebra_announce(dest, p, old_select,
+					bgp_zebra_route_install(dest, old_select,
-							   bgp, afi, safi);
+								bgp, true);
 			}
 		}
 
@@ -3521,10 +3521,10 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
 			 */
 			if (old_select &&
 			    is_route_parent_evpn(old_select))
-				bgp_zebra_withdraw(p, old_select, bgp, afi,
+				bgp_zebra_route_install(dest, old_select, bgp,
-						   safi);
+							false);
 
-			bgp_zebra_announce(dest, p, new_select, bgp, afi, safi);
+			bgp_zebra_route_install(dest, new_select, bgp, true);
 		} else {
 			/* Withdraw the route from the kernel. */
 			if (old_select && old_select->type == ZEBRA_ROUTE_BGP
@@ -3532,8 +3532,8 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
 				|| old_select->sub_type == BGP_ROUTE_AGGREGATE
 				|| old_select->sub_type == BGP_ROUTE_IMPORTED))
 
-				bgp_zebra_withdraw(p, old_select, bgp, afi,
+				bgp_zebra_route_install(dest, old_select, bgp,
-						   safi);
+							false);
 		}
 	}
 
@@ -4430,7 +4430,7 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
 	if (pi && pi->attr->rmap_table_id != new_attr.rmap_table_id) {
 		if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED))
 			/* remove from RIB previous entry */
-			bgp_zebra_withdraw(p, pi, bgp, afi, safi);
+			bgp_zebra_route_install(dest, pi, bgp, false);
 	}
 
 	if (peer->sort == BGP_PEER_EBGP) {
@@ -6056,8 +6056,7 @@ static void bgp_cleanup_table(struct bgp *bgp, struct bgp_table *table,
 				|| pi->sub_type == BGP_ROUTE_IMPORTED)) {
 
 				if (bgp_fibupd_safi(safi))
-					bgp_zebra_withdraw(p, pi, bgp, afi,
+					bgp_zebra_withdraw_actual(dest, pi, bgp);
-							   safi);
 			}
 
 			dest = bgp_path_info_reap(dest, pi);

bgpd/bgp_table.h

@@ -76,6 +76,9 @@ struct bgp_dest {
 
 	STAILQ_ENTRY(bgp_dest) pq;
 
+	struct zebra_announce_item zai;
+	struct bgp_path_info *za_bgp_pi;
+
 	uint64_t version;
 
 	mpls_label_t local_label;
@@ -91,12 +94,16 @@ struct bgp_dest {
 #define BGP_NODE_LABEL_REQUESTED        (1 << 7)
 #define BGP_NODE_SOFT_RECONFIG (1 << 8)
 #define BGP_NODE_PROCESS_CLEAR (1 << 9)
+#define BGP_NODE_SCHEDULE_FOR_INSTALL	(1 << 10)
+#define BGP_NODE_SCHEDULE_FOR_DELETE	(1 << 11)
 
 	struct bgp_addpath_node_data tx_addpath;
 
 	enum bgp_path_selection_reason reason;
 };
 
+DECLARE_LIST(zebra_announce, struct bgp_dest, zai);
+
 extern void bgp_delete_listnode(struct bgp_dest *dest);
 /*
  * bgp_table_iter_t

bgpd/bgp_zebra.c

@@ -1524,9 +1524,9 @@ static void bgp_debug_zebra_nh(struct zapi_route *api)
 	}
 }
 
-void bgp_zebra_announce(struct bgp_dest *dest, const struct prefix *p,
+static enum zclient_send_status
-			struct bgp_path_info *info, struct bgp *bgp, afi_t afi,
+bgp_zebra_announce_actual(struct bgp_dest *dest, struct bgp_path_info *info,
-			safi_t safi)
+			  struct bgp *bgp)
 {
 	struct bgp_path_info *bpi_ultimate;
 	struct zapi_route api = { 0 };
@@ -1539,34 +1539,19 @@ void bgp_zebra_announce(struct bgp_dest *dest, const struct prefix *p,
 	bool is_add;
 	uint32_t nhg_id = 0;
 	uint32_t recursion_flag = 0;
+	struct bgp_table *table = bgp_dest_table(dest);
+	const struct prefix *p = bgp_dest_get_prefix(dest);
 
-	/*
+	if (table->safi == SAFI_FLOWSPEC) {
-	 * BGP is installing this route and bgp has been configured
+		bgp_pbr_update_entry(bgp, p, info, table->afi, table->safi,
-	 * to suppress announcements until the route has been installed
+				     true);
-	 * let's set the fact that we expect this route to be installed
+		return ZCLIENT_SEND_SUCCESS;
-	 */
-	if (BGP_SUPPRESS_FIB_ENABLED(bgp))
-		SET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
-
-	/* Don't try to install if we're not connected to Zebra or Zebra doesn't
-	 * know of this instance.
-	 */
-	if (!bgp_install_info_to_zebra(bgp))
-		return;
-
-	if (bgp->main_zebra_update_hold)
-		return;
-
-	if (safi == SAFI_FLOWSPEC) {
-		bgp_pbr_update_entry(bgp, bgp_dest_get_prefix(dest), info, afi,
-				     safi, true);
-		return;
 	}
 
 	/* Make Zebra API structure. */
 	api.vrf_id = bgp->vrf_id;
 	api.type = ZEBRA_ROUTE_BGP;
-	api.safi = safi;
+	api.safi = table->safi;
 	api.prefix = *p;
 	SET_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP);
 
@@ -1603,8 +1588,8 @@ void bgp_zebra_announce(struct bgp_dest *dest, const struct prefix *p,
 	metric = info->attr->med;
 
 	bgp_zebra_announce_parse_nexthop(info, p, bgp, &api, &valid_nh_count,
-					 afi, safi, &nhg_id, &metric, &tag,
+					 table->afi, table->safi, &nhg_id,
-					 &allow_recursion);
+					 &metric, &tag, &allow_recursion);
 
 	is_add = (valid_nh_count || nhg_id) ? true : false;
 
@@ -1657,7 +1642,7 @@ void bgp_zebra_announce(struct bgp_dest *dest, const struct prefix *p,
 		api.tag = tag;
 	}
 
-	distance = bgp_distance_apply(p, info, afi, safi, bgp);
+	distance = bgp_distance_apply(p, info, table->afi, table->safi, bgp);
 	if (distance) {
 		SET_FLAG(api.message, ZAPI_MESSAGE_DISTANCE);
 		api.distance = distance;
@@ -1677,10 +1662,11 @@ void bgp_zebra_announce(struct bgp_dest *dest, const struct prefix *p,
 		zlog_debug("%s: %pFX: announcing to zebra (recursion %sset)",
 			   __func__, p, (recursion_flag ? "" : "NOT "));
 	}
-	zclient_route_send(is_add ? ZEBRA_ROUTE_ADD : ZEBRA_ROUTE_DELETE,
+	return zclient_route_send(is_add ? ZEBRA_ROUTE_ADD : ZEBRA_ROUTE_DELETE,
-			   zclient, &api);
+				  zclient, &api);
 }
 
+
 /* Announce all routes of a table to zebra */
 void bgp_zebra_announce_table(struct bgp *bgp, afi_t afi, safi_t safi)
 {
@@ -1706,9 +1692,7 @@ void bgp_zebra_announce_table(struct bgp *bgp, afi_t afi, safi_t safi)
 			     && (pi->sub_type == BGP_ROUTE_NORMAL
 				 || pi->sub_type == BGP_ROUTE_IMPORTED)))
 
-				bgp_zebra_announce(dest,
+				bgp_zebra_route_install(dest, pi, bgp, true);
-						   bgp_dest_get_prefix(dest),
-						   pi, bgp, afi, safi);
 }
 
 /* Announce routes of any bgp subtype of a table to zebra */
@@ -1730,39 +1714,29 @@ void bgp_zebra_announce_table_all_subtypes(struct bgp *bgp, afi_t afi,
 		for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next)
 			if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED) &&
 			    pi->type == ZEBRA_ROUTE_BGP)
-				bgp_zebra_announce(dest,
+				bgp_zebra_route_install(dest, pi, bgp, true);
-						   bgp_dest_get_prefix(dest),
-						   pi, bgp, afi, safi);
 }
 
-void bgp_zebra_withdraw(const struct prefix *p, struct bgp_path_info *info,
+enum zclient_send_status bgp_zebra_withdraw_actual(struct bgp_dest *dest,
-			struct bgp *bgp, afi_t afi, safi_t safi)
+						   struct bgp_path_info *info,
+						   struct bgp *bgp)
 {
 	struct zapi_route api;
 	struct peer *peer;
+	struct bgp_table *table = bgp_dest_table(dest);
+	const struct prefix *p = bgp_dest_get_prefix(dest);
 
-	/*
+	if (table->safi == SAFI_FLOWSPEC) {
-	 * If we are withdrawing the route, we don't need to have this
-	 * flag set.  So unset it.
-	 */
-	UNSET_FLAG(info->net->flags, BGP_NODE_FIB_INSTALL_PENDING);
-
-	/* Don't try to install if we're not connected to Zebra or Zebra doesn't
-	 * know of this instance.
-	 */
-	if (!bgp_install_info_to_zebra(bgp))
-		return;
-
-	if (safi == SAFI_FLOWSPEC) {
 		peer = info->peer;
-		bgp_pbr_update_entry(peer->bgp, p, info, afi, safi, false);
+		bgp_pbr_update_entry(peer->bgp, p, info, table->afi,
-		return;
+				     table->safi, false);
+		return ZCLIENT_SEND_SUCCESS;
 	}
 
 	memset(&api, 0, sizeof(api));
 	api.vrf_id = bgp->vrf_id;
 	api.type = ZEBRA_ROUTE_BGP;
-	api.safi = safi;
+	api.safi = table->safi;
 	api.prefix = *p;
 
 	if (info->attr->rmap_table_id) {
@@ -1774,7 +1748,169 @@ void bgp_zebra_withdraw(const struct prefix *p, struct bgp_path_info *info,
 		zlog_debug("Tx route delete VRF %u %pFX", bgp->vrf_id,
 			   &api.prefix);
 
-	zclient_route_send(ZEBRA_ROUTE_DELETE, zclient, &api);
+	return zclient_route_send(ZEBRA_ROUTE_DELETE, zclient, &api);
+}
+
+/*
+ * Walk the new Fifo list one by one and invoke bgp_zebra_announce/withdraw
+ * to install/withdraw the routes to zebra.
+ *
+ * If status = ZCLIENT_SEND_SUCCESS (Buffer empt)y i.e. Zebra is free to
+ * receive more incoming data, then pick the next item on the list and
+ * continue processing.
+ *
+ * If status = ZCLIENT_SEND_BUFFERED (Buffer pending) i.e. Zebra is busy,
+ * break and bail out of the function because once at some point when zebra
+ * is free, a callback is triggered which inturn call this same function and
+ * continue processing items on list.
+ */
+#define ZEBRA_ANNOUNCEMENTS_LIMIT 1000
+static void bgp_handle_route_announcements_to_zebra(struct event *e)
+{
+	uint32_t count = 0;
+	struct bgp_dest *dest = NULL;
+	struct bgp_table *table = NULL;
+	enum zclient_send_status status = ZCLIENT_SEND_SUCCESS;
+	bool install;
+
+	while (count < ZEBRA_ANNOUNCEMENTS_LIMIT) {
+		dest = zebra_announce_pop(&bm->zebra_announce_head);
+
+		if (!dest)
+			break;
+
+		table = bgp_dest_table(dest);
+		install = CHECK_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL);
+
+		if (BGP_DEBUG(zebra, ZEBRA))
+			zlog_debug("BGP %s route %pBD(%s) with dest %p and flags 0x%x to zebra",
+				   install ? "announcing" : "withdrawing", dest,
+				   table->bgp->name_pretty, dest, dest->flags);
+
+		if (install) {
+			status = bgp_zebra_announce_actual(dest, dest->za_bgp_pi,
+							   table->bgp);
+			UNSET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL);
+		} else {
+			status = bgp_zebra_withdraw_actual(dest, dest->za_bgp_pi,
+							   table->bgp);
+			UNSET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_DELETE);
+		}
+
+		bgp_path_info_unlock(dest->za_bgp_pi);
+		dest->za_bgp_pi = NULL;
+		bgp_dest_unlock_node(dest);
+
+		if (status == ZCLIENT_SEND_BUFFERED)
+			break;
+
+		count++;
+	}
+
+	if (status != ZCLIENT_SEND_BUFFERED &&
+	    zebra_announce_count(&bm->zebra_announce_head))
+		event_add_event(bm->master,
+				bgp_handle_route_announcements_to_zebra, NULL,
+				0, &bm->t_bgp_zebra_route);
+}
+
+/*
+ * Callback function invoked when zclient_flush_data() receives a BUFFER_EMPTY
+ * i.e. zebra is free to receive more incoming data.
+ */
+static void bgp_zebra_buffer_write_ready(void)
+{
+	bgp_handle_route_announcements_to_zebra(NULL);
+}
+
+/*
+ * BGP is now keeping a list of dests with the dest having a pointer
+ * to the bgp_path_info that it will be working on.
+ * Here is the sequence of events that should happen:
+ *
+ *  Current State      New State       Action
+ *  -------------      ---------       ------
+ *      ----           Install         Place dest on list, save pi, mark
+ *                                     as going to be installed
+ *      ----           Withdrawal      Place dest on list, save pi, mark
+ *                                     as going to be deleted
+ *
+ *    Install          Install         Leave dest on list, release old pi,
+ *                                     save new pi, mark as going to be
+ *                                     Installed
+ *    Install          Withdrawal      Leave dest on list, release old pi,
+ *                                     save new pi, mark as going to be
+ *                                     withdrawan, remove install flag
+ *
+ *    Withdrawal       Install         Special case, send withdrawal immediately
+ *                                     Leave dest on list, release old pi,
+ *                                     save new pi, mark as going to be
+ *                                     installed.  <see note about evpn
+ *                                     in bgp_route.c in bgp_process_main_one>
+ *    Withdrawal       Withdrawal      Leave dest on list, release old pi,
+ *                                     save new pi, mark as going to be
+ *                                     withdrawn.
+ */
+void bgp_zebra_route_install(struct bgp_dest *dest, struct bgp_path_info *info,
+			     struct bgp *bgp, bool install)
+{
+	/*
+	 * BGP is installing this route and bgp has been configured
+	 * to suppress announcements until the route has been installed
+	 * let's set the fact that we expect this route to be installed
+	 */
+	if (install) {
+		if (BGP_SUPPRESS_FIB_ENABLED(bgp))
+			SET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
+
+		if (bgp->main_zebra_update_hold)
+			return;
+	} else {
+		UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
+	}
+
+	/*
+	 * Don't try to install if we're not connected to Zebra or Zebra doesn't
+	 * know of this instance.
+	 */
+	if (!bgp_install_info_to_zebra(bgp))
+		return;
+
+	if (!CHECK_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL) &&
+	    !CHECK_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_DELETE)) {
+		zebra_announce_add_tail(&bm->zebra_announce_head, dest);
+		/*
+		 * If neither flag is set and za_bgp_pi is not set then it is a bug
+		 */
+		assert(!dest->za_bgp_pi);
+		bgp_path_info_lock(info);
+		bgp_dest_lock_node(dest);
+		dest->za_bgp_pi = info;
+	} else if (CHECK_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL)) {
+		assert(dest->za_bgp_pi);
+		bgp_path_info_unlock(dest->za_bgp_pi);
+		bgp_path_info_lock(info);
+		dest->za_bgp_pi = info;
+	} else if (CHECK_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_DELETE)) {
+		assert(dest->za_bgp_pi);
+		if (install)
+			bgp_zebra_withdraw_actual(dest, dest->za_bgp_pi, bgp);
+
+		bgp_path_info_unlock(dest->za_bgp_pi);
+		bgp_path_info_lock(info);
+		dest->za_bgp_pi = info;
+	}
+
+	if (install) {
+		UNSET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_DELETE);
+		SET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL);
+	} else {
+		UNSET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL);
+		SET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_DELETE);
+	}
+
+	event_add_event(bm->master, bgp_handle_route_announcements_to_zebra,
+			NULL, 0, &bm->t_bgp_zebra_route);
 }
 
 /* Withdraw all entries in a BGP instances RIB table from Zebra */
@@ -1795,8 +1931,7 @@ void bgp_zebra_withdraw_table_all_subtypes(struct bgp *bgp, afi_t afi, safi_t sa
 		for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) {
 			if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED)
 			    && (pi->type == ZEBRA_ROUTE_BGP))
-				bgp_zebra_withdraw(bgp_dest_get_prefix(dest),
+				bgp_zebra_route_install(dest, pi, bgp, false);
-						   pi, bgp, afi, safi);
 		}
 	}
 }
@@ -3471,6 +3606,7 @@ void bgp_zebra_init(struct event_loop *master, unsigned short instance)
 	zclient = zclient_new(master, &zclient_options_default, bgp_handlers,
 			      array_size(bgp_handlers));
 	zclient_init(zclient, ZEBRA_ROUTE_BGP, 0, &bgpd_privs);
+	zclient->zebra_buffer_write_ready = bgp_zebra_buffer_write_ready;
 	zclient->zebra_connected = bgp_zebra_connected;
 	zclient->zebra_capabilities = bgp_zebra_capabilities;
 	zclient->nexthop_update = bgp_nexthop_update;

bgpd/bgp_zebra.h

@@ -28,13 +28,10 @@ extern void bgp_zebra_destroy(void);
 extern int bgp_zebra_get_table_range(struct zclient *zc, uint32_t chunk_size,
 				     uint32_t *start, uint32_t *end);
 extern int bgp_if_update_all(void);
-extern void bgp_zebra_announce(struct bgp_dest *dest, const struct prefix *p,
+extern void bgp_zebra_route_install(struct bgp_dest *dest,
-			       struct bgp_path_info *path, struct bgp *bgp,
+				    struct bgp_path_info *path, struct bgp *bgp,
-			       afi_t afi, safi_t safi);
+				    bool install);
 extern void bgp_zebra_announce_table(struct bgp *bgp, afi_t afi, safi_t safi);
-extern void bgp_zebra_withdraw(const struct prefix *p,
-			       struct bgp_path_info *path, struct bgp *bgp,
-			       afi_t afi, safi_t safi);
 
 /* Announce routes of any bgp subtype of a table to zebra */
 extern void bgp_zebra_announce_table_all_subtypes(struct bgp *bgp, afi_t afi,
@@ -127,4 +124,7 @@ extern void bgp_zebra_send_nexthop_label(int cmd, mpls_label_t label,
 extern bool bgp_zebra_request_label_range(uint32_t base, uint32_t chunk_size,
 					  bool label_auto);
 extern void bgp_zebra_release_label_range(uint32_t start, uint32_t end);
+extern enum zclient_send_status
+bgp_zebra_withdraw_actual(struct bgp_dest *dest, struct bgp_path_info *info,
+			  struct bgp *bgp);
 #endif /* _QUAGGA_BGP_ZEBRA_H */

bgpd/bgpd.c

@@ -3855,10 +3855,20 @@ int bgp_delete(struct bgp *bgp)
 	afi_t afi;
 	safi_t safi;
 	int i;
+	struct bgp_dest *dest = NULL;
 	struct graceful_restart_info *gr_info;
 
 	assert(bgp);
 
+	while (zebra_announce_count(&bm->zebra_announce_head)) {
+		dest = zebra_announce_pop(&bm->zebra_announce_head);
+		if (dest->za_bgp_pi->peer->bgp == bgp) {
+			bgp_path_info_unlock(dest->za_bgp_pi);
+			bgp_dest_unlock_node(dest);
+		} else
+			zebra_announce_add_tail(&bm->zebra_announce_head, dest);
+	}
+
 	bgp_soft_reconfig_table_task_cancel(bgp, NULL, NULL);
 
 	/* make sure we withdraw any exported routes */
@@ -8298,6 +8308,8 @@ void bgp_master_init(struct event_loop *master, const int buffer_size,
 	memset(&bgp_master, 0, sizeof(bgp_master));
 
 	bm = &bgp_master;
+
+	zebra_announce_init(&bm->zebra_announce_head);
 	bm->bgp = list_new();
 	bm->listen_sockets = list_new();
 	bm->port = BGP_PORT_DEFAULT;
@@ -8316,6 +8328,7 @@ void bgp_master_init(struct event_loop *master, const int buffer_size,
 	bm->outq_limit = BM_DEFAULT_Q_LIMIT;
 	bm->t_bgp_sync_label_manager = NULL;
 	bm->t_bgp_start_label_manager = NULL;
+	bm->t_bgp_zebra_route = NULL;
 
 	bgp_mac_init();
 	/* init the rd id space.
@@ -8566,6 +8579,7 @@ void bgp_terminate(void)
 	EVENT_OFF(bm->t_rmap_update);
 	EVENT_OFF(bm->t_bgp_sync_label_manager);
 	EVENT_OFF(bm->t_bgp_start_label_manager);
+	EVENT_OFF(bm->t_bgp_zebra_route);
 
 	bgp_mac_finish();
 }

bgpd/bgpd.h

@@ -18,6 +18,8 @@
 #include "iana_afi.h"
 #include "asn.h"
 
+PREDECL_LIST(zebra_announce);
+
 /* For union sockunion.  */
 #include "queue.h"
 #include "sockunion.h"
@@ -173,8 +175,13 @@ struct bgp_master {
 	struct event *t_bgp_sync_label_manager;
 	struct event *t_bgp_start_label_manager;
 
+	struct event *t_bgp_zebra_route;
+
 	bool v6_with_v4_nexthops;
 
+	/* To preserve ordering of installations into zebra across all Vrfs */
+	struct zebra_announce_head zebra_announce_head;
+
 	QOBJ_FIELDS;
 };
 DECLARE_QOBJ_TYPE(bgp_master);

lib/zclient.c

@@ -282,6 +282,7 @@ static void zclient_flush_data(struct event *thread)
 				zclient->sock, &zclient->t_write);
 		break;
 	case BUFFER_EMPTY:
+		/* Currently only Sharpd and Bgpd has callbacks defined */
 		if (zclient->zebra_buffer_write_ready)
 			(*zclient->zebra_buffer_write_ready)();
 		break;