]> git.puffer.fish Git - mirror/frr.git/commitdiff
lib, yang: remove vrf from the interface list key 10110/head
authorIgor Ryzhov <iryzhov@nfware.com>
Mon, 8 Nov 2021 10:33:03 +0000 (13:33 +0300)
committerIgor Ryzhov <iryzhov@nfware.com>
Tue, 23 Nov 2021 09:57:52 +0000 (12:57 +0300)
This is needed for the following two reasons:

1. To be able to remove the northbound HACK in if_update_to_new_vrf. It
   is totally wrong to rewrite the configuration datastore when some
   operational state changes. It is a hard blocker for storing a
   configuration data in a management daemon which knows nothing about
   the operational state.
2. To allow changing the VRF of the interface using FRR CLI or any other
   frontend in the future. If the VRF is a part of the key, it can't be
   changed. If the VRF is a simple leaf, it becomes possible to change
   it and thus move the interface between VRFs. For now I mark the leaf
   as a "config false" as it's not yet possible to control it from FRR.

But we can't simply remove the VRF from the key, because it is needed to
distinguish interfaces when using netns based VRFs, as it is possible to
have multiple interfaces with the same name in different namespaces. To
handle this, I came up with an idea to store both VRF and an interface
name in the "name" leaf using the pattern "vrfname:ifname". For example,
if there's an interface "eth0" in VRF "red" then its "name" leaf will be
"red:eth0".

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
lib/if.c
lib/vrf.c
yang/frr-interface.yang
yang/ietf/frr-ietf-translator.json

index fe1d3ebb4ae07aeeb5b62194ede51345e10b2e15..99fa83719a25536c78327456bfa2d218adc4ad9e 100644 (file)
--- a/lib/if.c
+++ b/lib/if.c
@@ -245,36 +245,6 @@ void if_update_to_new_vrf(struct interface *ifp, vrf_id_t vrf_id)
 
        if (ifp->ifindex != IFINDEX_INTERNAL)
                IFINDEX_RB_INSERT(vrf, ifp);
-
-       /*
-        * HACK: Change the interface VRF in the running configuration directly,
-        * bypassing the northbound layer. This is necessary to avoid deleting
-        * the interface and readding it in the new VRF, which would have
-        * several implications.
-        */
-       if (!vrf_is_backend_netns() && yang_module_find("frr-interface")) {
-               struct lyd_node *if_dnode;
-               char oldpath[XPATH_MAXLEN];
-               char newpath[XPATH_MAXLEN];
-
-               snprintf(oldpath, sizeof(oldpath),
-                        "/frr-interface:lib/interface[name='%s'][vrf='%s']",
-                        ifp->name, old_vrf->name);
-               snprintf(newpath, sizeof(newpath),
-                        "/frr-interface:lib/interface[name='%s'][vrf='%s']",
-                        ifp->name, vrf->name);
-
-               if_dnode = yang_dnode_getf(running_config->dnode, "%s/vrf",
-                                          oldpath);
-
-               if (if_dnode) {
-                       yang_dnode_change_leaf(if_dnode, vrf->name);
-                       nb_running_move_tree(oldpath, newpath);
-                       running_config->version++;
-               }
-
-               vty_update_xpath(oldpath, newpath);
-       }
 }
 
 
