Skip to content

Commit

Permalink
*: rework renaming the default VRF
Browse files Browse the repository at this point in the history
Currently, it is possible to rename the default VRF either by passing
`-o` option to zebra or by creating a file in `/var/run/netns` and
binding it to `/proc/self/ns/net`.

In both cases, only zebra knows about the rename and other daemons learn
about it only after they connect to zebra. This is a problem, because
daemons may read their config before they connect to zebra. To handle
this rename after the config is read, we have some special code in every
single daemon, which is not very bad but not desirable in my opinion.
But things are getting worse when we need to handle this in northbound
layer as we have to manually rewrite the config nodes. This approach is
already hacky, but still works as every daemon handles its own NB
structures. But it is completely incompatible with the central
management daemon architecture we are aiming for, as mgmtd doesn't even
have a connection with zebra to learn from it. And it shouldn't have it,
because operational state changes should never affect configuration.

To solve the problem and simplify the code, I propose to expand the `-o`
option to all daemons. By using the startup option, we let daemons know
about the rename before they read their configs so we don't need any
special code to deal with it. There's an easy way to pass the option to
all daemons by using `frr_global_options` variable.

Unfortunately, the second way of renaming by creating a file in
`/var/run/netns` is incompatible with the new mgmtd architecture.
Theoretically, we could force daemons to read their configs only after
they connect to zebra, but it means adding even more code to handle a
very specific use-case. And anyway this won't work for mgmtd as it
doesn't have a connection with zebra. So I had to remove this option.

Signed-off-by: Igor Ryzhov <[email protected]>
  • Loading branch information
idryzhov committed Dec 21, 2021
1 parent a64829d commit ac2cb9b
Show file tree
Hide file tree
Showing 37 changed files with 83 additions and 380 deletions.
73 changes: 1 addition & 72 deletions bfdd/bfd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1946,19 +1946,6 @@ static int bfd_vrf_delete(struct vrf *vrf)
return 0;
}

static int bfd_vrf_update(struct vrf *vrf)
{
if (!vrf_is_enabled(vrf))
return 0;

if (bglobal.debug_zebra)
zlog_debug("VRF update: %s(%u)", vrf->name, vrf->vrf_id);

/* a different name is given; update bfd list */
bfdd_sessions_enable_vrf(vrf);
return 0;
}

static int bfd_vrf_enable(struct vrf *vrf)
{
struct bfd_vrf_global *bvrf;
Expand Down Expand Up @@ -2070,8 +2057,7 @@ static int bfd_vrf_disable(struct vrf *vrf)

void bfd_vrf_init(void)
{
vrf_init(bfd_vrf_new, bfd_vrf_enable, bfd_vrf_disable,
bfd_vrf_delete, bfd_vrf_update);
vrf_init(bfd_vrf_new, bfd_vrf_enable, bfd_vrf_disable, bfd_vrf_delete);
}

void bfd_vrf_terminate(void)
Expand All @@ -2096,63 +2082,6 @@ struct bfd_vrf_global *bfd_vrf_look_by_session(struct bfd_session *bfd)
return bfd->vrf->info;
}

void bfd_session_update_vrf_name(struct bfd_session *bs, struct vrf *vrf)
{
if (!vrf || !bs)
return;
/* update key */
hash_release(bfd_key_hash, bs);
/*
* HACK: Change the BFD VRF in the running configuration directly,
* bypassing the northbound layer. This is necessary to avoid deleting
* the BFD and readding it in the new VRF, which would have
* several implications.
*/
if (yang_module_find("frr-bfdd") && bs->key.vrfname[0]) {
struct lyd_node *bfd_dnode;
char xpath[XPATH_MAXLEN], xpath_srcaddr[XPATH_MAXLEN + 32];
char oldpath[XPATH_MAXLEN], newpath[XPATH_MAXLEN];
char addr_buf[INET6_ADDRSTRLEN];
int slen;

/* build xpath */
if (bs->key.mhop) {
inet_ntop(bs->key.family, &bs->key.local, addr_buf, sizeof(addr_buf));
snprintf(xpath_srcaddr, sizeof(xpath_srcaddr), "[source-addr='%s']",
addr_buf);
} else
xpath_srcaddr[0] = 0;
inet_ntop(bs->key.family, &bs->key.peer, addr_buf, sizeof(addr_buf));
slen = snprintf(xpath, sizeof(xpath),
"/frr-bfdd:bfdd/bfd/sessions/%s%s[dest-addr='%s']",
bs->key.mhop ? "multi-hop" : "single-hop", xpath_srcaddr,
addr_buf);
if (bs->key.ifname[0])
slen += snprintf(xpath + slen, sizeof(xpath) - slen,
"[interface='%s']", bs->key.ifname);
else
slen += snprintf(xpath + slen, sizeof(xpath) - slen,
"[interface='*']");
snprintf(xpath + slen, sizeof(xpath) - slen, "[vrf='%s']/vrf",
bs->key.vrfname);

bfd_dnode = yang_dnode_getf(running_config->dnode, xpath,
bs->key.vrfname);
if (bfd_dnode) {
yang_dnode_get_path(lyd_parent(bfd_dnode), oldpath,
sizeof(oldpath));
yang_dnode_change_leaf(bfd_dnode, vrf->name);
yang_dnode_get_path(lyd_parent(bfd_dnode), newpath,
sizeof(newpath));
nb_running_move_tree(oldpath, newpath);
running_config->version++;
}
}
memset(bs->key.vrfname, 0, sizeof(bs->key.vrfname));
strlcpy(bs->key.vrfname, vrf->name, sizeof(bs->key.vrfname));
hash_get(bfd_key_hash, bs, hash_alloc_intern);
}

