Fix handling of SR-IOV interface configuration

SR-IOV interfaces are currently only configured on charm
installation and not after seubsequent reboots.

The VFs need to be configured before the Neutron SR-IOV
agent is started. Charms should also really not be involved
in boot time system configuration. Due to these factors
this commit adds a init script and corrensponding systemd
unit file and upstart job to handle the boot-time configuration.

Keep configure_sriov function for runtime configuration. Add
warning about runtime configuration disrupting network service.

Add restart of Neutron SR-IOV agent after runtime configuration.

Cap value of sriov-numvfs at each interfaces sriov_totalvfs value.

Change-Id: I7bde7217bf027db09ded35a262c214ccb11d6d86
Closes-Bug: #1697572

.gitignore

@@ -6,3 +6,4 @@ tags
 *.sw[nop]
 *.pyc
 .unit-state.db
+.pydevproject

.pydevproject

@@ -1,9 +0,0 @@
-<?xml version="1.0" encoding="UTF-8" standalone="no"?>
-<?eclipse-pydev version="1.0"?><pydev_project>
-<pydev_property name="org.python.pydev.PYTHON_PROJECT_VERSION">python 2.7</pydev_property>
-<pydev_property name="org.python.pydev.PYTHON_PROJECT_INTERPRETER">Default</pydev_property>
-<pydev_pathproperty name="org.python.pydev.PROJECT_SOURCE_PATH">
-<path>/neutron-openvswitch/hooks</path>
-<path>/neutron-openvswitch/unit_tests</path>
-</pydev_pathproperty>
-</pydev_project>

config.yaml

@@ -207,6 +207,10 @@ options:
       .
       Multiple devices can be configured by providing multi values delimited by
       spaces.
+      .
+      NOTE: Changing this value will disrupt networking on all SR-IOV capable
+      interfaces for blanket configuration or listed interfaces when per-device
+      configuration is used.
   worker-multiplier:
     type: float
     default:

hooks/neutron_ovs_context.py

@@ -143,6 +143,20 @@ class OVSPluginContext(context.NeutronContext):
                 ','.join(sriov_mappings.split())
             )
 
+        enable_sriov = config('enable-sriov')
+        if enable_sriov:
+            ovs_ctxt['enable_sriov'] = True
+
+        sriov_numvfs = config('sriov-numvfs')
+        if sriov_numvfs:
+            try:
+                if sriov_numvfs != 'auto':
+                    int(sriov_numvfs)
+            except ValueError:
+                ovs_ctxt['sriov_vfs_list'] = sriov_numvfs
+            else:
+                ovs_ctxt['sriov_vfs_blanket'] = sriov_numvfs
+
         flat_providers = config('flat-network-providers')
         if flat_providers:
             ovs_ctxt['network_providers'] = ','.join(flat_providers.split())

hooks/neutron_ovs_hooks.py

@@ -73,8 +73,10 @@ def config_changed():
             git_install(config('openstack-origin-git'))
 
     configure_ovs()
-    configure_sriov()
     CONFIGS.write_all()
+    # NOTE(fnordahl): configure_sriov must be run after CONFIGS.write_all()
+    # to allow us to enable boot time execution of init script
+    configure_sriov()
     for rid in relation_ids('zeromq-configuration'):
         zeromq_configuration_relation_joined(rid)
     for rid in relation_ids('neutron-plugin'):

hooks/neutron_ovs_utils.py

@@ -147,6 +147,18 @@ OVS_DEFAULT = '/etc/default/openvswitch-switch'
 DPDK_INTERFACES = '/etc/dpdk/interfaces'
 NEUTRON_SRIOV_AGENT_CONF = os.path.join(NEUTRON_CONF_DIR,
                                         'plugins/ml2/sriov_agent.ini')
+NEUTRON_SRIOV_INIT_SCRIPT = os.path.join('/etc/init.d',
+                                         'neutron-openvswitch-'
+                                         'networking-sriov.sh')
+NEUTRON_SRIOV_INIT_DEFAULT = os.path.join('/etc/default',
+                                          'neutron-openvswitch-'
+                                          'networking-sriov')
+NEUTRON_SRIOV_SYSTEMD_UNIT = os.path.join('/lib/systemd/system',
+                                          'neutron-openvswitch-'
+                                          'networking-sriov.service')
+NEUTRON_SRIOV_UPSTART_CONF = os.path.join('/etc/init',
+                                          'neutron-openvswitch-'
+                                          'networking-sriov.conf')
 
 BASE_RESOURCE_MAP = OrderedDict([
     (NEUTRON_CONF, {
@@ -218,6 +230,22 @@ SRIOV_RESOURCE_MAP = OrderedDict([
         'services': ['neutron-sriov-agent'],
         'contexts': [neutron_ovs_context.OVSPluginContext()],
     }),
+    (NEUTRON_SRIOV_INIT_DEFAULT, {
+        'services': [],
+        'contexts': [neutron_ovs_context.OVSPluginContext()],
+    }),
+    (NEUTRON_SRIOV_INIT_SCRIPT, {
+        'services': [],
+        'contexts': [],
+    }),
+    (NEUTRON_SRIOV_SYSTEMD_UNIT, {
+        'services': [],
+        'contexts': [],
+    }),
+    (NEUTRON_SRIOV_UPSTART_CONF, {
+        'services': [],
+        'contexts': [],
+    }),
 ])
 
 TEMPLATES = 'templates/'
@@ -279,7 +307,7 @@ def determine_packages():
     if use_dpdk():
         pkgs.append('openvswitch-switch-dpdk')
 
-    if enable_sriov_agent():
+    if enable_sriov():
         if cmp_release >= 'mitaka':
             pkgs.append('neutron-sriov-agent')
         else:
@@ -331,7 +359,7 @@ def resource_map():
     else:
         drop_config.extend([OVS_CONF, DPDK_INTERFACES])
 
-    if enable_sriov_agent():
+    if enable_sriov():
         sriov_agent_name = 'neutron-sriov-agent'
         sriov_resource_map = deepcopy(SRIOV_RESOURCE_MAP)
 
@@ -459,10 +487,23 @@ def configure_ovs():
 
 
 def configure_sriov():
-    '''Configure SR-IOV devices based on provided configuration options'''
+    '''Configure SR-IOV devices based on provided configuration options
+
+    NOTE(fnordahl): Boot time configuration is done by init script
+    intalled by this charm.
+
+    This function only does runtime configuration!
+    '''
     charm_config = config()
-    enable_sriov = charm_config.get('enable-sriov')
+    if not enable_sriov():
-    if enable_sriov and charm_config.changed('sriov-numvfs'):
+        return
+
+    # make sure init script has correct mode and that boot time execution
+    # is enabled
+    os.chmod(NEUTRON_SRIOV_INIT_SCRIPT, 0o755)
+    service('enable', 'neutron-openvswitch-networking-sriov')
+
+    if charm_config.changed('sriov-numvfs'):
         devices = PCINetDevices()
         sriov_numvfs = charm_config.get('sriov-numvfs')
 
@@ -474,19 +515,31 @@ def configure_sriov():
                     log("Configuring SR-IOV device"
                         " {} with {} VF's".format(device.interface_name,
                                                   device.sriov_totalvfs))
+                    # NOTE(fnordahl): run-time change of numvfs is disallowed
+                    # without resetting to 0 first.
+                    device.set_sriov_numvfs(0)
                     device.set_sriov_numvfs(device.sriov_totalvfs)
         else:
             # Single int blanket configuration
             try:
-                sriov_numvfs = int(sriov_numvfs)
                 log('Configuring SR-IOV device VF functions'
                     ' with blanket setting')
                 for device in devices.pci_devices:
                     if device and device.sriov:
-                        log("Configuring SR-IOV device"
+                        numvfs = min(int(sriov_numvfs), device.sriov_totalvfs)
-                            " {} with {} VF's".format(device.interface_name,
+                        if int(sriov_numvfs) > device.sriov_totalvfs:
-                                                      sriov_numvfs))
+                            log('Requested value for sriov-numvfs ({}) too '
-                        device.set_sriov_numvfs(sriov_numvfs)
+                                'high for interface {}. Falling back to '
+                                'interface totalvfs '
+                                'value: {}'.format(sriov_numvfs,
+                                                   device.interface_name,
+                                                   device.sriov_totalvfs))
+                        log("Configuring SR-IOV device {} with {} "
+                            "VFs".format(device.interface_name, numvfs))
+                        # NOTE(fnordahl): run-time change of numvfs is
+                        # disallowed without resetting to 0 first.
+                        device.set_sriov_numvfs(0)
+                        device.set_sriov_numvfs(numvfs)
             except ValueError:
                 # <device>:<numvfs>[ <device>:numvfs] configuration
                 sriov_numvfs = sriov_numvfs.split()
@@ -496,14 +549,33 @@ def configure_sriov():
                     device = devices.get_device_from_interface_name(
                         interface_name)
                     if device and device.sriov:
-                        log("Configuring SR-IOV device"
+                        if int(numvfs) > device.sriov_totalvfs:
-                            " {} with {} VF's".format(device.interface_name,
+                            log('Requested value for sriov-numfs ({}) too '
-                                                      numvfs))
+                                'high for interface {}. Falling back to '
+                                'interface totalvfs '
+                                'value: {}'.format(numvfs,
+                                                   device.interface_name,
+                                                   device.sriov_totalvfs))
+                            numvfs = device.sriov_totalvfs
+                        log("Configuring SR-IOV device {} with {} "
+                            "VF's".format(device.interface_name, numvfs))
+                        # NOTE(fnordahl): run-time change of numvfs is
+                        # disallowed without resetting to 0 first.
+                        device.set_sriov_numvfs(0)
                         device.set_sriov_numvfs(int(numvfs))
 
         # Trigger remote restart in parent application
         remote_restart('neutron-plugin', 'nova-compute')
 
+        # Restart of SRIOV agent is required after changes to system runtime
+        # VF configuration
+        cmp_release = CompareOpenStackReleases(
+            os_release('neutron-common', base='icehouse'))
+        if cmp_release >= 'mitaka':
+            service_restart('neutron-sriov-agent')
+        else:
+            service_restart('neutron-plugin-sriov-agent')
+
 
 def get_shared_secret():
     ctxt = neutron_ovs_context.SharedSecretContext()()
@@ -533,8 +605,8 @@ def use_dpdk():
     return (cmp_release >= 'mitaka' and config('enable-dpdk'))
 
 
-def enable_sriov_agent():
+def enable_sriov():
-    '''Determine with SR-IOV agent should be used'''
+    '''Determine whether SR-IOV is enabled and supported'''
     cmp_release = CompareOpenStackReleases(
         os_release('neutron-common', base='icehouse'))
     return (cmp_release >= 'kilo' and config('enable-sriov'))

templates/kilo/neutron-openvswitch-networking-sriov

@@ -0,0 +1,26 @@
+# Set to 1 to configure Virtual Functions on system startup.
+{% if enable_sriov -%}
+ENABLE=1
+{% else -%}
+ENABLE=0
+{% endif -%}
+
+# Blanket configuration for number of Virtual Functions across all NICs
+#
+# Possible configurations:
+# auto - Set sriov_numvfs to value of sriov_totalvfs for each interface
+# N    - Set sriov_numvfs on all interfaces to N or value of sriov_totalvfs
+#        if sriov_totalvfs is less than N for that interface
+#
+{% if sriov_vfs_blanket -%}
+VFS_BLANKET={{ sriov_vfs_blanket }}
+{% else -%}
+VFS_BLANKET=auto
+{% endif -%}
+
+# List of <device>:<numvfs> tuples for configuration of specific NICs
+#
+#VFS_LIST=ens3p0:16 ens4p0:16
+{% if sriov_vfs_list -%}
+VFS_LIST={{ sriov_vfs_list }}
+{% endif -%}

templates/kilo/neutron-openvswitch-networking-sriov.conf

@@ -0,0 +1,11 @@
+description "Configure SRIOV Virtual Functions"
+author "Frode Nordahl <frode.nordahl@gmail.com>"
+
+start on virtual-filesystems
+
+task
+console log
+script
+    [ -x "/etc/init.d/neutron-openvswitch-networking-sriov.sh" ] || exit 0
+    exec /etc/init.d/neutron-openvswitch-networking-sriov.sh start
+end script

templates/kilo/neutron-openvswitch-networking-sriov.service

@@ -0,0 +1,19 @@
+[Unit]
+Description=Configure SRIOV Virtual Functions
+DefaultDependencies=no
+Wants=network.target
+After=local-fs.target network-pre.target apparmor.service systemd-sysctl.service systemd-modules-load.service
+Before=network.target shutdown.target network-online.target
+Conflicts=shutdown.target
+
+[Install]
+WantedBy=multi-user.target
+WantedBy=network-online.target
+
+[Service]
+Type=oneshot
+EnvironmentFile=-/etc/default/networking-sriov
+ExecStart=/etc/init.d/neutron-openvswitch-networking-sriov.sh systemd-start
+ExecStop=/etc/init.d/neutron-openvswitch-networking-sriov.sh systemd-stop
+RemainAfterExit=true
+TimeoutStartSec=5min

templates/kilo/neutron-openvswitch-networking-sriov.sh

@@ -0,0 +1,121 @@
+#!/bin/sh
+
+### BEGIN INIT INFO
+# Provides:          networking-sriov
+# Required-Start:    mountkernfs $local_fs
+# Required-Stop:     $local_fs
+# Default-Start:     S
+# Default-Stop:      0 6
+# Short-Description: Configure SRIOV Virtual Functions
+### END INIT INFO
+
+# Authors: Frode Nordahl <frode.nordahl@gmail.com>
+
+DESC="Configure SRIOV Virtual Functions"
+
+. /lib/lsb/init-functions
+
+# Include defaults if available
+if [ -f /etc/default/neutron-openvswitch-networking-sriov ] ; then
+    . /etc/default/neutron-openvswitch-networking-sriov
+fi
+
+if [ -z "${ENABLE}" ]; then
+    ENABLE=0
+fi
+
+if [ -z "${VFS_BLANKET}" ]; then
+    VFS_BLANKET=auto
+fi
+
+# Exit if feature is not enabled
+[ $ENABLE -gt 0 ] || exit 0
+
+do_wait_for_vfs() {
+    # Wait for VFs to be created
+    PCI_DEVICE_PATH=$1
+    NUMVFS=$2
+    [ -z "${DEBUG}" ] || echo "do_wait_for_vfs ${PCI_DEVICE_PATH} ${NUMVFS}"
+    while [ `ls -1 "${PCI_DEVICE_PATH}/" | wc -l` -lt "${NUMVFS}" ]; do
+        [ -z "${DEBUG}" ] || echo wait...
+        sleep 0.05
+    done
+}
+
+do_configure_vfs() {
+    if [ ! -z "${VFS_LIST}" ]; then
+        # Do configuration according to list of <device>:<numvfs> tuples
+        OIFS="$IFS"
+        echo "${VFS_LIST}" | \
+            while IFS=':' read DEVICE NUMVFS; do
+                # Check whether we should stop
+                [ -z "${STOP}" ] || NUMVFS=0
+                [ -z "${DEBUG}" ] || echo "echo ${NUMVFS} \> /sys/class/net/${DEVICE}/device/sriov_numvfs"
+                echo "${NUMVFS}" > "/sys/class/net/${DEVICE}/device/sriov_numvfs"
+                do_wait_for_vfs "/sys/class/net/${DEVICE}/device" "${NUMVFS}"
+            done
+        IFS="$OIFS"
+    else
+        # Do blanket configuration
+        SYSFS_LST=`find /sys/devices -name sriov_totalvfs`
+        for ENT in $SYSFS_LST; do
+            DENT=`dirname $ENT`
+            if [ -d ${DENT}/net ]; then
+                TOTALVFS=`cat "${ENT}"`
+                [ -z "${STOP}" ] || VFS_BLANKET=0
+                if [ "${VFS_BLANKET}" = "auto" ]; then
+                    # Set sriov_numvfs to value of sriov_toatlvfs for "auto"
+                    NUMVFS=$TOTALVFS
+                elif [ "${VFS_BLANKET}" -gt "${TOTALVFS}" ]; then
+                    # Set sriov_numvfs to value of sriov_totalvfs if
+                    # requested number is larger than sriov_totalvfs
+                        NUMVFS=$TOTALVFS
+                else
+                        NUMVFS=$VFS_BLANKET
+                fi
+                # Set sriov_numvfs to requested number
+                [ -z "${DEBUG}" ] || echo "echo ${NUMVFS} \> ${DENT}/sriov_numvfs"
+                echo "${NUMVFS}" > "${DENT}/sriov_numvfs"
+                do_wait_for_vfs "${DENT}" "${NUMVFS}"
+            fi
+        done
+    fi
+}
+
+do_start() {
+    do_configure_vfs
+}
+
+do_stop() {
+    STOP=1
+    do_configure_vfs
+}
+
+
+case "$1" in
+  start)
+      log_daemon_msg "$DESC"
+      do_start
+      ;;
+  stop)
+      log_daemon_msg "Un-$DESC"
+      do_stop
+      ;;
+  systemd-start)
+      do_start
+      ;;
+  systemd-stop)
+      do_stop
+      ;;
+  restart)
+      log_daemon_msg "Re-$DESC"
+      do_stop
+      do_start
+      ;;
+  *)
+      N=/etc/init.d/neutron-openvswitch-networking-sriov.sh
+      echo "Usage: $N {start|stop|restart|systemd-start|systemd-stop}" >&2
+      ;;
+esac
+
+exit 0

unit_tests/test_neutron_ovs_context.py

@@ -243,6 +243,7 @@ class OVSPluginContextTest(CharmTestCase):
             'overlay_network_type': 'gre',
             'polling_interval': 2,
             'rpc_response_timeout': 60,
+            'sriov_vfs_blanket': 'auto',
             'report_interval': 30,
             'bridge_mappings': 'physnet1:br-data',
             'vlan_ranges': 'physnet1:1000:2000',

unit_tests/test_neutron_ovs_utils.py

@@ -108,6 +108,7 @@ class TestNeutronOVSUtils(CharmTestCase):
 
     @patch.object(nutils, 'determine_packages')
     def test_install_packages(self, _determine_packages):
+        self.os_release.return_value = 'mitaka'
         _determine_packages.return_value = 'randompkg'
         nutils.install_packages()
         self.apt_update.assert_called_with()
@@ -116,6 +117,7 @@ class TestNeutronOVSUtils(CharmTestCase):
 
     @patch.object(nutils, 'determine_packages')
     def test_install_packages_dkms_needed(self, _determine_packages):
+        self.os_release.return_value = 'mitaka'
         _determine_packages.return_value = 'randompkg'
         self.determine_dkms_package.return_value = \
             ['openvswitch-datapath-dkms']
@@ -289,11 +291,11 @@ class TestNeutronOVSUtils(CharmTestCase):
         [self.assertIn(q_conf, _map.keys()) for q_conf in confs]
         self.assertEqual(_map[nutils.NEUTRON_CONF]['services'], svcs)
 
-    @patch.object(nutils, 'enable_sriov_agent')
+    @patch.object(nutils, 'enable_sriov')
     @patch.object(nutils, 'use_dvr')
-    def test_resource_map_kilo_sriov(self, _use_dvr, _enable_sriov_agent):
+    def test_resource_map_kilo_sriov(self, _use_dvr, _enable_sriov):
         _use_dvr.return_value = False
-        _enable_sriov_agent.return_value = True
+        _enable_sriov.return_value = True
         self.os_release.return_value = 'kilo'
         self.lsb_release.return_value = {'DISTRIB_CODENAME': 'trusty'}
         _map = nutils.resource_map()
@@ -316,11 +318,11 @@ class TestNeutronOVSUtils(CharmTestCase):
         [self.assertIn(q_conf, _map.keys()) for q_conf in confs]
         self.assertEqual(_map[nutils.NEUTRON_CONF]['services'], svcs)
 
-    @patch.object(nutils, 'enable_sriov_agent')
+    @patch.object(nutils, 'enable_sriov')
     @patch.object(nutils, 'use_dvr')
-    def test_resource_map_mitaka_sriov(self, _use_dvr, _enable_sriov_agent):
+    def test_resource_map_mitaka_sriov(self, _use_dvr, _enable_sriov):
         _use_dvr.return_value = False
