From 96c1788e94d15600386a375a774ac854ba589331 Mon Sep 17 00:00:00 2001 From: James Page Date: Wed, 22 Aug 2018 16:20:03 +0100 Subject: [PATCH] Misc updates for DPDK support Fix use of OVS DPDK context by direct use of methods on context for OVS table values. For modern OVS versions that require the PCI address of the DPDK device for type=dpdk ports, use a hash of the PCI address for the port name rather than the index of the PCI device in the current list of devices to use; this is idempotent in the event that the configuration changes and new devices appear in the list of devices to use for DPDK. Only set OVS table values if the value has changed; OVS will try to re-allocate hugepage memory, irrespective as to whether the table value actually changed. Switch to using /run/libvirt-vhost-user for libvirt created DPDK sockets, allowing libvirt to directly create the socket as part of instance creation; Use systemd-tmpfiles to ensure that the vhost-user subdirectory is re-created on boot with the correct permissions. Scan data-port and dpdk-bond-mappings for PCI devices to use for DPDK to avoid having to replicate all PCI devices in data-port configuration when DPDK bonds are in use. Change-Id: I2964046bc8681fa870d61c6cd23b6ad6fee47bf4 --- README.md | 18 ++- files/nova-ovs-vhost-user.conf | 2 + hooks/neutron_ovs_context.py | 6 +- hooks/neutron_ovs_hooks.py | 2 + hooks/neutron_ovs_utils.py | 171 +++++++++++++++++-------- templates/mitaka/openvswitch_agent.ini | 1 + unit_tests/test_neutron_ovs_context.py | 25 ++-- unit_tests/test_neutron_ovs_hooks.py | 1 + unit_tests/test_neutron_ovs_utils.py | 74 ++++++++--- 9 files changed, 216 insertions(+), 84 deletions(-) create mode 100644 files/nova-ovs-vhost-user.conf diff --git a/README.md b/README.md index a7e7bde4..f6c7292a 100644 --- a/README.md +++ b/README.md @@ -82,12 +82,28 @@ DPDK requires the use of hugepages, which is not directly configured in the neut By default, the charm will configure Open vSwitch/DPDK to consume a processor core + 1G of RAM from each NUMA node on the unit being deployed; this can be tuned using the dpdk-socket-memory and dpdk-socket-cores configuration options of the charm. The userspace kernel driver can be configured using the dpdk-driver option. See config.yaml for more details. -**NOTE:** Changing dpdk-socket-* configuration options will trigger a restart of Open vSwitch, which currently causes connectivity to running instances to be lost - connectivity can only be restored with a stop/start of each instance. +**NOTE:** Changing dpdk-socket-\* configuration options will trigger a restart of Open vSwitch, which currently causes connectivity to running instances to be lost - connectivity can only be restored with a stop/start of each instance. **NOTE:** Enabling DPDK support automatically disables security groups for instances. [dpdk-nics]: http://dpdk.org/doc/nics +# DPDK bonding + +For deployments using Open vSwitch 2.6.0 or later (OpenStack Ocata on Ubuntu 16.04 onwards), its also possible to use native Open vSwitch DPDK bonding to provide increased resilience for DPDK based deployments. + +This feature is configured using the `dpdk-bond-mappings` and `dpdk-bond-config` options of this charm, for example: + + neutron-openvswitch: + enable-dpdk: True + data-port: "br-phynet1:dpdk-bond0" + dpdk-bond-mappings: "dpdk-bond0:a8:9d:21:cf:93:fc dpdk-bond0:a8:9d:21:cf:93:fd" + dpdk-bond-config: ":balance-slb:off:fast" + +In this example, the PCI devices associated with the two MAC addresses provided will be configured as an OVS DPDK bond device named `dpdk-bond0`; this bond device is then used in br-phynet1 to provide resilient connectivity to the underlying network fabric. + +The charm will automatically detect which PCI devices are on each unit of the application based on the `dpdk-bond-mappings` configuration provided, supporting use in environments where network device naming may not be consistent across units. + # Port Configuration **NOTE:** External port configuration only applies when DVR mode is enabled. diff --git a/files/nova-ovs-vhost-user.conf b/files/nova-ovs-vhost-user.conf new file mode 100644 index 00000000..4c79466a --- /dev/null +++ b/files/nova-ovs-vhost-user.conf @@ -0,0 +1,2 @@ +# Create libvirt writeable directory for vhost-user sockets +d /run/libvirt-vhost-user 0770 libvirt-qemu kvm - - \ No newline at end of file diff --git a/hooks/neutron_ovs_context.py b/hooks/neutron_ovs_context.py index a9519483..571e1e4a 100644 --- a/hooks/neutron_ovs_context.py +++ b/hooks/neutron_ovs_context.py @@ -335,7 +335,11 @@ class DPDKDeviceContext(OSContextGenerator): driver = config('dpdk-driver') if driver is None: return {} - return {'devices': resolve_dpdk_bridges(), + # Resolve PCI devices for both directly used devices (_bridges) + # and devices for use in dpdk bonds (_bonds) + pci_devices = resolve_dpdk_bridges() + pci_devices.update(resolve_dpdk_bonds()) + return {'devices': pci_devices, 'driver': driver} diff --git a/hooks/neutron_ovs_hooks.py b/hooks/neutron_ovs_hooks.py index eca523bc..62ad3e78 100755 --- a/hooks/neutron_ovs_hooks.py +++ b/hooks/neutron_ovs_hooks.py @@ -47,6 +47,7 @@ from neutron_ovs_utils import ( install_packages, purge_packages, assess_status, + install_tmpfilesd, ) hooks = Hooks() @@ -88,6 +89,7 @@ def upgrade_charm(): @restart_on_change(restart_map()) def config_changed(): install_packages() + install_tmpfilesd() configure_ovs() CONFIGS.write_all() diff --git a/hooks/neutron_ovs_utils.py b/hooks/neutron_ovs_utils.py index ec165789..da4d536b 100644 --- a/hooks/neutron_ovs_utils.py +++ b/hooks/neutron_ovs_utils.py @@ -12,8 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import hashlib +import json import os from itertools import chain +import shutil import subprocess from charmhelpers.contrib.openstack.neutron import neutron_plugin_attribute @@ -40,7 +43,6 @@ from charmhelpers.contrib.network.ovs import ( full_restart, enable_ipfix, disable_ipfix, - set_Open_vSwitch_column_value ) from charmhelpers.core.hookenv import ( config, @@ -56,6 +58,7 @@ from charmhelpers.contrib.openstack.context import ( ExternalPortContext, DataPortContext, WorkerConfigContext, + parse_data_port_mappings, ) from charmhelpers.core.host import ( lsb_release, @@ -63,6 +66,7 @@ from charmhelpers.core.host import ( service_restart, service_running, CompareHostReleases, + init_is_systemd, ) from charmhelpers.fetch import ( @@ -145,7 +149,7 @@ BASE_RESOURCE_MAP = OrderedDict([ ['neutron-plugin', 'neutron-control'])], }), (DPDK_INTERFACES, { - 'services': ['dpdk'], + 'services': ['dpdk', 'openvswitch-switch'], 'contexts': [neutron_ovs_context.DPDKDeviceContext()], }), (PHY_NIC_MTU_CONF, { @@ -378,24 +382,69 @@ OVS_DPDK_BIN = '/usr/lib/openvswitch-switch-dpdk/ovs-vswitchd-dpdk' OVS_DEFAULT_BIN = '/usr/lib/openvswitch-switch/ovs-vswitchd' +# TODO(jamespage): rework back to charmhelpers +def set_Open_vSwitch_column_value(column, value): + """ + Calls ovs-vsctl and sets the 'column=value' in the Open_vSwitch table. + + :param column: colume name to set value for + :param value: value to set + See http://www.openvswitch.org//ovs-vswitchd.conf.db.5.pdf for + details of the relevant values. + :type str + :returns bool: indicating if a column value was changed + :raises CalledProcessException: possibly ovsdb-server is not running + """ + current_value = None + try: + current_value = json.loads(subprocess.check_output( + ['ovs-vsctl', 'get', 'Open_vSwitch', '.', column] + )) + except subprocess.CalledProcessError: + pass + + if current_value != value: + log('Setting {}:{} in the Open_vSwitch table'.format(column, value)) + subprocess.check_call(['ovs-vsctl', 'set', 'Open_vSwitch', + '.', '{}={}'.format(column, + value)]) + return True + return False + + def enable_ovs_dpdk(): '''Enables the DPDK variant of ovs-vswitchd and restarts it''' subprocess.check_call(UPDATE_ALTERNATIVES + [OVS_DPDK_BIN]) + values_changed = [] if ovs_has_late_dpdk_init(): - ctxt = neutron_ovs_context.OVSDPDKDeviceContext() - set_Open_vSwitch_column_value( - 'other_config:dpdk-init=true') - set_Open_vSwitch_column_value( - 'other_config:dpdk-lcore-mask={}'.format(ctxt['cpu_mask'])) - set_Open_vSwitch_column_value( - 'other_config:dpdk-socket-mem={}'.format(ctxt['socket_memory'])) - set_Open_vSwitch_column_value( - 'other_config:dpdk-extra=--vhost-owner' - ' libvirt-qemu:kvm --vhost-perm 0660') - if not is_unit_paused_set(): + dpdk_context = neutron_ovs_context.OVSDPDKDeviceContext() + other_config = OrderedDict([ + ('pmd-cpu-mask', dpdk_context.cpu_mask()), + ('dpdk-socket-mem', dpdk_context.socket_memory()), + ('dpdk-extra', + '--vhost-owner libvirt-qemu:kvm --vhost-perm 0660'), + ('dpdk-init', 'true'), + ]) + for column, value in other_config.items(): + values_changed.append( + set_Open_vSwitch_column_value( + 'other_config:{}'.format(column), + value + ) + ) + if ((values_changed and any(values_changed)) and + not is_unit_paused_set()): service_restart('openvswitch-switch') +def install_tmpfilesd(): + '''Install systemd-tmpfiles configuration for ovs vhost-user sockets''' + if init_is_systemd(): + shutil.copy('files/nova-ovs-vhost-user.conf', + '/etc/tmpfiles.d') + subprocess.check_call(['systemd-tmpfiles', '--create']) + + def configure_ovs(): status_set('maintenance', 'Configuring ovs') if not service_running('openvswitch-switch'): @@ -409,6 +458,8 @@ def configure_ovs(): if ext_port_ctx and ext_port_ctx['ext_port']: add_bridge_port(EXT_BRIDGE, ext_port_ctx['ext_port']) + modern_ovs = ovs_has_late_dpdk_init() + bridgemaps = None if not use_dpdk(): portmaps = DataPortContext()() @@ -428,25 +479,42 @@ def configure_ovs(): # NOTE: when in dpdk mode, add based on pci bus order # with type 'dpdk' bridgemaps = neutron_ovs_context.resolve_dpdk_bridges() - bondmaps = neutron_ovs_context.resolve_dpdk_bonds() device_index = 0 - bridge_bond_map = DPDKBridgeBondMap() for pci_address, br in bridgemaps.items(): add_bridge(br, datapath_type) - portname = 'dpdk{}'.format(device_index) - if pci_address in bondmaps: - bond = bondmaps[pci_address] - bridge_bond_map.add_port(br, bond, portname, pci_address) + if modern_ovs: + portname = 'dpdk-{}'.format( + hashlib.sha1(pci_address.encode('UTF-8')).hexdigest()[:7] + ) else: - dpdk_add_bridge_port(br, portname, - pci_address) + portname = 'dpdk{}'.format(device_index) + + dpdk_add_bridge_port(br, portname, + pci_address) device_index += 1 - bond_configs = DPDKBondsConfig() - for br, bonds in bridge_bond_map.items(): - for bond, t in bonds.items(): - dpdk_add_bridge_bond(br, bond, *t) - dpdk_set_bond_config(bond, bond_configs.get_bond_config(bond)) + if modern_ovs: + bondmaps = neutron_ovs_context.resolve_dpdk_bonds() + bridge_bond_map = DPDKBridgeBondMap() + portmap = parse_data_port_mappings(config('data-port')) + for pci_address, bond in bondmaps.items(): + if bond in portmap: + add_bridge(portmap[bond], datapath_type) + portname = 'dpdk-{}'.format( + hashlib.sha1(pci_address.encode('UTF-8')) + .hexdigest()[:7] + ) + bridge_bond_map.add_port(portmap[bond], bond, + portname, pci_address) + + bond_configs = DPDKBondsConfig() + for br, bonds in bridge_bond_map.items(): + for bond, port_map in bonds.items(): + dpdk_add_bridge_bond(br, bond, port_map) + dpdk_set_bond_config( + bond, + bond_configs.get_bond_config(bond) + ) target = config('ipfix-target') bridges = [INT_BRIDGE, EXT_BRIDGE] @@ -620,36 +688,34 @@ def dpdk_add_bridge_port(name, port, pci_address=None): subprocess.check_call(cmd) -def dpdk_add_bridge_bond(bridge_name, bond_name, port_list, pci_address_list): +def dpdk_add_bridge_bond(bridge_name, bond_name, port_map): ''' Add ports to a bond attached to the named openvswitch bridge ''' - if ovs_has_late_dpdk_init(): - cmd = ["ovs-vsctl", "--may-exist", - "add-bond", bridge_name, bond_name] - for port in port_list: - cmd.append(port) - id = 0 - for pci_address in pci_address_list: - cmd.extend(["--", "set", "Interface", port_list[id], - "type=dpdk", - "options:dpdk-devargs={}".format(pci_address)]) - id += 1 - else: - raise Exception("Bond's not supported for OVS pre-2.6.0") + if not ovs_has_late_dpdk_init(): + raise Exception("Bonds are not supported for OVS pre-2.6.0") + + cmd = ["ovs-vsctl", "--may-exist", + "add-bond", bridge_name, bond_name] + for portname in port_map.keys(): + cmd.append(portname) + for portname, pci_address in port_map.items(): + cmd.extend(["--", "set", "Interface", portname, + "type=dpdk", + "options:dpdk-devargs={}".format(pci_address)]) subprocess.check_call(cmd) def dpdk_set_bond_config(bond_name, config): - if ovs_has_late_dpdk_init(): - cmd = ["ovs-vsctl", - "--", "set", "port", bond_name, - "bond_mode={}".format(config['mode']), - "--", "set", "port", bond_name, - "lacp={}".format(config['lacp']), - "--", "set", "port", bond_name, - "other_config:lacp-time=={}".format(config['lacp-time']), - ] - else: + if not ovs_has_late_dpdk_init(): raise Exception("Bonds are not supported for OVS pre-2.6.0") + + cmd = ["ovs-vsctl", + "--", "set", "port", bond_name, + "bond_mode={}".format(config['mode']), + "--", "set", "port", bond_name, + "lacp={}".format(config['lacp']), + "--", "set", "port", bond_name, + "other_config:lacp-time=={}".format(config['lacp-time']), + ] subprocess.check_call(cmd) @@ -743,9 +809,8 @@ class DPDKBridgeBondMap(): if bridge not in self.map: self.map[bridge] = {} if bond not in self.map[bridge]: - self.map[bridge][bond] = ([], []) - self.map[bridge][bond][0].append(portname) - self.map[bridge][bond][1].append(pci_address) + self.map[bridge][bond] = {} + self.map[bridge][bond][portname] = pci_address def items(self): return list(self.map.items()) diff --git a/templates/mitaka/openvswitch_agent.ini b/templates/mitaka/openvswitch_agent.ini index 6a12091d..83cb6bca 100644 --- a/templates/mitaka/openvswitch_agent.ini +++ b/templates/mitaka/openvswitch_agent.ini @@ -10,6 +10,7 @@ local_ip = {{ local_ip }} bridge_mappings = {{ bridge_mappings }} {% if enable_dpdk -%} datapath_type = netdev +vhostuser_socket_dir = /run/libvirt-vhost-user {% endif -%} [agent] diff --git a/unit_tests/test_neutron_ovs_context.py b/unit_tests/test_neutron_ovs_context.py index bcabd33b..9085a3b2 100644 --- a/unit_tests/test_neutron_ovs_context.py +++ b/unit_tests/test_neutron_ovs_context.py @@ -17,6 +17,7 @@ from test_utils import patch_open from mock import patch, Mock import neutron_ovs_context as context import charmhelpers +import copy _LSB_RELEASE_XENIAL = { 'DISTRIB_CODENAME': 'xenial', @@ -566,6 +567,7 @@ DPDK_PATCH = [ 'parse_cpu_list', 'numa_node_cores', 'resolve_dpdk_bridges', + 'resolve_dpdk_bonds', 'glob', ] @@ -645,29 +647,34 @@ class TestOVSDPDKDeviceContext(CharmTestCase): class TestDPDKDeviceContext(CharmTestCase): + _dpdk_bridges = { + '0000:00:1c.0': 'br-data', + '0000:00:1d.0': 'br-physnet1', + } + _dpdk_bonds = { + '0000:00:1c.1': 'dpdk-bond0', + '0000:00:1d.1': 'dpdk-bond0', + } + def setUp(self): super(TestDPDKDeviceContext, self).setUp(context, TO_PATCH + DPDK_PATCH) self.config.side_effect = self.test_config.get self.test_context = context.DPDKDeviceContext() + self.resolve_dpdk_bridges.return_value = self._dpdk_bridges + self.resolve_dpdk_bonds.return_value = self._dpdk_bonds def test_context(self): self.test_config.set('dpdk-driver', 'uio_pci_generic') - self.resolve_dpdk_bridges.return_value = [ - '0000:00:1c.0', - '0000:00:1d.0' - ] + devices = copy.deepcopy(self._dpdk_bridges) + devices.update(self._dpdk_bonds) self.assertEqual(self.test_context(), { - 'devices': ['0000:00:1c.0', '0000:00:1d.0'], + 'devices': devices, 'driver': 'uio_pci_generic' }) self.config.assert_called_with('dpdk-driver') def test_context_none_driver(self): - self.resolve_dpdk_bridges.return_value = [ - '0000:00:1c.0', - '0000:00:1d.0' - ] self.assertEqual(self.test_context(), {}) self.config.assert_called_with('dpdk-driver') diff --git a/unit_tests/test_neutron_ovs_hooks.py b/unit_tests/test_neutron_ovs_hooks.py index c278c4cd..8bd74d5d 100644 --- a/unit_tests/test_neutron_ovs_hooks.py +++ b/unit_tests/test_neutron_ovs_hooks.py @@ -45,6 +45,7 @@ TO_PATCH = [ 'purge_packages', 'enable_nova_metadata', 'enable_local_dhcp', + 'install_tmpfilesd', ] NEUTRON_CONF_DIR = "/etc/neutron" diff --git a/unit_tests/test_neutron_ovs_utils.py b/unit_tests/test_neutron_ovs_utils.py index 8319270b..3f784598 100644 --- a/unit_tests/test_neutron_ovs_utils.py +++ b/unit_tests/test_neutron_ovs_utils.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import hashlib + from mock import MagicMock, patch, call from collections import OrderedDict import charmhelpers.contrib.openstack.templating as templating @@ -58,6 +60,7 @@ TO_PATCH = [ 'enable_ipfix', 'disable_ipfix', 'ovs_has_late_dpdk_init', + 'parse_data_port_mappings', ] head_pkg = 'linux-headers-3.15.0-5-generic' @@ -476,18 +479,31 @@ class TestNeutronOVSUtils(CharmTestCase): def _run_configure_ovs_dpdk(self, mock_config, _use_dvr, _resolve_dpdk_bridges, _resolve_dpdk_bonds, _late_init, _test_bonds): - _resolve_dpdk_bridges.return_value = OrderedDict([ - ('0000:001c.01', 'br-phynet1'), - ('0000:001c.02', 'br-phynet2'), - ('0000:001c.03', 'br-phynet3'), - ]) + def _resolve_port_name(pci_address, device_index, late_init): + if late_init: + return 'dpdk-{}'.format( + hashlib.sha1(pci_address.encode('UTF-8')).hexdigest()[:7] + ) + else: + return 'dpdk{}'.format(device_index) if _test_bonds: + _resolve_dpdk_bridges.return_value = OrderedDict() _resolve_dpdk_bonds.return_value = OrderedDict([ ('0000:001c.01', 'bond0'), ('0000:001c.02', 'bond1'), ('0000:001c.03', 'bond2'), ]) + self.parse_data_port_mappings.return_value = OrderedDict([ + ('bond0', 'br-phynet1'), + ('bond1', 'br-phynet2'), + ('bond2', 'br-phynet3'), + ]) else: + _resolve_dpdk_bridges.return_value = OrderedDict([ + ('0000:001c.01', 'br-phynet1'), + ('0000:001c.02', 'br-phynet2'), + ('0000:001c.03', 'br-phynet3'), + ]) _resolve_dpdk_bonds.return_value = OrderedDict() _use_dvr.return_value = True self.use_dpdk.return_value = True @@ -506,9 +522,15 @@ class TestNeutronOVSUtils(CharmTestCase): ) if _test_bonds: self.dpdk_add_bridge_bond.assert_has_calls([ - call('br-phynet1', 'bond0', ['dpdk0'], ['0000:001c.01']), - call('br-phynet2', 'bond1', ['dpdk1'], ['0000:001c.02']), - call('br-phynet3', 'bond2', ['dpdk2'], ['0000:001c.03'])], + call('br-phynet1', 'bond0', + {_resolve_port_name('0000:001c.01', + 0, _late_init): '0000:001c.01'}), + call('br-phynet2', 'bond1', + {_resolve_port_name('0000:001c.02', + 1, _late_init): '0000:001c.02'}), + call('br-phynet3', 'bond2', + {_resolve_port_name('0000:001c.03', + 2, _late_init): '0000:001c.03'})], any_order=True ) self.dpdk_set_bond_config.assert_has_calls([ @@ -528,9 +550,18 @@ class TestNeutronOVSUtils(CharmTestCase): ) else: self.dpdk_add_bridge_port.assert_has_calls([ - call('br-phynet1', 'dpdk0', '0000:001c.01'), - call('br-phynet2', 'dpdk1', '0000:001c.02'), - call('br-phynet3', 'dpdk2', '0000:001c.03')], + call('br-phynet1', + _resolve_port_name('0000:001c.01', + 0, _late_init), + '0000:001c.01'), + call('br-phynet2', + _resolve_port_name('0000:001c.02', + 1, _late_init), + '0000:001c.02'), + call('br-phynet3', + _resolve_port_name('0000:001c.03', + 2, _late_init), + '0000:001c.03')], any_order=True ) @@ -766,15 +797,18 @@ class TestDPDKBridgeBondMap(CharmTestCase): ctx.add_port("br1", "bond2", "port3", "00:00:00:00:00:03") ctx.add_port("br1", "bond2", "port4", "00:00:00:00:00:04") - expected = [('br1', - {'bond1': - (['port1', 'port2'], - ['00:00:00:00:00:01', '00:00:00:00:00:02']), - 'bond2': - (['port3', 'port4'], - ['00:00:00:00:00:03', '00:00:00:00:00:04']) - }) - ] + expected = [( + 'br1', { + 'bond1': { + 'port1': '00:00:00:00:00:01', + 'port2': '00:00:00:00:00:02' + }, + 'bond2': { + 'port3': '00:00:00:00:00:03', + 'port4': '00:00:00:00:00:04', + }, + }, + )] self.assertEqual(ctx.items(), expected)