@@ -1185,13 +1155,6 @@ DEFPY_YANG_NOSH (interface,
        struct vrf *vrf;
        int ret, count;
 
-       /*
-        * This command requires special handling to maintain backward
-        * compatibility. If a VRF name is not specified, it means we're willing
-        * to accept any interface with the given name on any VRF. If no
-        * interface is found, then a new one should be created on the default
-        * VRF.
-        */
        if (vrf_is_backend_netns()) {
                /*
                 * For backward compatibility, if the VRF name is not specified
@@ -1203,25 +1166,15 @@ DEFPY_YANG_NOSH (interface,
                        if (count != 1)
                                vrf_name = VRF_DEFAULT_NAME;
                }
+
+               snprintf(xpath_list, XPATH_MAXLEN,
+                        "/frr-interface:lib/interface[name='%s:%s']", vrf_name,
+                        ifname);
        } else {
-               /*
-                * If the interface already exists, use its VRF regardless of
-                * what user specified. We can't have same interface name in
-                * different VRFs with VRF-lite backend.
-                */
-               ifp = if_lookup_by_name_all_vrf(ifname);
-               if (ifp) {
-                       vrf_name = ifp->vrf->name;
-               } else {
-                       if (!vrf_name)
-                               vrf_name = VRF_DEFAULT_NAME;
-               }
+               snprintf(xpath_list, XPATH_MAXLEN,
+                        "/frr-interface:lib/interface[name='%s']", ifname);
        }
 
-       snprintf(xpath_list, sizeof(xpath_list),
-                "/frr-interface:lib/interface[name='%s'][vrf='%s']", ifname,
-                vrf_name);
-
        nb_cli_enqueue_change(vty, ".", NB_OP_CREATE, NULL);
        ret = nb_cli_apply_changes_clear_pending(vty, xpath_list);
        if (ret == CMD_SUCCESS) {
@@ -1257,27 +1210,72 @@ DEFPY_YANG (no_interface,
        "Interface's name\n"
        VRF_CMD_HELP_STR)
 {
-       if (!vrf_name)
-               vrf_name = VRF_DEFAULT_NAME;
+       char xpath_list[XPATH_MAXLEN];
+       int count;
+
+       if (vrf_is_backend_netns()) {
+               /*
+                * For backward compatibility, if the VRF name is not specified
+                * and there is exactly one interface with this name in the
+                * system, use its VRF. Otherwise fallback to the default VRF.
+                */
+               if (!vrf_name) {
+                       count = vrfname_by_ifname(ifname, &vrf_name);
+                       if (count != 1)
+                               vrf_name = VRF_DEFAULT_NAME;
+               }
+
+               snprintf(xpath_list, XPATH_MAXLEN,
+                        "/frr-interface:lib/interface[name='%s:%s']", vrf_name,
+                        ifname);
+       } else {
+               snprintf(xpath_list, XPATH_MAXLEN,
+                        "/frr-interface:lib/interface[name='%s']", ifname);
+       }
 
        nb_cli_enqueue_change(vty, ".", NB_OP_DESTROY, NULL);
 
-       return nb_cli_apply_changes(
-               vty, "/frr-interface:lib/interface[name='%s'][vrf='%s']",
-               ifname, vrf_name);
+       return nb_cli_apply_changes(vty, xpath_list);
+}
+
+static void netns_ifname_split(const char *xpath, char *ifname, char *vrfname)
+{
+       char *delim;
+       int len;
+
+       assert(vrf_is_backend_netns());
+
+       delim = strchr(xpath, ':');
+       assert(delim);
+
+       len = delim - xpath;
+       memcpy(vrfname, xpath, len);
+       vrfname[len] = 0;
+
+       strlcpy(ifname, delim + 1, XPATH_MAXLEN);
 }
 
 static void cli_show_interface(struct vty *vty, const struct lyd_node *dnode,
                               bool show_defaults)
 {
-       const char *vrf;
+       vty_out(vty, "!\n");
 
-       vrf = yang_dnode_get_string(dnode, "./vrf");
+       if (vrf_is_backend_netns()) {
+               char ifname[XPATH_MAXLEN];
+               char vrfname[XPATH_MAXLEN];
+
+               netns_ifname_split(yang_dnode_get_string(dnode, "./name"),
+                                  ifname, vrfname);
+
+               vty_out(vty, "interface %s", ifname);
+               if (!strmatch(vrfname, VRF_DEFAULT_NAME))
+                       vty_out(vty, " vrf %s", vrfname);
+       } else {
+               const char *ifname = yang_dnode_get_string(dnode, "./name");
+
+               vty_out(vty, "interface %s", ifname);
+       }
 
-       vty_out(vty, "!\n");
-       vty_out(vty, "interface %s", yang_dnode_get_string(dnode, "./name"));
-       if (!strmatch(vrf, VRF_DEFAULT_NAME))
-               vty_out(vty, " vrf %s", vrf);
        vty_out(vty, "\n");
 }
 
@@ -1406,19 +1404,55 @@ void if_zapi_callbacks(int (*create)(struct interface *ifp),
 static int lib_interface_create(struct nb_cb_create_args *args)
 {
        const char *ifname;
-       const char *vrfname;
        struct interface *ifp;
 
        ifname = yang_dnode_get_string(args->dnode, "./name");
-       vrfname = yang_dnode_get_string(args->dnode, "./vrf");
 
        switch (args->event) {
        case NB_EV_VALIDATE:
+               if (vrf_is_backend_netns()) {
+                       char ifname_ns[XPATH_MAXLEN];
+                       char vrfname_ns[XPATH_MAXLEN];
+
+                       netns_ifname_split(ifname, ifname_ns, vrfname_ns);
+
+                       if (strlen(ifname_ns) > 16) {
+                               snprintf(
+                                       args->errmsg, args->errmsg_len,
+                                       "Maximum interface name length is 16 characters");
+                               return NB_ERR_VALIDATION;
+                       }
+                       if (strlen(vrfname_ns) > 36) {
+                               snprintf(
+                                       args->errmsg, args->errmsg_len,
+                                       "Maximum VRF name length is 36 characters");
+                               return NB_ERR_VALIDATION;
+                       }
+               } else {
+                       if (strlen(ifname) > 16) {
+                               snprintf(
+                                       args->errmsg, args->errmsg_len,
+                                       "Maximum interface name length is 16 characters");
+                               return NB_ERR_VALIDATION;
+                       }
+               }
+               break;
        case NB_EV_PREPARE:
        case NB_EV_ABORT:
                break;
        case NB_EV_APPLY:
-               ifp = if_get_by_name(ifname, VRF_UNKNOWN, vrfname);
+               if (vrf_is_backend_netns()) {
+                       char ifname_ns[XPATH_MAXLEN];
+                       char vrfname_ns[XPATH_MAXLEN];
+
+                       netns_ifname_split(ifname, ifname_ns, vrfname_ns);
+
+                       ifp = if_get_by_name(ifname_ns, VRF_UNKNOWN,
+                                            vrfname_ns);
+               } else {
+                       ifp = if_get_by_name(ifname, VRF_UNKNOWN,
+                                            VRF_DEFAULT_NAME);
+               }
 
                ifp->configured = true;
                nb_running_set_entry(args->dnode, ifp);
@@ -1491,9 +1525,14 @@ static int lib_interface_get_keys(struct nb_cb_get_keys_args *args)
 {
        const struct interface *ifp = args->list_entry;
 
-       args->keys->num = 2;
-       strlcpy(args->keys->key[0], ifp->name, sizeof(args->keys->key[0]));
-       strlcpy(args->keys->key[1], ifp->vrf->name, sizeof(args->keys->key[1]));
+       args->keys->num = 1;
+
+       if (vrf_is_backend_netns())
+               snprintf(args->keys->key[0], sizeof(args->keys->key[0]),
+                        "%s:%s", ifp->vrf->name, ifp->name);
+       else
+               snprintf(args->keys->key[0], sizeof(args->keys->key[0]), "%s",
+                        ifp->name);
 
        return NB_OK;
 }
@@ -1501,11 +1540,19 @@ static int lib_interface_get_keys(struct nb_cb_get_keys_args *args)
 static const void *
 lib_interface_lookup_entry(struct nb_cb_lookup_entry_args *args)
 {
-       const char *ifname = args->keys->key[0];
-       const char *vrfname = args->keys->key[1];
-       struct vrf *vrf = vrf_lookup_by_name(vrfname);
+       if (vrf_is_backend_netns()) {
+               char ifname[XPATH_MAXLEN];
+               char vrfname[XPATH_MAXLEN];
+               struct vrf *vrf;
+
+               netns_ifname_split(args->keys->key[0], ifname, vrfname);
 
-       return vrf ? if_lookup_by_name(ifname, vrf->vrf_id) : NULL;
+               vrf = vrf_lookup_by_name(vrfname);
+
+               return vrf ? if_lookup_by_name(ifname, vrf->vrf_id) : NULL;
+       } else {
+               return if_lookup_by_name_all_vrf(args->keys->key[0]);
+       }
 }
 
 /*
@@ -1540,6 +1587,17 @@ static int lib_interface_description_destroy(struct nb_cb_destroy_args *args)
        return NB_OK;
 }
 
+/*
+ * XPath: /frr-interface:lib/interface/vrf
+ */
+static struct yang_data *
+lib_interface_vrf_get_elem(struct nb_cb_get_elem_args *args)
+{
+       const struct interface *ifp = args->list_entry;
+
+       return yang_data_new_string(args->xpath, ifp->vrf->name);
+}
+
 /*
  * XPath: /frr-interface:lib/interface/state/if-index
  */
@@ -1653,6 +1711,12 @@ const struct frr_yang_module_info frr_interface_info = {
                                .cli_show = cli_show_interface_desc,
                        },
                },
+               {
+                       .xpath = "/frr-interface:lib/interface/vrf",
+                       .cbs = {
+                               .get_elem = lib_interface_vrf_get_elem,
+                       }
+               },
                {
                        .xpath = "/frr-interface:lib/interface/state/if-index",
                        .cbs = {
index 5dc4e058557b6c0ec755fe5d9be2b9f71d250dfc..b5f06048b784144852d10018ba06342db3edc027 100644 (file)
--- a/lib/vrf.c
+++ b/lib/vrf.c
@@ -690,15 +690,10 @@ DEFUN_YANG (no_vrf,
 
        if (vrf_get_backend() == VRF_BACKEND_VRF_LITE) {
                /*
-                * Remove the VRF interface config. Currently, we allow to
-                * remove only inactive VRFs, so we use VRF_DEFAULT_NAME here,
-                * because when the VRF is removed from kernel, the interface
-                * is moved to the default VRF. If we ever allow removing
-                * active VRFs, this code have to be updated accordingly.
+                * Remove the VRF interface config when removing the VRF.
                 */
                snprintf(xpath_list, sizeof(xpath_list),
-                        "/frr-interface:lib/interface[name='%s'][vrf='%s']",
-                        vrfname, VRF_DEFAULT_NAME);
+                        "/frr-interface:lib/interface[name='%s']", vrfname);
                nb_cli_enqueue_change(vty, xpath_list, NB_OP_DESTROY, NULL);
        }
 
index 46c03a1d1f692ad8c413bf36704840331f077efa..4a152aaddab4e5df22c8ab290a4b4412fc40c019 100644 (file)
@@ -288,13 +288,11 @@ module frr-interface {
 
   container lib {
     list interface {
-      key "name vrf";
+      key "name";
       description
         "Interface.";
       leaf name {
-        type string {
-          length "1..16";
-        }
+        type string;
         description
           "Interface name.";
       }
@@ -303,6 +301,7 @@ module frr-interface {
         type frr-vrf:vrf-ref;
         description
           "VRF this interface is associated with.";
+        config false;
       }
 
       leaf description {
index 38a609982fa9afba3dbf6e38f2b826c5664df7ab..36d6ddc8afbc7bfd72b5f7b48a2784e246458a20 100644 (file)
@@ -2,20 +2,6 @@
   "frr-module-translator:frr-module-translator": {
     "family": "ietf",
     "module": [
-      {
-        "name": "ietf-interfaces@2018-01-09",
-        "deviations": "frr-deviations-ietf-interfaces",
-        "mappings": [
-          {
-            "custom": "/ietf-interfaces:interfaces/interface[name='KEY1']",
-            "native": "/frr-interface:lib/interface[name='KEY1'][vrf='default']"
-          },
-          {
-            "custom": "/ietf-interfaces:interfaces/interface[name='KEY1']/description",
-            "native": "/frr-interface:lib/interface[name='KEY1'][vrf='default']/description"
-          }
-        ]
-      },
       {
         "name": "ietf-routing@2018-01-25",
         "deviations": "frr-deviations-ietf-routing",
@@ -60,7 +46,7 @@
           },
           {
             "custom": "/ietf-routing:routing/control-plane-protocols/control-plane-protocol[type='ietf-rip:ripv2'][name='main']/ietf-rip:rip/interfaces/interface[interface='KEY1']/split-horizon",
-            "native": "/frr-interface:lib/interface[name='KEY1'][vrf='default']/frr-ripd:rip/split-horizon"
+            "native": "/frr-interface:lib/interface[name='KEY1']/frr-ripd:rip/split-horizon"
           },
           {
             "custom": "/ietf-routing:routing/control-plane-protocols/control-plane-protocol[type='ietf-rip:ripv2'][name='main']/ietf-rip:rip/ipv4/neighbors/neighbor[ipv4-address='KEY1']",