From: Igor Ryzhov Date: Thu, 6 May 2021 23:49:40 +0000 (+0300) Subject: lib: fix binding to a vrf X-Git-Tag: base_8.1~468^2~2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=36eef8586d80173a9d64e4b802584014b3780558;p=matthieu%2Ffrr.git 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 Signed-off-by: Igor Ryzhov --- diff --git a/lib/vrf.c b/lib/vrf.c index d99ec12ba8..185a11664a 100644 --- a/lib/vrf.c +++ b/lib/vrf.c @@ -994,25 +994,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 c79dd99b9a..7ce03079dd 100644 --- 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 92ee8513e1..a17819f747 100644 --- 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 fa2784ee7e..e1857abc4f 100755 --- 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))