87d178
From 08ac9f7f55c138678c6415139e7510a05a75b81d Mon Sep 17 00:00:00 2001
87d178
From: =?UTF-8?q?Michal=20Sekleta=CC=81r?= <msekleta@redhat.com>
87d178
Date: Wed, 14 Oct 2020 16:57:44 +0200
87d178
Subject: [PATCH] udev: introduce udev net_id "naming schemes"
87d178
MIME-Version: 1.0
87d178
Content-Type: text/plain; charset=UTF-8
87d178
Content-Transfer-Encoding: 8bit
87d178
87d178
With this we can stabilize how naming works for network interfaces. A
87d178
user can request through a kernel cmdline option or an env var which
87d178
scheme to follow. The idea is that installers use this to set into stone
87d178
(a very soft stone though) the scheme used during installation so that
87d178
interface naming doesn't change afterwards anymore.
87d178
87d178
Why use env vars and kernel cmdline options, and not a config file of
87d178
its own?
87d178
87d178
Well, first of all there's no obvious existing one to use. But more
87d178
importantly: I have the feeling that this logic is kind of an incomplete
87d178
hack, and I simply don't want to do advertise this as a perfectly
87d178
working solution. So far we used env vars for the non-so-official
87d178
options and proper config files for the official stuff. Given how
87d178
incomplete this logic is (i.e. the big variable for naming remains the
87d178
kernel, which might expose sysfs attributes in newer versions that we
87d178
check for and didn't exist in older versions — and other problems like
87d178
this), I am simply not confident in giving this first-class exposure in
87d178
a primary configuration file.
87d178
87d178
Fixes: #10448
87d178
87d178
(cherry-picked from commit f7e81fd96fdfe0ac6dcdb72de43f7cb4720e363a)
87d178
87d178
Related: #1827462
87d178
87d178
[msekleta: note that we are introducing our own naming schemes based on
87d178
RHEL-8 minor versions. Also we are not backporting all naming scheme
87d178
features that appeared in the original commit. We are backporting only
87d178
features relevant for v239 while original commit also converted
87d178
changes introduced in v240 into naming scheme flags.]
87d178
---
87d178
 doc/ENVIRONMENT.md             |   9 +++
87d178
 man/kernel-command-line.xml    |   1 +
87d178
 man/systemd-udevd.service.xml  |  16 +++++
87d178
 src/udev/udev-builtin-net_id.c | 106 ++++++++++++++++++++++++++++++++-
87d178
 4 files changed, 130 insertions(+), 2 deletions(-)
87d178
87d178
diff --git a/doc/ENVIRONMENT.md b/doc/ENVIRONMENT.md
87d178
index 39a36a52cc..1a4aa01ef4 100644
87d178
--- a/doc/ENVIRONMENT.md
87d178
+++ b/doc/ENVIRONMENT.md
87d178
@@ -76,6 +76,15 @@ systemd-logind:
87d178
   hibernation is available even if the swap devices do not provide enough room
87d178
   for it.
87d178
 
87d178
+* `$NET_NAMING_SCHEME=` – if set, takes a network naming scheme (i.e. one of
87d178
+  v238, v239, v240 …) as parameter. If specified udev's net_id builtin will
87d178
+  follow the specified naming scheme when determining stable network interface
87d178
+  names. This may be used to revert to naming schemes of older udev versions,
87d178
+  in order to provide more stable naming across updates. This environment
87d178
+  variable takes precedence over the kernel command line option
87d178
+  `net.naming-scheme=`, except if the value is prefixed with `:` in which case
87d178
+  the kernel command line option takes precedence, if it is specified as well.
87d178
+
87d178
 installed systemd tests:
87d178
 
87d178
 * `$SYSTEMD_TEST_DATA` — override the location of test data. This is useful if
87d178
diff --git a/man/kernel-command-line.xml b/man/kernel-command-line.xml
87d178
index 4d8cb4e50e..b753d0592c 100644
87d178
--- a/man/kernel-command-line.xml
87d178
+++ b/man/kernel-command-line.xml
87d178
@@ -246,6 +246,7 @@
87d178
         <term><varname>udev.event_timeout=</varname></term>
87d178
         <term><varname>rd.udev.event_timeout=</varname></term>
87d178
         <term><varname>net.ifnames=</varname></term>
87d178
+        <term><varname>net.naming-scheme=</varname></term>
87d178
 
87d178
         <listitem>
87d178
           <para>Parameters understood by the device event managing
87d178
diff --git a/man/systemd-udevd.service.xml b/man/systemd-udevd.service.xml
87d178
index 73c77ea690..6449103441 100644
87d178
--- a/man/systemd-udevd.service.xml
87d178
+++ b/man/systemd-udevd.service.xml
87d178
@@ -170,6 +170,22 @@
87d178
           when possible. It is enabled by default; specifying 0 disables it.</para>
87d178
         </listitem>
87d178
       </varlistentry>
