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

@@ -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.

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 - -

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}
 
 

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()

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()
+        dpdk_context = neutron_ovs_context.OVSDPDKDeviceContext()
-        set_Open_vSwitch_column_value(
+        other_config = OrderedDict([
-            'other_config:dpdk-init=true')
+            ('pmd-cpu-mask', dpdk_context.cpu_mask()),
-        set_Open_vSwitch_column_value(
+            ('dpdk-socket-mem', dpdk_context.socket_memory()),
-            'other_config:dpdk-lcore-mask={}'.format(ctxt['cpu_mask']))
+            ('dpdk-extra',
-        set_Open_vSwitch_column_value(
+             '--vhost-owner libvirt-qemu:kvm --vhost-perm 0660'),
-            'other_config:dpdk-socket-mem={}'.format(ctxt['socket_memory']))
+            ('dpdk-init', 'true'),
-        set_Open_vSwitch_column_value(
+        ])
-            'other_config:dpdk-extra=--vhost-owner'
+        for column, value in other_config.items():
-            ' libvirt-qemu:kvm --vhost-perm 0660')
+            values_changed.append(
-    if not is_unit_paused_set():
+                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 modern_ovs:
-            if pci_address in bondmaps:
+                portname = 'dpdk-{}'.format(
-                bond = bondmaps[pci_address]
+                    hashlib.sha1(pci_address.encode('UTF-8')).hexdigest()[:7]
-                bridge_bond_map.add_port(br, bond, portname, pci_address)
+                )
             else:
-                dpdk_add_bridge_port(br, portname,
+                portname = 'dpdk{}'.format(device_index)
-                                     pci_address)
+
+            dpdk_add_bridge_port(br, portname,
+                                 pci_address)
             device_index += 1
 
-        bond_configs = DPDKBondsConfig()
+        if modern_ovs:
-        for br, bonds in bridge_bond_map.items():
+            bondmaps = neutron_ovs_context.resolve_dpdk_bonds()
-            for bond, t in bonds.items():
+            bridge_bond_map = DPDKBridgeBondMap()
-                dpdk_add_bridge_bond(br, bond, *t)
+            portmap = parse_data_port_mappings(config('data-port'))
-                dpdk_set_bond_config(bond, bond_configs.get_bond_config(bond))
+            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():
+    if not ovs_has_late_dpdk_init():
-        cmd = ["ovs-vsctl", "--may-exist",
+        raise Exception("Bonds are not supported for OVS pre-2.6.0")
-               "add-bond", bridge_name, bond_name]
+
-        for port in port_list:
+    cmd = ["ovs-vsctl", "--may-exist",
-            cmd.append(port)
+           "add-bond", bridge_name, bond_name]
-        id = 0
+    for portname in port_map.keys():
-        for pci_address in pci_address_list:
+        cmd.append(portname)
-            cmd.extend(["--", "set", "Interface", port_list[id],
+    for portname, pci_address in port_map.items():
-                        "type=dpdk",
+        cmd.extend(["--", "set", "Interface", portname,
-                        "options:dpdk-devargs={}".format(pci_address)])
+                    "type=dpdk",
-            id += 1
+                    "options:dpdk-devargs={}".format(pci_address)])
-    else:
-        raise Exception("Bond's not supported for OVS pre-2.6.0")
     subprocess.check_call(cmd)
 
 
 def dpdk_set_bond_config(bond_name, config):
-    if ovs_has_late_dpdk_init():
+    if not 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:
         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] = {}
-        self.map[bridge][bond][0].append(portname)
+        self.map[bridge][bond][portname] = pci_address
-        self.map[bridge][bond][1].append(pci_address)
 
     def items(self):
         return list(self.map.items())

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]

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 = [
+        devices = copy.deepcopy(self._dpdk_bridges)
-            '0000:00:1c.0',
+        devices.update(self._dpdk_bonds)
-            '0000:00:1d.0'
-        ]
         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')
 

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"
 

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([
+        def _resolve_port_name(pci_address, device_index, late_init):
-            ('0000:001c.01', 'br-phynet1'),
+            if late_init:
-            ('0000:001c.02', 'br-phynet2'),
+                return 'dpdk-{}'.format(
-            ('0000:001c.03', 'br-phynet3'),
+                    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-phynet1', 'bond0',
-                call('br-phynet2', 'bond1', ['dpdk1'], ['0000:001c.02']),
+                     {_resolve_port_name('0000:001c.01',
-                call('br-phynet3', 'bond2', ['dpdk2'], ['0000:001c.03'])],
+                                         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-phynet1',
-                call('br-phynet2', 'dpdk1', '0000:001c.02'),
+                     _resolve_port_name('0000:001c.01',
-                call('br-phynet3', 'dpdk2', '0000:001c.03')],
+                                        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',
+        expected = [(
-                     {'bond1':
+            'br1', {
-                      (['port1', 'port2'],
+                'bond1': {
-                       ['00:00:00:00:00:01', '00:00:00:00:00:02']),
+                    'port1': '00:00:00:00:00:01',
-                      'bond2':
+                    'port2': '00:00:00:00:00:02'
-                          (['port3', 'port4'],
+                },
-                           ['00:00:00:00:00:03', '00:00:00:00:00:04'])
+                'bond2': {
-                      })
+                    'port3': '00:00:00:00:00:03',
-                    ]
+                    'port4': '00:00:00:00:00:04',
+                },
+            },
+        )]
 
         self.assertEqual(ctx.items(), expected)