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>

diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c
index 4c35d7c96c6d6800125507a3f262147208b9ff5e..d8cabf62be036f8bd669bc4abc03da8ce2a218c1 100644 (file)
--- 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);