87d178
+      <varlistentry>
87d178
+        <term><varname>net.naming-scheme=</varname></term>
87d178
+        <listitem>
87d178
+          <para>Network interfaces are renamed to give them predictable names when possible (unless
87d178
+          <varname>net.ifnames=0</varname> is specified, see above). The names are derived from various device metadata
87d178
+          fields. Newer versions of <filename>systemd-udevd.service</filename> take more of these fields into account,
87d178
+          improving (and thus possibly changing) the names used for the same devices. With this kernel command line
87d178
+          option it is possible to pick a specific version of this algorithm. It expects a naming scheme identifier as
87d178
+          argument. Currently the following identifiers are known: <literal>v238</literal>, <literal>v239</literal>,
87d178
+          <literal>v240</literal> which each implement the naming scheme that was the default in the indicated systemd
87d178
+          version. Note that selecting a specific scheme is not sufficient to fully stabilize interface naming: the
87d178
+          naming is generally derived from driver attributes exposed by the kernel. As the kernel is updated,
87d178
+          previously missing attributes <filename>systemd-udevd.service</filename> is checking might appear, which
87d178
+          affects older name derivation algorithms, too.</para>
87d178
+        </listitem>
87d178
+      </varlistentry>
87d178
     </variablelist>
87d178
     
87d178
          in kernel-command-line.xml -->
87d178
diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c
87d178
index 147e04ab8c..148696183e 100644
87d178
--- a/src/udev/udev-builtin-net_id.c
87d178
+++ b/src/udev/udev-builtin-net_id.c
87d178
@@ -96,6 +96,7 @@
87d178
 #include "fileio.h"
87d178
 #include "fs-util.h"
87d178
 #include "parse-util.h"
87d178
+#include "proc-cmdline.h"
87d178
 #include "stdio-util.h"
87d178
 #include "string-util.h"
87d178
 #include "udev.h"
87d178
@@ -103,6 +104,52 @@
87d178
 
87d178
 #define ONBOARD_INDEX_MAX (16*1024-1)
87d178
 
