sharpd, zebra, lib: add recursive support for protocol nhid

Nexthops with an undefined interface index are not supported, whereas recursive routing can be unlocked in zebra. Add the 'allow-recursion' command under nexthop-group, to autorise nexthops with undefined interface to be sent to zebra. Handle the ALLOW_RECURSION flag in nexthop groups, and dynamically resolve the nexthop with a resolved nexthop, if needed. Signed-off-by: Philippe Guibert <[email protected]>
FRRouting · Dec 8, 2023 · 309e922 · 309e922
1 parent 9e27760
commit 309e922
Show file tree

Hide file tree

Showing 9 changed files with 71 additions and 16 deletions.
diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c
@@ -649,6 +649,30 @@ DEFPY(nexthop_group_backup, nexthop_group_backup_cmd,
 	return CMD_SUCCESS;
 }
 
+DEFPY(nexthop_group_allow_recursion,
+      nexthop_group_allow_recursion_cmd,
+      "[no] allow-recursion",
+      NO_STR
+      "Allow recursion for nexthops with no interface presence\n")
+{
+	VTY_DECLVAR_CONTEXT(nexthop_group_cmd, nhgc);
+
+	if (!!no == !CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_ALLOW_RECURSION))
+		return CMD_SUCCESS;
+
+	if (no)
+		UNSET_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_ALLOW_RECURSION);
+	else
+		SET_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_ALLOW_RECURSION);
+
+	SET_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_RESET_NHG);
+	if (nhg_hooks.modify)
+		nhg_hooks.modify(nhgc);
+	UNSET_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_RESET_NHG);
+
+	return CMD_SUCCESS;
+}
+
 DEFPY(no_nexthop_group_backup, no_nexthop_group_backup_cmd,
       "no backup-group [WORD$name]",
       NO_STR
@@ -1172,6 +1196,9 @@ static int nexthop_group_write(struct vty *vty)
 
 		vty_out(vty, "nexthop-group %s\n", nhgc->name);
 
+		if (CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_ALLOW_RECURSION))
+			vty_out(vty, " allow-recursion\n");
+
 		if (nhgc->nhg.nhgr.buckets)
 			vty_out(vty,
 				" resilient buckets %u idle-timer %u unbalanced-timer %u\n",
@@ -1373,6 +1400,7 @@ void nexthop_group_init(void (*new)(const char *name),
 
 	install_element(NH_GROUP_NODE, &nexthop_group_resilience_cmd);
 	install_element(NH_GROUP_NODE, &no_nexthop_group_resilience_cmd);
+	install_element(NH_GROUP_NODE, &nexthop_group_allow_recursion_cmd);
 
 	memset(&nhg_hooks, 0, sizeof(nhg_hooks));
 

diff --git a/lib/nexthop_group.h b/lib/nexthop_group.h
@@ -34,6 +34,11 @@ struct nexthop_group {
 	struct nexthop *nexthop;
 
 	struct nhg_resilience nhgr;
+
+	/* nexthop group flags */
+#define NEXTHOP_GROUP_ALLOW_RECURSION (1 << 1)
+#define NEXTHOP_GROUP_RESET_NHG	      (1 << 2)
+	uint8_t flags;
 };
 
 struct nexthop_group *nexthop_group_new(void);

diff --git a/lib/zclient.c b/lib/zclient.c
@@ -1173,6 +1173,8 @@ static int zapi_nhg_encode(struct stream *s, int cmd, struct zapi_nhg *api_nhg)
 	stream_putl(s, api_nhg->resilience.idle_timer);
 	stream_putl(s, api_nhg->resilience.unbalanced_timer);
 
+	stream_putc(s, api_nhg->flags);
+
 	if (cmd == ZEBRA_NHG_ADD) {
 		/* Nexthops */
 		zapi_nexthop_group_sort(api_nhg->nexthops,

diff --git a/lib/zclient.h b/lib/zclient.h
@@ -486,6 +486,9 @@ struct zapi_nhg {
 
 	uint16_t backup_nexthop_num;
 	struct zapi_nexthop backup_nexthops[MULTIPATH_NUM];
+
+	/* nexthop group flags */
+	uint8_t flags;
 };
 
 /*

diff --git a/sharpd/sharp_nht.c b/sharpd/sharp_nht.c
@@ -126,6 +126,11 @@ static void sharp_nhgroup_modify_cb(const struct nexthop_group_cmd *nhgc)
 	if (!nhgc->nhg.nexthop)
 		return;
 
+	if (CHECK_FLAG(nhgc->nhg.flags, NEXTHOP_GROUP_RESET_NHG)) {
+		/* nexthops must be removed before being re-added */
+		nhg_del(snhg->id);
+	}
+
 	if (nhgc->backup_list_name[0])
 		bnhgc = nhgc_find(nhgc->backup_list_name);
 

diff --git a/sharpd/sharp_zebra.c b/sharpd/sharp_zebra.c
@@ -540,6 +540,7 @@ void nhg_add(uint32_t id, const struct nexthop_group *nhg,
 
 	api_nhg.id = id;
 
+	api_nhg.flags = nhg->flags;
 	api_nhg.resilience = nhg->nhgr;
 
 	for (ALL_NEXTHOPS_PTR(nhg, nh)) {
@@ -550,10 +551,11 @@ void nhg_add(uint32_t id, const struct nexthop_group *nhg,
 			break;
 		}
 
-		/* Unresolved nexthops will lead to failure - only send
-		 * nexthops that zebra will consider valid.
+		/* Unresolved nexthops will lead to failure, unless
+		 * ALLOW_RECURSION flag is set
 		 */
-		if (nh->ifindex == 0)
+		if (nh->ifindex == 0 &&
+		    !CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_ALLOW_RECURSION))
 			continue;
 
 		api_nh = &api_nhg.nexthops[api_nhg.nexthop_num];

diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c
@@ -1895,6 +1895,8 @@ static int zapi_nhg_decode(struct stream *s, int cmd, struct zapi_nhg *api_nhg)
 	STREAM_GETL(s, api_nhg->resilience.idle_timer);
 	STREAM_GETL(s, api_nhg->resilience.unbalanced_timer);
 
+	STREAM_GETC(s, api_nhg->flags);
+
 	/* Nexthops */
 	STREAM_GETW(s, api_nhg->nexthop_num);
 
@@ -2013,6 +2015,7 @@ static void zread_nhg_add(ZAPI_HANDLER_ARGS)
 	nhe = zebra_nhg_alloc();
 	nhe->id = api_nhg.id;
 	nhe->type = api_nhg.proto;
+	nhe->nhg.flags = api_nhg.flags;
 	nhe->zapi_instance = client->instance;
 	nhe->zapi_session = client->session_id;
 

diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c
@@ -2347,8 +2347,10 @@ static int nexthop_active(struct nexthop *nexthop, struct nhg_hash_entry *nhe,
 		 * if specified) - i.e., we cannot have a nexthop NH1 is
 		 * resolved by a route NH1. The exception is if the route is a
 		 * host route.
+		 * This control will not work by using nexthop groups, and will
+		 * have to be handled at protocol level
 		 */
-		if (prefix_same(&rn->p, top))
+		if (top && prefix_same(&rn->p, top))
 			if (((afi == AFI_IP)
 			     && (rn->p.prefixlen != IPV4_MAX_BITLEN))
 			    || ((afi == AFI_IP6)
@@ -2593,10 +2595,8 @@ static unsigned nexthop_active_check(struct route_node *rn,
 		family = AFI_IP6;
 	else
 		family = AF_UNSPEC;
-
 	if (IS_ZEBRA_DEBUG_NHG_DETAIL)
 		zlog_debug("%s: re %p, nexthop %pNHv", __func__, re, nexthop);
-
 	vrf_id = zvrf_id(rib_dest_vrf(rib_dest_from_rnode(rn)));
 
 	/*
@@ -3366,7 +3366,7 @@ struct nhg_hash_entry *zebra_nhg_proto_add(struct nhg_hash_entry *nhe,
 	struct nexthop *newhop;
 	bool replace = false;
 	int ret = 0, type;
-	uint32_t id, session;
+	uint32_t id, session, api_message = 0;
 	uint16_t instance;
 
 	id = nhe->id;
@@ -3414,14 +3414,23 @@ struct nhg_hash_entry *zebra_nhg_proto_add(struct nhg_hash_entry *nhe,
 			return NULL;
 		}
 
-		if (!newhop->ifindex) {
+		if (!newhop->ifindex &&
+		    !CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_ALLOW_RECURSION)) {
 			if (IS_ZEBRA_DEBUG_NHG)
-				zlog_debug(
-					"%s: id %u, nexthop without ifindex is not supported",
-					__func__, id);
+				zlog_debug("%s: id %u, nexthop without ifindex and allow-recursion is not supported",
+					   __func__, id);
 			return NULL;
 		}
-		SET_FLAG(newhop->flags, NEXTHOP_FLAG_ACTIVE);
+
+		/* Check that the route may be recursively resolved */
+		if (CHECK_FLAG(nhg->flags, NEXTHOP_GROUP_ALLOW_RECURSION))
+			api_message = ZEBRA_FLAG_ALLOW_RECURSION;
+
+		if (nexthop_active(newhop, nhe, NULL, 0, api_message, NULL,
+				   newhop->vrf_id))
+			SET_FLAG(newhop->flags, NEXTHOP_FLAG_ACTIVE);
+		else
+			UNSET_FLAG(newhop->flags, NEXTHOP_FLAG_ACTIVE);
 	}
 
 	zebra_nhe_init(&lookup, afi, nhg->nexthop);

diff --git a/zebra/zebra_nhg.h b/zebra/zebra_nhg.h
@@ -322,10 +322,8 @@ zebra_nhg_rib_find_nhe(struct nhg_hash_entry *rt_nhe, afi_t rt_afi);
  *
  * Returns allocated NHE on success, otherwise NULL.
  */
-struct nhg_hash_entry *zebra_nhg_proto_add(uint32_t id, int type,
-					   uint16_t instance, uint32_t session,
-					   struct nexthop_group *nhg,
-					   afi_t afi);
+struct nhg_hash_entry *zebra_nhg_proto_add(struct nhg_hash_entry *nhe,
+					   struct nexthop_group *nhg, afi_t afi);
 
 /*
  * Del NHE.