From e423ec9583c7e276845df3b0c76533ddaea50d4f Mon Sep 17 00:00:00 2001
From: Will Szumski <will@stackhpc.com>
Date: Thu, 8 Aug 2024 17:39:01 +0100
Subject: [PATCH] Use Systemd to activate virtual functions

Rationale:

- These can be used by other units to define an ordering e.g you may
  want to run some before the virtual functions are created and/or
  run something after.

- The udev method isn't working for me on Rocky 9.4 with Infiniband
  virtual functions.
---
 roles/sriov/README.md                         |  7 +++
 roles/sriov/defaults/main.yml                 |  7 +++
 roles/sriov/tasks/config.yml                  | 43 ++++++++++++++++++-
 .../templates/virtual-function.service.j2     | 25 +++++++++++
 4 files changed, 81 insertions(+), 1 deletion(-)
 create mode 100644 roles/sriov/templates/virtual-function.service.j2

diff --git a/roles/sriov/README.md b/roles/sriov/README.md
index 18482a8..7f8bb59 100644
--- a/roles/sriov/README.md
+++ b/roles/sriov/README.md
@@ -26,9 +26,16 @@ Example Playbook
 - name: configure sr-iov
   hosts: compute
   vars:
+    # Use deprecated udev numvfs driver by default
+    sriov_numvfs_driver: udev
     sriov_devices:
       - name: p4p1
         numvfs: 63
+        # Per device override of sriov_numvfs_driver
+        numvfs_driver: systemd
+        # Do not start docker until virtual functions are loaded
+        numvfs_required_by:
+          - docker.service
       - name: p3p1
         numvfs: 8
         # Don't add a udev rule to set numvfs. This can be useful if you use an alternative method
diff --git a/roles/sriov/defaults/main.yml b/roles/sriov/defaults/main.yml
index 17376ce..26cbc0d 100644
--- a/roles/sriov/defaults/main.yml
+++ b/roles/sriov/defaults/main.yml
@@ -21,6 +21,13 @@ sriov_restart_handler: reboot
 
 sriov_numvfs: 8
 
+# One of: systemd, or udev. Default is 'systemd'.
+sriov_numvfs_driver: systemd
+# List of services that should wait until numvfs has been set. Only applicable
+# to the systemd numvfs driver.
+sriov_numvfs_required_by: []
+
+# Path to udev rules when using  srivov_numvfs_driver == 'udev'.
 sriov_udev_rule_path: /etc/udev/rules.d/70-sriov.rules
 
 sriov_mellanox_vendor_ids:
diff --git a/roles/sriov/tasks/config.yml b/roles/sriov/tasks/config.yml
index 357dfaf..0c4e104 100644
--- a/roles/sriov/tasks/config.yml
+++ b/roles/sriov/tasks/config.yml
@@ -4,7 +4,7 @@
     path: "{{ sriov_udev_rule_path }}"
     block: |
       {% for device in sriov_devices %}
-      {% if device.on_boot_configuration_enabled | default(true) | bool %}
+      {% if device.on_boot_configuration_enabled | default(true) | bool and device.numvfs_driver | default(sriov_numvfs_driver) == 'udev' %}
       SUBSYSTEM=="net", ACTION=="add", KERNEL=="{{ device.name }}", RUN+="/usr/bin/sh -c 'echo {{ device.numvfs | default(sriov_numvfs) }} > /sys/class/net/{{ device.name }}/device/sriov_numvfs'"
       {% endif %}
       {% endfor %}
@@ -16,6 +16,47 @@
   become: true
   notify: "{{ sriov_restart_handler }}"
 
+- name: Persist sriov_numvfs with systemd unit
+  ansible.builtin.template:
+    dest: "/etc/systemd/system/virtual-functions-{{ device.name }}.service"
+    src: "virtual-function.service.j2"
+    mode: "0644"
+    owner: root
+    group: root
+  loop: "{{ sriov_devices }}"
+  loop_control:
+    loop_var: device
+  become: true
+  when:
+    - device.numvfs_driver | default(sriov_numvfs_driver) == 'systemd'
+    - device.on_boot_configuration_enabled | default(true)
+
+- name: Ensure systemd unit is removed if on boot configuration is disabled
+  ansible.builtin.file:
+    path: "/etc/systemd/system/virtual-functions-{{ device.name }}.service"
+    state: absent
+  loop: "{{ sriov_devices }}"
+  loop_control:
+    loop_var: device
+  become: true
+  when:
+    - not device.on_boot_configuration_enabled | default(true)
+
+- name: Enable sriov_numvfs systemd unit
+  ansible.builtin.systemd:
+    name: "virtual-functions-{{ device.name }}.service"
+    enabled: true
+    daemon_reload: true
+  loop: "{{ sriov_devices }}"
+  loop_control:
+    loop_var: device
+  become: true
+  notify:
+    - "{{ sriov_restart_handler }}"
+  when:
+    - device.numvfs_driver | default(sriov_numvfs_driver) == 'systemd'
+    - device.on_boot_configuration_enabled | default(true)
+
 - name: Add iommu to kernel command line (Intel)
   ansible.builtin.include_role:
     name: stackhpc.linux.grubcmdline
diff --git a/roles/sriov/templates/virtual-function.service.j2 b/roles/sriov/templates/virtual-function.service.j2
new file mode 100644
index 0000000..4cce4e9
--- /dev/null
+++ b/roles/sriov/templates/virtual-function.service.j2
@@ -0,0 +1,25 @@
+[Unit]
+Description=Adds virtual functions for {{ device.name }}
+Requires=sys-subsystem-net-devices-{{ device.name }}.device
+After=sys-subsystem-net-devices-{{ device.name }}.device
+Requires=network-pre.target
+Before=network-pre.target
+{% if ansible_facts.os_family == "Debian" %}
+Requires=systemd-networkd.service
+Before=systemd-networkd.service
+{% endif %}
+
+[Service]
+Type=oneshot
+ExecStart=/usr/bin/sh -c 'echo {{ device.numvfs | default(sriov_numvfs) }} > /sys/class/net/{{ device.name }}/device/sriov_numvfs'
+RemainAfterExit=yes
+
+[Install]
+WantedBy=network-pre.target
+{% if ansible_facts.os_family == "Debian" %}
+WantedBy=systemd-networkd.service
+{% endif %}
+{% set required_by = device.numvfs_required_by | default(sriov_numvfs_required_by) %}
+{% for item in required_by %}
+RequiredBy={{ item }}
+{% endfor %}
\ No newline at end of file