lib: fix binding to a vrf

There are two possible use-cases for the `vrf_bind` function:
- bind socket to an interface in a vrf
- bind socket to a vrf device

For the former case, there's one problem - success is returned when the
interface is not found. In that case, the socket is left unbound without
throwing an error.

For the latter case, there are multiple possible problems:
- If the name is not set, then the socket is left unbound (zebra, vrrp).
- If the name is "default" and there's an interface with that name in the
  default VRF, then the socket is bound to that interface.
- In most daemons, if the router is configured before the VRF is actually
  created, we're trying to open and bind the socket right after the
  daemon receives a VRF registration from zebra. We may not receive the
  VRF-interface registration from zebra yet at that point. Therefore,
  `if_lookup_by_name` fails, and the socket is left unbound.

This commit fixes all the issues and updates the function description.

Suggested-by: Pat Ruddy <pat@voltanet.io>
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>

diff --git a/lib/vrf.c b/lib/vrf.c
index c01f2c636decf07910e4ab7d347c24f26cf53341..a44cc50f9234954cc06aa56dd1d41b4161e94d83 100644 (file)
--- a/lib/vrf.c
+++ b/lib/vrf.c
@@ -993,25 +993,50 @@ const char *vrf_get_default_name(void)
        return vrf_default_name;
 }
 
-int vrf_bind(vrf_id_t vrf_id, int fd, const char *name)
+int vrf_bind(vrf_id_t vrf_id, int fd, const char *ifname)
 {
        int ret = 0;
        struct interface *ifp;
+       struct vrf *vrf;
+
+       if (fd < 0)
+               return -1;
+
+       if (vrf_id == VRF_UNKNOWN)
+               return -1;
+
+       /* can't bind to a VRF that doesn't exist */
+       vrf = vrf_lookup_by_id(vrf_id);
+       if (!vrf_is_enabled(vrf))
+               return -1;
+
+       if (ifname && strcmp(ifname, vrf->name)) {
+               /* binding to a regular interface */
+
+               /* can't bind to an interface that doesn't exist */
+               ifp = if_lookup_by_name(ifname, vrf_id);
+               if (!ifp)
+                       return -1;
+       } else {
+               /* binding to a VRF device */
+
+               /* nothing to do for netns */
+               if (vrf_is_backend_netns())
+                       return 0;
+
+               /* nothing to do for default vrf */
+               if (vrf_id == VRF_DEFAULT)
+                       return 0;
+
+               ifname = vrf->name;
+       }
 
-       if (fd < 0 || name == NULL)
-               return fd;
-       /* the device should exist
-        * otherwise we should return
-        * case ifname = vrf in netns mode => return
-        */
-       ifp = if_lookup_by_name(name, vrf_id);
-       if (!ifp)
-               return fd;
 #ifdef SO_BINDTODEVICE
-       ret = setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, name, strlen(name)+1);
+       ret = setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, ifname,
+                        strlen(ifname) + 1);
        if (ret < 0)
-               zlog_debug("bind to interface %s failed, errno=%d", name,
-                          errno);
+               zlog_err("bind to interface %s failed, errno=%d", ifname,
+                        errno);
 #endif /* SO_BINDTODEVICE */
        return ret;
 }

diff --git a/lib/vrf.h b/lib/vrf.h
index c79dd99b9a282da30db515337846a02ee8db0304..7ce03079dd8f1ca4fbf3db9a54a14aa7ee031c1f 100644 (file)
--- a/lib/vrf.h
+++ b/lib/vrf.h
@@ -251,15 +251,14 @@ extern int vrf_sockunion_socket(const union sockunion *su, vrf_id_t vrf_id,
                                const char *name);
 
 /*
- * Binds a socket to a VRF device.
+ * Binds a socket to an interface (ifname) in a VRF (vrf_id).
  *
- * If name is null, the socket is not bound, irrespective of any other
- * arguments.
+ * If ifname is NULL or is equal to the VRF name then bind to a VRF device.
+ * Otherwise, bind to the specified interface in the specified VRF.
  *
- * name should be the name of the VRF device. vrf_id should be the
- * corresponding vrf_id (the ifindex of the device).
+ * Returns 0 on success and -1 on failure.
  */
