openstack
/
charm-neutron-openvswitch
Code Issues Proposed changes

SR-IOV: match on PCI address, don't do runtime config 

Replace in-charm SR-IOV code with the common ``SRIOVContext``

Do not do run-time configuration of SR-IOV or hardware adaption
for hardware offload. In addition to being detrimental to any
virtual machine instance consuming the VF this will break NIC
firmware in some configurations.

The task is delegated to the installed packages and their systemd
services and configuration will occur at system bootup time.

We may consider adding an action to perform the configuration at
run-time if the operator really wants to, but it is very
complicated to get right. For example if you are using bonding
and hardware offload the virtual functions and hardware specific
setup has to happen _BEFORE_ netplan applies network configuration
to the system.

Closes-Bug: #1908351
Change-Id: Id0b81848658a3bd34470440bd68928ae9f6682e4
This commit is contained in:
Frode Nordahl 2021-02-09 21:25:39 +01:00
parent 9d62a38d71
commit a88259a768
6 changed files with 66 additions and 299 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
config.yaml
View File

@ -212,6 +212,10 @@ options:
Open vSwitch networking options.
.
NOTE: This configuration option will be ignored on Trusty and older.
.
NOTE: Changing this value will have no effect on runtime configuration. A
manual restart of the `sriov-netplan-shim` service or reboot of the
system is required to apply configuration.
sriov-device-mappings:
type: string
default:
@ -259,6 +263,10 @@ options:
supported by OVS or the Linux kernel).
.
This option must not be enabled with either enable-sriov or enable-dpdk.
.
NOTE: Changing this value will not perform hardware specific adaption. A
manual restart of the hardware specific adaption service or reboot of the
system is required to apply configuration.
networking-tools-source:
type: string
default: ppa:openstack-charmers/networking-tools

10
hooks/neutron_ovs_hooks.py
View File

