diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index 059bbe56970..483df8ba3ef 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -377,6 +377,7 @@ LSP_OPTIONS_VIF_PLUG_REPRESENTOR_VF_NUM_KEY = 'vif-plug:representor:vf-num' LSP_OPTIONS_REQUESTED_CHASSIS_KEY = 'requested-chassis' LSP_OPTIONS_MCAST_FLOOD_REPORTS = 'mcast_flood_reports' LSP_OPTIONS_MCAST_FLOOD = 'mcast_flood' +LSP_OPTIONS_QOS_MIN_RATE = 'qos_min_rate' LRP_OPTIONS_RESIDE_REDIR_CH = 'reside-on-redirect-chassis' diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py index 936b0cfdd05..213f6f87395 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py @@ -199,6 +199,34 @@ class SetLSwitchPortCommand(command.BaseCommand): setattr(port, col, val) +class UpdateLSwitchPortQosOptionsCommand(command.BaseCommand): + def __init__(self, api, lport, if_exists, **qos): + super().__init__(api) + self.lport = lport + self.if_exists = if_exists + self.qos = qos + + def run_idl(self, txn): + if isinstance(self.lport, command.BaseCommand): + port_id = self.lport.result + else: + port_id = self.lport.uuid + + try: + port = self.api.lookup('Logical_Switch_Port', port_id) + except idlutils.RowNotFound: + if self.if_exists: + return + raise RuntimeError(_('Logical Switch Port %s does not exist') % + port_id) + + for key, value in self.qos.items(): + if value is None: + port.delkey('options', key) + else: + port.setkey('options', key, value) + + class DelLSwitchPortCommand(command.BaseCommand): def __init__(self, api, lport, lswitch, if_exists): super(DelLSwitchPortCommand, self).__init__(api) 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 edac787735c..af41bef65e2 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 @@ -31,6 +31,7 @@ from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf LOG = logging.getLogger(__name__) OVN_QOS_DEFAULT_RULE_PRIORITY = 2002 +_MIN_RATE = ovn_const.LSP_OPTIONS_QOS_MIN_RATE class OVNClientQosExtension(object): @@ -92,9 +93,13 @@ class OVNClientQosExtension(object): r = {rule.rule_type: {'dscp_mark': rule.dscp_mark}} qos_rules[constants.EGRESS_DIRECTION].update(r) elif isinstance(rule, qos_rule.QosMinimumBandwidthRule): - # Rule supported for Placement scheduling but not enforced in - # the driver. - pass + if rule.direction == constants.INGRESS_DIRECTION: + LOG.warning('ML2/OVN QoS driver does not support minimum ' + 'bandwidth rules enforcement with ingress ' + 'direction') + else: + r = {rule.rule_type: {'min_kbps': rule.min_kbps}} + qos_rules[constants.EGRESS_DIRECTION].update(r) else: LOG.warning('Rule type %(rule_type)s from QoS policy ' '%(policy_id)s is not supported in OVN', @@ -133,7 +138,8 @@ class OVNClientQosExtension(object): :param rules_direction: (string) rules direction (egress, ingress). :param rules: (dict) {bw_limit: {max_kbps, max_burst_kbps}, - dscp: {dscp_mark}} + dscp: {dscp_mark}, + minimum_bandwidth: {min_kbps}} :param port_id: (string) port ID; for L3 floating IP bandwidth limit this is the router gateway port ID. :param network_id: (string) network ID. @@ -190,9 +196,38 @@ class OVNClientQosExtension(object): ovn_qos_rule['burst'] = rule['max_burst_kbps'] elif rule_type == qos_consts.RULE_TYPE_DSCP_MARKING: ovn_qos_rule.update({'dscp': rule['dscp_mark']}) + elif rule_type == qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH: + # NOTE(ralonsoh): minimum bandwidth rules are only supported + # for fixed IP ports (although this check is redundant, that + # ensures only fixed IP ports have this rule type in the + # returned dictionary). + if key == ovn_const.OVN_PORT_EXT_ID_KEY: + ovn_qos_rule[_MIN_RATE] = str(rule['min_kbps']) return ovn_qos_rule + def _update_lsp_qos_options(self, txn, lsp, port_id, min_qos_value): + """Update the LSP QoS options + + :param txn: the ovsdbapp transaction object. + :param lsp: (AddLSwitchPortCommand) logical switch port command, passed + when the port is being created. Because this method is + called inside the OVN DB transaction, the LSP has not been + created yet nor update in the IDL local cache. + :param port_id: (str) Neutron port ID that matches the LSP.name. + If the port ID is None, the OVN QoS rule does not + apply to a LSP but to a router gateway port or a + floating IP. + :param min_qos_value: (str) minimum bandwidth rule value in kbps; it is + a string because LSP.options is a {str:str} dict. + """ + lsp = lsp or self.nb_idl.lsp_get(port_id).execute() + if not lsp: + return + + options = {_MIN_RATE: min_qos_value} + txn.add(self.nb_idl.update_lswitch_qos_options(lsp, **options)) + @staticmethod def port_effective_qos_policy_id(port): """Return port effective QoS policy @@ -209,16 +244,21 @@ class OVNClientQosExtension(object): else: return port['qos_network_policy_id'], 'network' - def _delete_port_qos_rules(self, txn, port_id, network_id): + def _delete_port_qos_rules(self, txn, port_id, network_id, lsp=None, + port_deleted=False): # Generate generic deletion rules for both directions. In case of # creating deletion rules, the rule content is irrelevant. for ovn_rule in [self._ovn_qos_rule(direction, {}, port_id, network_id, delete=True) for direction in constants.VALID_DIRECTIONS]: + min_qos_value = ovn_rule.pop(_MIN_RATE, None) txn.add(self.nb_idl.qos_del(**ovn_rule)) + if not port_deleted: + self._update_lsp_qos_options(txn, lsp, port_id, + min_qos_value) def _add_port_qos_rules(self, txn, port_id, network_id, qos_policy_id, - qos_rules): + qos_rules, lsp=None): # NOTE(ralonsoh): we don't use the transaction context because the # QoS policy could belong to another user (network QoS policy). admin_context = n_context.get_admin_context() @@ -231,6 +271,7 @@ class OVNClientQosExtension(object): # generate a "ovn_rule" to be used as input in a "qos_del" method. ovn_rule = self._ovn_qos_rule(direction, rules, port_id, network_id, delete=not rules) + min_qos_value = ovn_rule.pop(_MIN_RATE, None) if rules: # NOTE(ralonsoh): with "may_exist=True", the "qos_add" will # create the QoS OVN rule or update the existing one. @@ -238,23 +279,25 @@ class OVNClientQosExtension(object): else: # Delete, if exists, the QoS rule in this direction. txn.add(self.nb_idl.qos_del(**ovn_rule, if_exists=True)) + self._update_lsp_qos_options(txn, lsp, port_id, min_qos_value) def _update_port_qos_rules(self, txn, port_id, network_id, qos_policy_id, - qos_rules): + qos_rules, lsp=None, port_deleted=False): if not qos_policy_id: - self._delete_port_qos_rules(txn, port_id, network_id) + self._delete_port_qos_rules(txn, port_id, network_id, lsp=lsp, + port_deleted=port_deleted) else: self._add_port_qos_rules(txn, port_id, network_id, qos_policy_id, - qos_rules) + qos_rules, lsp=lsp) - def create_port(self, txn, port): - self.update_port(txn, port, None, reset=True) + def create_port(self, txn, port, lsp): + self.update_port(txn, port, None, reset=True, lsp=lsp) 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): + qos_rules=None, lsp=None): if utils.is_port_external(port): # External ports (SR-IOV) QoS is handled by the SR-IOV agent QoS # extension. @@ -276,7 +319,8 @@ class OVNClientQosExtension(object): return # No QoS policy change self._update_port_qos_rules(txn, port['id'], port['network_id'], - qos_policy_id, qos_rules) + qos_policy_id, qos_rules, lsp=lsp, + port_deleted=delete) def update_network(self, txn, network, original_network, reset=False, qos_rules=None): diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py index 0a6ba8f774d..940afa04781 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py @@ -281,6 +281,10 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): return cmd.SetLSwitchPortCommand(self, lport_name, if_exists, **columns) + def update_lswitch_qos_options(self, port, if_exists=True, **qos): + return cmd.UpdateLSwitchPortQosOptionsCommand(self, port, if_exists, + **qos) + def delete_lswitch_port(self, lport_name=None, lswitch_name=None, ext_id=None, if_exists=True): if lport_name is not None: 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 9d4c6ab7fe1..6eb1eb4f096 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 @@ -584,7 +584,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) + self._qos_driver.create_port(txn, port, port_cmd) db_rev.bump_revision(context, port, ovn_const.TYPE_PORTS) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py index 50b555e34d1..d90da496a19 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py @@ -1240,7 +1240,7 @@ class OvnNbSynchronizer(OvnDbSynchronizer): for port in self.core_plugin.get_ports(ctx): if not ovn_qos_ext.port_effective_qos_policy_id(port)[0]: continue - ovn_qos_ext.create_port(txn, port) + ovn_qos_ext.create_port(txn, port, None) LOG.debug('Port QoS policies migration task finished') diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py index 57b7a06bdbf..60222c9cf21 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py @@ -17,7 +17,9 @@ from unittest import mock from neutron_lib import constants from neutron_lib.services.qos import constants as qos_constants +from ovsdbapp.backend.ovs_idl import idlutils +from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import utils as ovn_utils from neutron.db import l3_db from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.extensions \ @@ -29,6 +31,16 @@ QOS_RULE_BW_1 = {'max_kbps': 200, 'max_burst_kbps': 100} QOS_RULE_BW_2 = {'max_kbps': 300} QOS_RULE_DSCP_1 = {'dscp_mark': 16} QOS_RULE_DSCP_2 = {'dscp_mark': 20} +QOS_RULE_MINBW_1 = {'min_kbps': 500} + +QOS_RULES_0 = { + constants.EGRESS_DIRECTION: { + qos_constants.RULE_TYPE_BANDWIDTH_LIMIT: QOS_RULE_BW_1, + qos_constants.RULE_TYPE_DSCP_MARKING: QOS_RULE_DSCP_1, + qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH: QOS_RULE_MINBW_1}, + constants.INGRESS_DIRECTION: { + qos_constants.RULE_TYPE_BANDWIDTH_LIMIT: QOS_RULE_BW_2} +} QOS_RULES_1 = { constants.EGRESS_DIRECTION: { @@ -75,6 +87,12 @@ class TestOVNClientQosExtension(base.TestOVNFunctionalBase): with self.nb_api.transaction(check_error=True) as txn: txn.add(self.nb_api.ls_add(ovn_utils.ovn_name(self.network_1))) + def _add_logical_switch_port(self, port_id): + with self.nb_api.transaction(check_error=True) as txn: + txn.add(self.nb_api.lsp_add( + ovn_utils.ovn_name(self.network_1), port_id, + options={'requested-chassis': 'compute1'})) + def _check_rules(self, rules, port_id, network_id, fip_id=None, ip_address=None): egress_ovn_rule = self.qos_driver._ovn_qos_rule( @@ -88,6 +106,16 @@ class TestOVNClientQosExtension(base.TestOVNFunctionalBase): with self.nb_api.transaction(check_error=True): ls = self.qos_driver.nb_idl.lookup( 'Logical_Switch', ovn_utils.ovn_name(self.network_1)) + try: + lsp = self.qos_driver.nb_idl.lsp_get(port_id).execute( + check_error=True) + except idlutils.RowNotFound: + # A LSP is created only in the tests that apply QoS rules to + # an internal port. Any L3 QoS test (router gateway port or + # floating IP), won't have a LSP associated and won't check + # min-rate rules. + pass + self.assertEqual(len(rules), len(ls.qos_rules)) for rule in ls.qos_rules: ref_rule = (egress_ovn_rule if rule.direction == 'from-lport' @@ -103,9 +131,15 @@ class TestOVNClientQosExtension(base.TestOVNFunctionalBase): self.assertIn(port_id, rule.match) self.assertEqual(action, rule.action) self.assertEqual(bandwidth, rule.bandwidth) + min_rate = rules.get(constants.EGRESS_DIRECTION, {}).get( + qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH) + if min_rate is not None: + min_ovn = lsp.options.get(ovn_const.LSP_OPTIONS_QOS_MIN_RATE) + self.assertEqual(str(min_rate['min_kbps']), min_ovn) def test__update_port_qos_rules(self): port = 'port1' + self._add_logical_switch_port(port) def update_and_check(qos_rules): with self.nb_api.transaction(check_error=True) as txn: @@ -117,6 +151,7 @@ class TestOVNClientQosExtension(base.TestOVNFunctionalBase): txn, port, self.network_1, 'qos1', None) self._check_rules(qos_rules, port, self.network_1) + update_and_check(QOS_RULES_0) update_and_check(QOS_RULES_1) update_and_check(QOS_RULES_2) update_and_check(QOS_RULES_3) 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 6525edc4e44..71ebd2f4140 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 @@ -49,6 +49,7 @@ QOS_RULE_BW_2 = {'max_kbps': 300} QOS_RULE_DSCP_1 = {'dscp_mark': 16} QOS_RULE_DSCP_2 = {'dscp_mark': 20} QOS_RULE_MINBW_1 = {'min_kbps': 500} +QOS_RULE_MINBW_2 = {'min_kbps': 700} class _Context(object): @@ -202,18 +203,25 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): rule_obj.QosBandwidthLimitRule( direction=constants.INGRESS_DIRECTION, **QOS_RULE_BW_2), rule_obj.QosDscpMarkingRule(**QOS_RULE_DSCP_1), - rule_obj.QosMinimumBandwidthRule(**QOS_RULE_MINBW_1)] + rule_obj.QosMinimumBandwidthRule( + direction=constants.EGRESS_DIRECTION, **QOS_RULE_MINBW_1), + rule_obj.QosMinimumBandwidthRule( + direction=constants.INGRESS_DIRECTION, **QOS_RULE_MINBW_2), + ] mock_get_rules.return_value = rules expected = { constants.EGRESS_DIRECTION: { qos_constants.RULE_TYPE_BANDWIDTH_LIMIT: QOS_RULE_BW_1, - qos_constants.RULE_TYPE_DSCP_MARKING: QOS_RULE_DSCP_1}, + qos_constants.RULE_TYPE_DSCP_MARKING: QOS_RULE_DSCP_1, + qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH: QOS_RULE_MINBW_1}, constants.INGRESS_DIRECTION: { qos_constants.RULE_TYPE_BANDWIDTH_LIMIT: QOS_RULE_BW_2} } self.assertEqual(expected, self.qos_driver._qos_rules(mock.ANY, 'policy_id1')) - mock_warning.assert_not_called() + mock_warning.assert_called_once_with( + 'ML2/OVN QoS driver does not support minimum bandwidth rules ' + 'enforcement with ingress direction') @mock.patch.object(rule_obj, 'get_rules') def test__qos_rules_no_rules(self, mock_get_rules): @@ -317,14 +325,16 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): original_port.qos_policy_id = self.qos_policies[0].id self.qos_driver.update_port(mock.ANY, port, original_port) self.mock_rules.assert_called_once_with( - mock.ANY, port.id, port.network_id, None, None) + mock.ANY, port.id, port.network_id, None, None, lsp=None, + port_deleted=False) # Change from port policy (qos_policy0) to network policy (qos_policy1) self.mock_rules.reset_mock() port.qos_network_policy_id = self.qos_policies[1].id self.qos_driver.update_port(mock.ANY, port, original_port) self.mock_rules.assert_called_once_with( - mock.ANY, port.id, port.network_id, self.qos_policies[1].id, None) + mock.ANY, port.id, port.network_id, self.qos_policies[1].id, None, + lsp=None, port_deleted=False) # No change (qos_policy0) self.mock_rules.reset_mock() @@ -345,7 +355,8 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): # Reset (no policy) self.qos_driver.update_port(mock.ANY, port, original_port, reset=True) self.mock_rules.assert_called_once_with( - mock.ANY, port.id, port.network_id, None, None) + mock.ANY, port.id, port.network_id, None, None, lsp=None, + port_deleted=False) # Reset (qos_policy0, regardless of being the same a in the previous # state) @@ -354,7 +365,8 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): original_port.qos_policy_id = self.qos_policies[1].id self.qos_driver.update_port(mock.ANY, port, original_port, reset=True) self.mock_rules.assert_called_once_with( - mock.ANY, port.id, port.network_id, self.qos_policies[0].id, None) + mock.ANY, port.id, port.network_id, self.qos_policies[0].id, None, + lsp=None, port_deleted=False) # External port, OVN QoS extension does not apply. self.mock_rules.reset_mock() @@ -374,7 +386,8 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): # Assert that rules are deleted self.mock_rules.assert_called_once_with( - mock.ANY, self.ports[1].id, self.ports[1].network_id, None, None) + mock.ANY, self.ports[1].id, self.ports[1].network_id, None, None, + lsp=None, port_deleted=True) def test_update_network(self): """Test update network (internal ports). @@ -554,10 +567,17 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): mock.patch.object(self.qos_driver, 'update_router') as \ mock_update_router: self.qos_driver.update_policy(self.ctx, self.qos_policies[0]) - updated_ports = [self.ports[1], self.ports[3], self.ports[4]] + # Ports updated from "update_port": self.ports[1], self.ports[4] + updated_ports = [self.ports[1], self.ports[4]] calls = [mock.call(self.txn, port.id, port.network_id, - self.qos_policies[0].id, mock_qos_rules) + self.qos_policies[0].id, mock_qos_rules, + lsp=None, port_deleted=False) for port in updated_ports] + # Port updated from "update_network": self.ports[3] + calls.append(mock.call(self.txn, self.ports[3].id, + self.ports[3].network_id, + self.qos_policies[0].id, mock_qos_rules)) + # We can't ensure the call order because we are not enforcing any order # when retrieving the port and the network list. self.mock_rules.assert_has_calls(calls, any_order=True) diff --git a/releasenotes/notes/ovn-qos-minimum-bandwidth-74d51f63a536440a.yaml b/releasenotes/notes/ovn-qos-minimum-bandwidth-74d51f63a536440a.yaml new file mode 100644 index 00000000000..b5bc3e4bcdc --- /dev/null +++ b/releasenotes/notes/ovn-qos-minimum-bandwidth-74d51f63a536440a.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Added support for QoS minimum bandwidth rules (egress only) in ML2/OVN. + OVN supports setting these rule types in the logical switch ports since + `release 22.06.0 `_.