sriov: generate rebind systemd service if needed

V2: * simplify self.assert_sriov() in generator/base.py * add new netplan_state_finish_sriov_write() and netplan_sriov_cleanup() API * move SR-IOV rebind service generation into netplan_state_finish_sriov_write() V3: * move 'any_sriov' udev rules logic into netplan_state_finish_sriov_write()
canonical · Feb 9, 2022 · ef56b3a · ef56b3a
1 parent 57fb971
commit ef56b3a
Show file tree

Hide file tree

Showing 6 changed files with 167 additions and 19 deletions.
diff --git a/src/abi_compat.c b/src/abi_compat.c
@@ -29,6 +29,7 @@
 #include "names.h"
 #include "networkd.h"
 #include "nm.h"
+#include "sriov.h"
 #include "openvswitch.h"
 #include "util.h"
 
@@ -186,6 +187,19 @@ cleanup_ovs_conf(const char* rootdir)
 {
     netplan_ovs_cleanup(rootdir);
 }
+
+NETPLAN_INTERNAL void
+write_sriov_conf_finish(const char* rootdir)
+{
+    /* Original implementation had no error possible!! */
+    g_assert(netplan_state_finish_sriov_write(&global_state, rootdir, NULL));
+}
+
+NETPLAN_INTERNAL void
+cleanup_sriov_conf(const char* rootdir)
+{
+    netplan_sriov_cleanup(rootdir);
+}
 // LCOV_EXCL_STOP
 
 gboolean

diff --git a/src/generate.c b/src/generate.c
@@ -38,7 +38,6 @@
 static gchar* rootdir;
 static gchar** files;
 static gboolean any_networkd = FALSE;
