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>

lib/vrf.c

@@ -994,24 +994,49 @@ 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 || name == NULL)
+	if (fd < 0)
-		return fd;
+		return -1;
-	/* the device should exist
+
-	 * otherwise we should return
+	if (vrf_id == VRF_UNKNOWN)
-	 * case ifname = vrf in netns mode => return
+		return -1;
-	 */
+
-	ifp = if_lookup_by_name(name, vrf_id);
+	/* 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 fd;
+			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;
+	}
+
 #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,
+		zlog_err("bind to interface %s failed, errno=%d", ifname,
 			 errno);
 #endif /* SO_BINDTODEVICE */
 	return ret;

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
+ * If ifname is NULL or is equal to the VRF name then bind to a VRF device.
- * arguments.
+ * Otherwise, bind to the specified interface in the specified VRF.
  *
- * name should be the name of the VRF device. vrf_id should be the
+ * Returns 0 on success and -1 on failure.
- * corresponding vrf_id (the ifindex of the device).
  */
-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,

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))

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 (
+from lib.common_config import required_linux_kernel_version
-    required_linux_kernel_version,
-    adjust_router_l3mdev,
-)
 
 #####################################################
 ##
@@ -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))