-extern int vrf_bind(vrf_id_t vrf_id, int fd, const char *name);
+extern int vrf_bind(vrf_id_t vrf_id, int fd, const char *ifname);
 
 /* VRF ioctl operations */
 extern int vrf_getaddrinfo(const char *node, const char *service,

diff --git a/tests/topotests/bgp_vrf_lite_ipv6_rtadv/test_bgp_vrf_lite_ipv6_rtadv.py b/tests/topotests/bgp_vrf_lite_ipv6_rtadv/test_bgp_vrf_lite_ipv6_rtadv.py
index 92ee8513e166f3ece84b9824315c9d8221a0cc04..a17819f74751e8c4718739226743cbfbf29ffabe 100644 (file)
--- a/tests/topotests/bgp_vrf_lite_ipv6_rtadv/test_bgp_vrf_lite_ipv6_rtadv.py
+++ b/tests/topotests/bgp_vrf_lite_ipv6_rtadv/test_bgp_vrf_lite_ipv6_rtadv.py
@@ -42,7 +42,7 @@ sys.path.append(os.path.join(CWD, "../"))
 from lib import topotest
 from lib.topogen import Topogen, TopoRouter, get_topogen
 from lib.topolog import logger
-from lib.common_config import adjust_router_l3mdev
+from lib.common_config import required_linux_kernel_version
 
 # Required to instantiate the topology builder class.
 from mininet.topo import Topo
@@ -66,6 +66,12 @@ class BGPIPV6RTADVVRFTopo(Topo):
 
 def setup_module(mod):
     "Sets up the pytest environment"
+
+    # Required linux kernel version for this suite to run.
+    result = required_linux_kernel_version("5.0")
+    if result is not True:
+        pytest.skip("Kernel requirements are not met")
+
     tgen = Topogen(BGPIPV6RTADVVRFTopo, mod.__name__)
     tgen.start_topology()
 
@@ -84,9 +90,6 @@ def setup_module(mod):
         for cmd in cmds:
             output = tgen.net[rname].cmd(cmd.format(rname))
 
-        # adjust handling of vrf traffic
-        adjust_router_l3mdev(tgen, rname)
-
     for rname, router in router_list.items():
         router.load_config(
             TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname))

diff --git a/tests/topotests/ospf6_topo1_vrf/test_ospf6_topo1_vrf.py b/tests/topotests/ospf6_topo1_vrf/test_ospf6_topo1_vrf.py
index fa2784ee7edc89682f8710f642674a32eee74119..e1857abc4f509da49371c7e9d102bc9dd8ab44be 100755 (executable)
--- a/tests/topotests/ospf6_topo1_vrf/test_ospf6_topo1_vrf.py
+++ b/tests/topotests/ospf6_topo1_vrf/test_ospf6_topo1_vrf.py
@@ -92,10 +92,7 @@ from lib import topotest
 from lib.topogen import Topogen, TopoRouter, get_topogen
 from lib.topolog import logger
 from lib.topotest import iproute2_is_vrf_capable
-from lib.common_config import (
-    required_linux_kernel_version,
-    adjust_router_l3mdev,
-)
+from lib.common_config import required_linux_kernel_version
 
 #####################################################
 ##
@@ -159,6 +156,11 @@ class NetworkTopo(Topo):
 def setup_module(mod):
     "Sets up the pytest environment"
 
+    # Required linux kernel version for this suite to run.
+    result = required_linux_kernel_version("5.0")
+    if result is not True:
+        pytest.skip("Kernel requirements are not met")
+
     tgen = Topogen(NetworkTopo, mod.__name__)
     tgen.start_topology()
 
@@ -197,9 +199,6 @@ def setup_module(mod):
             for cmd in cmds2:
                 output = tgen.net[rname].cmd(cmd.format(rname))
 
-        # adjust handling of vrf traffic
-        adjust_router_l3mdev(tgen, rname)
-
     for rname, router in tgen.routers().items():
         router.load_config(
             TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname))