From 4877f2f6856366734a9419aaf9616785242c935a Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 26 Dec 2024 23:08:21 +0200 Subject: [PATCH 1/4] lib: remove VRF_BACKEND_UNKNOWN The backend type cannot be unknown. It is configured to VRF_LITE by default in zebra anyway, so just init to VRF_LITE in the lib and remove the UNKNOWN type. Signed-off-by: Igor Ryzhov --- lib/if.c | 2 -- lib/vrf.c | 7 +------ lib/vrf.h | 1 - zebra/main.c | 2 -- 4 files changed, 1 insertion(+), 11 deletions(-) diff --git a/lib/if.c b/lib/if.c index 796929ef0d..68724a65e9 100644 --- a/lib/if.c +++ b/lib/if.c @@ -416,7 +416,6 @@ static struct interface *if_lookup_by_ifindex(ifindex_t ifindex, struct interface *if_lookup_by_index(ifindex_t ifindex, vrf_id_t vrf_id) { switch (vrf_get_backend()) { - case VRF_BACKEND_UNKNOWN: case VRF_BACKEND_NETNS: return(if_lookup_by_ifindex(ifindex, vrf_id)); case VRF_BACKEND_VRF_LITE: @@ -686,7 +685,6 @@ struct interface *if_get_by_name(const char *name, vrf_id_t vrf_id, struct vrf *vrf; switch (vrf_get_backend()) { - case VRF_BACKEND_UNKNOWN: case VRF_BACKEND_NETNS: vrf = vrf_get(vrf_id, vrf_name); assert(vrf); diff --git a/lib/vrf.c b/lib/vrf.c index 9be8a9faae..e576111163 100644 --- a/lib/vrf.c +++ b/lib/vrf.c @@ -42,8 +42,7 @@ RB_GENERATE(vrf_name_head, vrf, name_entry, vrf_name_compare); struct vrf_id_head vrfs_by_id = RB_INITIALIZER(&vrfs_by_id); struct vrf_name_head vrfs_by_name = RB_INITIALIZER(&vrfs_by_name); -static int vrf_backend; -static int vrf_backend_configured; +static int vrf_backend = VRF_BACKEND_VRF_LITE; static char vrf_default_name[VRF_NAMSIZ] = VRF_DEFAULT_NAME_INTERNAL; /* @@ -654,8 +653,6 @@ int vrf_is_backend_netns(void) int vrf_get_backend(void) { - if (!vrf_backend_configured) - return VRF_BACKEND_UNKNOWN; return vrf_backend; } @@ -663,7 +660,6 @@ int vrf_configure_backend(enum vrf_backend_type backend) { /* Work around issue in old gcc */ switch (backend) { - case VRF_BACKEND_UNKNOWN: case VRF_BACKEND_NETNS: case VRF_BACKEND_VRF_LITE: break; @@ -672,7 +668,6 @@ int vrf_configure_backend(enum vrf_backend_type backend) } vrf_backend = backend; - vrf_backend_configured = 1; return 0; } diff --git a/lib/vrf.h b/lib/vrf.h index ad302de9ba..46d72910c2 100644 --- a/lib/vrf.h +++ b/lib/vrf.h @@ -94,7 +94,6 @@ DECLARE_QOBJ_TYPE(vrf); enum vrf_backend_type { VRF_BACKEND_VRF_LITE, VRF_BACKEND_NETNS, - VRF_BACKEND_UNKNOWN, VRF_BACKEND_MAX, }; diff --git a/zebra/main.c b/zebra/main.c index d189d1e0a0..f84bfa6eb0 100644 --- a/zebra/main.c +++ b/zebra/main.c @@ -360,8 +360,6 @@ int main(int argc, char **argv) if_notify_oper_changes = true; vrf_notify_oper_changes = true; - vrf_configure_backend(VRF_BACKEND_VRF_LITE); - frr_preinit(&zebra_di, argc, argv); frr_opt_add("baz:e:rK:s:R:" From 6f214d97d1a7883ea8d5866e10db31daffa0325a Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Fri, 27 Dec 2024 15:10:27 +0200 Subject: [PATCH 2/4] lib, zebra: move ns context intialization to zebra vrf->ns_ctxt is only ever used in zebra, so move its initialization to zebra's callback. Ideally this pointer shouldn't even be a part of library's vrf struct, and moved to zebra-specific struct, but this is the first step. Signed-off-by: Igor Ryzhov --- lib/vrf.c | 9 --------- zebra/zebra_vrf.c | 8 ++++++++ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/vrf.c b/lib/vrf.c index e576111163..0b39d93602 100644 --- a/lib/vrf.c +++ b/lib/vrf.c @@ -581,15 +581,6 @@ void vrf_init(int (*create)(struct vrf *), int (*enable)(struct vrf *), "vrf_init: failed to create the default VRF!"); exit(1); } - if (vrf_is_backend_netns()) { - struct ns *ns; - - strlcpy(default_vrf->data.l.netns_name, - VRF_DEFAULT_NAME, NS_NAMSIZ); - ns = ns_lookup(NS_DEFAULT); - ns->vrf_ctxt = default_vrf; - default_vrf->ns_ctxt = ns; - } /* Enable the default VRF. */ if (!vrf_enable(default_vrf)) { diff --git a/zebra/zebra_vrf.c b/zebra/zebra_vrf.c index c7781e86d8..7bfe07b4cf 100644 --- a/zebra/zebra_vrf.c +++ b/zebra/zebra_vrf.c @@ -98,6 +98,14 @@ static int zebra_vrf_new(struct vrf *vrf) zvrf = zebra_vrf_alloc(vrf); if (!vrf_is_backend_netns()) zvrf->zns = zebra_ns_lookup(NS_DEFAULT); + else if (vrf->vrf_id == VRF_DEFAULT) { + struct ns *ns; + + strlcpy(vrf->data.l.netns_name, VRF_DEFAULT_NAME, NS_NAMSIZ); + ns = ns_lookup(NS_DEFAULT); + ns->vrf_ctxt = vrf; + vrf->ns_ctxt = ns; + } otable_init(&zvrf->other_tables); From 300f8dbda4bc7efdab90809fbff5f9799c0ec2aa Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Fri, 27 Dec 2024 21:33:39 +0200 Subject: [PATCH 3/4] lib: introduce global -w option for VRF netns backend Current -n option is only for zebra and mgmtd. All other daemons receive the VRF backend configuration from zebra upon connection to it. This leads to a potential race condition - daemons need to know the backend before they start reading their config, but they can be not connected to zebra yet at this point. As the VRF backend cannot change during runtime, let's introduce a new global -w option for setting netns backend, to make sure that all daemons know their VRF backend immediately after start. The reason for introducing a new option instead of making -n global is that ospfd already uses -n for another purposes. Signed-off-by: Igor Ryzhov --- doc/manpages/frr-zebra.rst | 2 ++ doc/user/basic.rst | 11 +++++++++++ doc/user/zebra.rst | 2 ++ lib/libfrr.c | 14 ++++++++++++++ mgmtd/mgmt_main.c | 9 +++++---- zebra/main.c | 4 +++- 6 files changed, 37 insertions(+), 5 deletions(-) diff --git a/doc/manpages/frr-zebra.rst b/doc/manpages/frr-zebra.rst index 6cc46b806d..356c128e30 100644 --- a/doc/manpages/frr-zebra.rst +++ b/doc/manpages/frr-zebra.rst @@ -38,6 +38,8 @@ OPTIONS available for the |DAEMON| command: Enable namespace VRF backend. By default, the VRF backend relies on VRF-lite support from the Linux kernel. This option permits discovering Linux named network namespaces and mapping it to FRR VRF contexts. + This option is deprecated. Please use the global -w option instead. + ROUTES ------ diff --git a/doc/user/basic.rst b/doc/user/basic.rst index 5fdd1887fa..b2d47a38eb 100644 --- a/doc/user/basic.rst +++ b/doc/user/basic.rst @@ -754,6 +754,17 @@ These options apply to all |PACKAGE_NAME| daemons. be added to all files that use the statedir. If you have "/var/run/frr" as the default statedir then it will become "/var/run/frr/". +.. option:: -w, --vrfwnetns + + Enable namespace VRF backend. By default, the VRF backend relies on VRF-lite + support from the Linux kernel. This option permits discovering Linux named + network namespaces and mapping them to FRR VRF contexts. This option must be + the same for all running daemons. The easiest way to pass the same option to + all daemons is to use the ``frr_global_options`` variable in the + :ref:`Daemons Configuration File `. + + .. seealso:: :ref:`zebra-vrf` + .. option:: -o, --vrfdefaultname Set the name used for the *Default VRF* in CLI commands and YANG models. diff --git a/doc/user/zebra.rst b/doc/user/zebra.rst index ac29b1c7d4..ef3a619853 100644 --- a/doc/user/zebra.rst +++ b/doc/user/zebra.rst @@ -53,6 +53,8 @@ Besides the common invocation options (:ref:`common-invocation-options`), the VRF defined by *Zebra*, as usual. If this option is specified when running *Zebra*, one must also specify the same option for *mgmtd*. + This options is deprecated. Please use the global -w option instead. + .. seealso:: :ref:`zebra-vrf` .. option:: -z , --socket diff --git a/lib/libfrr.c b/lib/libfrr.c index d1a9f0b1cb..261d3aa87e 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -108,6 +108,9 @@ static const struct option lo_always[] = { { "module", no_argument, NULL, 'M' }, { "profile", required_argument, NULL, 'F' }, { "pathspace", required_argument, NULL, 'N' }, +#ifdef HAVE_NETLINK + { "vrfwnetns", no_argument, NULL, 'w' }, +#endif { "vrfdefaultname", required_argument, NULL, 'o' }, { "graceful_restart", optional_argument, NULL, 'K' }, { "vty_socket", required_argument, NULL, OPTION_VTYSOCK }, @@ -120,6 +123,9 @@ static const struct option lo_always[] = { { NULL } }; static const struct optspec os_always = { +#ifdef HAVE_NETLINK + "w" +#endif "hvdM:F:N:o:K::", " -h, --help Display this help and exit\n" " -v, --version Print program version\n" @@ -127,6 +133,9 @@ static const struct optspec os_always = { " -M, --module Load specified module\n" " -F, --profile Use specified configuration profile\n" " -N, --pathspace Insert prefix into config & socket paths\n" +#ifdef HAVE_NETLINK + " -w, --vrfwnetns Use network namespaces for VRFs\n" +#endif " -o, --vrfdefaultname Set default VRF name.\n" " -K, --graceful_restart FRR starting in Graceful Restart mode, with optional route-cleanup timer\n" " --vty_socket Override vty socket path\n" @@ -516,6 +525,11 @@ static int frr_opt(int opt) snprintf(frr_zclientpath, sizeof(frr_zclientpath), ZAPI_SOCK_NAME); break; +#ifdef HAVE_NETLINK + case 'w': + vrf_configure_backend(VRF_BACKEND_NETNS); + break; +#endif case 'o': vrf_set_default_name(optarg); break; diff --git a/mgmtd/mgmt_main.c b/mgmtd/mgmt_main.c index 7d909446c3..e3fc6b7f29 100644 --- a/mgmtd/mgmt_main.c +++ b/mgmtd/mgmt_main.c @@ -238,10 +238,9 @@ int main(int argc, char **argv) int buffer_size = MGMTD_SOCKET_BUF_SIZE; frr_preinit(&mgmtd_di, argc, argv); - frr_opt_add( - "s:n" DEPRECATED_OPTIONS, longopts, - " -s, --socket_size Set MGMTD peer socket send buffer size\n" - " -n, --vrfwnetns Use NetNS as VRF backend\n"); + frr_opt_add("s:n" DEPRECATED_OPTIONS, longopts, + " -s, --socket_size Set MGMTD peer socket send buffer size\n" + " -n, --vrfwnetns Use NetNS as VRF backend (deprecated, use -w)\n"); /* Command line argument treatment. */ while (1) { @@ -264,6 +263,8 @@ int main(int argc, char **argv) buffer_size = atoi(optarg); break; case 'n': + fprintf(stderr, + "The -n option is deprecated, please use global -w option instead.\n"); vrf_configure_backend(VRF_BACKEND_NETNS); break; default: diff --git a/zebra/main.c b/zebra/main.c index f84bfa6eb0..fd242e762a 100644 --- a/zebra/main.c +++ b/zebra/main.c @@ -377,7 +377,7 @@ int main(int argc, char **argv) " --v6-with-v4-nexthops Underlying dataplane supports v6 routes with v4 nexthops\n" #ifdef HAVE_NETLINK " -s, --nl-bufsize Set netlink receive buffer size\n" - " -n, --vrfwnetns Use NetNS as VRF backend\n" + " -n, --vrfwnetns Use NetNS as VRF backend (deprecated, use -w)\n" " --v6-rr-semantics Use v6 RR semantics\n" #else " -s, Set kernel socket receive buffer size\n" @@ -438,6 +438,8 @@ int main(int argc, char **argv) break; #ifdef HAVE_NETLINK case 'n': + fprintf(stderr, + "The -n option is deprecated, please use global -w option instead.\n"); vrf_configure_backend(VRF_BACKEND_NETNS); break; case OPTION_V6_RR_SEMANTICS: From 754b949889c2fe4eb2b96f591925d274ccfd1477 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Wed, 1 Jan 2025 23:22:52 +0200 Subject: [PATCH 4/4] tests: use global -w option instead of per-daemon -n Add ability to enable -w option for all daemons in a topotest and use this option instead of the deprecated -n. Signed-off-by: Igor Ryzhov --- tests/topotests/bfd_vrf_topo1/test_bfd_vrf_topo1.py | 6 ++---- tests/topotests/bgp_evpn_rt5/test_bgp_evpn.py | 6 ++---- tests/topotests/bgp_vrf_netns/test_bgp_vrf_netns_topo.py | 6 ++---- tests/topotests/lib/topogen.py | 6 ++++++ tests/topotests/lib/topotest.py | 6 ++++++ tests/topotests/ospf_netns_vrf/test_ospf_netns_vrf.py | 6 ++---- 6 files changed, 20 insertions(+), 16 deletions(-) diff --git a/tests/topotests/bfd_vrf_topo1/test_bfd_vrf_topo1.py b/tests/topotests/bfd_vrf_topo1/test_bfd_vrf_topo1.py index f6adff61d0..f00af34e39 100644 --- a/tests/topotests/bfd_vrf_topo1/test_bfd_vrf_topo1.py +++ b/tests/topotests/bfd_vrf_topo1/test_bfd_vrf_topo1.py @@ -84,11 +84,9 @@ def setup_module(mod): router.net.set_intf_netns(rname + "-eth2", ns, up=True) for rname, router in router_list.items(): - router.load_config(TopoRouter.RD_MGMTD, None, "--vrfwnetns") + router.use_netns_vrf() router.load_config( - TopoRouter.RD_ZEBRA, - os.path.join(CWD, "{}/zebra.conf".format(rname)), - "--vrfwnetns", + TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname)) ) router.load_config( TopoRouter.RD_BFD, os.path.join(CWD, "{}/bfdd.conf".format(rname)) diff --git a/tests/topotests/bgp_evpn_rt5/test_bgp_evpn.py b/tests/topotests/bgp_evpn_rt5/test_bgp_evpn.py index a9636a92f4..c874cbed66 100644 --- a/tests/topotests/bgp_evpn_rt5/test_bgp_evpn.py +++ b/tests/topotests/bgp_evpn_rt5/test_bgp_evpn.py @@ -136,11 +136,9 @@ def setup_module(mod): for rname, router in router_list.items(): if rname == "r1": - router.load_config(TopoRouter.RD_MGMTD, None, "--vrfwnetns") + router.use_netns_vrf() router.load_config( - TopoRouter.RD_ZEBRA, - os.path.join(CWD, "{}/zebra.conf".format(rname)), - "--vrfwnetns", + TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname)) ) else: router.load_config( diff --git a/tests/topotests/bgp_vrf_netns/test_bgp_vrf_netns_topo.py b/tests/topotests/bgp_vrf_netns/test_bgp_vrf_netns_topo.py index 028bc35358..4aa4404134 100644 --- a/tests/topotests/bgp_vrf_netns/test_bgp_vrf_netns_topo.py +++ b/tests/topotests/bgp_vrf_netns/test_bgp_vrf_netns_topo.py @@ -94,11 +94,9 @@ def setup_module(module): router.net.set_intf_netns("r1-eth0", ns, up=True) # run daemons - router.load_config(TopoRouter.RD_MGMTD, None, "--vrfwnetns") + router.use_netns_vrf() router.load_config( - TopoRouter.RD_ZEBRA, - os.path.join(CWD, "{}/zebra.conf".format("r1")), - "--vrfwnetns", + TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format("r1")) ) router.load_config( TopoRouter.RD_BGP, os.path.join(CWD, "{}/bgpd.conf".format("r1")) diff --git a/tests/topotests/lib/topogen.py b/tests/topotests/lib/topogen.py index 0a9a84a4bb..8b1bd6e1a5 100644 --- a/tests/topotests/lib/topogen.py +++ b/tests/topotests/lib/topogen.py @@ -819,6 +819,12 @@ class TopoRouter(TopoGear): gear += " TopoRouter<>" return gear + def use_netns_vrf(self): + """ + Use netns as VRF backend. + """ + self.net.useNetnsVRF() + def check_capability(self, daemon, param): """ Checks a capability daemon against an argument option diff --git a/tests/topotests/lib/topotest.py b/tests/topotests/lib/topotest.py index ca6723aecb..e2c70cdccd 100644 --- a/tests/topotests/lib/topotest.py +++ b/tests/topotests/lib/topotest.py @@ -1467,6 +1467,7 @@ class Router(Node): self.daemons_options = {"zebra": ""} self.reportCores = True self.version = None + self.use_netns_vrf = False self.ns_cmd = "sudo nsenter -a -t {} ".format(self.pid) try: @@ -1622,6 +1623,9 @@ class Router(Node): # breakpoint() # assert False, "can't remove IPs %s" % str(ex) + def useNetnsVRF(self): + self.use_netns_vrf = True + def checkCapability(self, daemon, param): if param is not None: daemon_path = os.path.join(self.daemondir, daemon) @@ -1908,6 +1912,8 @@ class Router(Node): def start_daemon(daemon, instance=None): daemon_opts = self.daemons_options.get(daemon, "") + if self.use_netns_vrf: + daemon_opts += " -w" # get pid and vty filenames and remove the files m = re.match(r"(.* |^)-n (\d+)( ?.*|$)", daemon_opts) diff --git a/tests/topotests/ospf_netns_vrf/test_ospf_netns_vrf.py b/tests/topotests/ospf_netns_vrf/test_ospf_netns_vrf.py index 718445f01f..c0a4689d46 100644 --- a/tests/topotests/ospf_netns_vrf/test_ospf_netns_vrf.py +++ b/tests/topotests/ospf_netns_vrf/test_ospf_netns_vrf.py @@ -87,11 +87,9 @@ def setup_module(mod): router.net.set_intf_netns(rname + "-eth0", ns, up=True) router.net.set_intf_netns(rname + "-eth1", ns, up=True) - router.load_config(TopoRouter.RD_MGMTD, None, "--vrfwnetns") + router.use_netns_vrf() router.load_config( - TopoRouter.RD_ZEBRA, - os.path.join(CWD, "{}/zebra.conf".format(rname)), - "--vrfwnetns", + TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname)) ) router.load_config( TopoRouter.RD_OSPF, os.path.join(CWD, "{}/ospfd.conf".format(rname))