From 6d5366278551d62d0d44f9bfb400f8546e4b0d48 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 4 Mar 2025 16:05:10 +0000 Subject: [PATCH] [OVN] Do not call ``QoSAddCommand`` without max-bw/DSCP rule The ML2/OVN QoS extension formats the Neutron QoS rules into the corresponding QoS registers and LSP.options values. The OVN QoS registers allow to define filters for a specific traffic, matching the direction and the source port. This registers can have a "rate" and a "dscp" values defined. It is mandatory to at least define one of these parameters. If, for example, the Neutron rules only define a min-bw rule, none of these parameters will be set in the dictionary with the OVN QoS rules. In that case, ``qos_add`` is not called. Closes-Bug: #2101063 Change-Id: Ia4f1d61753594c54f55bd146ad6dc06195a01485 --- .../ovn/mech_driver/ovsdb/extensions/qos.py | 68 +++++++++++-------- .../mech_driver/ovsdb/extensions/test_qos.py | 30 ++++++-- ...qos-min-bw-rule-only-25a0699c1510c4c4.yaml | 6 ++ 3 files changed, 68 insertions(+), 36 deletions(-) create mode 100644 releasenotes/notes/ovn-qos-min-bw-rule-only-25a0699c1510c4c4.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 bd9ba8f2505..456040ae02a 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 @@ -175,6 +175,21 @@ class OVNClientQosExtension: # Any specific rule parameter is left undefined. return ovn_qos_rule + for rule_type, rule in rules.items(): + if rule_type == qos_consts.RULE_TYPE_BANDWIDTH_LIMIT: + ovn_qos_rule['rate'] = rule['max_kbps'] + if rule.get('max_burst_kbps'): + ovn_qos_rule['burst'] = rule['max_burst_kbps'] + elif rule_type == qos_consts.RULE_TYPE_DSCP_MARKING: + ovn_qos_rule['dscp'] = rule['dscp_mark'] + # NOTE(ralonsoh): OVN QoS registers don't have minimum rate rules. + + if (ovn_qos_rule.get('rate') is None and + ovn_qos_rule.get('dscp') is None): + # Any specific rule parameter is left undefined, no OVN QoS rules + # defined. + return ovn_qos_rule + # All OVN QoS rules have an external ID reference to the port or the # FIP that are attached to. # 1) L3 floating IP ports. @@ -188,15 +203,6 @@ class OVNClientQosExtension: key, value = ovn_const.OVN_PORT_EXT_ID_KEY, port_id ovn_qos_rule['external_ids'] = {key: value} - for rule_type, rule in rules.items(): - if rule_type == qos_consts.RULE_TYPE_BANDWIDTH_LIMIT: - ovn_qos_rule['rate'] = rule['max_kbps'] - if rule.get('max_burst_kbps'): - 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']}) - # NOTE(ralonsoh): OVN QoS registers don't have minimum rate rules. - return ovn_qos_rule def _ovn_lsp_rule(self, rules): @@ -242,6 +248,26 @@ class OVNClientQosExtension: ovn_lsp_rule[ovn_const.LSP_OPTIONS_QOS_MIN_RATE] = qos_min_rate return ovn_lsp_rule + def _apply_ovn_rule_qos(self, txn, rules, ovn_rule_qos): + """Add or remove the OVN QoS rules (for max-bw and DSCP rules only). + + :param txn: the ovsdbapp transaction object. + :param rules: Neutron QoS rules (per direction). + :param ovn_rule_qos: dictionary with the Neutron QoS rules with the + parameters needed to call ``qos_add`` or + ``qos_del`` commands. + """ + if rules and not (ovn_rule_qos.get('rate') is None and + ovn_rule_qos.get('dscp') is None): + # NOTE(ralonsoh): with "may_exist=True", the "qos_add" will + # create the QoS OVN rule or update the existing one. + # NOTE(ralonsoh): if the Neutron QoS rules don't have at least + # a max-bw rule or a DSCP rule, skip this command. + txn.add(self.nb_idl.qos_add(**ovn_rule_qos, may_exist=True)) + else: + # Delete, if exists, the QoS rule in this direction. + txn.add(self.nb_idl.qos_del(**ovn_rule_qos, if_exists=True)) + def _update_lsp_qos_options(self, txn, lsp, port_id, ovn_rule_lsp): """Update the LSP QoS options @@ -313,13 +339,7 @@ class OVNClientQosExtension: ovn_rule_qos = self._ovn_qos_rule(direction, rules, port_id, network_id) - if rules: - # NOTE(ralonsoh): with "may_exist=True", the "qos_add" will - # create the QoS OVN rule or update the existing one. - txn.add(self.nb_idl.qos_add(**ovn_rule_qos, may_exist=True)) - else: - # Delete, if exists, the QoS rule in this direction. - txn.add(self.nb_idl.qos_del(**ovn_rule_qos, if_exists=True)) + self._apply_ovn_rule_qos(txn, rules, ovn_rule_qos) def _update_port_qos_rules(self, txn, port_id, network_id, network_type, qos_policy_id, qos_rules, lsp=None): @@ -452,13 +472,7 @@ class OVNClientQosExtension: floatingip['floating_network_id'], fip_id=floatingip['id'], ip_address=floatingip['floating_ip_address'], resident_port=resident_port) - if rules: - # NOTE(ralonsoh): with "may_exist=True", the "qos_add" will - # create the QoS OVN rule or update the existing one. - txn.add(self.nb_idl.qos_add(**ovn_rule_qos, may_exist=True)) - else: - # Delete, if exists, the QoS rule in this direction. - txn.add(self.nb_idl.qos_del(**ovn_rule_qos, if_exists=True)) + self._apply_ovn_rule_qos(txn, rules, ovn_rule_qos) def delete_floatingip(self, txn, floatingip): self.update_floatingip(txn, floatingip) @@ -489,13 +503,7 @@ class OVNClientQosExtension: ovn_rule_qos = self._ovn_qos_rule( direction, rules, gw_port_id, gw_network_id, router_id=router_id) - if rules: - # NOTE(ralonsoh): with "may_exist=True", the "qos_add" will - # create the QoS OVN rule or update the existing one. - txn.add(self.nb_idl.qos_add(**ovn_rule_qos, may_exist=True)) - else: - # Delete, if exists, the QoS rule in this direction. - txn.add(self.nb_idl.qos_del(**ovn_rule_qos, if_exists=True)) + self._apply_ovn_rule_qos(txn, rules, ovn_rule_qos) def update_policy(self, context, policy): updated_port_ids = set() 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 bfc25877ae9..2a9e01e835b 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 @@ -64,6 +64,11 @@ QOS_RULES_3 = { qos_constants.RULE_TYPE_BANDWIDTH_LIMIT: QOS_RULE_BW_1} } +QOS_RULES_4 = { + constants.INGRESS_DIRECTION: { + qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH: QOS_RULE_MINBW_1} +} + class _TestOVNClientQosExtensionBase(base.TestOVNFunctionalBase): def setUp(self, maintenance_worker=False): @@ -74,7 +79,8 @@ class _TestOVNClientQosExtensionBase(base.TestOVNFunctionalBase): fip_id=None, ip_address=None, expected_ext_ids=None): qos_rules = copy.deepcopy(rules) if network_type in (constants.TYPE_VLAN, constants.TYPE_FLAT): - # Remove the egress max-rate and min-rate rules. + # Remove the egress max-rate and min-rate rules, these are defined + # in the LSP.options field for a physical network. try: qos_rules[constants.EGRESS_DIRECTION].pop( qos_constants.RULE_TYPE_BANDWIDTH_LIMIT, None) @@ -82,6 +88,17 @@ class _TestOVNClientQosExtensionBase(base.TestOVNFunctionalBase): qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH, None) except KeyError: pass + + # Remove the min-bw rule from the qos_rules because this is not added + # to the OVN QoS registers. + for _, rules in qos_rules.items(): + rules.pop(qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH, None) + + # Remove any direction in qos_rules without defined rules. + for direction in copy.deepcopy(qos_rules): + if not qos_rules[direction]: + qos_rules.pop(direction) + egress_ovn_rule = self.qos_driver._ovn_qos_rule( constants.EGRESS_DIRECTION, qos_rules.get(constants.EGRESS_DIRECTION), @@ -179,7 +196,7 @@ class TestOVNClientQosExtension(_TestOVNClientQosExtensionBase): _qos_rules = copy.deepcopy(qos_rules) for direction in constants.VALID_DIRECTIONS: _qos_rules[direction] = _qos_rules.get(direction, {}) - self.mock_qos_rules.return_value = _qos_rules + self.mock_qos_rules.return_value = copy.deepcopy(_qos_rules) self.qos_driver._update_port_qos_rules( txn, port, self.network_1, network_type, 'qos1', None) self._check_rules_qos(qos_rules, port, self.network_1, @@ -190,6 +207,7 @@ class TestOVNClientQosExtension(_TestOVNClientQosExtensionBase): update_and_check(QOS_RULES_1) update_and_check(QOS_RULES_2) update_and_check(QOS_RULES_3) + update_and_check(QOS_RULES_4) update_and_check({}) def _update_fip_and_check(self, fip, qos_rules): @@ -197,7 +215,7 @@ class TestOVNClientQosExtension(_TestOVNClientQosExtensionBase): _qos_rules = copy.deepcopy(qos_rules) for direction in constants.VALID_DIRECTIONS: _qos_rules[direction] = _qos_rules.get(direction, {}) - self.mock_qos_rules.return_value = _qos_rules + self.mock_qos_rules.return_value = copy.deepcopy(_qos_rules) self.qos_driver.update_floatingip(txn, fip) self._check_rules_qos(qos_rules, self.gw_port_id, self.network_1, '', fip_id='fip_id', ip_address='1.2.3.4') @@ -292,7 +310,7 @@ class TestOVNClientQosExtensionEndToEnd(_TestOVNClientQosExtensionBase): _qos_rules = copy.deepcopy(QOS_RULES_1) for direction in constants.VALID_DIRECTIONS: _qos_rules[direction] = _qos_rules.get(direction, {}) - self.mock_qos_rules.return_value = _qos_rules + self.mock_qos_rules.return_value = copy.deepcopy(_qos_rules) network = self._create_ext_network( utils.get_rand_name(), 'flat', 'physnet4', @@ -310,7 +328,7 @@ class TestOVNClientQosExtensionEndToEnd(_TestOVNClientQosExtensionBase): _qos_rules = copy.deepcopy(QOS_RULES_1) for direction in constants.VALID_DIRECTIONS: _qos_rules[direction] = _qos_rules.get(direction, {}) - self.mock_qos_rules.return_value = _qos_rules + self.mock_qos_rules.return_value = copy.deepcopy(_qos_rules) network = self._create_ext_network( utils.get_rand_name(), 'flat', 'physnet4', @@ -350,7 +368,7 @@ class TestOVNClientQosExtensionEndToEnd(_TestOVNClientQosExtensionBase): _qos_rules = copy.deepcopy(qos_rules) for direction in constants.VALID_DIRECTIONS: _qos_rules[direction] = _qos_rules.get(direction, {}) - self.mock_qos_rules.return_value = _qos_rules + self.mock_qos_rules.return_value = copy.deepcopy(_qos_rules) self.l3_plugin.update_router( self.context, router['id'], {'router': {'admin_state_up': False}}) diff --git a/releasenotes/notes/ovn-qos-min-bw-rule-only-25a0699c1510c4c4.yaml b/releasenotes/notes/ovn-qos-min-bw-rule-only-25a0699c1510c4c4.yaml new file mode 100644 index 00000000000..4bef9b0a230 --- /dev/null +++ b/releasenotes/notes/ovn-qos-min-bw-rule-only-25a0699c1510c4c4.yaml @@ -0,0 +1,6 @@ +--- +issues: + - | + Using ML2/OVN, it is now possible to define a QoS policy with a single minimum + bandwidth rule and apply it to a bound port. The ML2/OVN QoS extension + will not try to add an OVN QoS register in that case.