Merge pull request systemd#14467 from poettering/nspawn-short-names-rework

poettering · web-flow · commit eb253fbdbd6e · 2020-01-03T09:55:29.000+01:00
nspawn: change how we truncate --network-veth names
diff --git a/man/systemd-nspawn.xml b/man/systemd-nspawn.xml
@@ -835,7 +835,11 @@
         container names may have a length up to 64 characters. As this option derives the host-side interface
         name from the container name the name is possibly truncated. Thus, care needs to be taken to ensure
         that interface names remain unique in this case, or even better container names are generally not
-        chosen longer than 12 characters, to avoid the truncation. Alternatively, the
+        chosen longer than 12 characters, to avoid the truncation. If the name is truncated,
+        <command>systemd-nspawn</command> will automatically append a 4-digit hash value to the name to
+        reduce the chance of collisions. However, the hash algorithm is not collision-free. (See
+        <citerefentry><refentrytitle>systemd.net-naming-scheme</refentrytitle><manvolnum>7</manvolnum></citerefentry>
+        for details on older naming algorithms for this interface). Alternatively, the
         <option>--network-veth-extra=</option> option may be used, which allows free configuration of the
         host-side interface name independently of the container name — but might require a bit more
         additional configuration in case bridging in a fashion similar to <option>--network-bridge=</option>
diff --git a/man/systemd.net-naming-scheme.xml b/man/systemd.net-naming-scheme.xml
@@ -46,6 +46,11 @@
     devices based on those properties. See the description of <varname>NamePolicy=</varname> and
     <varname>MACAddressPolicy=</varname> in
     <citerefentry><refentrytitle>systemd.link</refentrytitle><manvolnum>5</manvolnum></citerefentry>.</para>
+
+    <para>Note that while the concept of network interface naming schemes is primarily relevant in the
+    context of <filename>systemd-udevd.service</filename>, the
+    <citerefentry><refentrytitle>systemd-nspawn</refentrytitle><manvolnum>1</manvolnum></citerefentry>
+    container manager also takes it into account when naming network interfaces, see below.</para>
   </refsect1>
 
   <refsect1>
@@ -329,7 +334,21 @@
           <para>Previously two-letter interface type prefix was prepended to
           <varname>ID_NET_LABEL_ONBOARD=</varname>. This is not done anymore.</para></listitem>
         </varlistentry>
