bgpd: Cleanup multipath figuring out in bgp

Currently bgp multipath has these properties:

a) mp_info may or may not be on a single path, based
upon path perturbations in the past.
b) mp_info->count started counting at 0( meaning 1 ).  As that the
bestpath path_info was never included in the count
c) The first mp_info in the list held the multipath data associated
with the multipath.  As such if you were at any other node that data
was not filled in.
d) As such the mp_info's that are not first on the list basically
were just pointers to the corresponding bgp_path_info that was in
the multipath.
e) On bestpath calculation, a linklist(struct linklist *) of bgp_path_info's was
created.
f) This linklist was passed in to a comparison function that took the
old mpinfo list and compared it item by item to the linklist and
doing magic to figure out how to create a new mp_info list.
g) the old mp_info and the link list had to be memory managed and
freed up.
h) BGP_PATH_MULTIPATH is only set on non bestpath nodes in the
multipath.

This is really complicated.  Let's change the algorithm to this:

a) When running bestpath, mark a bgp_path_info node that could be in the ecmp path as
BGP_PATH_MULTIPATH_NEW.
b) When running multipath, just walk the list of bgp_path_info's and if
it has BGP_PATH_MULTIPATH_NEW on it, decide if it is in BGP_MULTIPATH.
If we run out of space to put in the ecmp, clear the flag on the rest.
c) Clean up the counting of sometimes adding 1 to the mpath count.
d) Only allocate a mpath_info node for the bestpath.  Clean it up
when done with it.
e) remove the unneeded list management associated with the linklist and
the mp_list.

This greatly simplifies multipath computation for bgp and reduces memory
load for large scale deployments.

2 full feeds in work_queue_run prior:

    0      56367.471      1123    50193    493695    50362    493791         0         0          0    TE   work_queue_run

BGP multipath info            :  1941844     48   110780992  1941844 110780992

2 full feeds in work_queue_run after change:

    1      52924.931      1296    40837    465968    41025    487390         0         0          1    TE   work_queue_run

BGP multipath info            :   970860     32    38836880   970866  38837120

Aproximately 4 seconds of saved cpu time for convergence and ~75 mb
smaller run time.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd/bgp_mpath.h

@@ -2,8 +2,9 @@
 /*
  * BGP Multipath
  * Copyright (C) 2010 Google Inc.
+ *               2024 Nvidia Corporation
  *
- * This file is part of Quagga
+ * This file is part of FRR
  */
 
 #ifndef _FRR_BGP_MPATH_H
@@ -13,12 +14,6 @@
  * multipath selections, lazily allocated to save memory
  */
 struct bgp_path_info_mpath {
-	/* Points to the first multipath (on bestpath) or the next multipath */
-	struct bgp_path_info_mpath *mp_nexta;
-
-	/* Points to the previous multipath or NULL on bestpath */
-	struct bgp_path_info_mpath *mp_prev;
-
 	/* Points to bgp_path_info associated with this multipath info */
 	struct bgp_path_info *mp_info;
 
@@ -51,14 +46,10 @@ extern int bgp_maximum_paths_unset(struct bgp *bgp, afi_t afi, safi_t safi,
  */
 extern int bgp_path_info_nexthop_cmp(struct bgp_path_info *bpi1,
 				     struct bgp_path_info *bpi2);
-extern void bgp_mp_list_init(struct list *mp_list);
-extern void bgp_mp_list_clear(struct list *mp_list);
-extern void bgp_mp_list_add(struct list *mp_list, struct bgp_path_info *mpinfo);
 extern void bgp_mp_dmed_deselect(struct bgp_path_info *dmed_best);
 extern void bgp_path_info_mpath_update(struct bgp *bgp, struct bgp_dest *dest,
 				       struct bgp_path_info *new_best,
-				       struct bgp_path_info *old_best,
-				       struct list *mp_list,
+				       struct bgp_path_info *old_best, uint32_t num_candidates,
 				       struct bgp_maxpaths_cfg *mpath_cfg);
 extern void
 bgp_path_info_mpath_aggregate_update(struct bgp_path_info *new_best,

bgpd/bgp_route.c

@@ -2173,8 +2173,7 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi,
 	from = pi->peer;
 	filter = &peer->filter[afi][safi];
 	bgp = SUBGRP_INST(subgrp);
-	piattr = bgp_path_info_mpath_count(pi) ? bgp_path_info_mpath_attr(pi)
-					       : pi->attr;
+	piattr = bgp_path_info_mpath_count(pi) > 1 ? bgp_path_info_mpath_attr(pi) : pi->attr;
 
 	if (CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_MAX_PREFIX_OUT) &&
 	    peer->pmax_out[afi][safi] != 0 &&
@@ -2854,13 +2853,12 @@ void bgp_best_selection(struct bgp *bgp, struct bgp_dest *dest,
 	struct bgp_path_info *pi2;
 	int paths_eq, do_mpath;
 	bool debug, any_comparisons;
-	struct list mp_list;
 	char pfx_buf[PREFIX2STR_BUFFER] = {};
 	char path_buf[PATH_ADDPATH_STR_BUFFER];
 	enum bgp_path_selection_reason reason = bgp_path_selection_none;
 	bool unsorted_items = true;
+	uint32_t num_candidates = 0;
 
-	bgp_mp_list_init(&mp_list);
 	do_mpath =
 		(mpath_cfg->maxpaths_ebgp > 1 || mpath_cfg->maxpaths_ibgp > 1);
 
@@ -3235,7 +3233,8 @@ void bgp_best_selection(struct bgp *bgp, struct bgp_dest *dest,
 						"%pBD(%s): %s is the bestpath, add to the multipath list",
 						dest, bgp->name_pretty,
 						path_buf);
-				bgp_mp_list_add(&mp_list, pi);
+				SET_FLAG(pi->flags, BGP_PATH_MULTIPATH_NEW);
+				num_candidates++;
 				continue;
 			}
 
@@ -3258,15 +3257,14 @@ void bgp_best_selection(struct bgp *bgp, struct bgp_dest *dest,
 						"%pBD(%s): %s is equivalent to the bestpath, add to the multipath list",
 						dest, bgp->name_pretty,
 						path_buf);
-				bgp_mp_list_add(&mp_list, pi);
+				SET_FLAG(pi->flags, BGP_PATH_MULTIPATH_NEW);
+				num_candidates++;
 			}
 		}
 	}
 