87d178
+/* So here's the deal: net_id is supposed to be an excercise in providing stable names for network devices. However, we
87d178
+ * also want to keep updating the naming scheme used in future versions of net_id. These two goals of course are
87d178
+ * contradictory: on one hand we want things to not change and on the other hand we want them to improve. Our way out
87d178
+ * of this dilemma is to introduce the "naming scheme" concept: each time we improve the naming logic we define a new
87d178
+ * flag for it. Then, we keep a list of schemes, each identified by a name associated with the flags it implements. Via
87d178
+ * a kernel command line and environment variable we then allow the user to pick the scheme they want us to follow:
87d178
+ * installers could "freeze" the used scheme at the moment of installation this way.
87d178
+ *
87d178
+ * Developers: each time you tweak the naming logic here, define a new flag below, and condition the tweak with
87d178
+ * it. Each time we do a release we'll then add a new scheme entry and include all newly defined flags.
87d178
+ *
87d178
+ * Note that this is only half a solution to the problem though: not only udev/net_id gets updated all the time, the
87d178
+ * kernel gets too. And thus a kernel that previously didn't expose some sysfs attribute we look for might eventually
87d178
+ * do, and thus affect our naming scheme too. Thus, enforcing a naming scheme will make interfacing more stable across
87d178
+ * OS versions, but not fully stabilize them. */
87d178
+typedef enum NamingSchemeFlags {
87d178
+        /* First, the individual features */
87d178
+        NAMING_SR_IOV_V        = 1 << 0, /* Use "v" suffix for SR-IOV, see 609948c7043a40008b8299529c978ed8e11de8f6*/
87d178
+        NAMING_NPAR_ARI        = 1 << 1, /* Use NPAR "ARI", see 6bc04997b6eab35d1cb9fa73889892702c27be09 */
87d178
+
87d178
+        /* And now the masks that combine the features above */
87d178
+        NAMING_V238 = 0,
87d178
+        NAMING_V239 = NAMING_V238|NAMING_SR_IOV_V|NAMING_NPAR_ARI,
87d178
+        NAMING_RHEL_8_0 = NAMING_V239,
87d178
+        NAMING_RHEL_8_1 = NAMING_V239,
87d178
+        NAMING_RHEL_8_2 = NAMING_V239,
87d178
+        NAMING_RHEL_8_3 = NAMING_V239,
87d178
+
87d178
+        _NAMING_SCHEME_FLAGS_INVALID = -1,
87d178
+} NamingSchemeFlags;
87d178
+
87d178
+typedef struct NamingScheme {
87d178
+        const char *name;
87d178
+        NamingSchemeFlags flags;
87d178
+} NamingScheme;
87d178
+
87d178
+static const NamingScheme naming_schemes[] = {
87d178
+        { "v238", NAMING_V238 },
87d178
+        { "v239", NAMING_V239 },
87d178
+        { "rhel-8.0", NAMING_RHEL_8_0 },
87d178
+        { "rhel-8.1", NAMING_RHEL_8_1 },
87d178
+        { "rhel-8.2", NAMING_RHEL_8_2 },
87d178
+        { "rhel-8.3", NAMING_RHEL_8_3 },
87d178
+        /* … add more schemes here, as the logic to name devices is updated … */
87d178
+};
87d178
+
87d178
 enum netname_type{
87d178
         NET_UNDEF,
87d178
         NET_PCI,
87d178
@@ -138,6 +185,56 @@ struct virtfn_info {
87d178
         char suffix[IFNAMSIZ];
87d178
 };
87d178
 
87d178
+static const NamingScheme* naming_scheme(void) {
87d178
+        static const NamingScheme *cache = NULL;
87d178
+        _cleanup_free_ char *buffer = NULL;
87d178
+        const char *e, *k;
87d178
+
87d178
+        if (cache)
87d178
+                return cache;
87d178
+
87d178
+        /* Acquire setting from the kernel command line */
87d178
+        (void) proc_cmdline_get_key("net.naming-scheme", 0, &buffer);
87d178
+
87d178
+        /* Also acquire it from an env var */
87d178
+        e = getenv("NET_NAMING_SCHEME");
87d178
+        if (e) {
87d178
+                if (*e == ':') {
87d178
+                        /* If prefixed with ':' the kernel cmdline takes precedence */
87d178
+                        k = buffer ?: e + 1;
87d178
+                } else
87d178
+                        k = e; /* Otherwise the env var takes precedence */
87d178
+        } else
87d178
+                k = buffer;
87d178
+
87d178
+        if (k) {
87d178
+                size_t i;
87d178
+
87d178
+                for (i = 0; i < ELEMENTSOF(naming_schemes); i++)
87d178
+                        if (streq(naming_schemes[i].name, k)) {
87d178
+                                cache = naming_schemes + i;
87d178
+                                break;
87d178
+                        }
87d178
+
87d178
+                if (!cache)
87d178
+                        log_warning("Unknown interface naming scheme '%s' requested, ignoring.", k);
87d178
+        }
87d178
+
87d178
+        if (cache)
87d178
+                log_info("Using interface naming scheme '%s'.", cache->name);
87d178
+        else {
87d178
+                /* RHEL-only: here we differ from the upstream and if no naming scheme was selected we default to naming from systemd-239 */
87d178
+                cache = &naming_schemes[2];
87d178
+                log_info("Using default interface naming scheme '%s'.", cache->name);
87d178
+        }
87d178
+
87d178
+        return cache;
87d178
+}
87d178
+
87d178
+static bool naming_scheme_has(NamingSchemeFlags flags) {
87d178
+        return FLAGS_SET(naming_scheme()->flags, flags);
87d178
+}
87d178
+
87d178
 /* skip intermediate virtio devices */
87d178
 static struct udev_device *skip_virtio(struct udev_device *dev) {
87d178
         struct udev_device *parent = dev;
87d178
@@ -299,7 +396,9 @@ static int dev_pci_slot(struct udev_device *dev, struct netnames *names) {
87d178
 
87d178
         if (sscanf(udev_device_get_sysname(names->pcidev), "%x:%x:%x.%u", &domain, &bus, &slot, &func) != 4)
87d178
                 return -ENOENT;
87d178
-        if (is_pci_ari_enabled(names->pcidev))
87d178
+
87d178
+        if (naming_scheme_has(NAMING_NPAR_ARI) &&
87d178
+            is_pci_ari_enabled(names->pcidev))
87d178
                 /* ARI devices support up to 256 functions on a single device ("slot"), and interpret the
87d178
                  * traditional 5-bit slot and 3-bit function number as a single 8-bit function number,
87d178
                  * where the slot makes up the upper 5 bits. */
87d178
@@ -494,7 +593,8 @@ static int names_pci(struct udev_device *dev, struct netnames *names) {
87d178
                         return -ENOENT;
87d178
         }
87d178
 
87d178
-        if (get_virtfn_info(dev, names, &vf_info) >= 0) {
87d178
+        if (naming_scheme_has(NAMING_SR_IOV_V) &&
87d178
+            get_virtfn_info(dev, names, &vf_info) >= 0) {
87d178
                 /* If this is an SR-IOV virtual device, get base name using physical device and add virtfn suffix. */
87d178
                 vf_names.pcidev = vf_info.physfn_pcidev;
87d178
                 dev_pci_onboard(dev, &vf_names);
87d178
@@ -741,6 +841,8 @@ static int builtin_net_id(struct udev_device *dev, int argc, char *argv[], bool
87d178
                         prefix = "ww";
87d178
         }
87d178
 
87d178
+        udev_builtin_add_property(dev, test, "ID_NET_NAMING_SCHEME", naming_scheme()->name);
87d178
+
87d178
         err = names_mac(dev, &names);
87d178
         if (err >= 0 && names.mac_valid) {
87d178
                 char str[IFNAMSIZ];