diff --git a/config.yaml b/config.yaml index c8f38f4a..5045d8ab 100644 --- a/config.yaml +++ b/config.yaml @@ -238,6 +238,14 @@ options: 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. + networking-tools-source: + type: string + default: ppa:openstack-charmers/networking-tools + description: | + Package archive source to use for utilities associated with configuring + SR-IOV VF's and switchdev mode in Mellanox network adapters. + . + This PPA can be mirrored for offline deployments. worker-multiplier: type: float default: diff --git a/hooks/neutron_ovs_hooks.py b/hooks/neutron_ovs_hooks.py index f9cf29c7..cdeebee5 100755 --- a/hooks/neutron_ovs_hooks.py +++ b/hooks/neutron_ovs_hooks.py @@ -71,8 +71,6 @@ from neutron_ovs_utils import ( pause_unit_helper, resume_unit_helper, determine_purge_packages, - install_sriov_systemd_files, - enable_sriov, use_fqdn_hint, ) @@ -116,10 +114,6 @@ def upgrade_charm(): # migrating. if 'Service restart triggered' not in f.read(): CONFIGS.write(OVS_DEFAULT) - # Ensure that the SR-IOV systemd files are copied if a charm-upgrade - # happens - if enable_sriov(): - install_sriov_systemd_files() @hooks.hook('neutron-plugin-relation-changed') @@ -150,9 +144,11 @@ def config_changed(): configure_ovs() 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('neutron-plugin'): neutron_plugin_joined( relation_id=rid, diff --git a/hooks/neutron_ovs_utils.py b/hooks/neutron_ovs_utils.py index 8cc1fb8a..f6699773 100644 --- a/hooks/neutron_ovs_utils.py +++ b/hooks/neutron_ovs_utils.py @@ -18,6 +18,7 @@ import os from itertools import chain import shutil import subprocess +import yaml from charmhelpers.contrib.openstack.neutron import neutron_plugin_attribute from copy import deepcopy @@ -66,7 +67,6 @@ from charmhelpers.contrib.openstack.context import ( ) from charmhelpers.core.host import ( lsb_release, - service, service_restart, service_running, CompareHostReleases, @@ -74,6 +74,7 @@ from charmhelpers.core.host import ( group_exists, user_exists, is_container, + restart_on_change ) from charmhelpers.core.kernel import ( modprobe, @@ -86,7 +87,8 @@ from charmhelpers.fetch import ( filter_installed_packages, filter_missing_packages, apt_autoremove, - get_upstream_version + get_upstream_version, + add_source, ) from pci import PCINetDevices @@ -259,6 +261,13 @@ DATA_BRIDGE = 'br-data' def install_packages(): + # NOTE(jamespage): + # networking-tools-source provides general tooling for configuration + # of SR-IOV VF's and Mellanox ConnectX switchdev capable adapters + # The default PPA published packages back to Xenial, which covers + # all target series for this charm. + if config('networking-tools-source'): + add_source(config('networking-tools-source')) apt_update() # NOTE(jamespage): install neutron-common package so we always # get a clear signal on which OS release is @@ -345,6 +354,7 @@ def determine_packages(): pkgs.append('neutron-sriov-agent') else: pkgs.append('neutron-plugin-sriov-agent') + pkgs.append('sriov-netplan-shim') if cmp_release >= 'rocky': pkgs = [p for p in pkgs if not p.startswith('python-')] @@ -546,14 +556,16 @@ def install_tmpfilesd(): subprocess.check_call(['systemd-tmpfiles', '--create']) -def install_sriov_systemd_files(): - '''Install SR-IOV systemd files''' - shutil.copy('files/neutron_openvswitch_networking_sriov.py', - '/usr/local/bin') - shutil.copy('files/neutron-openvswitch-networking-sriov.sh', - '/usr/local/bin') - shutil.copy('files/neutron-openvswitch-networking-sriov.service', - '/lib/systemd/system') +def purge_sriov_systemd_files(): + '''Purge obsolete SR-IOV configuration scripts''' + old_paths = [ + '/usr/local/bin/neutron_openvswitch_networking_sriov.py', + '/usr/local/bin/neutron-openvswitch-networking-sriov.sh', + '/lib/systemd/system/neutron-openvswitch-networking-sriov.service' + ] + for path in old_paths: + if os.path.exists(path): + os.remove(path) def configure_ovs(): @@ -573,6 +585,11 @@ def configure_ovs(): bridgemaps = None if not use_dpdk(): + # NOTE(jamespage): + # Its possible to support both hardware offloaded 'direct' ports + # and default 'openvswitch' ports on the same hypervisor, so + # configure bridge mappings in addition to any hardware offload + # enablement. portmaps = DataPortContext()() bridgemaps = parse_bridge_mappings(config('bridge-mappings')) for br in bridgemaps.values(): @@ -586,7 +603,8 @@ def configure_ovs(): add_bridge_port(br, port, promisc=True) else: add_ovsbridge_linuxbridge(br, port) - else: + + if use_dpdk(): log('Configuring bridges with DPDK', level=DEBUG) global_mtu = ( neutron_ovs_context.NeutronAPIContext()()['global_physnet_mtu']) @@ -678,7 +696,8 @@ def configure_ovs(): # provided. # NOTE(ajkavanagh) for pause/resume we don't gate this as it's not a # running service, but rather running a few commands. - service_restart('os-charm-phy-nic-mtu') + if not init_is_systemd(): + service_restart('os-charm-phy-nic-mtu') def _get_interfaces_from_mappings(sriov_mappings): @@ -692,90 +711,125 @@ def _get_interfaces_from_mappings(sriov_mappings): return interfaces +SRIOV_NETPLAN_SHIM_CONF = '/etc/sriov-netplan-shim/interfaces.yaml' + + +# TODO(jamespage) +# Drop all of this once MAAS and netplan can actually do SR-IOV +# configuration natively. def configure_sriov(): '''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! + This function writes out the configuration file for sriov-netplan-shim + which actually takes care of SR-IOV VF device configuration both + during configuration and post reboot. ''' - charm_config = config() - if not enable_sriov(): - return - - install_sriov_systemd_files() - # make sure that boot time execution is enabled - service('enable', 'neutron-openvswitch-networking-sriov') - - devices = PCINetDevices() - sriov_numvfs = charm_config.get('sriov-numvfs') - - # automatic configuration of all SR-IOV devices - if sriov_numvfs == 'auto': - interfaces = _get_interfaces_from_mappings( - charm_config.get('sriov-device-mappings')) - log('Configuring SR-IOV device VF functions in auto mode') - for device in devices.pci_devices: - if device and device.sriov: - if interfaces and device.interface_name not in interfaces: - log("Excluding configuration of SR-IOV device {}.".format( - device.interface_name)) - else: + @restart_on_change({SRIOV_NETPLAN_SHIM_CONF: ['sriov-netplan-shim']}) + def _write_interfaces_yaml(): + charm_config = config() + devices = PCINetDevices() + sriov_numvfs = charm_config.get('sriov-numvfs') + section = 'interfaces' + interfaces_yaml = { + section: {} + } + numvfs_changed = False + # automatic configuration of all SR-IOV devices + if sriov_numvfs == 'auto': + interfaces = _get_interfaces_from_mappings( + charm_config.get('sriov-device-mappings')) + log('Configuring SR-IOV device VF functions in auto mode') + for device in devices.pci_devices: + if device and device.sriov: + if interfaces and device.interface_name not in interfaces: + log("Excluding configuration of SR-IOV" + " device {}.".format(device.interface_name)) + continue log("Configuring SR-IOV device" " {} with {} VF's".format(device.interface_name, device.sriov_totalvfs)) - device.set_sriov_numvfs(device.sriov_totalvfs) - else: - # Single int blanket configuration - try: - log('Configuring SR-IOV device VF functions' - ' with blanket setting') - for device in devices.pci_devices: - if device and device.sriov: - 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)) - device.set_sriov_numvfs(numvfs) - except ValueError: - # :[ :numvfs] configuration - sriov_numvfs = sriov_numvfs.split() - for device_config in sriov_numvfs: - log('Configuring SR-IOV device VF functions per interface') - interface_name, numvfs = device_config.split(':') - device = devices.get_device_from_interface_name( - interface_name) - if device and device.sriov: - 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)) - device.set_sriov_numvfs(int(numvfs)) + interfaces_yaml[section][device.interface_name] = { + 'num_vfs': int(device.sriov_totalvfs) + } + numvfs_changed = ( + (device.sriov_totalvfs != device.sriov_numvfs) or + numvfs_changed + ) + else: + # Single int blanket configuration + try: + log('Configuring SR-IOV device VF functions' + ' with blanket setting') + for device in devices.pci_devices: + if device and device.sriov: + 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)) + interfaces_yaml[section][device.interface_name] = { + 'num_vfs': numvfs + } + numvfs_changed = ( + (numvfs != device.sriov_numvfs) or + numvfs_changed + ) + except ValueError: + # :[ :numvfs] configuration + sriov_numvfs = sriov_numvfs.split() + for device_config in sriov_numvfs: + log('Configuring SR-IOV device VF functions per interface') + interface_name, numvfs = device_config.split(':') + device = devices.get_device_from_interface_name( + interface_name) + if device and device.sriov: + 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)) + interfaces_yaml[section][device.interface_name] = { + 'num_vfs': int(numvfs) + } + numvfs_changed = ( + (numvfs != device.sriov_numvfs) or + numvfs_changed + ) + with open(SRIOV_NETPLAN_SHIM_CONF, 'w') as _conf: + yaml.dump(interfaces_yaml, _conf) + return numvfs_changed + + if not enable_sriov(): + return + + # Tidy up any prior installation of obsolete sriov startup + # scripts + purge_sriov_systemd_files() + + numvfs_changed = _write_interfaces_yaml() # 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') + if numvfs_changed: + # 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(): diff --git a/hooks/pci.py b/hooks/pci.py index 647cbc1e..abb347f9 100644 --- a/hooks/pci.py +++ b/hooks/pci.py @@ -174,28 +174,6 @@ class PCINetDevice(object): self.sriov_totalvfs = interface['sriov_totalvfs'] self.sriov_numvfs = interface['sriov_numvfs'] - def _set_sriov_numvfs(self, numvfs): - sdevice = os.path.join('/sys/class/net', - self.interface_name, - 'device', 'sriov_numvfs') - with open(sdevice, 'w') as sh: - sh.write(str(numvfs)) - self.update_attributes() - - def set_sriov_numvfs(self, numvfs): - """Set the number of VF devices for a SR-IOV PF - - Assuming the device is an SR-IOV device, this function will attempt - to change the number of VF's created by the PF. - - @param numvfs: integer to set the current number of VF's to - """ - if self.sriov and numvfs != self.sriov_numvfs: - # NOTE(fnordahl): run-time change of numvfs is disallowed - # without resetting to 0 first. - self._set_sriov_numvfs(0) - self._set_sriov_numvfs(numvfs) - class PCINetDevices(object): diff --git a/unit_tests/test_neutron_ovs_hooks.py b/unit_tests/test_neutron_ovs_hooks.py index 4ecbe1f9..cd4c0d42 100644 --- a/unit_tests/test_neutron_ovs_hooks.py +++ b/unit_tests/test_neutron_ovs_hooks.py @@ -85,7 +85,6 @@ class NeutronOVSHooksTests(CharmTestCase): _kv.assert_called_once_with() fake_dict.set.assert_called_once_with(hooks.USE_FQDN_KEY, True) - @patch('neutron_ovs_hooks.enable_sriov', MagicMock(return_value=False)) @patch.object(hooks, 'restart_map') @patch.object(hooks, 'restart_on_change') def test_migrate_ovs_default_file(self, mock_restart, mock_restart_map): diff --git a/unit_tests/test_neutron_ovs_utils.py b/unit_tests/test_neutron_ovs_utils.py index 66657b76..d9391658 100644 --- a/unit_tests/test_neutron_ovs_utils.py +++ b/unit_tests/test_neutron_ovs_utils.py @@ -13,7 +13,9 @@ # limitations under the License. import hashlib +import io import subprocess +import yaml from mock import MagicMock, patch, call from collections import OrderedDict @@ -26,6 +28,7 @@ import neutron_ovs_context from test_utils import ( CharmTestCase, + patch_open, ) import charmhelpers import charmhelpers.core.hookenv as hookenv @@ -41,6 +44,7 @@ TO_PATCH = [ 'dpdk_set_bond_config', 'dpdk_set_mtu_request', 'dpdk_set_interfaces_mtu', + 'add_source', 'apt_install', 'apt_update', 'config', @@ -50,7 +54,6 @@ TO_PATCH = [ 'lsb_release', 'neutron_plugin_attribute', 'full_restart', - 'service', 'service_restart', 'service_running', 'ExternalPortContext', @@ -365,6 +368,7 @@ class TestNeutronOVSUtils(CharmTestCase): head_pkg, 'neutron-plugin-openvswitch-agent', 'neutron-plugin-sriov-agent', + 'sriov-netplan-shim', ] self.assertEqual(pkg_list, expect) @@ -386,6 +390,7 @@ class TestNeutronOVSUtils(CharmTestCase): head_pkg, 'neutron-openvswitch-agent', 'neutron-sriov-agent', + 'sriov-netplan-shim', ] self.assertEqual(pkg_list, expect) @@ -933,14 +938,22 @@ class TestNeutronOVSUtils(CharmTestCase): } self._configure_sriov_base(_config) - nutils.configure_sriov() + with patch_open() as (_open, _file): + mock_stringio = io.StringIO() + _file.write.side_effect = mock_stringio.write + nutils.configure_sriov() + self.assertEqual( + yaml.load(mock_stringio.getvalue()), + { + 'interfaces': { + 'ens0': {'num_vfs': 64}, + 'ens49': {'num_vfs': 64} + } + } + ) - self.mock_sriov_device.set_sriov_numvfs.assert_called_with( - self.mock_sriov_device.sriov_totalvfs - ) - self.mock_sriov_device2.set_sriov_numvfs.assert_called_with( - self.mock_sriov_device2.sriov_totalvfs - ) + self.mock_sriov_device.set_sriov_numvfs.assert_not_called() + self.mock_sriov_device2.set_sriov_numvfs.assert_not_called() self.assertTrue(self.remote_restart.called) @patch('shutil.copy') @@ -954,12 +967,21 @@ class TestNeutronOVSUtils(CharmTestCase): } self._configure_sriov_base(_config) - nutils.configure_sriov() + with patch_open() as (_open, _file): + mock_stringio = io.StringIO() + _file.write.side_effect = mock_stringio.write + nutils.configure_sriov() + self.assertEqual( + yaml.load(mock_stringio.getvalue()), + { + 'interfaces': { + 'ens49': {'num_vfs': 64} + } + } + ) - self.assertFalse(self.mock_sriov_device.set_sriov_numvfs.called) - self.mock_sriov_device2.set_sriov_numvfs.assert_called_with( - self.mock_sriov_device2.sriov_totalvfs - ) + self.mock_sriov_device.set_sriov_numvfs.assert_not_called() + self.mock_sriov_device2.set_sriov_numvfs.assert_not_called() self.assertTrue(self.remote_restart.called) @patch('shutil.copy') @@ -972,11 +994,22 @@ class TestNeutronOVSUtils(CharmTestCase): } self._configure_sriov_base(_config) - nutils.configure_sriov() - - self.mock_sriov_device.set_sriov_numvfs.assert_called_with(32) - self.mock_sriov_device2.set_sriov_numvfs.assert_called_with(32) + with patch_open() as (_open, _file): + mock_stringio = io.StringIO() + _file.write.side_effect = mock_stringio.write + nutils.configure_sriov() + self.assertEqual( + yaml.load(mock_stringio.getvalue()), + { + 'interfaces': { + 'ens0': {'num_vfs': 32}, + 'ens49': {'num_vfs': 32} + } + } + ) + self.mock_sriov_device.set_sriov_numvfs.assert_not_called() + self.mock_sriov_device2.set_sriov_numvfs.assert_not_called() self.assertTrue(self.remote_restart.called) @patch('shutil.copy') @@ -989,30 +1022,21 @@ class TestNeutronOVSUtils(CharmTestCase): } self._configure_sriov_base(_config) - nutils.configure_sriov() + with patch_open() as (_open, _file): + mock_stringio = io.StringIO() + _file.write.side_effect = mock_stringio.write + nutils.configure_sriov() + self.assertEqual( + yaml.load(mock_stringio.getvalue()), + { + 'interfaces': { + 'ens0': {'num_vfs': 32}, + } + } + ) - self.mock_sriov_device.set_sriov_numvfs.assert_called_with(32) + self.mock_sriov_device.set_sriov_numvfs.assert_not_called() self.mock_sriov_device2.set_sriov_numvfs.assert_not_called() - - self.assertTrue(self.remote_restart.called) - - @patch('shutil.copy') - @patch('os.chmod') - def test_configure_sriov_auto_avoid_recall(self, _os_chmod, _sh_copy): - self.os_release.return_value = 'mitaka' - _config = { - 'enable-sriov': True, - 'sriov-numvfs': 'auto' - } - self._configure_sriov_base(_config) - - nutils.configure_sriov() - - self.mock_sriov_device2.sriov_numvfs = 64 - self.mock_sriov_device2.set_sriov_numvfs.assert_called_with( - self.mock_sriov_device2.sriov_totalvfs) - self.mock_sriov_device2._set_sriov_numvfs.assert_not_called() - self.assertTrue(self.remote_restart.called) @patch.object(nutils, 'subprocess') diff --git a/unit_tests/test_pci.py b/unit_tests/test_pci.py index e388f0d5..a5043b35 100644 --- a/unit_tests/test_pci.py +++ b/unit_tests/test_pci.py @@ -21,7 +21,7 @@ from test_pci_helper import ( mocked_islink, mocked_realpath, ) -from mock import patch, MagicMock, call +from mock import patch, MagicMock import pci TO_PATCH = [ @@ -178,56 +178,6 @@ class PCINetDeviceTest(CharmTestCase): self.assertEqual( pci.get_sysnet_interface('/sys/class/net/eth3'), 'eth3') - @patch('pci.get_sysnet_interfaces_and_macs') - def test__set_sriov_numvfs(self, mock_sysnet_ints): - mock_sysnet_ints.side_effect = [{ - 'interface': 'eth2', - 'mac_address': 'a8:9d:21:cf:93:fc', - 'pci_address': '0000:10:00.0', - 'state': 'up', - 'sriov': True, - 'sriov_totalvfs': 7, - 'sriov_numvfs': 0 - }], [{ - 'interface': 'eth2', - 'mac_address': 'a8:9d:21:cf:93:fc', - 'pci_address': '0000:10:00.0', - 'state': 'up', - 'sriov': True, - 'sriov_totalvfs': 7, - 'sriov_numvfs': 4 - }] - dev = pci.PCINetDevice('0000:10:00.0') - self.assertEqual('eth2', dev.interface_name) - self.assertTrue(dev.sriov) - self.assertEqual(7, dev.sriov_totalvfs) - self.assertEqual(0, dev.sriov_numvfs) - - with patch_open() as (mock_open, mock_file): - dev._set_sriov_numvfs(4) - mock_open.assert_called_with( - '/sys/class/net/eth2/device/sriov_numvfs', 'w') - mock_file.write.assert_called_with("4") - self.assertTrue(dev.sriov) - self.assertEqual(7, dev.sriov_totalvfs) - self.assertEqual(4, dev.sriov_numvfs) - - @patch('pci.PCINetDevice._set_sriov_numvfs') - def test_set_sriov_numvfs(self, mock__set_sriov_numvfs): - dev = pci.PCINetDevice('0000:10:00.0') - dev.sriov = True - dev.set_sriov_numvfs(4) - mock__set_sriov_numvfs.assert_has_calls([ - call(0), call(4)]) - - @patch('pci.PCINetDevice._set_sriov_numvfs') - def test_set_sriov_numvfs_avoid_call(self, mock__set_sriov_numvfs): - dev = pci.PCINetDevice('0000:10:00.0') - dev.sriov = True - dev.sriov_numvfs = 4 - dev.set_sriov_numvfs(4) - self.assertFalse(mock__set_sriov_numvfs.called) - class PCINetDevicesTest(CharmTestCase):