From 43c10aae867c6ccd6ed76b4c68ef58c9aa8a0726 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 20 Apr 2021 17:37:40 +0000 Subject: [PATCH] [OVN] External network ports (SR-IOV) QoS is handled by SR-IOV agent This commit is similar to [1]. The OVN QoS extension does not manage the external type ports (SR-IOV); the port QoS policy of those ports is handled by the SR-IOV agent QoS extension. When a network is updated, the OVN QoS extension should check the type of port bound. In case of having an "external" port (SR-IOV), the QoS extension should not handle it. [1]https://review.opendev.org/c/openstack/neutron/+/780054 Closes-Bug: #1925218 Change-Id: Idf85e304897c9d1073091c6cabb86be39640f332 --- neutron/common/ovn/utils.py | 33 +++++++++++++++ .../drivers/ovn/mech_driver/mech_driver.py | 4 +- .../ovn/mech_driver/ovsdb/extensions/qos.py | 13 +++--- .../ovn/mech_driver/ovsdb/ovn_client.py | 11 ++--- .../mech_driver/ovsdb/extensions/test_qos.py | 41 ++++++++++++++++++- 5 files changed, 83 insertions(+), 19 deletions(-) diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index cd6a12e41ce..1decf02a28c 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -30,6 +30,7 @@ from neutron_lib.plugins import directory from neutron_lib.utils import net as n_utils from oslo_config import cfg from oslo_log import log +from oslo_serialization import jsonutils from oslo_utils import netutils from oslo_utils import strutils from ovsdbapp import constants as ovsdbapp_const @@ -37,6 +38,8 @@ from ovsdbapp import constants as ovsdbapp_const from neutron._i18n import _ from neutron.common.ovn import constants from neutron.common.ovn import exceptions as ovn_exc +from neutron.db import models_v2 +from neutron.objects import ports as ports_obj LOG = log.getLogger(__name__) @@ -552,3 +555,33 @@ def parse_ovn_lb_port_forwarding(ovn_rtr_lb_pfs): def get_network_name_from_datapath(datapath): return datapath.external_ids['name'].replace('neutron-', '') + + +def is_port_external(port): + # This port is represented in OVN DB as lsp.type=external + capabilities = [] + vnic_type = portbindings.VNIC_NORMAL + + if isinstance(port, dict): + capabilities = get_port_capabilities(port) + vnic_type = port.get(portbindings.VNIC_TYPE, + portbindings.VNIC_NORMAL) + else: + if isinstance(port, models_v2.Port): + bindings = port.port_bindings + elif isinstance(port, ports_obj.Port): + bindings = port.bindings + else: # What else could be "port"? + bindings = [] + + if bindings: + profile = bindings[0].get('profile') + if profile: + # DB object, not OVO, stores the dict in JSON. + profile = (jsonutils.loads(profile) if isinstance(profile, str) + else profile) + capabilities = profile.get('capabilities', []) + vnic_type = bindings[0].get('vnic_type', portbindings.VNIC_NORMAL) + + return (vnic_type in constants.EXTERNAL_PORT_TYPES and + constants.PORT_CAP_SWITCHDEV not in capabilities) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index efbf23d226d..1cd4dc37700 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -858,9 +858,7 @@ class OVNMechanismDriver(api.MechanismDriver): {'port_id': port['id'], 'vnic_type': vnic_type}) return - capabilities = ovn_utils.get_port_capabilities(port) - if (vnic_type in ovn_const.EXTERNAL_PORT_TYPES and - ovn_const.PORT_CAP_SWITCHDEV not in capabilities): + if ovn_utils.is_port_external(port): LOG.debug("Refusing to bind port due to unsupported vnic_type: %s " "with no switchdev capability", vnic_type) return diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py index 3c36600cf04..ae68e48500a 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py @@ -209,16 +209,16 @@ class OVNClientQosExtension(object): if ovn_rule: txn.add(self._driver._nb_idl.qos_add(**ovn_rule)) - def create_port(self, txn, port, port_type=None): - self.update_port(txn, port, None, reset=True, port_type=port_type) + def create_port(self, txn, port): + self.update_port(txn, port, None, reset=True) def delete_port(self, txn, port): self.update_port(txn, port, None, delete=True) def update_port(self, txn, port, original_port, reset=False, delete=False, - qos_rules=None, port_type=None): - if port_type == ovn_const.LSP_TYPE_EXTERNAL: - # External ports (SR-IOV) QoS is handled the SR-IOV agent QoS + qos_rules=None): + if utils.is_port_external(port): + # External ports (SR-IOV) QoS is handled by the SR-IOV agent QoS # extension. return @@ -259,7 +259,8 @@ class OVNClientQosExtension(object): admin_context = n_context.get_admin_context() for port in qos_binding.QosPolicyPortBinding.get_ports_by_network_id( admin_context, network['id']): - if utils.is_network_device_port(port): + if (utils.is_network_device_port(port) or + utils.is_port_external(port)): continue self._update_port_qos_rules(txn, port['id'], network['id'], diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index 8d7c0f0ec6d..2457c309556 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -265,11 +265,7 @@ class OVNClient(object): not utils.is_neutron_dhcp_agent_port(port)): port_type = 'localport' - capabilities = utils.get_port_capabilities(port) - vnic_type = port.get(portbindings.VNIC_TYPE, - portbindings.VNIC_NORMAL) - if (vnic_type in ovn_const.EXTERNAL_PORT_TYPES and - ovn_const.PORT_CAP_SWITCHDEV not in capabilities): + if utils.is_port_external(port): if self.is_external_ports_supported(): port_type = ovn_const.LSP_TYPE_EXTERNAL else: @@ -425,7 +421,7 @@ class OVNClient(object): if self.is_dns_required_for_port(port): self.add_txns_to_sync_port_dns_records(txn, port) - self._qos_driver.create_port(txn, port, port_type=port_info.type) + self._qos_driver.create_port(txn, port) db_rev.bump_revision(context, port, ovn_const.TYPE_PORTS) @@ -570,8 +566,7 @@ class OVNClient(object): utils.is_lsp_trusted(port)): self._del_port_from_drop_port_group(port['id'], txn) - self._qos_driver.update_port(txn, port, port_object, - port_type=port_info.type) + self._qos_driver.update_port(txn, port, port_object) if self.is_dns_required_for_port(port): self.add_txns_to_sync_port_dns_records( diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py index facad0a5151..e04c6ed2ca1 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py @@ -15,6 +15,7 @@ from unittest import mock import netaddr +from neutron_lib.api.definitions import portbindings as portbindings_api from neutron_lib.api.definitions import qos as qos_api from neutron_lib import constants from neutron_lib import context @@ -317,8 +318,13 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): # External port, OVN QoS extension does not apply. self.mock_rules.reset_mock() port.qos_policy_id = self.qos_policies[0].id - self.qos_driver.update_port(mock.ANY, port, original_port, - port_type=ovn_const.LSP_TYPE_EXTERNAL) + port_obj.PortBinding(self.ctx, port_id=port.id, host='host', + profile={}, vif_type='', + vnic_type=portbindings_api.VNIC_DIRECT).create() + # NOTE(ralonsoh): this OVO retrieval must include, in the port object, + # the port binding register created. + port = port_obj.Port.get_object(self.ctx, id=port.id) + self.qos_driver.update_port(mock.ANY, port, original_port) self.mock_rules.assert_not_called() def test_delete_port(self): @@ -403,6 +409,37 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): self.mock_rules.assert_has_calls(calls) self.mock_rules.reset_mock() + def test_update_network_external_ports(self): + """Test update network with external ports. + + - port10: no QoS port policy + - port11: no QoS port policy but external + - port12: qos_policy0 + """ + policies_ports = [(self.qos_policies[0].id, {self.ports[0].id})] + self.ports[2].qos_policy_id = self.qos_policies[0].id + self.ports[2].update() + port_obj.PortBinding(self.ctx, port_id=self.ports[1].id, host='host', + profile={}, vif_type='', + vnic_type=portbindings_api.VNIC_DIRECT).create() + with mock.patch.object(self.qos_driver._driver._nb_idl, + 'get_lswitch_port') as mock_lsp: + mock_lsp.side_effect = [ + mock.Mock(type=ovn_const.LSP_TYPE_LOCALNET), + mock.Mock(type=ovn_const.LSP_TYPE_EXTERNAL)] + for qos_policy_id, reference_ports in policies_ports: + self.networks[0].qos_policy_id = qos_policy_id + self.networks[0].update() + original_network = {'qos_policy_id': self.qos_policies[0]} + reviewed_port_ids = self.qos_driver.update_network( + mock.ANY, self.networks[0], original_network, reset=True) + self.assertEqual(reference_ports, reviewed_port_ids) + calls = [mock.call( + mock.ANY, self.ports[0].id, self.ports[0].network_id, + qos_policy_id, None)] + self.mock_rules.assert_has_calls(calls) + self.mock_rules.reset_mock() + def test_update_policy(self): """Test update QoS policy, networks and ports bound are updated.