From 012327bb4dfa8ab1817fb03cac530a8daa109f98 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 6 May 2025 10:08:36 +0000 Subject: [PATCH] [OVN] Change the OVN QoS rule priority for floating IPs The OVN QoS rules for router and floating IPs have the same priority and similar matches. For the router rule, the match is the gateway Logical_Router_Port, that always matches the traffic for the floating IP too. If two QoS rules with the same priority match, the selected one is not defined [1]. This patch is increasing the priority of the floating IP OVN QoS rules to match before the router QoS rules. NOTE: the floating IP QoS rules are deleted matching the QoS external_ids [2], instead of creating the same QoS rule, thus the priority is not needed for the deletion. [1]https://www.ovn.org/support/dist-docs/ovn-nb.5.html [2]https://github.com/openstack/neutron/blob/123bd115f3b65ba09560685ad6cf68c6934a6535/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py#L451-L456 Closes-Bug: #2110018 Change-Id: If01a8783ac998b2a1f1249ab6f555dd1a5148ea8 --- .../ovn/mech_driver/ovsdb/extensions/qos.py | 8 ++++-- .../ovn/mech_driver/ovsdb/maintenance.py | 26 ++++++++++++++++++ .../ovn/mech_driver/ovsdb/test_maintenance.py | 27 +++++++++++++++++++ .../mech_driver/ovsdb/extensions/test_qos.py | 12 +++++---- ...os-fip-rule-priority-16ad3908790dfa7d.yaml | 8 ++++++ 5 files changed, 74 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/ovn-qos-fip-rule-priority-16ad3908790dfa7d.yaml 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 d3fe2c666a6..12f1c05d365 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 @@ -33,6 +33,7 @@ from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf LOG = logging.getLogger(__name__) OVN_QOS_DEFAULT_RULE_PRIORITY = 2002 +OVN_QOS_FIP_RULE_PRIORITY = 2003 _MIN_RATE = ovn_const.LSP_OPTIONS_QOS_MIN_RATE # NOTE(ralonsoh): this constant will be in neutron_lib.constants TYPE_PHYSICAL = (constants.TYPE_FLAT, constants.TYPE_VLAN) @@ -167,8 +168,11 @@ class OVNClientQosExtension: match = self._ovn_qos_rule_match(rules_direction, port_id, ip_address, resident_port) - ovn_qos_rule = {'switch': lswitch_name, 'direction': direction, - 'priority': OVN_QOS_DEFAULT_RULE_PRIORITY, + priority = (OVN_QOS_FIP_RULE_PRIORITY if fip_id else + OVN_QOS_DEFAULT_RULE_PRIORITY) + ovn_qos_rule = {'switch': lswitch_name, + 'direction': direction, + 'priority': priority, 'match': match} if not rules: diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index 0c003e3ebea..e640581c7e6 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -32,6 +32,7 @@ from oslo_log import log from oslo_utils import strutils from oslo_utils import timeutils from ovsdbapp.backend.ovs_idl import event as row_event +from ovsdbapp.backend.ovs_idl import rowview from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import utils @@ -42,6 +43,8 @@ from neutron.db import ovn_revision_numbers_db as revision_numbers_db from neutron.objects import network as network_obj from neutron.objects import ports as ports_obj from neutron.objects import router as router_obj +from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.extensions import qos \ + as qos_extension from neutron import service from neutron.services.logapi.drivers.ovn import driver as log_driver @@ -1174,6 +1177,29 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): raise periodics.NeverAgain() + # TODO(ralonsoh): to remove in E+4 cycle (2nd next SLURP release) + @has_lock_periodic( + periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT, + spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING, + run_immediately=True) + def update_qos_fip_rule_priority(self): + """The new QoS FIP rule priority is OVN_QOS_FIP_RULE_PRIORITY""" + cmds = [] + table = self._nb_idl.tables['QoS'] + for qos_rule in table.rows.values(): + qos_rule = rowview.RowView(qos_rule) + if qos_rule.external_ids.get(ovn_const.OVN_FIP_EXT_ID_KEY): + cmds.append(self._nb_idl.db_set( + 'QoS', qos_rule.uuid, + ('priority', qos_extension.OVN_QOS_FIP_RULE_PRIORITY))) + + if cmds: + with self._nb_idl.transaction(check_error=True) as txn: + for cmd in cmds: + txn.add(cmd) + + raise periodics.NeverAgain() + class HashRingHealthCheckPeriodics: diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index 0a4528dba1a..37de5f345ad 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -34,6 +34,8 @@ from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import utils from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf as ovn_config from neutron.db import ovn_revision_numbers_db as db_rev +from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.extensions import qos \ + as qos_extension from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import maintenance from neutron.services.portforwarding import constants as pf_consts from neutron.tests.functional import base @@ -1472,6 +1474,31 @@ class TestMaintenance(_TestMaintenanceHelper): self.assertEqual( ls_dns_record.options.get('ovn-owned'), 'true') + def test_update_qos_fip_rule_priority(self): + def_prio = qos_extension.OVN_QOS_DEFAULT_RULE_PRIORITY + fip_prio = qos_extension.OVN_QOS_FIP_RULE_PRIORITY + neutron_net = self._create_network('network1') + ls_name = utils.ovn_name(neutron_net['id']) + self.nb_api.qos_add( + ls_name, 'from-lport', def_prio, "outport == 1", + 1000, 800, None, None, + external_ids={ovn_const.OVN_ROUTER_ID_EXT_ID_KEY: 1}) + self.nb_api.qos_add( + ls_name, 'from-lport', def_prio, "outport == 1", + 1000, 800, None, None, + external_ids={ovn_const.OVN_FIP_EXT_ID_KEY: 1}) + + self.assertRaises( + periodics.NeverAgain, + self.maint.update_qos_fip_rule_priority) + + for qos_rule in self.nb_api.qos_list(ls_name).execute( + check_errors=True): + if qos_rule.external_ids.get(ovn_const.OVN_FIP_EXT_ID_KEY): + self.assertEqual(fip_prio, qos_rule.priority) + else: + self.assertEqual(def_prio, qos_rule.priority) + class TestLogMaintenance(_TestMaintenanceHelper, test_log_driver.LogApiTestCaseBase): 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 5e352a32736..ee6da135717 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 @@ -263,9 +263,11 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): rule = {qos_constants.RULE_TYPE_BANDWIDTH_LIMIT: QOS_RULE_BW_1} match = self.qos_driver._ovn_qos_rule_match( direction, 'port_id', ip_address, 'resident_port') + priority = (qos_extension.OVN_QOS_FIP_RULE_PRIORITY if fip_id else + qos_extension.OVN_QOS_DEFAULT_RULE_PRIORITY) expected = {'burst': 100, 'rate': 200, 'direction': 'to-lport', 'match': match, - 'priority': qos_extension.OVN_QOS_DEFAULT_RULE_PRIORITY, + 'priority': priority, 'switch': 'neutron-network_id', 'external_ids': external_ids} result = self.qos_driver._ovn_qos_rule( @@ -288,10 +290,11 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): rule = {qos_constants.RULE_TYPE_DSCP_MARKING: QOS_RULE_DSCP_1} match = self.qos_driver._ovn_qos_rule_match( direction, 'port_id', ip_address, 'resident_port') + priority = (qos_extension.OVN_QOS_FIP_RULE_PRIORITY if fip_id else + qos_extension.OVN_QOS_DEFAULT_RULE_PRIORITY) expected = {'direction': 'from-lport', 'match': match, 'dscp': 16, 'switch': 'neutron-network_id', - 'priority': qos_extension.OVN_QOS_DEFAULT_RULE_PRIORITY, - 'external_ids': external_ids} + 'priority': priority, 'external_ids': external_ids} result = self.qos_driver._ovn_qos_rule( direction, rule, 'port_id', 'network_id', fip_id=fip_id, ip_address=ip_address, resident_port='resident_port') @@ -301,8 +304,7 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): qos_constants.RULE_TYPE_DSCP_MARKING: QOS_RULE_DSCP_2} expected = {'direction': 'from-lport', 'match': match, 'rate': 300, 'dscp': 20, 'switch': 'neutron-network_id', - 'priority': qos_extension.OVN_QOS_DEFAULT_RULE_PRIORITY, - 'external_ids': external_ids} + 'priority': priority, 'external_ids': external_ids} result = self.qos_driver._ovn_qos_rule( direction, rule, 'port_id', 'network_id', fip_id=fip_id, ip_address=ip_address, resident_port='resident_port') diff --git a/releasenotes/notes/ovn-qos-fip-rule-priority-16ad3908790dfa7d.yaml b/releasenotes/notes/ovn-qos-fip-rule-priority-16ad3908790dfa7d.yaml new file mode 100644 index 00000000000..d37d5361af6 --- /dev/null +++ b/releasenotes/notes/ovn-qos-fip-rule-priority-16ad3908790dfa7d.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + The OVN QoS floating IP rule has precedence over the OVN QoS router + rule. If both are present in the same router and port (the one + assigned to the floating IP), the floating IP rule will now apply. + For more information, see bug + `2110018 `_.