From f6d956c1f5daa6272145955e52986de12ba86bc4 Mon Sep 17 00:00:00 2001 From: Moshe Levi Date: Wed, 30 Aug 2017 18:35:48 +0300 Subject: [PATCH] ovs mech: bind only if user request switchdev In I77650be5f04775a72e2bdf694f93988825a84b72 we added vnic_type direct to the ovs mechanism drivers supported vnic_types. This cause problems when working with ovs and sriovnicswitch mechanism drivers in that order. In this case the ovs will bind the direct port instead of the sriovnicswitch. This change make ovs mech driver to bind the direct port only if user requested --binding-profile '{"capabilities": ["switchdev"]}' in the direct port if a user don't request this capability the SR-IOV legacy NIC mode is used. When enable-sriov-nic-features will be implemented in nova and libvirt will expose the switchdev capability then nova will be able to select a host which supports SR-IOV nic with switchdev mode. [1] - https://review.openstack.org/#/c/435954/11/specs/pike/approved/enable-sriov-nic-features.rst [2] - https://www.redhat.com/archives/libvir-list/2017-August/msg00583.html Closes-Bug: #1713590 Change-Id: I0b5f062bcbf02381bdf4f694fc039f9bb17a2db5 (cherry picked from commit b184558ab6a61571160346818dcf220d925c5b30) --- .../mech_sriov/mech_driver/mech_driver.py | 9 ++++++ .../mech_driver/mech_openvswitch.py | 16 +++++++++++ .../unit/plugins/ml2/_test_mech_agent.py | 6 ++-- .../mech_driver/test_mech_sriov_nic_switch.py | 28 +++++++++++-------- .../mech_driver/test_mech_openvswitch.py | 22 +++++++++++---- 5 files changed, 62 insertions(+), 19 deletions(-) diff --git a/neutron/plugins/ml2/drivers/mech_sriov/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/mech_sriov/mech_driver/mech_driver.py index 094e4b7e0a3..b6e9a1dd2d5 100644 --- a/neutron/plugins/ml2/drivers/mech_sriov/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/mech_sriov/mech_driver/mech_driver.py @@ -76,8 +76,17 @@ class SriovNicSwitchMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase): "network %(network)s", {'port': context.current['id'], 'network': context.network.current['id']}) + profile = context.current.get(portbindings.PROFILE) vnic_type = context.current.get(portbindings.VNIC_TYPE, portbindings.VNIC_NORMAL) + capabilities = [] + if profile: + capabilities = profile.get('capabilities', []) + if (vnic_type == portbindings.VNIC_DIRECT and + 'switchdev' in capabilities): + LOG.debug("Refusing to bind due to unsupported vnic_type: %s " + "with switchdev capability", portbindings.VNIC_DIRECT) + return if vnic_type not in self.supported_vnic_types: LOG.debug("Refusing to bind due to unsupported vnic_type: %s", vnic_type) diff --git a/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py b/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py index f6af8ab5429..dc03189df94 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py +++ b/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py @@ -20,6 +20,7 @@ from neutron_lib.callbacks import events from neutron_lib.callbacks import registry from neutron_lib import constants from oslo_config import cfg +from oslo_log import log from neutron.agent import securitygroups_rpc from neutron.plugins.common import constants as p_constants @@ -28,6 +29,7 @@ from neutron.plugins.ml2.drivers.openvswitch.agent.common \ import constants as a_const from neutron.services.qos.drivers.openvswitch import driver as ovs_qos_driver +LOG = log.getLogger(__name__) IPTABLES_FW_DRIVER_FULL = ("neutron.agent.linux.iptables_firewall." "OVSHybridIptablesFirewallDriver") @@ -74,6 +76,20 @@ class OpenvswitchMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase): """Currently Openvswitch driver doesn't support vlan transparency.""" return False + def bind_port(self, context): + vnic_type = context.current.get(portbindings.VNIC_TYPE, + portbindings.VNIC_NORMAL) + profile = context.current.get(portbindings.PROFILE) + capabilities = [] + if profile: + capabilities = profile.get('capabilities', []) + if (vnic_type == portbindings.VNIC_DIRECT and + 'switchdev' not in capabilities): + LOG.debug("Refusing to bind due to unsupported vnic_type: %s with " + "no switchdev capability", portbindings.VNIC_DIRECT) + return + super(OpenvswitchMechanismDriver, self).bind_port(context) + def get_vif_type(self, context, agent, segment): caps = agent['configurations'].get('ovs_capabilities', {}) if (any(x in caps.get('iface_types', []) for x diff --git a/neutron/tests/unit/plugins/ml2/_test_mech_agent.py b/neutron/tests/unit/plugins/ml2/_test_mech_agent.py index 1d5593c494a..011892365bc 100644 --- a/neutron/tests/unit/plugins/ml2/_test_mech_agent.py +++ b/neutron/tests/unit/plugins/ml2/_test_mech_agent.py @@ -43,11 +43,12 @@ class FakeNetworkContext(api.NetworkContext): class FakePortContext(api.PortContext): def __init__(self, agent_type, agents, segments, vnic_type=portbindings.VNIC_NORMAL, - original=None): + original=None, profile=None): self._agent_type = agent_type self._agents = agents self._network_context = FakeNetworkContext(segments) self._bound_vnic_type = vnic_type + self._bound_profile = profile self._bound_segment_id = None self._bound_vif_type = None self._bound_vif_details = None @@ -56,7 +57,8 @@ class FakePortContext(api.PortContext): @property def current(self): return {'id': PORT_ID, - portbindings.VNIC_TYPE: self._bound_vnic_type} + portbindings.VNIC_TYPE: self._bound_vnic_type, + portbindings.PROFILE: self._bound_profile} @property def original(self): diff --git a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/mech_driver/test_mech_sriov_nic_switch.py b/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/mech_driver/test_mech_sriov_nic_switch.py index bc3a0498b8f..40661d3b4ab 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/mech_driver/test_mech_sriov_nic_switch.py +++ b/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/mech_driver/test_mech_sriov_nic_switch.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import mock from neutron_lib.api.definitions import portbindings from neutron_lib import constants from neutron_lib.plugins.ml2 import api @@ -24,25 +25,16 @@ from neutron.plugins.ml2.drivers.mech_sriov.mech_driver \ from neutron.plugins.ml2.drivers.mech_sriov.mech_driver import mech_driver from neutron.tests.unit.plugins.ml2 import _test_mech_agent as base -MELLANOX_CONNECTX3_PCI_INFO = '15b3:1004' - class TestFakePortContext(base.FakePortContext): def __init__(self, agent_type, agents, segments, vnic_type=portbindings.VNIC_NORMAL, - profile={'pci_vendor_info': - MELLANOX_CONNECTX3_PCI_INFO}): + profile=None): super(TestFakePortContext, self).__init__(agent_type, agents, segments, - vnic_type) - self._bound_profile = profile - - @property - def current(self): - return {'id': base.PORT_ID, - portbindings.VNIC_TYPE: self._bound_vnic_type, - portbindings.PROFILE: self._bound_profile} + vnic_type=vnic_type, + profile=profile) def set_binding(self, segment_id, vif_type, vif_details, state): self._bound_segment_id = segment_id @@ -145,6 +137,18 @@ class SriovSwitchMechVnicTypeTestCase(SriovNicSwitchMechanismBaseTestCase): self._check_vif_type_for_vnic_type(portbindings.VNIC_DIRECT_PHYSICAL, portbindings.VIF_TYPE_HOSTDEV_PHY) + @mock.patch.object(mech_driver.SriovNicSwitchMechanismDriver, + 'try_to_bind_segment_for_agent') + def test_vnic_type_direct_with_switchdev_cap(self, mocked_bind_segment): + profile = {'capabilities': ['switchdev']} + context = TestFakePortContext(self.AGENT_TYPE, + self.AGENTS, + self.VLAN_SEGMENTS, + portbindings.VNIC_DIRECT, + profile) + self.driver.bind_port(context) + mocked_bind_segment.assert_not_called() + class SriovSwitchMechVifDetailsTestCase(SriovNicSwitchMechanismBaseTestCase): VLAN_SEGMENTS = [{api.ID: 'vlan_segment_id', diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/mech_driver/test_mech_openvswitch.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/mech_driver/test_mech_openvswitch.py index 3486450e49e..ddf67cb4e68 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/mech_driver/test_mech_openvswitch.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/mech_driver/test_mech_openvswitch.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import mock from neutron_lib.api.definitions import portbindings from neutron_lib.callbacks import events from neutron_lib.callbacks import registry @@ -243,12 +244,23 @@ class OpenvswitchMechanismDPDKTestCase(OpenvswitchMechanismBaseTestCase): class OpenvswitchMechanismSRIOVTestCase(OpenvswitchMechanismBaseTestCase): - def _make_port_ctx(self, agents): + def _make_port_ctx(self, agents, profile=None): segments = [{api.ID: 'local_segment_id', api.NETWORK_TYPE: 'local'}] return base.FakePortContext(self.AGENT_TYPE, agents, segments, - vnic_type=portbindings.VNIC_DIRECT) + vnic_type=portbindings.VNIC_DIRECT, + profile=profile) - def test_get_vif_type(self): + @mock.patch('neutron.plugins.ml2.drivers.mech_agent.' + 'SimpleAgentMechanismDriverBase.bind_port') + def test_bind_port_sriov_legacy(self, mocked_bind_port): context = self._make_port_ctx(self.AGENTS) - result = self.driver.get_vif_type(context, self.AGENTS[0], None) - self.assertEqual(self.VIF_TYPE, result) + self.driver.bind_port(context) + mocked_bind_port.assert_not_called() + + @mock.patch('neutron.plugins.ml2.drivers.mech_agent.' + 'SimpleAgentMechanismDriverBase.bind_port') + def test_bind_port_sriov_switchdev(self, mocked_bind_port): + profile = {'capabilities': ['switchdev']} + context = self._make_port_ctx(self.AGENTS, profile=profile) + self.driver.bind_port(context) + mocked_bind_port.assert_called()