From 4ffbc2fe25400abf55719a370f3a2cd37f90c99d Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Wed, 23 Aug 2017 12:35:47 +0200 Subject: [PATCH] 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 | 1 + .pydevproject | 9 -- config.yaml | 4 + hooks/neutron_ovs_context.py | 14 ++ hooks/neutron_ovs_hooks.py | 4 +- hooks/neutron_ovs_utils.py | 102 ++++++++++++--- .../kilo/neutron-openvswitch-networking-sriov | 26 ++++ .../neutron-openvswitch-networking-sriov.conf | 11 ++ ...utron-openvswitch-networking-sriov.service | 19 +++ .../neutron-openvswitch-networking-sriov.sh | 121 ++++++++++++++++++ unit_tests/test_neutron_ovs_context.py | 1 + unit_tests/test_neutron_ovs_utils.py | 30 +++-- 12 files changed, 307 insertions(+), 35 deletions(-) delete mode 100644 .pydevproject create mode 100644 templates/kilo/neutron-openvswitch-networking-sriov create mode 100644 templates/kilo/neutron-openvswitch-networking-sriov.conf create mode 100644 templates/kilo/neutron-openvswitch-networking-sriov.service create mode 100755 templates/kilo/neutron-openvswitch-networking-sriov.sh diff --git a/.gitignore b/.gitignore index 58a20d7c..5b57b4e1 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ tags *.sw[nop] *.pyc .unit-state.db +.pydevproject diff --git a/.pydevproject b/.pydevproject deleted file mode 100644 index d6fbb946..00000000 --- a/.pydevproject +++ /dev/null @@ -1,9 +0,0 @@ - - -python 2.7 -Default - -/neutron-openvswitch/hooks -/neutron-openvswitch/unit_tests - - diff --git a/config.yaml b/config.yaml index 5afc2490..08a649b8 100644 --- a/config.yaml +++ b/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: diff --git a/hooks/neutron_ovs_context.py b/hooks/neutron_ovs_context.py index 8d5df6b1..e5033b40 100644 --- a/hooks/neutron_ovs_context.py +++ b/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()) diff --git a/hooks/neutron_ovs_hooks.py b/hooks/neutron_ovs_hooks.py index d82774ab..4a0906cd 100755 --- a/hooks/neutron_ovs_hooks.py +++ b/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'): diff --git a/hooks/neutron_ovs_utils.py b/hooks/neutron_ovs_utils.py index 0fece95b..139ece3b 100644 --- a/hooks/neutron_ovs_utils.py +++ b/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 enable_sriov and charm_config.changed('sriov-numvfs'): + if not enable_sriov(): + 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" - " {} with {} VF's".format(device.interface_name, - sriov_numvfs)) - device.set_sriov_numvfs(sriov_numvfs) + numvfs = min(int(sriov_numvfs), device.sriov_totalvfs) + if int(sriov_numvfs) > device.sriov_totalvfs: + log('Requested value for sriov-numvfs ({}) too ' + '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: # :[ :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" - " {} with {} VF's".format(device.interface_name, - numvfs)) + if int(numvfs) > device.sriov_totalvfs: + log('Requested value for sriov-numfs ({}) too ' + '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(): - '''Determine with SR-IOV agent should be used''' +def enable_sriov(): + '''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')) diff --git a/templates/kilo/neutron-openvswitch-networking-sriov b/templates/kilo/neutron-openvswitch-networking-sriov new file mode 100644 index 00000000..f27f5800 --- /dev/null +++ b/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 : tuples for configuration of specific NICs +# +#VFS_LIST=ens3p0:16 ens4p0:16 +{% if sriov_vfs_list -%} +VFS_LIST={{ sriov_vfs_list }} +{% endif -%} diff --git a/templates/kilo/neutron-openvswitch-networking-sriov.conf b/templates/kilo/neutron-openvswitch-networking-sriov.conf new file mode 100644 index 00000000..a66cc5d6 --- /dev/null +++ b/templates/kilo/neutron-openvswitch-networking-sriov.conf @@ -0,0 +1,11 @@ +description "Configure SRIOV Virtual Functions" +author "Frode Nordahl " + +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 diff --git a/templates/kilo/neutron-openvswitch-networking-sriov.service b/templates/kilo/neutron-openvswitch-networking-sriov.service new file mode 100644 index 00000000..a6edf40a --- /dev/null +++ b/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 diff --git a/templates/kilo/neutron-openvswitch-networking-sriov.sh b/templates/kilo/neutron-openvswitch-networking-sriov.sh new file mode 100755 index 00000000..3576e278 --- /dev/null +++ b/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 + +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 : 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 diff --git a/unit_tests/test_neutron_ovs_context.py b/unit_tests/test_neutron_ovs_context.py index d54a8bcf..05836458 100644 --- a/unit_tests/test_neutron_ovs_context.py +++ b/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', diff --git a/unit_tests/test_neutron_ovs_utils.py b/unit_tests/test_neutron_ovs_utils.py index 0863d1af..bfda0287 100644 --- a/unit_tests/test_neutron_ovs_utils.py +++ b/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'