Merge ~davecore/charm-sysconfig:master-charm-sysconfig-mellanox-ringsize into charm-sysconfig:master

[David Coronel]

Added Mellanox ConnectX ringsize support

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Alvaro Uria (aluria)]

Hi David. I've added several comments inline. Could you please also add unit tests for the changes applied as well as file a bug this MP can be linked to?

Thank you.

review: Needs Fixing

[David Coronel (davecore)]

Hi Alvaro. Sorry for taking so long to get back to this. I added comments inline as well.

I filed this LP bug https://bugs.launchpad.net/charm-sysconfig/+bug/1869233

[Alvaro Uria (aluria)]

Hi David. I have added some replies inline. Thank you.

[Drew Freiberger (afreiberger)]

I'm really not sold on determining the nics in the charm. If this is part of networkd-dispatcher service, shouldn't the startup script probe current connectx nics at boot time and configure them all based on their current names. what happens if you add a new nic, or reseat a nic in the wrong slot.

I'd assume the charm wouldn't update the startup script until well after startup, which both triggers issues of timing of network card changes to when there are applications running, and potentially not having the changes take effect until a second reboot of the host taking place.

I might suggest moving all the syshelper hook logic into the script and only templatize the size to set, or even just drop the size as another file on disk rather than having a python/jinja generated shell script.

Also, +1 Alvaro's suggestion to have an action for "tune-connectx-rings" as an operator timed action to happen after the startup script and ring size configuration have landed.

Unmerged commits

a8e89de... by David Coronel on 2020-02-06

added removal of mellanox changes and missing install step

496af1c... by David Coronel on 2020-02-05

added exit 0 to script in case ringsize is already same size

7fbf73f... by David Coronel on 2020-02-04

Added Mellanox ConnectX ringsize support

diff --git a/src/config.yaml b/src/config.yaml
index bd60238..d30aee2 100644
--- a/src/config.yaml
+++ b/src/config.yaml
@@ -122,3 +122,9 @@ options:
                         Recommended option when Bios control power is set to the OS.
         - 'performance'
         - 'powersave'
+  mellanox-connectx-ringsize:
+    type: string
+    default: ""
+    description: |
+      Configure the value of RX/TX ring size for Mellanox Connect-X cards
+      to this value.
diff --git a/src/lib/lib_sysconfig.py b/src/lib/lib_sysconfig.py
index 51bf049..a58cfa7 100644
--- a/src/lib/lib_sysconfig.py
+++ b/src/lib/lib_sysconfig.py
@@ -16,11 +16,13 @@ GRUB_DEFAULT = 'Advanced options for Ubuntu>Ubuntu, with Linux {}'
 CPUFREQUTILS_TMPL = 'cpufrequtils.j2'
 GRUB_CONF_TMPL = 'grub.j2'
 SYSTEMD_SYSTEM_TMPL = 'etc.systemd.system.conf.j2'
+SET_RINGSIZE_SCRIPT_TMPL = 'set-ringsize.sh.j2'
 
 CPUFREQUTILS = '/etc/default/cpufrequtils'
 GRUB_CONF = '/etc/default/grub.d/90-sysconfig.cfg'
 SYSTEMD_SYSTEM = '/etc/systemd/system.conf'
 KERNEL = 'kernel'
+SET_RINGSIZE_SCRIPT = '/usr/lib/networkd-dispatcher/routable.d/set-ringsize.sh'
 
 
 def parse_config_flags(config_flags):
@@ -185,6 +187,27 @@ class SysConfigHelper:
         flags = config_flags_parser(self.charm_config['config-flags'])
         return flags
 
+    @property
+    def mellanox_connectx_ringsize(self):
+        """Return mellanox-connectx-ringsize config option."""
+        return self.charm_config['mellanox-connectx-ringsize']
+
+    @property
+    def mellanox_connectx_nics(self):
+        """Return Mellanoc ConnectX NICs."""
+        if not self.charm_config.get('mellanox-connectx-ringsize'):
+            return []
+        mellanoxdevices = []
+        for filename in os.listdir('/sys/class/net/'):
+            fullpath = '/sys/class/net/' + filename + '/device/uevent'
+            if os.path.isfile(fullpath):
+                with open(fullpath) as f:
+                    for line in f:
+                        if 'DRIVER=mlx5_core' in line:
+                            mellanoxdevices.append(filename)
+                            break
+        return mellanoxdevices
+
     def _render_boot_resource(self, source, target, context):
         """Render the template and set the resource as changed."""
         render(source=source, templates_dir='templates', target=target, context=context)
