lib: add support for interface nexthops on nexthop groups

This patch adds support to nexthops of type NEXTHOP_TYPE_IFINDEX to
nexthop-groups. This should be especially useful when dealing with
p2p interfaces like tunnels that don't have IP addresses assigned
to them.

NOTE: nh->addr can be NULL now, so we should always perform a null
check before dereferencing this pointer.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

1 files changed, 51 insertions, 33 deletions

diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c
index 4c35d7c96..d8cabf62b 100644
--- a/lib/nexthop_group.c
+++ b/lib/nexthop_group.c
@@ -188,11 +188,25 @@ static int nhgc_cmp_helper(const char *a, const char *b)
 	return strcmp(a, b);
 }
 
+static int nhgc_addr_cmp_helper(const union sockunion *a, const union sockunion *b)
+{
+	if (!a && !b)
+		return 0;
+
+	if (a && !b)
+		return -1;
+
+	if (!a && b)
+		return 1;
+
+	return sockunion_cmp(a, b);
+}
+
 static int nhgl_cmp(struct nexthop_hold *nh1, struct nexthop_hold *nh2)
 {
 	int ret;
 
-	ret = sockunion_cmp(nh1->addr, nh2->addr);
+	ret = nhgc_addr_cmp_helper(nh1->addr, nh2->addr);
 	if (ret)
 		return ret;
 
@@ -211,7 +225,8 @@ static void nhgl_delete(struct nexthop_hold *nh)
 	if (nh->nhvrf_name)
 		XFREE(MTYPE_TMP, nh->nhvrf_name);
 
-	sockunion_free(nh->addr);
+	if (nh->addr)
+		sockunion_free(nh->addr);
 
 	XFREE(MTYPE_TMP, nh);
 }
@@ -296,8 +311,8 @@ static void nexthop_group_save_nhop(struct nexthop_group_cmd *nhgc,
 		nh->nhvrf_name = XSTRDUP(MTYPE_TMP, nhvrf_name);
 	if (intf)
 		nh->intf = XSTRDUP(MTYPE_TMP, intf);
-
-	nh->addr = sockunion_dup(addr);
+	if (addr)
+		nh->addr = sockunion_dup(addr);
 
 	listnode_add_sort(nhgc->nhg_list, nh);
 }
@@ -312,7 +327,7 @@ static void nexthop_group_unsave_nhop(struct nexthop_group_cmd *nhgc,
 
 	for (ALL_LIST_ELEMENTS_RO(nhgc->nhg_list, node, nh)) {
 		if (nhgc_cmp_helper(nhvrf_name, nh->nhvrf_name) == 0 &&
-		    sockunion_cmp(addr, nh->addr) == 0 &&
+		    nhgc_addr_cmp_helper(addr, nh->addr) == 0 &&
 		    nhgc_cmp_helper(intf, nh->intf) == 0)
 			break;
 	}
@@ -345,36 +360,45 @@ static bool nexthop_group_parse_nexthop(struct nexthop *nhop,
 
 	nhop->vrf_id = vrf->vrf_id;
 
-	if (addr->sa.sa_family == AF_INET) {
-		nhop->gate.ipv4.s_addr = addr->sin.sin_addr.s_addr;
-		if (intf) {
-			nhop->type = NEXTHOP_TYPE_IPV4_IFINDEX;
-			nhop->ifindex = ifname2ifindex(intf, vrf->vrf_id);
-			if (nhop->ifindex == IFINDEX_INTERNAL)
-				return false;
-		} else
-			nhop->type = NEXTHOP_TYPE_IPV4;
-	} else {
-		memcpy(&nhop->gate.ipv6, &addr->sin6.sin6_addr, 16);
-		if (intf) {
-			nhop->type = NEXTHOP_TYPE_IPV6_IFINDEX;
-			nhop->ifindex = ifname2ifindex(intf, vrf->vrf_id);
-			if (nhop->ifindex == IFINDEX_INTERNAL)
-				return false;
-		} else
-			nhop->type = NEXTHOP_TYPE_IPV6;
+	if (intf) {
+		nhop->ifindex = ifname2ifindex(intf, vrf->vrf_id);
+		if (nhop->ifindex == IFINDEX_INTERNAL)
+			return false;
 	}
 
+	if (addr) {
+		if (addr->sa.sa_family == AF_INET) {
+			nhop->gate.ipv4.s_addr = addr->sin.sin_addr.s_addr;
+			if (intf)
+				nhop->type = NEXTHOP_TYPE_IPV4_IFINDEX;
+			else
+				nhop->type = NEXTHOP_TYPE_IPV4;
+		} else {
+			nhop->gate.ipv6 = addr->sin6.sin6_addr;
+			if (intf)
+				nhop->type = NEXTHOP_TYPE_IPV6_IFINDEX;
+			else
+				nhop->type = NEXTHOP_TYPE_IPV6;
+		}
+	} else
+		nhop->type = NEXTHOP_TYPE_IFINDEX;
+
 	return true;
 }
 
 DEFPY(ecmp_nexthops, ecmp_nexthops_cmd,
-      "[no] nexthop <A.B.C.D|X:X::X:X>$addr [INTERFACE]$intf [nexthop-vrf NAME$name]",
+      "[no] nexthop\
+        <\
+	  <A.B.C.D|X:X::X:X>$addr [INTERFACE$intf]\
+	  |INTERFACE$intf\
+	>\
+	[nexthop-vrf NAME$name]",
       NO_STR
       "Specify one of the nexthops in this ECMP group\n"
       "v4 Address\n"
       "v6 Address\n"
       "Interface to use\n"
+      "Interface to use\n"
       "If the nexthop is in a different vrf tell us\n"
       "The nexthop-vrf Name\n")
 {
@@ -383,13 +407,6 @@ DEFPY(ecmp_nexthops, ecmp_nexthops_cmd,
 	struct nexthop *nh;
 	bool legal;
 
-	/*
-	 * This is impossible to happen as that the cli parser refuses
-	 * to let you get here without an addr, but the SA system
-	 * does not understand this intricacy
-	 */
-	assert(addr);
-
 	legal = nexthop_group_parse_nexthop(&nhop, addr, intf, name);
 
 	if (nhop.type == NEXTHOP_TYPE_IPV6
@@ -478,9 +495,10 @@ static void nexthop_group_write_nexthop_internal(struct vty *vty,
 {
 	char buf[100];
 
-	vty_out(vty, "nexthop ");
+	vty_out(vty, "nexthop");
 
-	vty_out(vty, "%s", sockunion2str(nh->addr, buf, sizeof(buf)));
+	if (nh->addr)
+		vty_out(vty, " %s", sockunion2str(nh->addr, buf, sizeof(buf)));
 
 	if (nh->intf)
 		vty_out(vty, " %s", nh->intf);