-        _enable_sriov_agent.return_value = True
+        _enable_sriov.return_value = True
         self.os_release.return_value = 'mitaka'
         self.lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'}
         _map = nutils.resource_map()
@@ -745,7 +747,9 @@ class TestNeutronOVSUtils(CharmTestCase):
         mock_pci_devices.get_device_from_interface_name.side_effect = \
             lambda x: self.pci_devices.get(x)
 
-    def test_configure_sriov_no_changes(self):
+    @patch('os.chmod')
+    def test_configure_sriov_no_changes(self, _os_chmod):
+        self.os_release.return_value = 'kilo'
         _config = {
             'enable-sriov': True,
             'sriov-numvfs': 'auto'
@@ -756,7 +760,9 @@ class TestNeutronOVSUtils(CharmTestCase):
 
         self.assertFalse(self.remote_restart.called)
 
-    def test_configure_sriov_auto(self):
+    @patch('os.chmod')
+    def test_configure_sriov_auto(self, _os_chmod):
+        self.os_release.return_value = 'mitaka'
         _config = {
             'enable-sriov': True,
             'sriov-numvfs': 'auto'
@@ -773,7 +779,9 @@ class TestNeutronOVSUtils(CharmTestCase):
         )
         self.assertTrue(self.remote_restart.called)
 
-    def test_configure_sriov_numvfs(self):
+    @patch('os.chmod')
+    def test_configure_sriov_numvfs(self, _os_chmod):
+        self.os_release.return_value = 'mitaka'
         _config = {
             'enable-sriov': True,
             'sriov-numvfs': '32',
@@ -787,7 +795,9 @@ class TestNeutronOVSUtils(CharmTestCase):
 
         self.assertTrue(self.remote_restart.called)
 
-    def test_configure_sriov_numvfs_per_device(self):
+    @patch('os.chmod')
+    def test_configure_sriov_numvfs_per_device(self, _os_chmod):
+        self.os_release.return_value = 'kilo'
         _config = {
             'enable-sriov': True,
             'sriov-numvfs': 'ens0:32 sriov23:64'