@ -55,7 +55,6 @@ from neutron_ovs_utils import (
OVS_DEFAULT,
USE_FQDN_KEY,
configure_ovs,
configure_sriov,
get_shared_secret,
register_configs,
restart_map,
@ -71,6 +70,7 @@ from neutron_ovs_utils import (
pause_unit_helper,
resume_unit_helper,
determine_purge_packages,
purge_sriov_systemd_files,
use_fqdn_hint,
)
@ -95,6 +95,10 @@ def install():
# for the implications of modifications to the /etc/default/openvswitch-switch.
@hooks.hook('upgrade-charm')
def upgrade_charm():
# Tidy up any prior installation of obsolete sriov startup
# scripts
purge_sriov_systemd_files()
if OVS_DEFAULT in restart_map():
# In the 16.10 release of the charms, the code changed from managing
# the /etc/default/openvswitch-switch file only when dpdk was enabled
@ -159,10 +163,6 @@ def config_changed():
_restart_before_runtime_config_when_required()
configure_ovs()
# 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,

173
hooks/neutron_ovs_utils.py
View File

@ -18,7 +18,6 @@ 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
@ -30,7 +29,6 @@ from charmhelpers.contrib.openstack.utils import (
make_assess_status_func,
is_unit_paused_set,
os_application_version_set,
remote_restart,
CompareOpenStackReleases,
os_release,
)
@ -75,7 +73,6 @@ from charmhelpers.core.host import (
group_exists,
user_exists,
is_container,
restart_on_change
)
from charmhelpers.core.kernel import (
modprobe,
@ -92,8 +89,6 @@ from charmhelpers.fetch import (
add_source,
)
from pci import PCINetDevices
# The interface is said to be satisfied if anyone of the interfaces in the
# list has a complete context.
@ -145,6 +140,7 @@ DPDK_INTERFACES = '/etc/dpdk/interfaces'
NEUTRON_SRIOV_AGENT_CONF = os.path.join(NEUTRON_CONF_DIR,
'plugins/ml2/sriov_agent.ini')
USE_FQDN_KEY = 'neutron-ovs-charm-use-fqdn'
SRIOV_NETPLAN_SHIM_CONF = '/etc/sriov-netplan-shim/interfaces.yaml'
def use_fqdn_hint():
@ -417,6 +413,30 @@ def resource_map():
resource_map.update(sriov_resource_map)
resource_map[NEUTRON_CONF]['services'].append(
sriov_agent_name)
if enable_sriov() or use_hw_offload():
# We do late initialization of this as a call to
# ``context.SRIOVContext`` requires the ``sriov-netplan-shim`` package
# to already be installed on the system.
#
# Note that we also do not want the charm to manage the service, but
# only update the configuration for boot-time initialization.
# LP: #1908351
try:
resource_map.update(OrderedDict([
(SRIOV_NETPLAN_SHIM_CONF, {
# We deliberately omit service here as we only want changes
# to be applied at boot time.
'services': [],
'contexts': [SRIOVContext_adapter()],
}),
]))
except NameError:
# The resource_map is built at module import time and as such this
# function is called multiple times prior to the charm actually
# being installed. As the SRIOVContext depends on a Python module
# provided by the ``sriov-netplan-shim`` package gracefully ignore
# this to allow the package to be installed.
pass
# Use MAAS1.9 for MTU and external port config on xenial and above
if CompareHostReleases(lsb_release()['DISTRIB_CODENAME']) >= 'xenial':
@ -719,134 +739,6 @@ 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
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.
'''
@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))
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
# NOTE(jamespage)
# Hardware offload makes use of SR-IOV VF representor ports so
# makes use of the general SR-IOV configuration support in this
# charm
if not (enable_sriov() or use_hw_offload()):
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')
if not use_hw_offload() and 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')
if use_hw_offload() and numvfs_changed:
service_restart('mlnx-switchdev-mode')
def get_shared_secret():
ctxt = neutron_ovs_context.SharedSecretContext()()
if 'shared_secret' in ctxt:
@ -918,6 +810,21 @@ def enable_sriov():
return (cmp_release >= 'xenial' and config('enable-sriov'))
class SRIOVContext_adapter(object):
"""Adapt the SRIOVContext for use in a classic charm.
:returns: Dictionary with entry point to context map.
:rtype: Dict[str,SRIOVContext]
"""
interfaces = []
def __init__(self):
self._sriov_device = context.SRIOVContext()
def __call__(self):
return {'sriov_device': self._sriov_device}
# TODO: update into charm-helpers to add port_type parameter
def dpdk_add_bridge_port(name, port, pci_address=None):
''' Add a port to the named openvswitch bridge '''

7
templates/mitaka/interfaces.yaml Normal file
View File

@ -0,0 +1,7 @@
interfaces:
{% for _, pcidnvfs in sriov_device.get_map.items() -%}
{{ pcidnvfs.device.interface_name }}:
match:
pciaddress: '{{ pcidnvfs.device.pci_address }}'
num_vfs: {{ pcidnvfs.numvfs }}
{% endfor -%}

1
unit_tests/test_neutron_ovs_hooks.py
View File

@ -41,7 +41,6 @@ TO_PATCH = [
'relation_ids',
'relation_set',
'configure_ovs',
'configure_sriov',
'use_dvr',
'use_l3ha',
'install_packages',

166
unit_tests/test_neutron_ovs_utils.py
View File

@ -13,9 +13,7 @@
# limitations under the License.
import hashlib
import io
import subprocess
import yaml
from mock import MagicMock, patch, call
from collections import OrderedDict
@ -28,7 +26,6 @@ import neutron_ovs_context
from test_utils import (
CharmTestCase,
patch_open,
)
import charmhelpers
import charmhelpers.core.hookenv as hookenv
@ -63,8 +60,6 @@ TO_PATCH = [
'status_set',
'use_dpdk',
'os_application_version_set',
'remote_restart',
'PCINetDevices',
'enable_ipfix',
'disable_ipfix',
'ovs_has_late_dpdk_init',
@ -473,9 +468,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, 'SRIOVContext_adapter')
@patch.object(nutils, 'enable_sriov')
@patch.object(nutils, 'use_dvr')
def test_resource_map_kilo_sriov(self, _use_dvr, _enable_sriov):
def test_resource_map_kilo_sriov(self, _use_dvr, _enable_sriov,
_sriovcontext_adapter):
_use_dvr.return_value = False
_enable_sriov.return_value = True
self.os_release.return_value = 'kilo'
@ -500,9 +497,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, 'SRIOVContext_adapter')
@patch.object(nutils, 'enable_sriov')
@patch.object(nutils, 'use_dvr')
def test_resource_map_mitaka_sriov(self, _use_dvr, _enable_sriov):
def test_resource_map_mitaka_sriov(self, _use_dvr, _enable_sriov,
_sriovcontext_adapter):
_use_dvr.return_value = False
_enable_sriov.return_value = True
self.os_release.return_value = 'mitaka'
@ -924,159 +923,6 @@ class TestNeutronOVSUtils(CharmTestCase):
# ports=None whilst port checks are disabled.
f.assert_called_once_with('assessor', services='s1', ports=None)
def _configure_sriov_base(self, config):
self.mock_config = MagicMock()
self.config.side_effect = None
self.config.return_value = self.mock_config
self.mock_config.get.side_effect = lambda x: config.get(x)
self.mock_eth_device = MagicMock()
self.mock_eth_device.sriov = False
self.mock_eth_device.interface_name = 'eth0'
self.mock_eth_device.sriov_totalvfs = 0
self.mock_sriov_device = MagicMock()
self.mock_sriov_device.sriov = True
self.mock_sriov_device.interface_name = 'ens0'
self.mock_sriov_device.sriov_totalvfs = 64
self.mock_sriov_device2 = MagicMock()
self.mock_sriov_device2.sriov = True
self.mock_sriov_device2.interface_name = 'ens49'
self.mock_sriov_device2.sriov_totalvfs = 64
self.pci_devices = {
'eth0': self.mock_eth_device,
'ens0': self.mock_sriov_device,
'ens49': self.mock_sriov_device2,
}
mock_pci_devices = MagicMock()
mock_pci_devices.pci_devices = [
self.mock_eth_device,
self.mock_sriov_device,
self.mock_sriov_device2
]
self.PCINetDevices.return_value = mock_pci_devices
mock_pci_devices.get_device_from_interface_name.side_effect = \
lambda x: self.pci_devices.get(x)
@patch('shutil.copy')
@patch('os.chmod')
def test_configure_sriov_auto(self, _os_chmod, _sh_copy):
self.os_release.return_value = 'mitaka'
self.lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'}
_config = {
'enable-sriov': True,
'sriov-numvfs': 'auto'
}
self._configure_sriov_base(_config)
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_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_mapping(self, _os_chmod, _sh_copy):
self.os_release.return_value = 'mitaka'
self.lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'}
_config = {
'enable-sriov': True,
'sriov-numvfs': 'auto',
'sriov-device-mappings': 'net1:ens49'
}
self._configure_sriov_base(_config)
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.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_numvfs(self, _os_chmod, _sh_copy):
self.os_release.return_value = 'mitaka'
self.lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'}
_config = {
'enable-sriov': True,
'sriov-numvfs': '32',
}
self._configure_sriov_base(_config)
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')
@patch('os.chmod')
def test_configure_sriov_numvfs_per_device(self, _os_chmod, _sh_copy):
self.os_release.return_value = 'mitaka'
self.lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'}
_config = {
'enable-sriov': True,
'sriov-numvfs': 'ens0:32 sriov23:64'
}
self._configure_sriov_base(_config)
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_not_called()
self.mock_sriov_device2.set_sriov_numvfs.assert_not_called()
self.assertTrue(self.remote_restart.called)
@patch.object(nutils, 'subprocess')
@patch.object(nutils, 'shutil')
def test_install_tmpfilesd_lxd(self, mock_shutil, mock_subprocess):