@@ -313,6 +336,21 @@ class SysConfigHelper:
 
         host.service_restart('cpufrequtils')
 
+    def configure_mellanox_connectx_ringsize(self):
+        """Configure Mellanox ringsize as given by the mellanox-connectx-ringsize option.
+
+        Will generate networkd-dispatcher script and run it as well
+        """
+        context = {}
+        if self.mellanox_connectx_nics:
+            context['mellanox_connectx_nics'] = self.mellanox_connectx_nics
+        if self.mellanox_connectx_ringsize:
+            context['mellanox_connectx_ringsize'] = self.mellanox_connectx_ringsize
+        self._render_boot_resource(SET_RINGSIZE_SCRIPT_TMPL, SET_RINGSIZE_SCRIPT, context)
+        hookenv.log('Mellanox ConnectX ringsize script updated via networkd-dispatcher')
+        subprocess.check_call(['/bin/chmod', '755', SET_RINGSIZE_SCRIPT])
+        subprocess.check_call([SET_RINGSIZE_SCRIPT])
+
     def remove_grub_configuration(self):
         """Remove /etc/default/grub.d/90-sysconfig.cfg if exists.
 
@@ -361,3 +399,11 @@ class SysConfigHelper:
             hookenv.DEBUG
         )
         host.service_restart('cpufrequtils')
+
+    def remove_mellanox_connectx_ringsize(self):
+        """Remove Mellanox ringsize configuration.
+
+        Will remove networkd-dispatcher script.
+        """
+        os.remove(SET_RINGSIZE_SCRIPT)
+        hookenv.log('Mellanox ConnectX ringsize script removed')
diff --git a/src/reactive/sysconfig.py b/src/reactive/sysconfig.py
index eba0916..501ff0f 100644
--- a/src/reactive/sysconfig.py
+++ b/src/reactive/sysconfig.py
@@ -54,6 +54,7 @@ def install_sysconfig():
     syshelper.update_cpufreq()
     syshelper.update_grub_file()
     syshelper.update_systemd_system_file()
+    syshelper.configure_mellanox_connectx_ringsize()
     set_flag('sysconfig.installed')
     update_status()
 
@@ -103,6 +104,10 @@ def config_changed():
     )) or helpers.any_file_changed([SYSTEMD_SYSTEM]):
         syshelper.update_systemd_system_file()
 
+    # Mellanox ConnectX ringsize
+    if syshelper.charm_config.changed('mellanox-connectx-ringsize'):
+        syshelper.configure_mellanox_connectx_ringsize()
+
     update_status()
 
 
@@ -142,5 +147,6 @@ def remove_configuration():
     syshelper.remove_cpufreq_configuration()
     syshelper.remove_grub_configuration()
     syshelper.remove_systemd_configuration()
+    syshelper.remove_mellanox_connectx_ringsize()
     clear_flag('sysconfig.installed')
     clear_flag('sysconfig.unsupported')
diff --git a/src/templates/set-ringsize.sh.j2 b/src/templates/set-ringsize.sh.j2
new file mode 100644
index 0000000..024dfda
--- /dev/null
+++ b/src/templates/set-ringsize.sh.j2
@@ -0,0 +1,8 @@
+#!/bin/bash
+# Set rx-ringsize and tx-ringsize to card max
+{% if mellanox_connectx_nics is defined and mellanox_connectx_nics -%}
+{% for nic in mellanox_connectx_nics -%}
+/sbin/ethtool -G {{ nic }} rx {{ mellanox_connectx_ringsize }} tx {{ mellanox_connectx_ringsize }}
+{% endfor -%}
+{% endif -%}
+exit 0