Merge "Use the OVN policer for max-bw only QoS policies" into stable/2025.1

Author: Zuul

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py

@@ -348,14 +348,25 @@ def _add_port_qos_rules(self, txn, port_id, network_id, network_type,
         _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)

neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py

@@ -69,6 +69,13 @@
         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 @@ def setUp(self, maintenance_worker=False):
     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 @@ def _check_rules_qos(self, rules, port_id, network_id, network_type,
                 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 @@ def test_update_policy(self):
                                   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):
 

neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py

@@ -386,8 +386,13 @@ def test_create_router_port_multiple_routers(self):
                     # 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 @@ def _check_bw(port_id, max_kbps=None, max_burst_kbps=None):
             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'])
-
-                    # 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)
-
-                    # 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)
+            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, 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, min_kbps)

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.