-static gboolean any_sriov;
 static gchar* mapping_iface;
 
 static GOptionEntry options[] = {
@@ -254,8 +253,7 @@ int main(int argc, char** argv)
     netplan_networkd_cleanup(rootdir);
     netplan_nm_cleanup(rootdir);
     netplan_ovs_cleanup(rootdir);
-
-    cleanup_sriov_conf(rootdir);
+    netplan_sriov_cleanup(rootdir);
 
     if (mapping_iface && np_state->netdefs) {
         error_code = find_interface(mapping_iface, np_state->netdefs);
@@ -274,13 +272,10 @@ int main(int argc, char** argv)
 
             CHECK_CALL(netplan_netdef_write_ovs(np_state, def, rootdir, &has_been_written, &error));
             CHECK_CALL(netplan_netdef_write_nm(np_state, def, rootdir, &has_been_written, &error));
-
-            if (def->sriov_explicit_vf_count < G_MAXUINT || def->sriov_link)
-                any_sriov = TRUE;
         }
 
         CHECK_CALL(netplan_state_finish_nm_write(np_state, rootdir, &error));
-        if (any_sriov) write_sriov_conf_finish(rootdir);
+        CHECK_CALL(netplan_state_finish_sriov_write(np_state, rootdir, &error));
         /* We may have written .rules & .link files, thus we must
          * invalidate udevd cache of its config as by default it only
          * invalidates cache at most every 3 seconds. Not sure if this

diff --git a/src/sriov.c b/src/sriov.c
@@ -1,6 +1,7 @@
 /*
- * Copyright (C) 2020 Canonical, Ltd.
+ * Copyright (C) 2020-2022 Canonical, Ltd.
  * Author: Łukasz 'sil2100' Zemczak <lukasz.zemczak@canonical.com>
+ * Author: Lukas Märdian <slyon@ubuntu.com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -16,6 +17,7 @@
  */
 
 #include <unistd.h>
+#include <errno.h>
 
 #include <glib.h>
 #include <glib/gstdio.h>
@@ -24,18 +26,99 @@
 #include "util-internal.h"
 #include "sriov.h"
 
-void
-write_sriov_conf_finish(const char* rootdir)
+static gboolean
+write_sriov_rebind_systemd_unit(const GString* pfs, const char* rootdir, GError** error)
 {
-    /* For now we execute apply --sriov-only everytime there is a new
-       SR-IOV device appearing, which is fine as it's relatively fast */
-    GString *udev_rule = g_string_new("ACTION==\"add\", SUBSYSTEM==\"net\", ATTRS{sriov_totalvfs}==\"?*\", RUN+=\"/usr/sbin/netplan apply --sriov-only\"\n");
-    g_string_free_to_file(udev_rule, rootdir, "run/udev/rules.d/99-sriov-netplan-setup.rules", NULL);
+    g_autofree gchar* id_escaped = NULL;
+    g_autofree char* link = g_strjoin(NULL, rootdir ?: "", "/run/systemd/system/multi-user.target.wants/netplan-sriov-rebind.service", NULL);
+    g_autofree char* path = g_strjoin(NULL, "/run/systemd/system/netplan-sriov-rebind.service", NULL);
+    gchar** split = NULL;
+
+    GString* s = g_string_new("[Unit]\n");
+    g_string_append(s, "Description=(Re-)bind SR-IOV Virtual Functions to their driver\n");
+    g_string_append_printf(s, "After=network.target\n");
+
+    /* Run after udev */
+    split = g_strsplit(pfs->str, " ", 0);
+    for (unsigned i = 0; split[i]; ++i)
+        g_string_append_printf(s, "After=sys-subsystem-net-devices-%s.device\n",
+                               split[i]);
+    g_strfreev(split);
+
+    g_string_append(s, "\n[Service]\nType=oneshot\n");
+    g_string_append_printf(s, "ExecStart=" SBINDIR "/netplan rebind %s\n", pfs->str);
+
+    g_string_free_to_file(s, rootdir, path, NULL);
+
+    safe_mkdir_p_dir(link);
+    if (symlink(path, link) < 0 && errno != EEXIST) {
+        // LCOV_EXCL_START
+        g_set_error(error, G_MARKUP_ERROR, G_MARKUP_ERROR_INVALID_CONTENT,
+                    "failed to create enablement symlink: %m\n");
+        return FALSE;
+        // LCOV_EXCL_STOP
+    }
+    return TRUE;
+}
+
+/**
+ * Finalize the SR-IOV configuration (global config)
+ */
+gboolean
+netplan_state_finish_sriov_write(const NetplanState* np_state, const char* rootdir, GError** error)
+{
+    NetplanNetDefinition* def = NULL;
+    NetplanNetDefinition* pf = NULL;
+    gboolean any_sriov = FALSE;
+    gboolean ret = TRUE;
+
+    if (np_state) {
+        GString* pfs = g_string_new(NULL);
+        /* Find netdev interface names for SR-IOV PFs*/
+        for (GList* iterator = np_state->netdefs_ordered; iterator; iterator = iterator->next) {
+            def = (NetplanNetDefinition*) iterator->data;
+            pf = NULL;
+            if (def->sriov_explicit_vf_count < G_MAXUINT || def->sriov_link) {
+                any_sriov = TRUE;
+                if (def->sriov_explicit_vf_count < G_MAXUINT)
+                    pf = def;
+                else if (def->sriov_link)
+                    pf = def->sriov_link;
+            }
+
+            if (pf && pf->sriov_delay_virtual_functions_rebind) {
+                if (pf->set_name)
+                    g_string_append_printf(pfs, "%s ", pf->set_name);
+                else if (!pf->has_match) /* netdef_id == interface name */
+                    g_string_append_printf(pfs, "%s ", pf->id);
+                else
+                    g_warning("%s: Cannot rebind SR-IOV virtual functions, unknown interface name. "
+                              "Use 'netplan rebind <IFACE>' to rebind manually or use the 'set-name' stanza.",
+                              pf->id);
+            }
+        }
+        if (pfs->len > 0) {
+            g_string_truncate(pfs, pfs->len-1); /* cut trailing whitespace */
+            ret = write_sriov_rebind_systemd_unit(pfs, rootdir, NULL);
+        }
+        g_string_free(pfs, TRUE);
+    }
+
+    if (any_sriov) {
+        /* For now we execute apply --sriov-only everytime there is a new
+        SR-IOV device appearing, which is fine as it's relatively fast */
+        GString *udev_rule = g_string_new("ACTION==\"add\", SUBSYSTEM==\"net\", ATTRS{sriov_totalvfs}==\"?*\", RUN+=\"/usr/sbin/netplan apply --sriov-only\"\n");
+        g_string_free_to_file(udev_rule, rootdir, "run/udev/rules.d/99-sriov-netplan-setup.rules", NULL);
+    }
+
+    return ret;
 }
 
-void
-cleanup_sriov_conf(const char* rootdir)
+gboolean
+netplan_sriov_cleanup(const char* rootdir)
 {
-    g_autofree char* rulepath = g_strjoin(NULL, rootdir ?: "", "/run/udev/rules.d/99-sriov-netplan-setup.rules", NULL);
-    unlink(rulepath);
+    unlink_glob(rootdir, "/run/udev/rules.d/*-sriov-netplan-*.rules");
+    unlink_glob(rootdir, "/run/systemd/system/netplan-sriov-*.service");
+    return TRUE;
+
 }
diff --git a/src/sriov.h b/src/sriov.h
@@ -18,8 +18,17 @@
 #pragma once
 #include "netplan.h"
 
+NETPLAN_INTERNAL gboolean
+netplan_state_finish_sriov_write(
+        const NetplanState* np_state,
+        const char* rootdir,
+        GError** error);
+
+NETPLAN_INTERNAL gboolean
+netplan_sriov_cleanup(const char* rootdir);
+
+/* Deprecated API */
 NETPLAN_INTERNAL void
 write_sriov_conf_finish(const char* rootdir);
-
 NETPLAN_INTERNAL void
 cleanup_sriov_conf(const char* rootdir);
diff --git a/tests/generator/base.py b/tests/generator/base.py
@@ -456,3 +456,10 @@ def assert_ovs(self, file_contents_map):
                 self.assertEqual(link_target,
                                  os.path.join(
                                     '/', 'run', 'systemd', 'system', fname))
+
+    def assert_sriov(self, file_contents_map):
+        systemd_dir = os.path.join(self.workdir.name, 'run', 'systemd', 'system')
+        sriov_systemd_dir = glob.glob(os.path.join(systemd_dir, '*netplan-sriov-*.service'))
+        self.assertEqual(set(os.path.basename(file) for file in sriov_systemd_dir),
+                         {'netplan-sriov-' + f for f in file_contents_map})
+        self.assertEqual(set(os.listdir(self.workdir.name)) - {'lib'}, {'etc', 'run'})
diff --git a/tests/test_sriov.py b/tests/test_sriov.py
@@ -860,8 +860,48 @@ def test_eswitch_mode(self):
     engreen:
       embedded-switch-mode: switchdev
       delay-virtual-functions-rebind: true
+    enblue:
+      match: {driver: dummy_driver}
+      set-name: enblue
+      embedded-switch-mode: legacy
+      delay-virtual-functions-rebind: true
+      virtual-function-count: 4
+    sriov_vf0:
+      link: engreen''')
+        self.assert_sriov({'rebind.service': '''[Unit]
+Description=(Re-)bind SR-IOV Virtual Functions to their driver
+After=network.target
+After=sys-subsystem-net-devices-enblue.device
+After=sys-subsystem-net-devices-engreen.device
+
+[Service]
+Type=oneshot
+ExecStart=/usr/sbin/netplan rebind enblue engreen
+'''})
+
+    def test_rebind_not_delayed(self):
+        self.generate('''network:
+  version: 2
+  ethernets:
+    engreen:
+      embedded-switch-mode: switchdev
+      delay-virtual-functions-rebind: false
+    sriov_vf:
+      link: engreen''')
+        self.assert_sriov({})
+
+    def test_rebind_no_iface(self):
+        out = self.generate('''network:
+  version: 2
+  ethernets:
+    engreen:
+      match: {name: 'enp4f[1-3]'}
+      embedded-switch-mode: switchdev
+      delay-virtual-functions-rebind: true
     sriov_vf:
       link: engreen''')
+        self.assert_sriov({})
+        self.assertIn('engreen: Cannot rebind SR-IOV virtual functions, unknown interface name.', out)
 
     def test_invalid_not_a_pf(self):
         err = self.generate('''network: