openstack/charm-neutron-openvswitch
Code Issues Proposed changes

Refactor SR-IOV support

Browse Source 
Refactor SR-IOV VF configuration support to use sriov-netplan-shim
to configure VF's on PF's so the charm simply writes out the required
interfaces.yaml file and restarts the sriov-netplan-shim service
which is fully idempotent.

Change-Id: I7a3ddf91d4b2ae6aa0806d97c45b59e8a951f67f
This commit is contained in:
James Page 2019-09-30 10:04:54 +01:00
parent 91b86cb9eb
commit 7ba64f9412
7 changed files with 213 additions and 204 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
config.yaml
View File

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

8
hooks/neutron_ovs_hooks.py
View File

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

224
hooks/neutron_ovs_utils.py
View File

@ -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:
# <device>:<numvfs>[ <device>: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:
# <device>:<numvfs>[ <device>: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():

22
hooks/pci.py
View File

@ -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):

1
unit_tests/test_neutron_ovs_hooks.py
View File

@ -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):

102
unit_tests/test_neutron_ovs_utils.py
View File

@ -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')

52
unit_tests/test_pci.py
View File

@ -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):