-    </variablelist>
+
+        <varlistentry>
+          <term><constant>v245</constant></term>
+
+          <listitem><para>When
+          <citerefentry><refentrytitle>systemd-nspawn</refentrytitle><manvolnum>1</manvolnum></citerefentry>
+          derives the name for the host side of the network interface created with
+          <option>--network-veth</option> from the container name it previously simply truncated the result
+          at 15 characters if longer (since that's the maximum length for network interface names). From now
+          on, for any interface name that would be longer than 15 characters the last 4 characters are set to
+          a 24bit hash value of the full interface name. This way network interface name collisions between
+          multiple similarly named containers (who only differ in container name suffix) should be less
+          likely (but still possible, since the 24bit hash value is very small).</para></listitem>
+        </varlistentry>
+      </variablelist>
 
     <para>Note that <constant>latest</constant> may be used to denote the latest scheme known (to this
     particular version of systemd.</para>
@@ -428,7 +447,8 @@ ID_NET_NAME_PATH=encf5f0</programlisting>
     <para>
       <citerefentry><refentrytitle>udev</refentrytitle><manvolnum>7</manvolnum></citerefentry>,
       <citerefentry><refentrytitle>udevadm</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
-      <ulink url="https://systemd.io/PREDICTABLE_INTERFACE_NAMES">Predictable Network Interface Names</ulink>
+      <ulink url="https://systemd.io/PREDICTABLE_INTERFACE_NAMES">Predictable Network Interface Names</ulink>,
+      <citerefentry><refentrytitle>systemd-nspawn</refentrytitle><manvolnum>1</manvolnum></citerefentry>
     </para>
   </refsect1>
 
diff --git a/src/nspawn/nspawn-network.c b/src/nspawn/nspawn-network.c
@@ -11,6 +11,7 @@
 #include "ether-addr-util.h"
 #include "lockfile-util.h"
 #include "missing_network.h"
+#include "netif-naming-scheme.h"
 #include "netlink-util.h"
 #include "nspawn-network.h"
 #include "parse-util.h"
@@ -27,6 +28,7 @@
 #define VETH_EXTRA_HOST_HASH_KEY SD_ID128_MAKE(48,c7,f6,b7,ea,9d,4c,9e,b7,28,d4,de,91,d5,bf,66)
 #define VETH_EXTRA_CONTAINER_HASH_KEY SD_ID128_MAKE(af,50,17,61,ce,f9,4d,35,84,0d,2b,20,54,be,ce,59)
 #define MACVLAN_HASH_KEY SD_ID128_MAKE(00,13,6d,bc,66,83,44,81,bb,0c,f9,51,1f,24,a6,6f)
+#define SHORTEN_IFNAME_HASH_KEY SD_ID128_MAKE(e1,90,a4,04,a8,ef,4b,51,8c,cc,c3,3a,9f,11,fc,a2)
 
 static int remove_one_link(sd_netlink *rtnl, const char *name) {
         _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL;
@@ -169,23 +171,67 @@ static int add_veth(
         return 0;
 }
 
+/* This is almost base64char(), but not entirely, as it uses the "url and filename safe" alphabet, since we
+ * don't want "/" appear in interface names (since interfaces appear in sysfs as filenames). See section #5
+ * of RFC 4648. */
+static char urlsafe_base64char(int x) {
+        static const char table[64] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+                                      "abcdefghijklmnopqrstuvwxyz"
+                                      "0123456789-_";
+        return table[x & 63];
+}
+
+static void shorten_ifname(char *ifname) {
+        char new_ifname[IFNAMSIZ];
+
+        assert(ifname);
+
+        if (strlen(ifname) < IFNAMSIZ) /* Name is short enough */
+                return;
+
+        if (naming_scheme_has(NAMING_NSPAWN_LONG_HASH)) {
+                uint64_t h;
+
+                /* Calculate 64bit hash value */
+                h = siphash24(ifname, strlen(ifname), SHORTEN_IFNAME_HASH_KEY.bytes);
+
+                /* Set the final four bytes (i.e. 32bit) to the lower 24bit of the hash, encoded in url-safe base64 */
+                memcpy(new_ifname, ifname, IFNAMSIZ - 5);
+                new_ifname[IFNAMSIZ - 5] = urlsafe_base64char(h >> 18);
+                new_ifname[IFNAMSIZ - 4] = urlsafe_base64char(h >> 12);
+                new_ifname[IFNAMSIZ - 3] = urlsafe_base64char(h >> 6);
+                new_ifname[IFNAMSIZ - 2] = urlsafe_base64char(h);
+        } else
+                /* On old nspawn versions we just truncated the name, provide compatibility */
+                memcpy(new_ifname, ifname, IFNAMSIZ-1);
+
+        new_ifname[IFNAMSIZ - 1] = 0;
+
+        /* Log the incident to make it more discoverable */
+        log_warning("Network interface name '%s' has been changed to '%s' to fit length constraints.", ifname, new_ifname);
+
+        strcpy(ifname, new_ifname);
+}
+
 int setup_veth(const char *machine_name,
                pid_t pid,
                char iface_name[IFNAMSIZ],
                bool bridge) {
 
         _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL;
         struct ether_addr mac_host, mac_container;
-        int r, i;
+        unsigned u;
+        char *n;
+        int r;
 
         assert(machine_name);
         assert(pid > 0);
         assert(iface_name);
 
         /* Use two different interface name prefixes depending whether
          * we are in bridge mode or not. */
-        snprintf(iface_name, IFNAMSIZ - 1, "%s-%s",
-                 bridge ? "vb" : "ve", machine_name);
+        n = strjoina(bridge ? "vb-" : "ve-", machine_name);
+        shorten_ifname(n);
 
         r = generate_mac(machine_name, &mac_container, CONTAINER_HASH_KEY, 0);
         if (r < 0)
@@ -199,15 +245,16 @@ int setup_veth(const char *machine_name,
         if (r < 0)
                 return log_error_errno(r, "Failed to connect to netlink: %m");
 
-        r = add_veth(rtnl, pid, iface_name, &mac_host, "host0", &mac_container);
+        r = add_veth(rtnl, pid, n, &mac_host, "host0", &mac_container);
         if (r < 0)
                 return r;
 
-        r = parse_ifindex_or_ifname(iface_name, &i);
-        if (r < 0)
-                return log_error_errno(r, "Failed to resolve interface %s: %m", iface_name);
+        u = if_nametoindex(n);
+        if (u == 0)
+                return log_error_errno(errno, "Failed to resolve interface %s: %m", n);
 
-        return i;
+        strcpy(iface_name, n);
+        return (int) u;
 }
 
 int setup_veth_extra(
@@ -503,7 +550,7 @@ int setup_macvlan(const char *machine_name, pid_t pid, char **ifaces) {
                 if (!n)
                         return log_oom();
 
-                strshorten(n, IFNAMSIZ-1);
+                shorten_ifname(n);
 
                 r = sd_netlink_message_append_string(m, IFLA_IFNAME, n);
                 if (r < 0)
@@ -578,7 +625,7 @@ int setup_ipvlan(const char *machine_name, pid_t pid, char **ifaces) {
                 if (!n)
                         return log_oom();
 
-                strshorten(n, IFNAMSIZ-1);
+                shorten_ifname(n);
 
                 r = sd_netlink_message_append_string(m, IFLA_IFNAME, n);
                 if (r < 0)
diff --git a/src/shared/meson.build b/src/shared/meson.build
@@ -130,6 +130,8 @@ shared_sources = files('''
         module-util.h
         mount-util.c
         mount-util.h
+        netif-naming-scheme.c
+        netif-naming-scheme.h
         nscd-flush.c
         nscd-flush.h
         nsflags.c
diff --git a/src/shared/netif-naming-scheme.c b/src/shared/netif-naming-scheme.c
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: LGPL-2.1+ */
+
 #include "alloc-util.h"
-#include "naming-scheme.h"
+#include "netif-naming-scheme.h"
 #include "proc-cmdline.h"
 #include "string-util.h"
 
@@ -10,6 +11,7 @@ static const NamingScheme naming_schemes[] = {
         { "v240", NAMING_V240 },
         { "v241", NAMING_V241 },
         { "v243", NAMING_V243 },
+        { "v245", NAMING_V245 },
         /* … add more schemes here, as the logic to name devices is updated … */
 };
 
diff --git a/src/shared/netif-naming-scheme.h b/src/shared/netif-naming-scheme.h
@@ -30,13 +30,15 @@ typedef enum NamingSchemeFlags {
         NAMING_STABLE_VIRTUAL_MACS = 1 << 5, /* Use device name to generate MAC, see 6d3646406560 */
         NAMING_NETDEVSIM           = 1 << 6, /* Generate names for netdevsim devices, see eaa9d507d855 */
         NAMING_LABEL_NOPREFIX      = 1 << 7, /* Don't prepend ID_NET_LABEL_ONBOARD with interface type prefix */
+        NAMING_NSPAWN_LONG_HASH    = 1 << 8, /* Shorten nspawn interfaces by including 24bit hash, instead of simple truncation  */
 
         /* And now the masks that combine the features above */
         NAMING_V238 = 0,
         NAMING_V239 = NAMING_V238 | NAMING_SR_IOV_V | NAMING_NPAR_ARI,
         NAMING_V240 = NAMING_V239 | NAMING_INFINIBAND | NAMING_ZERO_ACPI_INDEX | NAMING_ALLOW_RERENAMES,
         NAMING_V241 = NAMING_V240 | NAMING_STABLE_VIRTUAL_MACS,
         NAMING_V243 = NAMING_V241 | NAMING_NETDEVSIM | NAMING_LABEL_NOPREFIX,
+        NAMING_V245 = NAMING_V243 | NAMING_NSPAWN_LONG_HASH,
 
         _NAMING_SCHEME_FLAGS_INVALID = -1,
 } NamingSchemeFlags;
diff --git a/src/udev/meson.build b/src/udev/meson.build
@@ -40,8 +40,6 @@ libudev_core_sources = '''
         udev-builtin-usb_id.c
         net/link-config.c
         net/link-config.h
-        net/naming-scheme.c
-        net/naming-scheme.h
 '''.split()
 
 if conf.get('HAVE_KMOD') == 1
diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c
@@ -16,7 +16,7 @@
 #include "link-config.h"
 #include "log.h"
 #include "memory-util.h"
-#include "naming-scheme.h"
+#include "netif-naming-scheme.h"
 #include "netlink-util.h"
 #include "network-internal.h"
 #include "parse-util.h"
diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c
@@ -27,7 +27,7 @@
 #include "fd-util.h"
 #include "fileio.h"
 #include "fs-util.h"
-#include "naming-scheme.h"
+#include "netif-naming-scheme.h"
 #include "parse-util.h"
 #include "proc-cmdline.h"
 #include "stdio-util.h"
