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 6e4bc330b41..f896f720614 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 @@ -348,14 +348,25 @@ class OVNClientQosExtension: _qos_rules = (copy.deepcopy(qos_rules) if qos_rules else self._qos_rules(admin_context, qos_policy_id)) for direction, rules in _qos_rules.items(): + min_bw = rules.get(qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH) + # NOTE(ralonsoh): the QoS rules are defined in the LSP.options + # dictionary if (1) direction=egress, (2) the network is physical + # and (3) there are min-bw rules. Otherwise, the OVN QoS registers + # are used (OVN BW policer). if (network_type in TYPE_PHYSICAL and direction == constants.EGRESS_DIRECTION): - ovn_rule_lsp = self._ovn_lsp_rule(rules) - self._update_lsp_qos_options(txn, lsp, port_id, ovn_rule_lsp) - # In this particular case, the QoS rules should be defined in - # LSP.options. Only DSCP rule will create a QoS entry. - rules.pop(qos_consts.RULE_TYPE_BANDWIDTH_LIMIT, None) - rules.pop(qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH, None) + if min_bw: + ovn_rule_lsp = self._ovn_lsp_rule(rules) + self._update_lsp_qos_options(txn, lsp, port_id, + ovn_rule_lsp) + # In this particular case, the QoS rules should be defined + # in LSP.options. Only DSCP rule will create a QoS entry. + rules.pop(qos_consts.RULE_TYPE_BANDWIDTH_LIMIT, None) + rules.pop(qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH, None) + else: + # Clear the LSP.options QoS rules. + self._update_lsp_qos_options(txn, lsp, port_id, + self._ovn_lsp_rule({})) ovn_rule_qos = self._ovn_qos_rule(direction, rules, port_id, network_id) 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 2a9e01e835b..3cf8db87ecc 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 @@ -69,6 +69,13 @@ QOS_RULES_4 = { qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH: QOS_RULE_MINBW_1} } +QOS_RULES_5 = { + 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}, +} + class _TestOVNClientQosExtensionBase(base.TestOVNFunctionalBase): def setUp(self, maintenance_worker=False): @@ -78,7 +85,9 @@ class _TestOVNClientQosExtensionBase(base.TestOVNFunctionalBase): def _check_rules_qos(self, rules, port_id, network_id, network_type, 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): + min_bw = qos_rules.get(constants.EGRESS_DIRECTION, {}).get( + qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH) + if network_type in constants.TYPE_PHYSICAL and min_bw: # Remove the egress max-rate and min-rate rules, these are defined # in the LSP.options field for a physical network. try: @@ -135,12 +144,13 @@ class _TestOVNClientQosExtensionBase(base.TestOVNFunctionalBase): self.assertEqual(bandwidth, rule.bandwidth) def _check_rules_lsp(self, rules, port_id, network_type): - if network_type not in (constants.TYPE_VLAN, constants.TYPE_FLAT): + egress_rules = rules.get(constants.EGRESS_DIRECTION, {}) + min_bw = egress_rules.get(qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH) + if not (network_type in constants.TYPE_PHYSICAL and min_bw): return # If there are no egress rules, it is checked that there are no # QoS parameters in the LSP.options dictionary. - egress_rules = rules.get(constants.EGRESS_DIRECTION, {}) qos_rule_lsp = self.qos_driver._ovn_lsp_rule(egress_rules) lsp = self.qos_driver.nb_idl.lsp_get(port_id).execute( check_error=True) @@ -267,6 +277,62 @@ class TestOVNClientQosExtension(_TestOVNClientQosExtensionBase): constants.TYPE_VLAN) self._check_rules_lsp(_qos_rules, port['id'], constants.TYPE_VLAN) + def test_set_and_update_physical_network_qos(self): + # The goal of this test is to check how the OVN QoS registers and + # LSP.options are set and deleted, depending on the QoS policy rules. + # Check LP#2115952 for more information. + port = uuidutils.generate_uuid() + self._add_logical_switch_port(port) + + def _apply_rules(qos_rules): + with self.nb_api.transaction(check_error=True) as txn: + _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 = copy.deepcopy(_qos_rules) + self.qos_driver._update_port_qos_rules( + txn, port, self.network_1, constants.TYPE_VLAN, 'qos1', + None) + + # Loop this test twice, to check that all the QoS registers and + # parameters are correctly created, set or removed, regardless of the + # previous state. + for _ in range(2): + # Apply QOS_RULES_5: egress with max-bw, min-bw and DSCP rules. + # * Check the OVN QoS rule created has only DSCP information. + # * Check the LSP.options have the correct fields. + _apply_rules(QOS_RULES_5) + lsp = self.qos_driver.nb_idl.lsp_get(port).execute( + check_error=True) + for _param in ('qos_max_rate', 'qos_burst', 'qos_min_rate'): + self.assertIn(_param, lsp.options) + ls = self.qos_driver.nb_idl.lookup( + 'Logical_Switch', ovn_utils.ovn_name(self.network_1)) + self.assertEqual(1, len(ls.qos_rules)) + rule = ls.qos_rules[0] + self.assertIn(port, rule.match) + self.assertEqual({'dscp': QOS_RULE_DSCP_1['dscp_mark']}, + rule.action) + self.assertEqual({}, rule.bandwidth) + + # Apply QOS_RULES_3: egress with max-bw only rule. + # * Check the OVN QoS rule created has only max-bw information. + # * Check the LSP.options has no QoS information. + _apply_rules(QOS_RULES_3) + lsp = self.qos_driver.nb_idl.lsp_get(port).execute( + check_error=True) + for _param in ('qos_max_rate', 'qos_burst', 'qos_min_rate'): + self.assertNotIn(_param, lsp.options) + ls = self.qos_driver.nb_idl.lookup( + 'Logical_Switch', ovn_utils.ovn_name(self.network_1)) + self.assertEqual(1, len(ls.qos_rules)) + rule = ls.qos_rules[0] + self.assertIn(port, rule.match) + self.assertEqual({}, rule.action) + self.assertEqual({'burst': QOS_RULE_BW_1['max_burst_kbps'], + 'rate': QOS_RULE_BW_1['max_kbps']}, + rule.bandwidth) + class TestOVNClientQosExtensionEndToEnd(_TestOVNClientQosExtensionBase): diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py index 7023869d4be..8d9fa3ea999 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py @@ -386,8 +386,13 @@ class TestOVNClient(testlib_api.MySQLTestCaseMixin, # GW network MTU=1400 self.assertEqual(1400, int(lrp.options['gateway_mtu'])) - def test_update_port_with_qos(self): - def _check_bw(port_id, max_kbps=None, max_burst_kbps=None): + def test_update_port_with_qos_in_lsp_options(self): + # The goal of this test is to check that a Neutron port that has a + # QoS policy defined in the LSP.options (physical network, egress + # rules, max+min BW rules) is correctly updated and the LSP.options + # are defined when the Neutron port is updated (see LP#2106231). + def _check_bw(port_id, max_kbps=None, max_burst_kbps=None, + min_kbps=None): lsp = self.nb_api.lookup('Logical_Switch_Port', port_id) if max_kbps: self.assertEqual( @@ -403,37 +408,47 @@ class TestOVNClient(testlib_api.MySQLTestCaseMixin, else: self.assertNotIn(ovn_const.LSP_OPTIONS_QOS_BURST, lsp.options) + if min_kbps: + self.assertEqual( + '{}'.format(min_kbps * 1000), + lsp.options[ovn_const.LSP_OPTIONS_QOS_MIN_RATE]) + else: + self.assertNotIn(ovn_const.LSP_OPTIONS_QOS_MIN_RATE, + lsp.options) res = self._create_qos_policy(self.fmt, is_admin=True) qos = self.deserialize(self.fmt, res)['policy'] - max_kbps, max_burst_kbps = 1000, 800 + max_kbps, max_burst_kbps, min_kbps = 1000, 800, 600 + self._create_qos_rule(self.fmt, qos['id'], qos_const.RULE_TYPE_BANDWIDTH_LIMIT, max_kbps=max_kbps, max_burst_kbps=max_burst_kbps, is_admin=True) + self._create_qos_rule(self.fmt, qos['id'], + qos_const.RULE_TYPE_MINIMUM_BANDWIDTH, + min_kbps=min_kbps, is_admin=True) net_args = {provider_net.NETWORK_TYPE: 'flat', provider_net.PHYSICAL_NETWORK: 'datacentre'} with self.network(uuidutils.generate_uuid(), arg_list=tuple(net_args.keys()), as_admin=True, **net_args) as net: - with self.subnet(net) as subnet: - with self.port(subnet) as port: - port_data = port['port'] - # Check no QoS options. - _check_bw(port_data['id']) + with self.subnet(net) as subnet, self.port(subnet) as port: + port_data = port['port'] + # Check no QoS options. + _check_bw(port_data['id']) - # Add QoS policy. - req = self.new_update_request( - 'ports', - {'port': {'qos_policy_id': qos['id']}}, - port_data['id']) - req.get_response(self.api) - _check_bw(port_data['id'], max_kbps, max_burst_kbps) + # Add QoS policy. + req = self.new_update_request( + 'ports', + {'port': {'qos_policy_id': qos['id']}}, + port_data['id']) + req.get_response(self.api) + _check_bw(port_data['id'], max_kbps, max_burst_kbps, min_kbps) - # Update port. - req = self.new_update_request( - 'ports', - {'port': {'name': uuidutils.generate_uuid()}}, - port_data['id']) - req.get_response(self.api) - _check_bw(port_data['id'], max_kbps, max_burst_kbps) + # Update port. + req = self.new_update_request( + 'ports', + {'port': {'name': uuidutils.generate_uuid()}}, + port_data['id']) + req.get_response(self.api) + _check_bw(port_data['id'], max_kbps, max_burst_kbps, min_kbps) diff --git a/releasenotes/notes/ovn-qos-max-bw-physical-networks-843dfce4a60fc38f.yaml b/releasenotes/notes/ovn-qos-max-bw-physical-networks-843dfce4a60fc38f.yaml new file mode 100644 index 00000000000..d42bba27a59 --- /dev/null +++ b/releasenotes/notes/ovn-qos-max-bw-physical-networks-843dfce4a60fc38f.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + When using the ML2/OVN mechanism driver, the QoS policies with maximum + bandwidth rules only are always enforced using the internal OVN policers, + regardless of the direction and network type. It is not relevant if the + QoS policy has or not DSCP rules.