*: Allow 16 bit size for nexthops

donaldsharp · donaldsharp · commit 9f8968fc5ae7 · 2024-10-08T09:26:57.000-04:00
Currently FRR is limiting the nexthop count to a uint8_t not a
uint16_t.  This leads to issues when the nexthop count is 256
which results in the count to overflow to 0 causing problems
in the code.

Signed-off-by: Donald Sharp &lt;sharpd@nvidia.com&gt;
diff --git a/bgpd/bgp_addpath.c b/bgpd/bgp_addpath.c
@@ -361,8 +361,7 @@ void bgp_addpath_type_changed(struct bgp *bgp)
 	}
 }
 
-int bgp_addpath_capability_action(enum bgp_addpath_strat addpath_type,
-				  uint8_t paths)
+int bgp_addpath_capability_action(enum bgp_addpath_strat addpath_type, uint16_t paths)
 {
 	int action = CAPABILITY_ACTION_UNSET;
 
@@ -392,8 +391,7 @@ int bgp_addpath_capability_action(enum bgp_addpath_strat addpath_type,
  * change take effect.
  */
 void bgp_addpath_set_peer_type(struct peer *peer, afi_t afi, safi_t safi,
-			       enum bgp_addpath_strat addpath_type,
-			       uint8_t paths)
+			       enum bgp_addpath_strat addpath_type, uint16_t paths)
 {
 	struct bgp *bgp = peer->bgp;
 	enum bgp_addpath_strat old_type;
diff --git a/bgpd/bgp_addpath.h b/bgpd/bgp_addpath.h
@@ -62,13 +62,11 @@ bool bgp_addpath_tx_path(enum bgp_addpath_strat strat,
  * Change the type of addpath used for a peer.
  */
 void bgp_addpath_set_peer_type(struct peer *peer, afi_t afi, safi_t safi,
-			       enum bgp_addpath_strat addpath_type,
-			       uint8_t paths);
+			       enum bgp_addpath_strat addpath_type, uint16_t paths);
 
 void bgp_addpath_update_ids(struct bgp *bgp, struct bgp_dest *dest, afi_t afi,
 			    safi_t safi);
 
 void bgp_addpath_type_changed(struct bgp *bgp);
-extern int bgp_addpath_capability_action(enum bgp_addpath_strat addpath_type,
-					 uint8_t paths);
+extern int bgp_addpath_capability_action(enum bgp_addpath_strat addpath_type, uint16_t paths);
 #endif
diff --git a/bgpd/bgp_nexthop.h b/bgpd/bgp_nexthop.h
@@ -38,7 +38,7 @@ struct bgp_nexthop_cache {
 	uint32_t metric;
 
 	/* Nexthop number and nexthop linked list.*/
-	uint8_t nexthop_num;
+	uint16_t nexthop_num;
 
 	/* This flag is set to TRUE for a bnc that is gateway IP overlay index
 	 * nexthop.
diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c
@@ -761,7 +761,7 @@ static void bgp_nht_ifp_table_handle(struct bgp *bgp,
 {
 	struct bgp_nexthop_cache *bnc;
 	struct nexthop *nhop;
-	uint8_t other_nh_count;
+	uint16_t other_nh_count;
 	bool nhop_ll_found = false;
 	bool nhop_found = false;
 
diff --git a/pimd/pim_nht.c b/pimd/pim_nht.c
@@ -573,7 +573,7 @@ static int pim_ecmp_nexthop_search(struct pim_instance *pim,
 	ifindex_t first_ifindex;
 	struct interface *ifp = NULL;
 	uint32_t hash_val = 0, mod_val = 0;
-	uint8_t nh_iter = 0, found = 0;
+	uint16_t nh_iter = 0, found = 0;
 	uint32_t i, num_nbrs = 0;
 	struct pim_interface *pim_ifp;
 
@@ -947,7 +947,7 @@ int pim_ecmp_nexthop_lookup(struct pim_instance *pim,
 	struct interface *ifps[router->multipath], *ifp;
 	int first_ifindex;
 	int found = 0;
-	uint8_t i = 0;
+	uint16_t i = 0;
 	uint32_t hash_val = 0, mod_val = 0;
 	uint32_t num_nbrs = 0;
 	struct pim_interface *pim_ifp;
diff --git a/pimd/pim_nht.h b/pimd/pim_nht.h
@@ -23,7 +23,7 @@ struct pim_nexthop_cache {
 	uint32_t metric;
 	uint32_t distance;
 	/* Nexthop number and nexthop linked list. */
-	uint8_t nexthop_num;
+	uint16_t nexthop_num;
 	struct nexthop *nexthop;
 	int64_t last_update;
 	uint16_t flags;
diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c
@@ -591,12 +591,9 @@ parse_nexthop_unicast(ns_id_t ns_id, struct rtmsg *rtm, struct rtattr **tb,
 	return nh;
 }
 
-static uint8_t parse_multipath_nexthops_unicast(ns_id_t ns_id,
-						struct nexthop_group *ng,
-						struct rtmsg *rtm,
-						struct rtnexthop *rtnh,
-						struct rtattr **tb,
-						void *prefsrc, vrf_id_t vrf_id)
+static uint16_t parse_multipath_nexthops_unicast(ns_id_t ns_id, struct nexthop_group *ng,
+						 struct rtmsg *rtm, struct rtnexthop *rtnh,
+						 struct rtattr **tb, void *prefsrc, vrf_id_t vrf_id)
 {
 	void *gate = NULL;
 	struct interface *ifp = NULL;
@@ -721,7 +718,7 @@ static uint8_t parse_multipath_nexthops_unicast(ns_id_t ns_id,
 		rtnh = RTNH_NEXT(rtnh);
 	}
 
-	uint8_t nhop_num = nexthop_group_nexthop_num(ng);
+	uint16_t nhop_num = nexthop_group_nexthop_num(ng);
 
 	return nhop_num;
 }
@@ -1000,7 +997,7 @@ int netlink_route_change_read_unicast_internal(struct nlmsghdr *h,
 				(struct rtnexthop *)RTA_DATA(tb[RTA_MULTIPATH]);
 
 			if (!nhe_id) {
-				uint8_t nhop_num;
+				uint16_t nhop_num;
 
 				/* Use temporary list of nexthops; parse
 				 * message payload's nexthops.
@@ -2644,11 +2641,9 @@ int kernel_get_ipmr_sg_stats(struct zebra_vrf *zvrf, void *in)
 /* Char length to debug ID with */
 #define ID_LENGTH 10
 
-static bool _netlink_nexthop_build_group(struct nlmsghdr *n, size_t req_size,
-					 uint32_t id,
-					 const struct nh_grp *z_grp,
-					 const uint8_t count, bool resilient,
-					 const struct nhg_resilience *nhgr)
+static bool _netlink_nexthop_build_group(struct nlmsghdr *n, size_t req_size, uint32_t id,
+					 const struct nh_grp *z_grp, const uint16_t count,
+					 bool resilient, const struct nhg_resilience *nhgr)
 {
 	struct nexthop_grp grp[count];
 	/* Need space for max group size, "/", and null term */
@@ -3285,7 +3280,7 @@ static int netlink_nexthop_process_group(struct rtattr **tb,
 					 struct nh_grp *z_grp, int z_grp_size,
 					 struct nhg_resilience *nhgr)
 {
-	uint8_t count = 0;
+	uint16_t count = 0;
 	/* linux/nexthop.h group struct */
 	struct nexthop_grp *n_grp = NULL;
 
@@ -3358,7 +3353,7 @@ int netlink_nexthop_change(struct nlmsghdr *h, ns_id_t ns_id, int startup)
 	struct nexthop nh = {.weight = 1};
 	struct nh_grp grp[MULTIPATH_NUM] = {};
 	/* Count of nexthops in group array */
-	uint8_t grp_count = 0;
+	uint16_t grp_count = 0;
 	struct rtattr *tb[NHA_MAX + 1] = {};
 
 	frrtrace(3, frr_zebra, netlink_nexthop_change, h, ns_id, startup);
diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c
@@ -517,7 +517,7 @@ int zsend_redistribute_route(int cmd, struct zserv *client,
 	struct zapi_nexthop *api_nh;
 	struct nexthop *nexthop;
 	const struct prefix *p, *src_p;
-	uint8_t count = 0;
+	uint16_t count = 0;
 	afi_t afi;
 	size_t stream_size =
 		MAX(ZEBRA_MAX_PACKET_SIZ, sizeof(struct zapi_route));
diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
@@ -86,7 +86,7 @@ struct dplane_nexthop_info {
 
 	struct nexthop_group ng;
 	struct nh_grp nh_grp[MULTIPATH_NUM];
-	uint8_t nh_grp_count;
+	uint16_t nh_grp_count;
 };
 
 /*
@@ -2316,7 +2316,7 @@ dplane_ctx_get_nhe_nh_grp(const struct zebra_dplane_ctx *ctx)
 	return ctx->u.rinfo.nhe.nh_grp;
 }
 
-uint8_t dplane_ctx_get_nhe_nh_grp_count(const struct zebra_dplane_ctx *ctx)
+uint16_t dplane_ctx_get_nhe_nh_grp_count(const struct zebra_dplane_ctx *ctx)
 {
 	DPLANE_CTX_VALID(ctx);
 	return ctx->u.rinfo.nhe.nh_grp_count;
diff --git a/zebra/zebra_dplane.h b/zebra/zebra_dplane.h
@@ -597,7 +597,7 @@ const struct nexthop_group *
 dplane_ctx_get_nhe_ng(const struct zebra_dplane_ctx *ctx);
 const struct nh_grp *
 dplane_ctx_get_nhe_nh_grp(const struct zebra_dplane_ctx *ctx);
-uint8_t dplane_ctx_get_nhe_nh_grp_count(const struct zebra_dplane_ctx *ctx);
+uint16_t dplane_ctx_get_nhe_nh_grp_count(const struct zebra_dplane_ctx *ctx);
 
 /* Accessors for LSP information */
 
diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c
@@ -625,9 +625,8 @@ bool zebra_nhg_hash_id_equal(const void *arg1, const void *arg2)
 	return nhe1->id == nhe2->id;
 }
 
-static int zebra_nhg_process_grp(struct nexthop_group *nhg,
-				 struct nhg_connected_tree_head *depends,
-				 struct nh_grp *grp, uint8_t count,
+static int zebra_nhg_process_grp(struct nexthop_group *nhg, struct nhg_connected_tree_head *depends,
+				 struct nh_grp *grp, uint16_t count,
 				 struct nhg_resilience *resilience)
 {
 	nhg_connected_tree_init(depends);
@@ -982,7 +981,7 @@ static struct nexthop *nhg_ctx_get_nh(struct nhg_ctx *ctx)
 	return &ctx->u.nh;
 }
 
-static uint8_t nhg_ctx_get_count(const struct nhg_ctx *ctx)
+static uint16_t nhg_ctx_get_count(const struct nhg_ctx *ctx)
 {
 	return ctx->count;
 }
@@ -1028,9 +1027,8 @@ void nhg_ctx_free(struct nhg_ctx **ctx)
 	XFREE(MTYPE_NHG_CTX, *ctx);
 }
 
-static struct nhg_ctx *nhg_ctx_init(uint32_t id, struct nexthop *nh,
-				    struct nh_grp *grp, vrf_id_t vrf_id,
-				    afi_t afi, int type, uint8_t count,
+static struct nhg_ctx *nhg_ctx_init(uint32_t id, struct nexthop *nh, struct nh_grp *grp,
+				    vrf_id_t vrf_id, afi_t afi, int type, uint16_t count,
 				    struct nhg_resilience *resilience)
 {
 	struct nhg_ctx *ctx = NULL;
@@ -1204,7 +1202,7 @@ static int nhg_ctx_process_new(struct nhg_ctx *ctx)
 	struct nhg_hash_entry *nhe = NULL;
 
 	uint32_t id = nhg_ctx_get_id(ctx);
-	uint8_t count = nhg_ctx_get_count(ctx);
+	uint16_t count = nhg_ctx_get_count(ctx);
 	vrf_id_t vrf_id = nhg_ctx_get_vrf_id(ctx);
 	int type = nhg_ctx_get_type(ctx);
 	afi_t afi = nhg_ctx_get_afi(ctx);
@@ -1356,9 +1354,9 @@ int nhg_ctx_process(struct nhg_ctx *ctx)
 }
 
 /* Kernel-side, you either get a single new nexthop or a array of ID's */
-int zebra_nhg_kernel_find(uint32_t id, struct nexthop *nh, struct nh_grp *grp,
-			  uint8_t count, vrf_id_t vrf_id, afi_t afi, int type,
-			  int startup, struct nhg_resilience *nhgr)
+int zebra_nhg_kernel_find(uint32_t id, struct nexthop *nh, struct nh_grp *grp, uint16_t count,
+			  vrf_id_t vrf_id, afi_t afi, int type, int startup,
+			  struct nhg_resilience *nhgr)
 {
 	struct nhg_ctx *ctx = NULL;
 
@@ -3180,15 +3178,14 @@ int nexthop_active_update(struct route_node *rn, struct route_entry *re,
  * I'm pretty sure we only allow ONE level of group within group currently.
  * But making this recursive just in case that ever changes.
  */
-static uint8_t zebra_nhg_nhe2grp_internal(struct nh_grp *grp, uint8_t curr_index,
-					  struct nhg_hash_entry *nhe,
-					  struct nhg_hash_entry *original,
-					  int max_num)
+static uint16_t zebra_nhg_nhe2grp_internal(struct nh_grp *grp, uint16_t curr_index,
+					   struct nhg_hash_entry *nhe,
+					   struct nhg_hash_entry *original, int max_num)
 {
 	struct nhg_connected *rb_node_dep = NULL;
 	struct nhg_hash_entry *depend = NULL;
 	struct nexthop *nexthop;
-	uint8_t i = curr_index;
+	uint16_t i = curr_index;
 
 	frr_each(nhg_connected_tree, &nhe->nhg_depends, rb_node_dep) {
 		bool duplicate = false;
@@ -3309,8 +3306,7 @@ static uint8_t zebra_nhg_nhe2grp_internal(struct nh_grp *grp, uint8_t curr_index
 }
 
 /* Convert a nhe into a group array */
-uint8_t zebra_nhg_nhe2grp(struct nh_grp *grp, struct nhg_hash_entry *nhe,
-			  int max_num)
+uint16_t zebra_nhg_nhe2grp(struct nh_grp *grp, struct nhg_hash_entry *nhe, int max_num)
 {
 	/* Call into the recursive function */
 	return zebra_nhg_nhe2grp_internal(grp, 0, nhe, nhe, max_num);
diff --git a/zebra/zebra_nhg.h b/zebra/zebra_nhg.h
@@ -231,7 +231,7 @@ struct nhg_ctx {
 	int type;
 
 	/* If its a group array, how many? */
-	uint8_t count;
+	uint16_t count;
 
 	/* Its either a single nexthop or an array of ID's */
 	union {
@@ -317,10 +317,8 @@ extern int nhg_ctx_process(struct nhg_ctx *ctx);
 void nhg_ctx_free(struct nhg_ctx **ctx);
 
 /* Find via kernel nh creation */
-extern int zebra_nhg_kernel_find(uint32_t id, struct nexthop *nh,
-				 struct nh_grp *grp, uint8_t count,
-				 vrf_id_t vrf_id, afi_t afi, int type,
-				 int startup,
+extern int zebra_nhg_kernel_find(uint32_t id, struct nexthop *nh, struct nh_grp *grp,
+				 uint16_t count, vrf_id_t vrf_id, afi_t afi, int type, int startup,
 				 struct nhg_resilience *resilience);
 /* Del via kernel */
 extern int zebra_nhg_kernel_del(uint32_t id, vrf_id_t vrf_id);
@@ -379,8 +377,7 @@ extern void zebra_nhg_increment_ref(struct nhg_hash_entry *nhe);
 extern void zebra_nhg_check_valid(struct nhg_hash_entry *nhe);
 
 /* Convert nhe depends to a grp context that can be passed around safely */
-extern uint8_t zebra_nhg_nhe2grp(struct nh_grp *grp, struct nhg_hash_entry *nhe,
-				 int size);
+extern uint16_t zebra_nhg_nhe2grp(struct nh_grp *grp, struct nhg_hash_entry *nhe, int size);
 
 /* Dataplane install/uninstall */
 extern void zebra_nhg_install_kernel(struct nhg_hash_entry *nhe, uint8_t type);

Original file line number	Diff line number	Diff line change
`@@ -361,8 +361,7 @@ void bgp_addpath_type_changed(struct bgp *bgp)`
`361`	`361`	`}`
`362`	`362`	`}`
`363`	`363`
`364`		`-int bgp_addpath_capability_action(enum bgp_addpath_strat addpath_type,`
`365`		`- uint8_t paths)`
	`364`	`+int bgp_addpath_capability_action(enum bgp_addpath_strat addpath_type, uint16_t paths)`
`366`	`365`	`{`
`367`	`366`	`int action = CAPABILITY_ACTION_UNSET;`
`368`	`367`
`@@ -392,8 +391,7 @@ int bgp_addpath_capability_action(enum bgp_addpath_strat addpath_type,`
`392`	`391`	`* change take effect.`
`393`	`392`	`*/`
`394`	`393`	`void bgp_addpath_set_peer_type(struct peer *peer, afi_t afi, safi_t safi,`
`395`		`- enum bgp_addpath_strat addpath_type,`
`396`		`- uint8_t paths)`
	`394`	`+ enum bgp_addpath_strat addpath_type, uint16_t paths)`
`397`	`395`	`{`
`398`	`396`	`struct bgp *bgp = peer->bgp;`
`399`	`397`	`enum bgp_addpath_strat old_type;`
Original file line number	Diff line number	Diff line change
`@@ -761,7 +761,7 @@ static void bgp_nht_ifp_table_handle(struct bgp *bgp,`
`761`	`761`	`{`
`762`	`762`	`struct bgp_nexthop_cache *bnc;`
`763`	`763`	`struct nexthop *nhop;`
`764`		`- uint8_t other_nh_count;`
	`764`	`+ uint16_t other_nh_count;`
`765`	`765`	`bool nhop_ll_found = false;`
`766`	`766`	`bool nhop_found = false;`
`767`	`767`
Original file line number	Diff line number	Diff line change
`@@ -86,7 +86,7 @@ struct dplane_nexthop_info {`
`86`	`86`
`87`	`87`	`struct nexthop_group ng;`
`88`	`88`	`struct nh_grp nh_grp[MULTIPATH_NUM];`
`89`		`- uint8_t nh_grp_count;`
	`89`	`+ uint16_t nh_grp_count;`
`90`	`90`	`};`
`91`	`91`
`92`	`92`	`/*`
`@@ -2316,7 +2316,7 @@ dplane_ctx_get_nhe_nh_grp(const struct zebra_dplane_ctx *ctx)`
`2316`	`2316`	`return ctx->u.rinfo.nhe.nh_grp;`
`2317`	`2317`	`}`
`2318`	`2318`
`2319`		`-uint8_t dplane_ctx_get_nhe_nh_grp_count(const struct zebra_dplane_ctx *ctx)`
	`2319`	`+uint16_t dplane_ctx_get_nhe_nh_grp_count(const struct zebra_dplane_ctx *ctx)`
`2320`	`2320`	`{`
`2321`	`2321`	`DPLANE_CTX_VALID(ctx);`
`2322`	`2322`	`return ctx->u.rinfo.nhe.nh_grp_count;`