Merge pull request #16961 from donaldsharp/mp_info_changes

Mp info changes

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,27 +14,24 @@
  * 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_next;
-
-	/* 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;
 
 	/* When attached to best path, the number of selected multipaths */
 	uint16_t mp_count;
 
-	/* Flags - relevant as noted. */
+	/* Flags - relevant as noted, attached to bestpath. */
 	uint16_t mp_flags;
 #define BGP_MP_LB_PRESENT 0x1 /* Link-bandwidth present for >= 1 path */
 #define BGP_MP_LB_ALL 0x2 /* Link-bandwidth present for all multipaths */
 
-	/* Aggregated attribute for advertising multipath route */
+	/*
+	 * Aggregated attribute for advertising multipath route,
+	 * attached to bestpath
+	 */
 	struct attr *mp_attr;
 
-	/* Cumulative bandiwdth of all multipaths - attached to best path. */
+	/* Cumulative bandiwdth of all multipaths - attached to bestpath. */
 	uint64_t cum_bw;
 };
 
@@ -47,23 +45,17 @@ extern int bgp_maximum_paths_unset(struct bgp *bgp, afi_t afi, safi_t safi,
 /* Functions used by bgp_best_selection to record current
  * multipath selections
  */
-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 int bgp_path_info_nexthop_cmp(struct bgp_path_info *bpi1, struct bgp_path_info *bpi2);
 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,
 				     struct bgp_path_info *old_best);
 
 /* Unlink and free multipath information associated with a bgp_path_info */
-extern void bgp_path_info_mpath_dequeue(struct bgp_path_info *path);
 extern void bgp_path_info_mpath_free(struct bgp_path_info_mpath **mpath);
 
 /* Walk list of multipaths associated with a best path */

bgpd/bgp_route.c

@@ -525,8 +525,6 @@ struct bgp_dest *bgp_path_info_reap(struct bgp_dest *dest,
 	else
 		bgp_dest_set_bgp_path_info(dest, pi->next);
 
-	bgp_path_info_mpath_dequeue(pi);
-
 	pi->next = NULL;
 	pi->prev = NULL;
 
@@ -541,8 +539,6 @@ struct bgp_dest *bgp_path_info_reap(struct bgp_dest *dest,
 static struct bgp_dest *bgp_path_info_reap_unsorted(struct bgp_dest *dest,
 						    struct bgp_path_info *pi)
 {
-	bgp_path_info_mpath_dequeue(pi);
-
 	pi->next = NULL;
 	pi->prev = NULL;
 
@@ -2173,8 +2169,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 +2849,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 +3229,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 +3253,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 +11183,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