bgpd: backpressure - Optimize EVPN L3VNI remote routes processing

Anytime BGP gets a L3 VNI ADD/DEL from zebra, - Walking the entire global routing table per L3VNI is very expensive. - The next read (say of another VNI ADD/DEL) from the socket does not proceed unless this walk is complete. So for triggers where a bulk of L3VNI's are flapped, this results in huge output buffer FIFO growth spiking up the memory in zebra since bgp is slow/busy processing the first message. To avoid this, idea is to hookup the BGP-VRF off the struct bgp_master and maintain a struct bgp FIFO list which is processed later on, where we walk a chunk of BGP-VRFs and do the remote route install/uninstall. Ticket :#3864372 Signed-off-by: Rajasekar Raja <[email protected]>
FRRouting · Nov 28, 2024 · f10f3f9 · f10f3f9
1 parent e0de2c3
commit f10f3f9
Show file tree

Hide file tree

Showing 10 changed files with 288 additions and 63 deletions.
diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c
@@ -79,6 +79,8 @@ static void bgp_evpn_remote_ip_hash_unlink_nexthop(struct hash_bucket *bucket,
 						   void *args);
 static struct in_addr zero_vtep_ip;
 
+static void bgp_evpn_local_l3vni_del_post_processing(struct bgp *bgp_vrf);
+
 /*
  * Private functions.
  */