-	bgp_path_info_mpath_update(bgp, dest, new_select, old_select, &mp_list,
-				   mpath_cfg);
+	bgp_path_info_mpath_update(bgp, dest, new_select, old_select, num_candidates, mpath_cfg);
 	bgp_path_info_mpath_aggregate_update(new_select, old_select);
-	bgp_mp_list_clear(&mp_list);
 
 	bgp_addpath_update_ids(bgp, dest, afi, safi);
 
@@ -11189,9 +11187,8 @@ void route_vty_out_detail(struct vty *vty, struct bgp *bgp, struct bgp_dest *bn,
 			vty_out(vty, ", otc %u", attr->otc);
 	}
 
-	if (CHECK_FLAG(path->flags, BGP_PATH_MULTIPATH)
-	    || (CHECK_FLAG(path->flags, BGP_PATH_SELECTED)
-		&& bgp_path_info_mpath_count(path))) {
+	if (CHECK_FLAG(path->flags, BGP_PATH_MULTIPATH) ||
+	    (CHECK_FLAG(path->flags, BGP_PATH_SELECTED) && bgp_path_info_mpath_count(path) > 1)) {
 		if (json_paths)
 			json_object_boolean_true_add(json_path, "multipath");
 		else

bgpd/bgp_route.h

@@ -313,6 +313,11 @@ struct bgp_path_info {
 #define BGP_PATH_STALE (1 << 8)
 #define BGP_PATH_REMOVED (1 << 9)
 #define BGP_PATH_COUNTED (1 << 10)
+/*
+ * A BGP_PATH_MULTIPATH flag is not set on the best path
+ * it is set on every other node that is part of ECMP
+ * for that particular dest
+ */
 #define BGP_PATH_MULTIPATH (1 << 11)
 #define BGP_PATH_MULTIPATH_CHG (1 << 12)
 #define BGP_PATH_RIB_ATTR_CHG (1 << 13)
@@ -322,6 +327,15 @@ struct bgp_path_info {
 #define BGP_PATH_MPLSVPN_LABEL_NH (1 << 17)
 #define BGP_PATH_MPLSVPN_NH_LABEL_BIND (1 << 18)
 #define BGP_PATH_UNSORTED (1 << 19)
+/*
+ * BGP_PATH_MULTIPATH_NEW is set on those bgp_path_info
+ * nodes that we have decided should possibly be in the
+ * ecmp path for a particular dest.  This flag is
+ * removed when the bgp_path_info's are looked at to
+ * decide on whether or not a bgp_path_info is on
+ * the actual ecmp path.
+ */
+#define BGP_PATH_MULTIPATH_NEW (1 << 20)
 
 	/* BGP route type.  This can be static, RIP, OSPF, BGP etc.  */
 	uint8_t type;

bgpd/bgp_routemap.c

@@ -3220,7 +3220,7 @@ route_set_ecommunity_lb(void *rule, const struct prefix *prefix, void *object)
 			return RMAP_OKAY;
 
 		bw_bytes = (peer->bgp->lb_ref_bw * 1000 * 1000) / 8;
-		mpath_count = bgp_path_info_mpath_count(path) + 1;
+		mpath_count = bgp_path_info_mpath_count(path);
 		bw_bytes *= mpath_count;
 	}
 

tests/bgpd/subdir.am

@@ -52,17 +52,6 @@ tests_bgpd_test_mp_attr_LDADD = $(BGP_TEST_LDADD)
 tests_bgpd_test_mp_attr_SOURCES = tests/bgpd/test_mp_attr.c
 EXTRA_DIST += tests/bgpd/test_mp_attr.py
 
-
-if BGPD
-check_PROGRAMS += tests/bgpd/test_mpath
-endif
-tests_bgpd_test_mpath_CFLAGS = $(TESTS_CFLAGS)
-tests_bgpd_test_mpath_CPPFLAGS = $(TESTS_CPPFLAGS)
-tests_bgpd_test_mpath_LDADD = $(BGP_TEST_LDADD)
-tests_bgpd_test_mpath_SOURCES = tests/bgpd/test_mpath.c
-EXTRA_DIST += tests/bgpd/test_mpath.py
-
-
 if BGPD
 check_PROGRAMS += tests/bgpd/test_packet
 endif