Merge pull request #3776 from opensourcerouting/pbrd-interface-nexthops

pbrd: add support for interface nexthops

1 files changed, 57 insertions, 43 deletions

diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c
index f940418d83..27ab04279c 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;
 
@@ -209,6 +223,9 @@ static void nhgl_delete(struct nexthop_hold *nh)
 
 	XFREE(MTYPE_TMP, nh->nhvrf_name);
 
+	if (nh->addr)
+		sockunion_free(nh->addr);
+
 	XFREE(MTYPE_TMP, nh);
 }
 
@@ -292,8 +309,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 = *addr;
+	if (addr)
+		nh->addr = sockunion_dup(addr);
 
 	listnode_add_sort(nhgc->nhg_list, nh);
 }
@@ -308,7 +325,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;
 	}
@@ -320,13 +337,7 @@ static void nexthop_group_unsave_nhop(struct nexthop_group_cmd *nhgc,
 		return;
 
 	list_delete_node(nhgc->nhg_list, node);
-
-	if (nh->nhvrf_name)
-		XFREE(MTYPE_TMP, nh->nhvrf_name);
-	if (nh->intf)
-		XFREE(MTYPE_TMP, nh->intf);
-
-	XFREE(MTYPE_TMP, nh);
+	nhgl_delete(nh);
 }
 
 static bool nexthop_group_parse_nexthop(struct nexthop *nhop,
@@ -347,36 +358,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")
 {
@@ -385,13 +405,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
@@ -480,9 +493,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);
@@ -504,7 +518,7 @@ static int nexthop_group_write(struct vty *vty)
 		vty_out(vty, "nexthop-group %s\n", nhgc->name);
 
 		for (ALL_LIST_ELEMENTS_RO(nhgc->nhg_list, node, nh)) {
-			vty_out(vty, "  ");
+			vty_out(vty, " ");
 			nexthop_group_write_nexthop_internal(vty, nh);
 		}
 
@@ -526,7 +540,7 @@ void nexthop_group_enable_vrf(struct vrf *vrf)
 			struct nexthop nhop;
 			struct nexthop *nh;
 
-			if (!nexthop_group_parse_nexthop(&nhop, &nhh->addr,
+			if (!nexthop_group_parse_nexthop(&nhop, nhh->addr,
 							 nhh->intf,
 							 nhh->nhvrf_name))
 				continue;
@@ -562,7 +576,7 @@ void nexthop_group_disable_vrf(struct vrf *vrf)
 			struct nexthop nhop;
 			struct nexthop *nh;
 
-			if (!nexthop_group_parse_nexthop(&nhop, &nhh->addr,
+			if (!nexthop_group_parse_nexthop(&nhop, nhh->addr,
 							 nhh->intf,
 							 nhh->nhvrf_name))
 				continue;
@@ -600,7 +614,7 @@ void nexthop_group_interface_state_change(struct interface *ifp,
 				struct nexthop nhop;
 
 				if (!nexthop_group_parse_nexthop(
-					    &nhop, &nhh->addr, nhh->intf,
+					    &nhop, nhh->addr, nhh->intf,
 					    nhh->nhvrf_name))
 					continue;