@@ -3882,14 +3884,6 @@ int bgp_evpn_route_entry_install_if_vrf_match(struct bgp *bgp_vrf,
 	const struct prefix_evpn *evp =
 		(const struct prefix_evpn *)bgp_dest_get_prefix(pi->net);
 
-	/* Consider "valid" remote routes applicable for
-	 * this VRF.
-	 */
-	if (!(CHECK_FLAG(pi->flags, BGP_PATH_VALID)
-	      && pi->type == ZEBRA_ROUTE_BGP
-	      && pi->sub_type == BGP_ROUTE_NORMAL))
-		return 0;
-
 	if (is_route_matching_for_vrf(bgp_vrf, pi)) {
 		if (bgp_evpn_route_rmac_self_check(bgp_vrf, evp, pi))
 			return 0;
@@ -3916,26 +3910,67 @@ int bgp_evpn_route_entry_install_if_vrf_match(struct bgp *bgp_vrf,
 	return ret;
 }
 
+#define BGP_PROC_L3VNI_LIMIT 10
+static int install_uninstall_evpn_remote_route_per_l3vni(struct bgp_path_info *pi,
+							 const struct prefix_evpn *evp)
+{
+	int ret = 0;
+	uint8_t vni_iter = 0;
+	bool is_install = false;
+	struct bgp *bgp_to_proc = NULL;
+	struct bgp *bgp_to_proc_next = NULL;
+
+	for (bgp_to_proc = zebra_l3_vni_first(&bm->zebra_l3_vni_head);
+	     bgp_to_proc && vni_iter < BGP_PROC_L3VNI_LIMIT; bgp_to_proc = bgp_to_proc_next) {
+		bgp_to_proc_next = zebra_l3_vni_next(&bm->zebra_l3_vni_head, bgp_to_proc);
+		vni_iter++;
+		is_install = !!CHECK_FLAG(bgp_to_proc->flags, BGP_FLAG_L3VNI_SCHEDULE_FOR_INSTALL);
+
+		ret = bgp_evpn_route_entry_install_if_vrf_match(bgp_to_proc, pi, is_install);
+		if (ret) {
+			flog_err(EC_BGP_EVPN_FAIL,
+				 "%u: Failed to %s EVPN %s route in L3VNI %u during BP",
+				 bgp_to_proc->vrf_id, is_install ? "install" : "uninstall",
+				 bgp_evpn_route_type_str[evp->prefix.route_type].str,
+				 bgp_to_proc->l3vni);
+			zebra_l3_vni_del(&bm->zebra_l3_vni_head, bgp_to_proc);
+			if (!is_install)
+				bgp_evpn_local_l3vni_del_post_processing(bgp_to_proc);
+
+			return ret;
+		}
+	}
+
+	return 0;
+}
 /*
  * Install or uninstall mac-ip routes are appropriate for this
  * particular VRF.
  */
-static int install_uninstall_routes_for_vrf(struct bgp *bgp_vrf, bool install)
+int install_uninstall_routes_for_vrf(struct bgp *bgp_vrf, bool install)
 {
 	afi_t afi;
 	safi_t safi;
 	struct bgp_dest *rd_dest, *dest;
 	struct bgp_table *table;
 	struct bgp_path_info *pi;
-	int ret;
+	int ret = 0;
 	struct bgp *bgp_evpn = NULL;
+	uint8_t count = 0;
 
 	afi = AFI_L2VPN;
 	safi = SAFI_EVPN;
 	bgp_evpn = bgp_get_evpn();
-	if (!bgp_evpn)
+	if (!bgp_evpn) {
+		if (BGP_DEBUG(zebra, ZEBRA))
+			zlog_debug("No BGP EVPN instance found...");
+
 		return -1;
+	}
 
+	if (BGP_DEBUG(zebra, ZEBRA))
+		zlog_debug("%s: Total %u L3VNI BGP-VRFs pending to be processed for remote route installation",
+			   __func__, (uint32_t)zebra_l3_vni_count(&bm->zebra_l3_vni_head));
 	/* Walk entire global routing table and evaluate routes which could be
 	 * imported into this VRF. Note that we need to loop through all global
 	 * routes to determine which route matches the import rt on vrf
@@ -3952,30 +3987,73 @@ static int install_uninstall_routes_for_vrf(struct bgp *bgp_vrf, bool install)
 				(const struct prefix_evpn *)bgp_dest_get_prefix(
 					dest);
 
-			/* if not mac-ip route skip this route */
-			if (!(evp->prefix.route_type == BGP_EVPN_MAC_IP_ROUTE
-			      || evp->prefix.route_type
-					 == BGP_EVPN_IP_PREFIX_ROUTE))
-				continue;
-
-			/* if not a mac+ip route skip this route */
-			if (!(is_evpn_prefix_ipaddr_v4(evp)
-			      || is_evpn_prefix_ipaddr_v6(evp)))
+			/* Proceed only for MAC-IP and IP-Prefix routes */
+			switch (evp->prefix.route_type) {
+			case BGP_EVPN_MAC_IP_ROUTE:
+			case BGP_EVPN_IP_PREFIX_ROUTE:
+				if (!(is_evpn_prefix_ipaddr_v4(evp) ||
+				      is_evpn_prefix_ipaddr_v6(evp)))
+					continue;
+				break;
+			case BGP_EVPN_AD_ROUTE:
+			case BGP_EVPN_IMET_ROUTE:
+			case BGP_EVPN_ES_ROUTE:
 				continue;
+			}
 
 			for (pi = bgp_dest_get_bgp_path_info(dest); pi;
 			     pi = pi->next) {
-				ret = bgp_evpn_route_entry_install_if_vrf_match(
-					bgp_vrf, pi, install);
-				if (ret) {
-					bgp_dest_unlock_node(rd_dest);
-					bgp_dest_unlock_node(dest);
-					return ret;
+				/* Consider "valid" remote routes applicable for
+				 * this VRF */
+				if (!(CHECK_FLAG(pi->flags, BGP_PATH_VALID) &&
+				      pi->type == ZEBRA_ROUTE_BGP &&
+				      pi->sub_type == BGP_ROUTE_NORMAL))
+					continue;
+
+				if (!bgp_vrf) {
+					ret = install_uninstall_evpn_remote_route_per_l3vni(pi, evp);
+					if (ret) {
+						bgp_dest_unlock_node(rd_dest);
+						bgp_dest_unlock_node(dest);
+
+						return ret;
+					}
+				} else {
+					ret = bgp_evpn_route_entry_install_if_vrf_match(bgp_vrf, pi,
+											install);
+					if (ret) {
+						flog_err(EC_BGP_EVPN_FAIL,
+							 "%u: Failed to %s EVPN %s route in L3VNI %u",
+							 bgp_vrf->vrf_id,
+							 install ? "install" : "uninstall",
+							 bgp_evpn_route_type_str[evp->prefix.route_type]
+								 .str,
+							 bgp_vrf->l3vni);
+						bgp_dest_unlock_node(rd_dest);
+						bgp_dest_unlock_node(dest);
+
+						return ret;
+					}
 				}
 			}
 		}
 	}
 
+	if (!bgp_vrf) {
+		while (count < BGP_PROC_L3VNI_LIMIT) {
+			struct bgp *bgp_to_proc = zebra_l3_vni_pop(&bm->zebra_l3_vni_head);
+
+			if (!bgp_to_proc)
+				return 0;
+
+			if (CHECK_FLAG(bgp_to_proc->flags, BGP_FLAG_L3VNI_SCHEDULE_FOR_DELETE))
+				bgp_evpn_local_l3vni_del_post_processing(bgp_to_proc);
+
+			UNSET_FLAG(bgp_to_proc->flags, BGP_FLAG_L3VNI_SCHEDULE_FOR_INSTALL);
+			count++;
+		}
+	}
+
 	return 0;
 }
 
@@ -6857,6 +6935,53 @@ static void link_l2vni_hash_to_l3vni(struct hash_bucket *bucket,
 		bgpevpn_link_to_l3vni(vpn);
 }
 
+static void bgp_evpn_l3vni_remote_route_processing(struct bgp *bgp, bool install)
+{
+	/*
+	 * Anytime BGP gets a Bulk of L3 VNI ADD/DEL from zebra,
+	 *  - Walking the entire global routing table per VNI is very expensive.
+	 *  - The next read (say of another VNI ADD/DEL) from the socket does
+	 *    not proceed unless this walk is complete.
+	 * This results in huge output buffer FIFO growth spiking up the
+	 * memory in zebra.
+	 *
+	 * To avoid this, idea is to hookup the BGP-VRF off the struct
+	 * bgp_master and maintain a struct bgp FIFO list which is processed
+	 * later on, where we walk a chunk of BGP-VRFs and do the remote route
+	 * install/uninstall.
+	 */
+	if (!CHECK_FLAG(bgp->flags, BGP_FLAG_L3VNI_SCHEDULE_FOR_INSTALL) &&
+	    !CHECK_FLAG(bgp->flags, BGP_FLAG_L3VNI_SCHEDULE_FOR_DELETE))
+		zebra_l3_vni_add_tail(&bm->zebra_l3_vni_head, bgp);
+
+	if (install) {
+		SET_FLAG(bgp->flags, BGP_FLAG_L3VNI_SCHEDULE_FOR_INSTALL);
+		UNSET_FLAG(bgp->flags, BGP_FLAG_L3VNI_SCHEDULE_FOR_DELETE);
+	} else {
+		SET_FLAG(bgp->flags, BGP_FLAG_L3VNI_SCHEDULE_FOR_DELETE);
+		UNSET_FLAG(bgp->flags, BGP_FLAG_L3VNI_SCHEDULE_FOR_INSTALL);
+	}
+
+	if (BGP_DEBUG(zebra, ZEBRA))
+		zlog_debug("Scheduling L3VNI %s to be processed later for %s VNI %u",
+			   install ? "ADD" : "DEL", bgp->name_pretty, bgp->l3vni);
+	/*
+	 * If there are no BGP-VRFs's in the bm L3VNI FIFO list i.e. an update
+	 * for an already processed L3VNI comes in, schedule the remote route
+	 * install immediately.
+	 *
+	 * In all other cases, it is ok to schedule the remote route un/install
+	 * after a small sleep. This is to give benefit of doubt in case more
+	 * L3VNI events come.
+	 */
+	if (zebra_l3_vni_count(&bm->zebra_l3_vni_head))
+		event_add_timer_msec(bm->master, bgp_zebra_process_remote_routes_for_l3vrf, NULL,
+				     20, &bm->t_bgp_zebra_l3_vni);
+	else
+		event_add_event(bm->master, bgp_zebra_process_remote_routes_for_l3vrf, NULL, 0,
+				&bm->t_bgp_zebra_l3_vni);
+}
+
 int bgp_evpn_local_l3vni_add(vni_t l3vni, vrf_id_t vrf_id,
 			     struct ethaddr *svi_rmac,
 			     struct ethaddr *vrr_rmac,
@@ -7002,52 +7127,36 @@ int bgp_evpn_local_l3vni_add(vni_t l3vni, vrf_id_t vrf_id,
 	/* advertise type-5 routes if needed */
 	update_advertise_vrf_routes(bgp_vrf);
 
-	/* install all remote routes belonging to this l3vni into correspondng
-	 * vrf */
-	install_routes_for_vrf(bgp_vrf);
+	bgp_evpn_l3vni_remote_route_processing(bgp_vrf, true);
 
 	return 0;
 }
 
-int bgp_evpn_local_l3vni_del(vni_t l3vni, vrf_id_t vrf_id)
+static void bgp_evpn_local_l3vni_del_post_processing(struct bgp *bgp_vrf)
 {
-	struct bgp *bgp_vrf = NULL; /* bgp vrf instance */
 	struct bgp *bgp_evpn = NULL; /* EVPN bgp instance */
 	struct listnode *node = NULL;
 	struct listnode *next = NULL;
 	struct bgpevpn *vpn = NULL;
 
-	bgp_vrf = bgp_lookup_by_vrf_id(vrf_id);
-	if (!bgp_vrf) {
-		flog_err(
-			EC_BGP_NO_DFLT,
-			"Cannot process L3VNI %u Del - Could not find BGP instance",
-			l3vni);
-		return -1;
-	}
-
 	bgp_evpn = bgp_get_evpn();
 	if (!bgp_evpn) {
-		flog_err(
-			EC_BGP_NO_DFLT,
-			"Cannot process L3VNI %u Del - Could not find EVPN BGP instance",
-			l3vni);
-		return -1;
+		flog_err(EC_BGP_NO_DFLT,
+			 "Cannot process L3VNI %u Del - Could not find EVPN BGP instance",
+			 bgp_vrf->l3vni);
+		return;
 	}
 
 	if (CHECK_FLAG(bgp_evpn->flags, BGP_FLAG_DELETE_IN_PROGRESS)) {
 		flog_err(EC_BGP_NO_DFLT,
-			  "Cannot process L3VNI %u ADD - EVPN BGP instance is shutting down",
-			  l3vni);
-		return -1;
+			 "Cannot process L3VNI %u ADD - EVPN BGP instance is shutting down",
+			 bgp_vrf->l3vni);
+		return;
 	}
 
-	/* Remove remote routes from BGT VRF even if BGP_VRF_AUTO is configured,
-	 * bgp_delete would not remove/decrement bgp_path_info of the ip_prefix
-	 * routes. This will uninstalling the routes from zebra and decremnt the
-	 * bgp info count.
-	 */
-	uninstall_routes_for_vrf(bgp_vrf);
+	if (BGP_DEBUG(zebra, ZEBRA))
+		zlog_debug("In %s for L3VNI %u after remote route installation", __func__,
+			   bgp_vrf->l3vni);
 
 	/* delete/withdraw all type-5 routes */
 	delete_withdraw_vrf_routes(bgp_vrf);
@@ -7093,10 +7202,44 @@ int bgp_evpn_local_l3vni_del(vni_t l3vni, vrf_id_t vrf_id)
 		bgpevpn_unlink_from_l3vni(vpn);
 
 	UNSET_FLAG(bgp_vrf->vrf_flags, BGP_VRF_L3VNI_PREFIX_ROUTES_ONLY);
+	UNSET_FLAG(bgp_vrf->flags, BGP_FLAG_L3VNI_SCHEDULE_FOR_DELETE);
 
 	/* Delete the instance if it was autocreated */
 	if (CHECK_FLAG(bgp_vrf->vrf_flags, BGP_VRF_AUTO))
 		bgp_delete(bgp_vrf);
+}
+
+int bgp_evpn_local_l3vni_del(vni_t l3vni, vrf_id_t vrf_id)
+{
+	struct bgp *bgp_evpn = NULL; /* EVPN bgp instance */
+	struct bgp *bgp_vrf = NULL;  /* bgp vrf instance */
+
+	bgp_vrf = bgp_lookup_by_vrf_id(vrf_id);
+	if (!bgp_vrf) {
+		flog_err(EC_BGP_NO_DFLT,
+			 "Cannot process L3VNI %u Del - Could not find BGP instance", l3vni);
+		return -1;
+	}
+
+	bgp_evpn = bgp_get_evpn();
+	if (!bgp_evpn) {
+		flog_err(EC_BGP_NO_DFLT,
+			 "Cannot process L3VNI %u Del - Could not find EVPN BGP instance", l3vni);
+		return -1;
+	}
+
+	if (CHECK_FLAG(bgp_evpn->flags, BGP_FLAG_DELETE_IN_PROGRESS)) {
+		flog_err(EC_BGP_NO_DFLT,
+			 "Cannot process L3VNI %u ADD - EVPN BGP instance is shutting down", l3vni);
+		return -1;
+	}
+
+	/*
+	 * Move all the l3vni_delete operation post the remote route
+	 * installation processing i.e. add the L3VNI DELETE item on the
+	 * BGP-VRFs FIFO and move on.
+	 */
+	bgp_evpn_l3vni_remote_route_processing(bgp_vrf, false);
 
 	return 0;
 }

diff --git a/bgpd/bgp_evpn.h b/bgpd/bgp_evpn.h
@@ -201,4 +201,5 @@ int uninstall_evpn_route_entry_in_vrf(struct bgp *bgp_vrf, const struct prefix_e
 				      struct bgp_path_info *parent_pi);
 extern void bgp_zebra_evpn_pop_items_from_announce_fifo(struct bgpevpn *vpn);
 extern int install_uninstall_routes_for_vni(struct bgp *bgp, struct bgpevpn *vpn, bool install);
+extern int install_uninstall_routes_for_vrf(struct bgp *bgp_vrf, bool install);
 #endif /* _QUAGGA_BGP_EVPN_H */
diff --git a/bgpd/bgp_main.c b/bgpd/bgp_main.c
@@ -208,6 +208,7 @@ static __attribute__((__noreturn__)) void bgp_exit(int status)
 
 	zebra_announce_fini(&bm->zebra_announce_head);
 	zebra_l2_vni_fini(&bm->zebra_l2_vni_head);
+	zebra_l3_vni_fini(&bm->zebra_l3_vni_head);
 
 	/* reverse bgp_dump_init */
 	bgp_dump_finish();

diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
@@ -1696,8 +1696,13 @@ DEFUN (no_router_bgp,
 		}
 
 		if (bgp->l3vni) {
-			vty_out(vty, "%% Please unconfigure l3vni %u\n",
-				bgp->l3vni);
+			if (CHECK_FLAG(bgp->flags, BGP_FLAG_L3VNI_SCHEDULE_FOR_DELETE))
+				vty_out(vty,
+					"%% L3VNI %u is scheduled to be deleted. Please give it few secs and retry the command\n",
+					bgp->l3vni);
+			else
+				vty_out(vty, "%% Please unconfigure l3vni %u\n", bgp->l3vni);
+
 			return CMD_WARNING_CONFIG_FAILED;
 		}