From 353d4a97eb82f67098b1749303cc117aa9c2b6e7 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Fri, 9 Jun 2017 10:08:30 +0200 Subject: [PATCH] Add support for setting sriov_supported_pci_vendor_devs This is required for SR-IOV to work on OpenStack versions Kilo, Liberty and Mitaka (unless you are the lucky owner of NICs with the default vendor/product IDs '15b3:1004', '8086:10ca'). The option is deprecated in Newton and the default behaviour onwards is to not perform the check (unless configured) and not overrule Nova's scheduling decision. (See LP #1611302) Re-work mechanism_driver templating. Current implementation treats mechanism drivers 'hyperv' and 'sriovnicswitch' as mutually exclusive for simplicity. This prohibits us from adding functional test for verifying sriov statements in configuration files. Due to how neutron init scripts are laid out on various Linux distributions put the [ml2_sriov] section in ml2_conf.ini instead of its default ml2_conf_sriov.ini location. Add a placeholder ml2_conf_sriov.ini with comment to point users in the right direction. Change-Id: I37da1c430a06417ff7f1bc9df2d984137688bba0 Closes-Bug: #1630387 --- config.yaml | 18 ++++++++++ hooks/neutron_api_context.py | 34 +++++++++++++++--- hooks/neutron_api_utils.py | 11 ++++++ templates/kilo/ml2_conf.ini | 13 ++++--- templates/kilo/ml2_conf_sriov.ini | 7 ++++ templates/mitaka/ml2_conf.ini | 13 ++++--- tests/basic_deployment.py | 42 +++++++++++++++------- unit_tests/test_neutron_api_context.py | 50 ++++++++++++++++++++++---- 8 files changed, 150 insertions(+), 38 deletions(-) create mode 100644 templates/kilo/ml2_conf_sriov.ini diff --git a/config.yaml b/config.yaml index 5a665eb9..43cd3081 100755 --- a/config.yaml +++ b/config.yaml @@ -221,6 +221,24 @@ options: default: False description: | Enable SR-IOV networking support across Neutron and Nova. + supported-pci-vendor-devs: + type: string + default: + description: | + NOTE: Required for OpenStack versions Kilo, Liberty and Mitaka. Ignored + for OpenStack Newton and newer. + . + Space-delimited list of supported Vendor PCI Devices, in format + vendor_id:product_id + . + Example: 15b3:1004 8086:10ca + . + This must be set to match available hardware on nova-compute units. + Neutron will refuse to bind ports to hardware with Vendor PCI IDs not + in this list. (Use lspci -nn to get IDs) + . + NOTE: The vendor_id:product_id of the PFs and VFs differ. Make sure to + enter the correct one for your use case. midonet-origin: type: string default: midonet-2015.06 diff --git a/hooks/neutron_api_context.py b/hooks/neutron_api_context.py index c7ed6cc1..62074f59 100644 --- a/hooks/neutron_api_context.py +++ b/hooks/neutron_api_context.py @@ -173,6 +173,27 @@ def get_dns_domain(): return dns_domain +def get_ml2_mechanism_drivers(): + """Build comma delimited list of mechanism drivers for use in Neutron + ml2_conf.ini. Which drivers to enable are deduced from OpenStack + release and charm configuration options. + """ + mechanism_drivers = [ + 'openvswitch', + ] + + cmp_release = CompareOpenStackReleases(os_release('neutron-server')) + if (cmp_release == 'kilo' or cmp_release >= 'mitaka'): + mechanism_drivers.append('hyperv') + + if get_l2population(): + mechanism_drivers.append('l2population') + + if (config('enable-sriov') and cmp_release >= 'kilo'): + mechanism_drivers.append('sriovnicswitch') + return ','.join(mechanism_drivers) + + class ApacheSSLContext(context.ApacheSSLContext): interfaces = ['https'] @@ -368,11 +389,6 @@ class NeutronCCContext(context.NeutronContext): ctxt['enable_sriov'] = config('enable-sriov') - if cmp_release == 'kilo' or cmp_release >= 'mitaka': - ctxt['enable_hyperv'] = True - else: - ctxt['enable_hyperv'] = False - if cmp_release >= 'mitaka': if config('global-physnet-mtu'): ctxt['global_physnet_mtu'] = config('global-physnet-mtu') @@ -381,6 +397,14 @@ class NeutronCCContext(context.NeutronContext): else: ctxt['path_mtu'] = config('global-physnet-mtu') + if 'kilo' <= cmp_release <= 'mitaka': + pci_vendor_devs = config('supported-pci-vendor-devs') + if pci_vendor_devs: + ctxt['supported_pci_vendor_devs'] = \ + ','.join(pci_vendor_devs.split()) + + ctxt['mechanism_drivers'] = get_ml2_mechanism_drivers() + return ctxt diff --git a/hooks/neutron_api_utils.py b/hooks/neutron_api_utils.py index 88c1e158..b0b5d891 100755 --- a/hooks/neutron_api_utils.py +++ b/hooks/neutron_api_utils.py @@ -159,6 +159,12 @@ NEUTRON_DEFAULT = '/etc/default/neutron-server' CA_CERT_PATH = '/usr/local/share/ca-certificates/keystone_juju_ca_cert.crt' MEMCACHED_CONF = '/etc/memcached.conf' API_PASTE_INI = '%s/api-paste.ini' % NEUTRON_CONF_DIR +# NOTE:(fnordahl) placeholder ml2_conf_srov.ini pointing users to ml2_conf.ini +# Due to how neutron init scripts are laid out on various Linux +# distributions we put the [ml2_sriov] section in ml2_conf.ini instead +# of its default ml2_conf_sriov.ini location. +ML2_SRIOV_INI = os.path.join(NEUTRON_CONF_DIR, + 'plugins/ml2/ml2_conf_sriov.ini') BASE_RESOURCE_MAP = OrderedDict([ (NEUTRON_CONF, { @@ -392,6 +398,11 @@ def resource_map(release=None): resource_map[conf]['contexts'].append( context.PostgresqlDBContext(database=config('database'))) + if ('kilo' <= CompareOpenStackReleases(release) <= 'mitaka' and + config('enable-sriov')): + resource_map[ML2_SRIOV_INI] = {} + resource_map[ML2_SRIOV_INI]['services'] = services + resource_map[ML2_SRIOV_INI]['contexts'] = [] else: resource_map[NEUTRON_CONF]['contexts'].append( neutron_api_context.NeutronApiSDNContext() diff --git a/templates/kilo/ml2_conf.ini b/templates/kilo/ml2_conf.ini index 8981f5e7..da762d6e 100644 --- a/templates/kilo/ml2_conf.ini +++ b/templates/kilo/ml2_conf.ini @@ -14,13 +14,7 @@ mechanism_drivers = calico {% else -%} type_drivers = {{ tenant_network_types }} tenant_network_types = {{ tenant_network_types }} -{% if enable_sriov %} -mechanism_drivers = openvswitch,l2population,sriovnicswitch -{% elif enable_hyperv %} -mechanism_drivers = openvswitch,hyperv,l2population -{% else %} -mechanism_drivers = openvswitch,l2population -{% endif %} +mechanism_drivers = {{ mechanism_drivers }} [ml2_type_gre] tunnel_id_ranges = 1:1000 @@ -53,3 +47,8 @@ firewall_driver = neutron.agent.linux.iptables_firewall.OVSHybridIptablesFirewal {% else -%} enable_security_group = False {% endif -%} + +{% if supported_pci_vendor_devs %} +[ml2_sriov] +supported_pci_vendor_devs = {{ supported_pci_vendor_devs }} +{% endif -%} diff --git a/templates/kilo/ml2_conf_sriov.ini b/templates/kilo/ml2_conf_sriov.ini new file mode 100644 index 00000000..4c4cbcdb --- /dev/null +++ b/templates/kilo/ml2_conf_sriov.ini @@ -0,0 +1,7 @@ +############################################################################### +# [ WARNING ] +# Configuration file maintained by Juju. Local changes may be overwritten. +# +# __NOTE__ [ml2_sriov] section is maintained in ml2_conf.ini +# +############################################################################### diff --git a/templates/mitaka/ml2_conf.ini b/templates/mitaka/ml2_conf.ini index bd4bd5e6..f1252526 100644 --- a/templates/mitaka/ml2_conf.ini +++ b/templates/mitaka/ml2_conf.ini @@ -14,13 +14,7 @@ mechanism_drivers = calico {% else -%} type_drivers = {{ tenant_network_types }} tenant_network_types = {{ tenant_network_types }} -{% if enable_sriov %} -mechanism_drivers = openvswitch,l2population,sriovnicswitch -{% elif enable_hyperv %} -mechanism_drivers = openvswitch,hyperv,l2population -{% else %} -mechanism_drivers = openvswitch,l2population -{% endif %} +mechanism_drivers = {{ mechanism_drivers }} {% if path_mtu -%} path_mtu = {{ path_mtu }} @@ -57,3 +51,8 @@ firewall_driver = neutron.agent.linux.iptables_firewall.OVSHybridIptablesFirewal {% else -%} enable_security_group = False {% endif -%} + +{% if supported_pci_vendor_devs %} +[ml2_sriov] +supported_pci_vendor_devs = {{ supported_pci_vendor_devs }} +{% endif -%} diff --git a/tests/basic_deployment.py b/tests/basic_deployment.py index df68b692..e8578528 100644 --- a/tests/basic_deployment.py +++ b/tests/basic_deployment.py @@ -171,6 +171,9 @@ class NeutronAPIBasicDeployment(OpenStackAmuletDeployment): neutron_api_config['openstack-origin-git'] = \ yaml.dump(openstack_origin_git) + neutron_api_config['enable-sriov'] = True + neutron_api_config['supported-pci-vendor-devs'] = '8086:1515' + keystone_config = {'admin-password': 'openstack', 'admin-token': 'ubuntutesting'} nova_cc_config = {'network-manager': 'Neutron'} @@ -211,11 +214,17 @@ class NeutronAPIBasicDeployment(OpenStackAmuletDeployment): service units.""" u.log.debug('Checking status of system services...') neutron_api_services = ['neutron-server'] + nova_cc_services = ['nova-api-os-compute', + 'nova-cert', + 'nova-scheduler', + 'nova-conductor'] + if self._get_openstack_release() >= self.xenial_newton: neutron_services = ['neutron-dhcp-agent', 'neutron-lbaasv2-agent', 'neutron-metadata-agent', 'neutron-openvswitch-agent'] + nova_cc_services.remove('nova-cert') elif self._get_openstack_release() >= self.trusty_mitaka and \ self._get_openstack_release() < self.xenial_newton: neutron_services = ['neutron-dhcp-agent', @@ -234,11 +243,6 @@ class NeutronAPIBasicDeployment(OpenStackAmuletDeployment): if self._get_openstack_release() < self.trusty_kilo: neutron_services.append('neutron-metering-agent') - nova_cc_services = ['nova-api-os-compute', - 'nova-cert', - 'nova-scheduler', - 'nova-conductor'] - services = { self.keystone_sentry: ['keystone'], self.nova_cc_sentry: nova_cc_services, @@ -607,19 +611,31 @@ class NeutronAPIBasicDeployment(OpenStackAmuletDeployment): }, 'securitygroup': { 'enable_security_group': 'False', - } + }, } - if self._get_openstack_release() == self.trusty_liberty: - # Liberty - expected['ml2'].update({ - 'mechanism_drivers': 'openvswitch,l2population' - }) - else: - # Earlier or later than Liberty + if self._get_openstack_release() < self.trusty_kilo: + # Pre-Kilo expected['ml2'].update({ 'mechanism_drivers': 'openvswitch,hyperv,l2population' }) + elif self._get_openstack_release() == self.trusty_liberty: + # Liberty + expected['ml2'].update({ + 'mechanism_drivers': 'openvswitch,l2population,sriovnicswitch' + }) + else: + # Juno, Kilo, Mitaka and newer + expected['ml2'].update({ + 'mechanism_drivers': 'openvswitch,hyperv,l2population' + ',sriovnicswitch' + }) + + if ('kilo' <= self._get_openstack_release() <= 'mitaka'): + # Kilo through Mitaka require supported_pci_vendor_devs set + expected['ml2_sriov'].update({ + 'supported_pci_vendor_devs': '8086:1515', + }) for section, pairs in expected.iteritems(): ret = u.validate_config_data(unit, conf, section, pairs) diff --git a/unit_tests/test_neutron_api_context.py b/unit_tests/test_neutron_api_context.py index 1c5724c4..bb0a0e21 100644 --- a/unit_tests/test_neutron_api_context.py +++ b/unit_tests/test_neutron_api_context.py @@ -224,6 +224,40 @@ class GeneralTests(CharmTestCase): self.test_config.set('dns-domain', value) self.assertRaises(ValueError, context.get_dns_domain) + def test_get_ml2_mechanism_drivers(self): + self.os_release.return_value = 'mitaka' + self.assertEquals(context.get_ml2_mechanism_drivers(), + 'openvswitch,hyperv,l2population') + + def test_get_ml2_mechanism_drivers_kilo(self): + self.os_release.return_value = 'kilo' + self.assertEquals(context.get_ml2_mechanism_drivers(), + 'openvswitch,hyperv,l2population') + + def test_get_ml2_mechanism_drivers_liberty(self): + self.os_release.return_value = 'liberty' + self.assertEquals(context.get_ml2_mechanism_drivers(), + 'openvswitch,l2population') + + def test_get_ml2_mechanism_drivers_no_l2pop(self): + self.os_release.return_value = 'mitaka' + self.test_config.set('l2-population', False) + self.assertEquals(context.get_ml2_mechanism_drivers(), + 'openvswitch,hyperv') + + def test_get_ml2_mechanism_drivers_sriov(self): + self.os_release.return_value = 'mitaka' + self.test_config.set('enable-sriov', True) + self.assertEquals(context.get_ml2_mechanism_drivers(), + 'openvswitch,hyperv,l2population,sriovnicswitch') + + def test_get_ml2_mechanism_drivers_no_l2pop_sriov(self): + self.os_release.return_value = 'mitaka' + self.test_config.set('enable-sriov', True) + self.test_config.set('l2-population', False) + self.assertEquals(context.get_ml2_mechanism_drivers(), + 'openvswitch,hyperv,sriovnicswitch') + class IdentityServiceContext(CharmTestCase): @@ -385,6 +419,7 @@ class NeutronCCContextTest(CharmTestCase): 'debug': True, 'enable_dvr': False, 'l3_ha': False, + 'mechanism_drivers': 'openvswitch,l2population', 'dhcp_agents_per_network': 3, 'enable_sriov': False, 'external_network': 'bob', @@ -407,7 +442,6 @@ class NeutronCCContextTest(CharmTestCase): 'vlan_ranges': 'physnet1:1000:2000', 'vni_ranges': '1001:2000', 'extension_drivers': 'port_security', - 'enable_hyperv': False } napi_ctxt = context.NeutronCCContext() self.os_release.return_value = 'havana' @@ -470,6 +504,7 @@ class NeutronCCContextTest(CharmTestCase): 'debug': True, 'enable_dvr': False, 'l3_ha': False, + 'mechanism_drivers': 'openvswitch,l2population', 'dhcp_agents_per_network': 3, 'enable_sriov': False, 'external_network': 'bob', @@ -493,7 +528,6 @@ class NeutronCCContextTest(CharmTestCase): 'vni_ranges': '1001:2000,3001:4000', 'network_providers': 'physnet2,physnet3', 'extension_drivers': 'port_security', - 'enable_hyperv': False } napi_ctxt = context.NeutronCCContext() self.os_release.return_value = 'havana' @@ -514,7 +548,7 @@ class NeutronCCContextTest(CharmTestCase): 'debug': True, 'enable_dvr': False, 'l3_ha': True, - 'enable_sriov': False, + 'mechanism_drivers': 'openvswitch', 'external_network': 'bob', 'neutron_bind_port': self.api_port, 'verbose': True, @@ -524,6 +558,7 @@ class NeutronCCContextTest(CharmTestCase): 'max_l3_agents_per_router': 2, 'min_l3_agents_per_router': 2, 'dhcp_agents_per_network': 3, + 'enable_sriov': False, 'quota_floatingip': 50, 'quota_health_monitors': -1, 'quota_member': -1, @@ -538,7 +573,6 @@ class NeutronCCContextTest(CharmTestCase): 'vlan_ranges': 'physnet1:1000:2000', 'vni_ranges': '1001:2000', 'extension_drivers': 'port_security', - 'enable_hyperv': False } napi_ctxt = context.NeutronCCContext() with patch.object(napi_ctxt, '_ensure_packages'): @@ -564,12 +598,17 @@ class NeutronCCContextTest(CharmTestCase): def test_neutroncc_context_sriov(self, _import, plugin, nm): plugin.return_value = None self.test_config.set('enable-sriov', True) + self.test_config.set('supported-pci-vendor-devs', + '1111:3333 2222:4444') ctxt_data = { 'debug': True, 'enable_dvr': False, 'l3_ha': False, + 'mechanism_drivers': 'openvswitch,hyperv,l2population' + ',sriovnicswitch', 'dhcp_agents_per_network': 3, 'enable_sriov': True, + 'supported_pci_vendor_devs': '1111:3333,2222:4444', 'external_network': 'bob', 'neutron_bind_port': self.api_port, 'verbose': True, @@ -590,10 +629,9 @@ class NeutronCCContextTest(CharmTestCase): 'vlan_ranges': 'physnet1:1000:2000', 'vni_ranges': '1001:2000', 'extension_drivers': 'port_security', - 'enable_hyperv': False } napi_ctxt = context.NeutronCCContext() - self.os_release.return_value = 'havana' + self.os_release.return_value = 'kilo' with patch.object(napi_ctxt, '_ensure_packages'): self.assertEquals(ctxt_data, napi_ctxt())