unsigned long bfd_get_session_count(void)
{
return bfd_key_hash->count;
Expand Down
1 change: 0 additions & 1 deletion bfdd/bfd.h
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,6 @@ void bfdd_zclient_unregister(vrf_id_t vrf_id);
void bfdd_zclient_register(vrf_id_t vrf_id);
void bfdd_sessions_enable_vrf(struct vrf *vrf);
void bfdd_sessions_disable_vrf(struct vrf *vrf);
void bfd_session_update_vrf_name(struct bfd_session *bs, struct vrf *vrf);

int ptm_bfd_notify(struct bfd_session *bs, uint8_t notify_state);

Expand Down
5 changes: 0 additions & 5 deletions bfdd/ptm_adapter.c
Original file line number Diff line number Diff line change
Expand Up @@ -721,11 +721,6 @@ void bfdd_sessions_enable_vrf(struct vrf *vrf)
/* it may affect configs without interfaces */
TAILQ_FOREACH(bso, &bglobal.bg_obslist, bso_entry) {
bs = bso->bso_bs;
/* update name */
if (bs->vrf && bs->vrf == vrf) {
if (!strmatch(bs->key.vrfname, vrf->name))
bfd_session_update_vrf_name(bs, vrf);
}
if (bs->vrf)
continue;
if (bs->key.vrfname[0] &&
Expand Down
16 changes: 1 addition & 15 deletions bgpd/bgp_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -294,19 +294,6 @@ static int bgp_vrf_enable(struct vrf *vrf)

bgp = bgp_lookup_by_name(vrf->name);
if (bgp && bgp->vrf_id != vrf->vrf_id) {
if (bgp->name && strmatch(vrf->name, VRF_DEFAULT_NAME)) {
XFREE(MTYPE_BGP, bgp->name);
XFREE(MTYPE_BGP, bgp->name_pretty);
bgp->name_pretty = XSTRDUP(MTYPE_BGP, "VRF default");
bgp->inst_type = BGP_INSTANCE_TYPE_DEFAULT;
#ifdef ENABLE_BGP_VNC
if (!bgp->rfapi) {
bgp->rfapi = bgp_rfapi_new(bgp);
assert(bgp->rfapi);
assert(bgp->rfapi_cfg);
}
#endif /* ENABLE_BGP_VNC */
}
old_vrf_id = bgp->vrf_id;
/* We have instance configured, link to VRF and make it "up". */
bgp_vrf_link(bgp, vrf);
Expand Down Expand Up @@ -367,8 +354,7 @@ static int bgp_vrf_disable(struct vrf *vrf)

static void bgp_vrf_init(void)
{
vrf_init(bgp_vrf_new, bgp_vrf_enable, bgp_vrf_disable,
bgp_vrf_delete, bgp_vrf_enable);
vrf_init(bgp_vrf_new, bgp_vrf_enable, bgp_vrf_disable, bgp_vrf_delete);
}

static void bgp_vrf_terminate(void)
Expand Down
8 changes: 8 additions & 0 deletions doc/user/basic.rst
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,14 @@ 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/<namespace>".

.. option:: -o, --vrfdefaultname <name>

Set the name used for the *Default VRF* in CLI commands and YANG models.
This option must be the same for all running daemons. By default, the name
is "default".

.. seealso:: :ref:`zebra-vrf`

.. option:: -v, --version

Print program version.
Expand Down
2 changes: 2 additions & 0 deletions doc/user/setup.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ of these buffers, pipe their contents through ``tr '\0' '\n'``. A blank line
marks the end of valid unwritten data (it will generally be followed by
garbled, older log messages since the buffer is not cleared.)

.. _daemons-configuration-file:

Daemons Configuration File
--------------------------
After a fresh install, starting FRR will do nothing. This is because daemons
Expand Down
48 changes: 7 additions & 41 deletions doc/user/zebra.rst
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,6 @@ Besides the common invocation options (:ref:`common-invocation-options`), the

.. seealso:: :ref:`zebra-vrf`

.. option:: -o, --vrfdefaultname

When *Zebra* starts with this option, the default VRF name is changed to the
parameter.

.. seealso:: :ref:`zebra-vrf`

.. option:: -z <path_to_socket>, --socket <path_to_socket>

If this option is supplied on the cli, the path to the zebra
Expand Down Expand Up @@ -363,7 +356,13 @@ separate for each set of VRF, and routing daemons can have their own context
for each VRF.

This conceptual view introduces the *Default VRF* case. If the user does not
configure any specific VRF, then by default, FRR uses the *Default VRF*.
configure any specific VRF, then by default, FRR uses the *Default VRF*. The
name "default" is used to refer to this VRF in various CLI commands and YANG
models. It is possible to change that name by passing the ``-o`` option to all
daemons, for example, one can use ``-o vrf0`` to change the name to "vrf0".
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 <daemons-configuration-file>`.

Configuring VRF networking contexts can be done in various ways on FRR. The VRF
interfaces can be configured by entering in interface configuration mode
Expand Down Expand Up @@ -440,39 +439,6 @@ commands in relationship to VRF. Here is an extract of some of those commands:
the default vrf and default table. If prefix is specified dump the
number of prefix routes.

By using the :option:`-n` option, the *Linux network namespace* will be mapped
over the *Zebra* VRF. One nice feature that is possible by handling *Linux
network namespace* is the ability to name default VRF. At startup, *Zebra*
discovers the available *Linux network namespace* by parsing folder
``/var/run/netns``. Each file stands for a *Linux network namespace*, but not all
*Linux network namespaces* are available under that folder. This is the case for
default VRF. It is possible to name the default VRF, by creating a file, by
executing following commands.

.. code-block:: shell
touch /var/run/netns/vrf0
mount --bind /proc/self/ns/net /var/run/netns/vrf0
Above command illustrates what happens when the default VRF is visible under
``/var/run/netns``. Here, the default VRF file is ``vrf0``.
At startup, FRR detects the presence of that file. It detects that the file
statistics information matches the same file statistics information as
``/proc/self/ns/net`` ( through stat() function). As statistics information
matches, then ``vrf0`` stands for the new default namespace name.
Consequently, the VRF naming ``Default`` will be overridden by the new discovered
namespace name ``vrf0``.

For those who don't use VRF backend with *Linux network namespace*, it is
possible to statically configure and recompile FRR. It is possible to choose an
alternate name for default VRF. Then, the default VRF naming will automatically
be updated with the new name. To illustrate, if you want to recompile with
``global`` value, use the following command:

.. code-block:: shell
./configure --with-defaultvrfname=global
.. _zebra-table-allocation:

Table Allocation
Expand Down
2 changes: 1 addition & 1 deletion eigrpd/eigrp_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ int main(int argc, char **argv, char **envp)

eigrp_error_init();
eigrp_vrf_init();
vrf_init(NULL, NULL, NULL, NULL, NULL);
vrf_init(NULL, NULL, NULL, NULL);

/*EIGRPd init*/
eigrp_if_init();
Expand Down
4 changes: 2 additions & 2 deletions eigrpd/eigrp_vrf.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,6 @@ static int eigrp_vrf_delete(struct vrf *vrf)

void eigrp_vrf_init(void)
{
vrf_init(eigrp_vrf_new, eigrp_vrf_enable,
eigrp_vrf_disable, eigrp_vrf_delete, NULL);
vrf_init(eigrp_vrf_new, eigrp_vrf_enable, eigrp_vrf_disable,
eigrp_vrf_delete);
}
47 changes: 1 addition & 46 deletions isisd/isisd.c
Original file line number Diff line number Diff line change
Expand Up @@ -646,51 +646,6 @@ static int isis_vrf_enable(struct vrf *vrf)
vrf->vrf_id);

isis = isis_lookup_by_vrfname(vrf->name);
if (!isis) {
char *old_vrf_name = NULL;

isis = (struct isis *)vrf->info;
if (!isis)
return 0;
/* update vrf name */
if (isis->name)
old_vrf_name = isis->name;
isis->name = XSTRDUP(MTYPE_ISIS_NAME, vrf->name);
/*
* HACK: Change the ISIS VRF in the running configuration
* directly, bypassing the northbound layer. This is necessary
* to avoid deleting the ISIS and readding it in the new VRF,
* which would have several implications.
*/
if (yang_module_find("frr-isisd") && old_vrf_name) {
struct lyd_node *isis_dnode;
struct isis_area *area;
char oldpath[XPATH_MAXLEN];
char newpath[XPATH_MAXLEN];
struct listnode *node, *nnode;

for (ALL_LIST_ELEMENTS(isis->area_list, node, nnode,
area)) {
isis_dnode = yang_dnode_getf(
running_config->dnode,
"/frr-isisd:isis/instance[area-tag='%s'][vrf='%s']/vrf",
area->area_tag, old_vrf_name);
if (isis_dnode) {
yang_dnode_get_path(
lyd_parent(isis_dnode), oldpath,
sizeof(oldpath));
yang_dnode_change_leaf(isis_dnode,
vrf->name);
yang_dnode_get_path(
lyd_parent(isis_dnode), newpath,
sizeof(newpath));
nb_running_move_tree(oldpath, newpath);
running_config->version++;
}
}
}
XFREE(MTYPE_ISIS_NAME, old_vrf_name);
}
if (isis && isis->vrf_id != vrf->vrf_id) {
old_vrf_id = isis->vrf_id;
/* We have instance configured, link to VRF and make it "up". */
Expand Down Expand Up @@ -742,7 +697,7 @@ static int isis_vrf_disable(struct vrf *vrf)
void isis_vrf_init(void)
{
vrf_init(isis_vrf_new, isis_vrf_enable, isis_vrf_disable,
isis_vrf_delete, isis_vrf_enable);
isis_vrf_delete);

vrf_cmd_init(NULL);
}
Expand Down
2 changes: 1 addition & 1 deletion ldpd/ldpd.c
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ main(int argc, char *argv[])

master = frr_init();

vrf_init(NULL, NULL, NULL, NULL, NULL);
vrf_init(NULL, NULL, NULL, NULL);
access_list_init();
ldp_vty_init();
ldp_zebra_init(master);
Expand Down
7 changes: 6 additions & 1 deletion lib/libfrr.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ static const struct option lo_always[] = {
{"module", no_argument, NULL, 'M'},
{"profile", required_argument, NULL, 'F'},
{"pathspace", required_argument, NULL, 'N'},
{"vrfdefaultname", required_argument, NULL, 'o'},
{"vty_socket", required_argument, NULL, OPTION_VTYSOCK},
{"moduledir", required_argument, NULL, OPTION_MODULEDIR},
{"scriptdir", required_argument, NULL, OPTION_SCRIPTDIR},
Expand All @@ -126,13 +127,14 @@ static const struct option lo_always[] = {
{"limit-fds", required_argument, NULL, OPTION_LIMIT_FDS},
{NULL}};
static const struct optspec os_always = {
"hvdM:F:N:",
"hvdM:F:N:o:",
" -h, --help Display this help and exit\n"
" -v, --version Print program version\n"
" -d, --daemon Runs in daemon mode\n"
" -M, --module Load specified module\n"
" -F, --profile Use specified configuration profile\n"
" -N, --pathspace Insert prefix into config & socket paths\n"
" -o, --vrfdefaultname Set default VRF name.\n"
" --vty_socket Override vty socket path\n"
" --moduledir Override modules directory\n"
" --scriptdir Override scripts directory\n"
Expand Down Expand Up @@ -495,6 +497,9 @@ static int frr_opt(int opt)
snprintf(pidfile_default, sizeof(pidfile_default), "%s/%s.pid",
frr_vtydir, di->name);
break;
case 'o':
vrf_set_default_name(optarg);
break;
#ifdef HAVE_SQLITE3
case OPTION_DB_FILE:
if (di->flags & FRR_NO_CFG_PID_DRY)
Expand Down
Loading

0 comments on commit ac2cb9b

Please sign in to comment.