diff --git a/neutron/agent/ovn/agent/ovsdb.py b/neutron/agent/ovn/agent/ovsdb.py index 366a4973ac6..9af6fd5161f 100644 --- a/neutron/agent/ovn/agent/ovsdb.py +++ b/neutron/agent/ovn/agent/ovsdb.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +from neutron_lib import constants from oslo_log import log from ovsdbapp.backend.ovs_idl import connection from ovsdbapp.backend.ovs_idl import idlutils @@ -20,6 +21,7 @@ from ovsdbapp.schema.open_vswitch import impl_idl as impl_idl_ovs from neutron.agent.ovsdb.native import connection as ovsdb_conn from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import utils as ovn_utils +from neutron.common import utils as n_utils from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf as config from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import impl_idl_ovn from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovsdb_monitor @@ -146,6 +148,10 @@ def get_ovs_port_name(ovs_idl, port_id): def get_port_qos(nb_idl, port_id): """Retrieve the QoS egress max-bw and min-bw values (in kbps) of a LSP + Depending on the network type (tunnelled or not), the max-bw value can be + defined in a QoS register (tunnelled network) or in the LSP.options + (physical network). + There could be max-bw rules ingress (to-lport) and egress (from-lport); this method is only returning the egress one. The min-bw rule is only implemented for egress traffic. @@ -167,7 +173,12 @@ def get_port_qos(nb_idl, port_id): max_kbps = int(qos_rule.bandwidth.get('rate', 0)) break else: - max_kbps = 0 - - min_kbps = int(lsp.options.get(ovn_const.LSP_OPTIONS_QOS_MIN_RATE, 0)) + # The "qos_max_rate" is stored in bits/s + max_kbps = n_utils.bits_to_kilobits( + int(lsp.options.get(ovn_const.LSP_OPTIONS_QOS_MAX_RATE, 0)), + constants.SI_BASE) + # The "qos_min_rate" is stored in bits/s + min_kbps = n_utils.bits_to_kilobits( + int(lsp.options.get(ovn_const.LSP_OPTIONS_QOS_MIN_RATE, 0)), + constants.SI_BASE) return max_kbps, min_kbps diff --git a/neutron/agent/ovn/extensions/qos_hwol.py b/neutron/agent/ovn/extensions/qos_hwol.py index c492bc9b66d..2a6dcec38b5 100644 --- a/neutron/agent/ovn/extensions/qos_hwol.py +++ b/neutron/agent/ovn/extensions/qos_hwol.py @@ -108,8 +108,8 @@ class QoSBandwidthLimitEvent(row_event.RowEvent): self.ovn_agent[EXT_NAME].update_egress(port_id, max_kbps, min_kbps) -class QoSMinimumBandwidthEvent(row_event.RowEvent): - LOG_MSG = 'Port ID %s, min_kbps: %s (event: %s)' +class QoSLogicalSwitchPortEvent(row_event.RowEvent): + LOG_MSG = 'Port ID %s, max_kbps, %s, min_kbps: %s (event: %s)' def __init__(self, ovn_agent): self.ovn_agent = ovn_agent @@ -121,11 +121,14 @@ class QoSMinimumBandwidthEvent(row_event.RowEvent): if not self.ovn_agent.sb_post_fork_event.is_set(): return False - # The "qos_min_rate" set on the LSP has always egress direction. - # Check if "options:qos_min_rate" has changed. + # The "qos_xxx_rate" keys are set on the LSP are always egress + # direction. Check if "options:qos_max_rate" or "options:qos_min_rate" + # have changed. try: - ovn_min_rate = ovn_const.LSP_OPTIONS_QOS_MIN_RATE - if row.options.get(ovn_min_rate) == old.options.get(ovn_min_rate): + _min_rate = ovn_const.LSP_OPTIONS_QOS_MIN_RATE + _max_rate = ovn_const.LSP_OPTIONS_QOS_MAX_RATE + if (row.options.get(_min_rate) == old.options.get(_min_rate) and + row.options.get(_max_rate) == old.options.get(_max_rate)): return False except (KeyError, AttributeError): return False @@ -138,7 +141,7 @@ class QoSMinimumBandwidthEvent(row_event.RowEvent): def run(self, event, row, old): max_kbps, min_kbps = agent_ovsdb.get_port_qos(self.ovn_agent.nb_idl, row.name) - LOG.debug(self.LOG_MSG, row.name, min_kbps, event) + LOG.debug(self.LOG_MSG, row.name, max_kbps, min_kbps, event) self.ovn_agent[EXT_NAME].update_egress(row.name, max_kbps, min_kbps) @@ -204,7 +207,7 @@ class QoSHardwareOffloadExtension(extension_manager.OVNAgentExtension): @property def nb_idl_events(self): return [QoSBandwidthLimitEvent, - QoSMinimumBandwidthEvent, + QoSLogicalSwitchPortEvent, ] @property diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index 4f173616730..bdedcc36680 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -405,6 +405,8 @@ 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_MAX_RATE = 'qos_max_rate' +LSP_OPTIONS_QOS_BURST = 'qos_burst' LSP_OPTIONS_QOS_MIN_RATE = 'qos_min_rate' LSP_OPTIONS_LOCALNET_LEARN_FDB = 'localnet_learn_fdb' diff --git a/neutron/common/utils.py b/neutron/common/utils.py index 31c4f825f5a..659c03ea83e 100644 --- a/neutron/common/utils.py +++ b/neutron/common/utils.py @@ -22,6 +22,7 @@ import functools import hashlib import hmac import importlib +import math import os import os.path import re @@ -31,6 +32,7 @@ import socket import sys import threading import time +import typing import uuid import eventlet @@ -844,9 +846,13 @@ def bytes_to_bits(value): return value * 8 -def bits_to_kilobits(value, base): - # NOTE(slaweq): round up that even 1 bit will give 1 kbit as a result - return int((value + (base - 1)) / base) +def bits_to_kilobits( + value: typing.Union[int, float], + base: int +) -> int: + # NOTE(slaweq): round up that even 1 bit will give 1 kbit as a result, but + # zero will return zero too. + return math.ceil(value / base) def disable_extension_by_service_plugin(core_plugin, service_plugin): 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 d8571eb5744..f32a8d4ce87 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py @@ -259,6 +259,8 @@ class UpdateLSwitchPortQosOptionsCommand(command.BaseCommand): raise RuntimeError(_('Logical Switch Port %s does not exist') % port_id) + # TODO(ralonsoh): add a check to only modify the QoS related keys: + # qos_max_rate, qos_burst and qos_min_rate. for key, value in self.qos.items(): if value is None: port.delkey('options', key) 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 26854b82186..7c74d716093 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 @@ -16,6 +16,7 @@ from neutron.objects.qos import binding as qos_binding from neutron.objects.qos import policy as qos_policy from neutron.objects.qos import rule as qos_rule from neutron_lib.api.definitions import l3 as l3_api +from neutron_lib.api.definitions import provider_net as pnet_api from neutron_lib import constants from neutron_lib import context as n_context from neutron_lib.plugins import constants as plugins_const @@ -32,6 +33,8 @@ 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 +# NOTE(ralonsoh): this constant will be in neutron_lib.constants +TYPE_PHYSICAL = (constants.TYPE_FLAT, constants.TYPE_VLAN) class OVNClientQosExtension: @@ -72,7 +75,8 @@ class OVNClientQosExtension: :return: (dict) nested dictionary of QoS rules, classified per direction and rule type {egress: {bw_limit: {max_kbps, max_burst_kbps}, - dscp: {dscp_mark} + dscp: {dscp_mark}, + min_kbps: {min_kbps}, ingress: {...} } """ qos_rules = {constants.EGRESS_DIRECTION: {}, @@ -196,17 +200,54 @@ class OVNClientQosExtension: 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']) + # NOTE(ralonsoh): OVN QoS registers don't have minimum rate rules. return ovn_qos_rule - def _update_lsp_qos_options(self, txn, lsp, port_id, min_qos_value): + def _ovn_lsp_rule(self, rules): + """Generate the OVN LSP.options for physical network ports (egress) + + The Logical_Switch_Port options field is a dictionary that can contain + the following options: + * qos_min_rate: (str) indicates the minimum guaranteed rate available + for data sent from this interface, in bit/s. + * qos_max_rate: (str) indicates the maximum rate for data sent from + this interface, in bit/s. + * qos_burst: (str) indicates the maximum burst size for data sent from + this interface, in bits. + (from https://www.ovn.org/support/dist-docs/ovn-nb.5.html) + + :param rules: (dict) {bw_limit: {max_kbps, max_burst_kbps}, + dscp: {dscp_mark}, + minimum_bandwidth: {min_kbps}} + An empty dictionary will create a deletion rule. + :param port_id: (string) port ID; for L3 floating IP bandwidth + limit this is the router gateway port ID. + :return: (dict) a dictionary with the QoS rules to be updated with the + LSP.options field. By default, the values of the QoS + parameters are None. In that case, the keys are removed from + the LSP.options dictionary (check + ``UpdateLSwitchPortQosOptionsCommand``). + """ + ovn_lsp_rule = {ovn_const.LSP_OPTIONS_QOS_MAX_RATE: None, + ovn_const.LSP_OPTIONS_QOS_BURST: None, + ovn_const.LSP_OPTIONS_QOS_MIN_RATE: None} + # NOTE(ralonsoh): the rate values must be defined in bits/s and bits. + # It is used the SI_BASE=1000 constant to convert from kbits/s and + # kbits. + for rule_type, rule in rules.items(): + if rule_type == qos_consts.RULE_TYPE_BANDWIDTH_LIMIT: + qos_max_rate = str(rule['max_kbps'] * constants.SI_BASE) + ovn_lsp_rule[ovn_const.LSP_OPTIONS_QOS_MAX_RATE] = qos_max_rate + if rule.get('max_burst_kbps'): + qos_burst = str(rule['max_burst_kbps'] * constants.SI_BASE) + ovn_lsp_rule[ovn_const.LSP_OPTIONS_QOS_BURST] = qos_burst + elif rule_type == qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH: + qos_min_rate = str(rule['min_kbps'] * constants.SI_BASE) + ovn_lsp_rule[ovn_const.LSP_OPTIONS_QOS_MIN_RATE] = qos_min_rate + return ovn_lsp_rule + + def _update_lsp_qos_options(self, txn, lsp, port_id, ovn_rule_lsp): """Update the LSP QoS options :param txn: the ovsdbapp transaction object. @@ -218,15 +259,14 @@ class OVNClientQosExtension: 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. + :param ovn_rule_lsp: (dict) dictionary with the QoS values to be set in + the LSP.options. If the values are None, the keys + are removed. """ 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)) + if lsp: + txn.add(self.nb_idl.update_lswitch_qos_options(lsp, + **ovn_rule_lsp)) @staticmethod def port_effective_qos_policy_id(port): @@ -243,21 +283,21 @@ class OVNClientQosExtension: return port['qos_policy_id'], 'port' return port['qos_network_policy_id'], 'network' - def _delete_port_qos_rules(self, txn, port_id, network_id, lsp=None, - port_deleted=False): + def _delete_port_qos_rules(self, txn, port_id, network_id, network_type, + lsp=None): # 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) + for ovn_rule_qos in (self._ovn_qos_rule(direction, {}, port_id, + network_id, delete=True) + for direction in constants.VALID_DIRECTIONS): + txn.add(self.nb_idl.qos_del(**ovn_rule_qos)) - def _add_port_qos_rules(self, txn, port_id, network_id, qos_policy_id, - qos_rules, lsp=None): + if network_type in TYPE_PHYSICAL: + self._update_lsp_qos_options(txn, lsp, port_id, + self._ovn_lsp_rule({})) + + def _add_port_qos_rules(self, txn, port_id, network_id, network_type, + qos_policy_id, 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() @@ -266,28 +306,36 @@ class OVNClientQosExtension: # the QoS rules can be retrieved only once. qos_rules = qos_rules or self._qos_rules(admin_context, qos_policy_id) for direction, rules in qos_rules.items(): - # "delete=not rule": that means, when we don't have rules, we - # 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 (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) + + # "delete=not rules": that means, when we don't have rules, we + # generate an "ovn_rule_qos" to be used as input in a "qos_del" + # method. + ovn_rule_qos = self._ovn_qos_rule(direction, rules, port_id, + network_id, delete=not rules) 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, may_exist=True)) + 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, if_exists=True)) - self._update_lsp_qos_options(txn, lsp, port_id, min_qos_value) + txn.add(self.nb_idl.qos_del(**ovn_rule_qos, if_exists=True)) - def _update_port_qos_rules(self, txn, port_id, network_id, qos_policy_id, - qos_rules, lsp=None, port_deleted=False): + def _update_port_qos_rules(self, txn, port_id, network_id, network_type, + qos_policy_id, qos_rules, lsp=None): if not qos_policy_id: - self._delete_port_qos_rules(txn, port_id, network_id, lsp=lsp, - port_deleted=port_deleted) + self._delete_port_qos_rules(txn, port_id, network_id, network_type, + lsp=lsp) else: - self._add_port_qos_rules(txn, port_id, network_id, qos_policy_id, - qos_rules, lsp=lsp) + self._add_port_qos_rules(txn, port_id, network_id, network_type, + qos_policy_id, qos_rules, lsp=lsp) def create_port(self, txn, port, lsp): self.update_port(txn, port, None, reset=True, lsp=lsp) @@ -317,9 +365,12 @@ class OVNClientQosExtension: if qos_policy_id == original_qos_policy_id: return # No QoS policy change + net_name = utils.ovn_name(port['network_id']) + ls = self.nb_idl.ls_get(net_name).execute(check_errors=True) + network_type = ls.external_ids[ovn_const.OVN_NETTYPE_EXT_ID_KEY] self._update_port_qos_rules(txn, port['id'], port['network_id'], - qos_policy_id, qos_rules, lsp=lsp, - port_deleted=delete) + network_type, qos_policy_id, qos_rules, + lsp=lsp) def update_network(self, txn, network, original_network, reset=False, qos_rules=None): @@ -346,9 +397,9 @@ class OVNClientQosExtension: if (utils.is_network_device_port(port) or utils.is_port_external(port)): continue - + network_type = network[pnet_api.NETWORK_TYPE] self._update_port_qos_rules(txn, port['id'], network['id'], - qos_policy_id, qos_rules) + network_type, qos_policy_id, qos_rules) updated_port_ids.add(port['id']) fips = qos_binding.QosPolicyFloatingIPBinding.get_fips_by_network_id( @@ -403,9 +454,10 @@ class OVNClientQosExtension: qos_rules = self._qos_rules(admin_context, qos_policy_id) for direction, rules in qos_rules.items(): - # "delete=not rule": that means, when we don't have rules, we - # generate a "ovn_rule" to be used as input in a "qos_del" method. - ovn_rule = self._ovn_qos_rule( + # "delete=not rules": that means, when we don't have rules, we + # generate an "ovn_rule_qos" to be used as input in a "qos_del" + # method. + ovn_rule_qos = self._ovn_qos_rule( direction, rules, gw_port_id, floatingip['floating_network_id'], fip_id=floatingip['id'], ip_address=floatingip['floating_ip_address'], @@ -413,10 +465,10 @@ class OVNClientQosExtension: 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, may_exist=True)) + 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, if_exists=True)) + txn.add(self.nb_idl.qos_del(**ovn_rule_qos, if_exists=True)) def delete_floatingip(self, txn, floatingip): self.update_floatingip(txn, floatingip) @@ -445,17 +497,18 @@ class OVNClientQosExtension: qos_rules = self._qos_rules(admin_context, qos_policy_id) for direction, rules in qos_rules.items(): # "delete=not rule": that means, when we don't have rules, we - # generate a "ovn_rule" to be used as input in a "qos_del" method. - ovn_rule = self._ovn_qos_rule( + # generate an "ovn_rule_qos" to be used as input in a "qos_del" + # method. + ovn_rule_qos = self._ovn_qos_rule( direction, rules, gw_port_id, gw_network_id, router_id=router_id, delete=not rules) 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, may_exist=True)) + 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, if_exists=True)) + txn.add(self.nb_idl.qos_del(**ovn_rule_qos, if_exists=True)) def update_policy(self, context, policy): updated_port_ids = set() @@ -471,7 +524,13 @@ class OVNClientQosExtension: # operations. with self.nb_idl.transaction(check_error=True) as txn: for network_id in bound_networks: - network = {'qos_policy_id': policy.id, 'id': network_id} + ls = self._nb_idl.ls_get(utils.ovn_name(network_id)).execute( + check_errors=True) + net_type = ls.external_ids[ovn_const.OVN_NETTYPE_EXT_ID_KEY] + network = {'qos_policy_id': policy.id, + 'id': network_id, + pnet_api.NETWORK_TYPE: net_type, + } port_ids, fip_ids, router_ids = self.update_network( txn, network, {}, reset=True, qos_rules=qos_rules) updated_port_ids.update(port_ids) diff --git a/neutron/tests/common/test_db_base_plugin_v2.py b/neutron/tests/common/test_db_base_plugin_v2.py index 8ae8701c412..e68e76930fa 100644 --- a/neutron/tests/common/test_db_base_plugin_v2.py +++ b/neutron/tests/common/test_db_base_plugin_v2.py @@ -22,6 +22,7 @@ from unittest import mock import eventlet import netaddr +from neutron_lib.api.definitions import external_net as enet_api from neutron_lib.callbacks import exceptions from neutron_lib.callbacks import registry from neutron_lib import constants @@ -456,7 +457,7 @@ class NeutronDbPluginV2TestCase(testlib_api.WebTestCase): 'admin_state_up': admin_state_up, 'tenant_id': tenant_id}} for arg in (('admin_state_up', 'tenant_id', 'shared', - 'vlan_transparent', 'mtu', + 'vlan_transparent', 'mtu', enet_api.EXTERNAL, 'availability_zone_hints') + (arg_list or ())): # Arg must be present if arg in kwargs: diff --git a/neutron/tests/functional/agent/ovn/agent/test_ovsdb.py b/neutron/tests/functional/agent/ovn/agent/test_ovsdb.py index 79575bb8100..17df6732c53 100644 --- a/neutron/tests/functional/agent/ovn/agent/test_ovsdb.py +++ b/neutron/tests/functional/agent/ovn/agent/test_ovsdb.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import ddt from neutron_lib import constants from neutron_lib.services.qos import constants as qos_consts from oslo_utils import uuidutils @@ -25,9 +26,47 @@ from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.extensions import qos \ from neutron.tests.functional import base +@ddt.ddt class GetPortQosTestCase(base.TestOVNFunctionalBase): - def test_get_port_qos(self): + def _set_lsp_qos(self, qos_param, qos_value, lsp_name): + if qos_value is not None: + options = {qos_param: str(qos_value * constants.SI_BASE)} + else: + options = {qos_param: None} + lsp = self.nb_api.lsp_get(lsp_name).execute(check_error=True) + self.nb_api.update_lswitch_qos_options(lsp, **options).execute( + check_error=True) + if qos_value is not None: + self.assertEqual(qos_value, + int(lsp.options[qos_param]) // constants.SI_BASE) + else: + self.assertNotIn(qos_param, lsp.options) + + def _add_max_kbps_rule(self, max_qos_value, network_id, lsp_name, + network_type): + if network_type in (constants.TYPE_VLAN, constants.TYPE_FLAT): + self._set_lsp_qos(ovn_const.LSP_OPTIONS_QOS_MAX_RATE, + max_qos_value, lsp_name) + else: + qos_extension = ovn_qos.OVNClientQosExtension() + rules = {qos_consts.RULE_TYPE_BANDWIDTH_LIMIT: + {'max_kbps': max_qos_value}} + ovn_rules = qos_extension._ovn_qos_rule( + constants.EGRESS_DIRECTION, rules, lsp_name, network_id) + self.nb_api.qos_add(**ovn_rules, may_exist=True).execute( + check_error=True) + + def _remove_max_kbps_rule(self, network_name, lsp_name, network_type): + if network_type in (constants.TYPE_VLAN, constants.TYPE_FLAT): + self._set_lsp_qos(ovn_const.LSP_OPTIONS_QOS_MAX_RATE, 0, lsp_name) + else: + ext_ids = {ovn_const.OVN_PORT_EXT_ID_KEY: lsp_name} + self.nb_api.qos_del_ext_ids( + network_name, ext_ids).execute(check_error=True) + + @ddt.data(constants.TYPE_VLAN, constants.TYPE_GENEVE) + def test_get_port_qos(self, network_type): network_id = uuidutils.generate_uuid() network_name = ovn_utils.ovn_name(network_id) ext_ids = {ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: network_name} @@ -38,41 +77,27 @@ class GetPortQosTestCase(base.TestOVNFunctionalBase): lsp = self.nb_api.lsp_get(lsp_name).execute(check_error=True) self.assertIsNone(lsp.options.get(ovn_const.LSP_OPTIONS_QOS_MIN_RATE)) + # Set the max-bw rule + max_qos_value = 50000 + self._add_max_kbps_rule(max_qos_value, network_id, lsp_name, + network_type) + # Set min-bw rule in the LSP. min_qos_value = 30000 - options = {ovn_const.LSP_OPTIONS_QOS_MIN_RATE: str(min_qos_value)} - self.nb_api.update_lswitch_qos_options(lsp, **options).execute( - check_error=True) - lsp = self.nb_api.lsp_get(lsp_name).execute(check_error=True) - self.assertEqual(min_qos_value, - int(lsp.options[ovn_const.LSP_OPTIONS_QOS_MIN_RATE])) - - # Create the QoS register with the max-bw rule. - qos_extension = ovn_qos.OVNClientQosExtension() - max_qos_value = 50000 - rules = { - qos_consts.RULE_TYPE_BANDWIDTH_LIMIT: {'max_kbps': max_qos_value}} - ovn_rules = qos_extension._ovn_qos_rule(constants.EGRESS_DIRECTION, - rules, lsp.name, network_id) - self.nb_api.qos_add(**ovn_rules, may_exist=True).execute( - check_error=True) + self._set_lsp_qos(ovn_const.LSP_OPTIONS_QOS_MIN_RATE, min_qos_value, + lsp_name) # Retrieve the min-bw and max-bw egress rules associated to a port. max_kbps, min_kbps = agent_ovsdb.get_port_qos(self.nb_api, lsp.name) self.assertEqual((max_qos_value, min_qos_value), (max_kbps, min_kbps)) # Remove the min-bw rule. - options = {ovn_const.LSP_OPTIONS_QOS_MIN_RATE: str(0)} - lsp = self.nb_api.lsp_get(lsp_name).execute(check_error=True) - self.nb_api.update_lswitch_qos_options(lsp, **options).execute( - check_error=True) + self._set_lsp_qos(ovn_const.LSP_OPTIONS_QOS_MIN_RATE, None, lsp_name) max_kbps, min_kbps = agent_ovsdb.get_port_qos(self.nb_api, lsp.name) self.assertEqual((max_qos_value, 0), (max_kbps, min_kbps)) # Remove the max-bw rule - ext_ids = {ovn_const.OVN_PORT_EXT_ID_KEY: lsp_name} - self.nb_api.qos_del_ext_ids( - network_name, ext_ids).execute(check_error=True) + self._remove_max_kbps_rule(network_name, lsp_name, network_type) max_kbps, min_kbps = agent_ovsdb.get_port_qos(self.nb_api, lsp.name) self.assertEqual((0, 0), (max_kbps, min_kbps)) diff --git a/neutron/tests/functional/agent/ovn/extensions/test_qos_hwol.py b/neutron/tests/functional/agent/ovn/extensions/test_qos_hwol.py index 02d5efc8a22..757ec521837 100644 --- a/neutron/tests/functional/agent/ovn/extensions/test_qos_hwol.py +++ b/neutron/tests/functional/agent/ovn/extensions/test_qos_hwol.py @@ -15,6 +15,7 @@ from unittest import mock +from neutron_lib import constants from oslo_utils import uuidutils from neutron.agent.ovn.agent import ovsdb as agent_ovsdb @@ -120,7 +121,7 @@ class QoSBandwidthLimitEventTestCase(base.TestOVNFunctionalBase): lambda: check_update_egress_called(ovn_rule['rate']), timeout=5) -class QoSMinimumBandwidthEventTestCase(base.TestOVNFunctionalBase): +class QoSLogicalSwitchPortEventTestCase(base.TestOVNFunctionalBase): def setUp(self, **kwargs): super().setUp(**kwargs) @@ -130,7 +131,7 @@ class QoSMinimumBandwidthEventTestCase(base.TestOVNFunctionalBase): res = self._create_port(self.fmt, self.net['id']) self.port = self.deserialize(self.fmt, res)['port'] - def test_qos_min_bw_created_and_updated(self): + def test_qos_created_and_updated(self): def check_update_egress_called(max_kbps, min_kbps): try: mock_agent[qos_hwol.EXT_NAME].update_egress.assert_has_calls( @@ -140,17 +141,20 @@ class QoSMinimumBandwidthEventTestCase(base.TestOVNFunctionalBase): return False mock_agent = mock.MagicMock(nb_idl=self.nb_api) - events = [qos_hwol.QoSMinimumBandwidthEvent(mock_agent)] + events = [qos_hwol.QoSLogicalSwitchPortEvent(mock_agent)] agent_ovsdb.MonitorAgentOvnNbIdl(qos_hwol.NB_IDL_TABLES, events).start() port_id = self.port['id'] - min_kbps = 5000 + max_kbps, min_kbps = 9000, 5000 + max_bps, min_bps = (max_kbps * constants.SI_BASE, + min_kbps * constants.SI_BASE) lsp = self.nb_api.lsp_get(port_id).execute(check_error=True) - options = {ovn_const.LSP_OPTIONS_QOS_MIN_RATE: str(min_kbps)} + options = {ovn_const.LSP_OPTIONS_QOS_MAX_RATE: str(max_bps), + ovn_const.LSP_OPTIONS_QOS_MIN_RATE: str(min_bps)} self.nb_api.update_lswitch_qos_options(lsp, **options).execute( check_error=True) n_utils.wait_until_true( - lambda: check_update_egress_called(0, min_kbps), timeout=5) + lambda: check_update_egress_called(max_kbps, min_kbps), timeout=5) class PortBindingChassisCreatedEventTestCase(base.TestOVNFunctionalBase): 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 005cb3f0656..872d3d440ff 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 @@ -15,18 +15,16 @@ import copy from unittest import mock +import ddt from neutron_lib.api.definitions import external_net from neutron_lib.api.definitions import provider_net as pnet 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.common import utils from neutron.db import l3_db -from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.extensions \ - import qos as qos_extension from neutron.tests.functional import base @@ -65,33 +63,36 @@ QOS_RULES_3 = { } -class TestOVNClientQosExtensionBase(base.TestOVNFunctionalBase): +class _TestOVNClientQosExtensionBase(base.TestOVNFunctionalBase): + def setUp(self, maintenance_worker=False): + super().setUp(maintenance_worker=maintenance_worker) + self.qos_driver = self.l3_plugin._ovn_client._qos_driver - def _check_rules(self, rules, port_id, network_id, fip_id=None, - ip_address=None, check_min_rate=True, - expected_ext_ids=None): + 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): + # Remove the egress max-rate and min-rate rules. + try: + qos_rules[constants.EGRESS_DIRECTION].pop( + qos_constants.RULE_TYPE_BANDWIDTH_LIMIT, None) + qos_rules[constants.EGRESS_DIRECTION].pop( + qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH, None) + except KeyError: + pass egress_ovn_rule = self.qos_driver._ovn_qos_rule( - constants.EGRESS_DIRECTION, rules.get(constants.EGRESS_DIRECTION), + constants.EGRESS_DIRECTION, + qos_rules.get(constants.EGRESS_DIRECTION), port_id, network_id, fip_id=fip_id, ip_address=ip_address) ingress_ovn_rule = self.qos_driver._ovn_qos_rule( constants.INGRESS_DIRECTION, - rules.get(constants.INGRESS_DIRECTION), port_id, network_id, + qos_rules.get(constants.INGRESS_DIRECTION), port_id, network_id, fip_id=fip_id, ip_address=ip_address) with self.nb_api.transaction(check_error=True): ls = self.qos_driver.nb_idl.lookup( 'Logical_Switch', ovn_utils.ovn_name(network_id)) - 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)) + self.assertEqual(len(qos_rules), len(ls.qos_rules)) for rule in ls.qos_rules: if expected_ext_ids: self.assertDictEqual(expected_ext_ids, rule.external_ids) @@ -108,21 +109,31 @@ class TestOVNClientQosExtensionBase(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 and check_min_rate: - min_ovn = lsp.options.get(ovn_const.LSP_OPTIONS_QOS_MIN_RATE) - self.assertEqual(str(min_rate['min_kbps']), min_ovn) + + def _check_rules_lsp(self, rules, port_id, network_type): + if network_type not in (constants.TYPE_VLAN, constants.TYPE_FLAT): + 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) + for param in ('qos_max_rate', 'qos_burst', 'qos_min_rate'): + if qos_rule_lsp[param] is None: + self.assertNotIn(param, lsp.options) + else: + self.assertEqual(qos_rule_lsp[param], lsp.options[param]) -class TestOVNClientQosExtension(TestOVNClientQosExtensionBase): +@ddt.ddt +class TestOVNClientQosExtension(_TestOVNClientQosExtensionBase): def setUp(self, maintenance_worker=False): super().setUp( maintenance_worker=maintenance_worker) self._add_logical_switch() - self.qos_driver = qos_extension.OVNClientQosExtension( - nb_idl=self.nb_api) self.gw_port_id = 'gw_port_id' self._mock_get_router = mock.patch.object(l3_db.L3_NAT_dbonly_mixin, '_get_router') @@ -146,7 +157,8 @@ class TestOVNClientQosExtension(TestOVNClientQosExtensionBase): ovn_utils.ovn_name(self.network_1), port_id, options={'requested-chassis': 'compute1'})) - def test__update_port_qos_rules(self): + @ddt.data(constants.TYPE_VLAN, constants.TYPE_GENEVE) + def test__update_port_qos_rules(self, network_type): port = 'port1' self._add_logical_switch_port(port) @@ -157,8 +169,10 @@ class TestOVNClientQosExtension(TestOVNClientQosExtensionBase): _qos_rules[direction] = _qos_rules.get(direction, {}) self.mock_qos_rules.return_value = _qos_rules self.qos_driver._update_port_qos_rules( - txn, port, self.network_1, 'qos1', None) - self._check_rules(qos_rules, port, self.network_1) + txn, port, self.network_1, network_type, 'qos1', None) + self._check_rules_qos(qos_rules, port, self.network_1, + network_type) + self._check_rules_lsp(qos_rules, port, network_type) update_and_check(QOS_RULES_0) update_and_check(QOS_RULES_1) @@ -173,8 +187,8 @@ class TestOVNClientQosExtension(TestOVNClientQosExtensionBase): _qos_rules[direction] = _qos_rules.get(direction, {}) self.mock_qos_rules.return_value = _qos_rules self.qos_driver.update_floatingip(txn, fip) - self._check_rules(qos_rules, self.gw_port_id, self.network_1, - fip_id='fip_id', ip_address='1.2.3.4') + self._check_rules_qos(qos_rules, self.gw_port_id, self.network_1, + '', fip_id='fip_id', ip_address='1.2.3.4') def test_create_floatingip(self): self._update_fip_and_check(self.fip, QOS_RULES_1) @@ -194,12 +208,11 @@ class TestOVNClientQosExtension(TestOVNClientQosExtensionBase): self._update_fip_and_check(fip_dict, {}) -class TestOVNClientQosExtensionEndToEnd(TestOVNClientQosExtensionBase): +class TestOVNClientQosExtensionEndToEnd(_TestOVNClientQosExtensionBase): def setUp(self, maintenance_worker=False): super().setUp( maintenance_worker=maintenance_worker) - self.qos_driver = self.l3_plugin._ovn_client._qos_driver self._mock_qos_rules = mock.patch.object(self.qos_driver, '_qos_rules') self.mock_qos_rules = self._mock_qos_rules.start() @@ -245,10 +258,8 @@ class TestOVNClientQosExtensionEndToEnd(TestOVNClientQosExtensionBase): gw_info = {'network_id': network['network']['id']} router = self._create_router(utils.get_rand_name(), gw_info=gw_info) - self._check_rules( - _qos_rules, router['gw_port_id'], - network['network']['id'], - check_min_rate=False, + self._check_rules_qos( + _qos_rules, router['gw_port_id'], network['network']['id'], '', expected_ext_ids={ ovn_const.OVN_ROUTER_ID_EXT_ID_KEY: router['id']}) self.l3_plugin.delete_router(self.context, router['id']) @@ -265,10 +276,8 @@ class TestOVNClientQosExtensionEndToEnd(TestOVNClientQosExtensionBase): gw_info = {'network_id': network['network']['id']} router = self._create_router(utils.get_rand_name(), gw_info=gw_info) - self._check_rules( - _qos_rules, router['gw_port_id'], - network['network']['id'], - check_min_rate=False, + self._check_rules_qos( + _qos_rules, router['gw_port_id'], network['network']['id'], '', expected_ext_ids={ ovn_const.OVN_ROUTER_ID_EXT_ID_KEY: router['id']}) ls = self.qos_driver.nb_idl.lookup( @@ -306,9 +315,8 @@ class TestOVNClientQosExtensionEndToEnd(TestOVNClientQosExtensionBase): self.l3_plugin.update_router( self.context, router['id'], {'router': {'admin_state_up': True}}) - self._check_rules( - qos_rules, router['gw_port_id'], network['network']['id'], - check_min_rate=False, + self._check_rules_qos( + qos_rules, router['gw_port_id'], network['network']['id'], '', expected_ext_ids={ ovn_const.OVN_ROUTER_ID_EXT_ID_KEY: router['id']}) 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 f2386f68135..5e352a32736 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 @@ -15,9 +15,12 @@ import random from unittest import mock +import ddt import netaddr +from neutron_lib.api.definitions import external_net as enet_api from neutron_lib.api.definitions import l3 as l3_apidef from neutron_lib.api.definitions import portbindings as portbindings_api +from neutron_lib.api.definitions import provider_net as pnet_api from neutron_lib.api.definitions import qos as qos_api from neutron_lib.api.definitions import qos_fip as qos_fip_apidef from neutron_lib import constants @@ -28,12 +31,13 @@ from oslo_config import cfg from oslo_utils import uuidutils from neutron.api import extensions +from neutron.common import config as common_config from neutron.common.ovn import constants as ovn_const +from neutron.conf.plugins.ml2.drivers import driver_type from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.core_extensions import qos as core_qos from neutron.db import l3_fip_qos from neutron.db import l3_gateway_ip_qos -from neutron.objects import network as network_obj from neutron.objects import ports as port_obj from neutron.objects.qos import policy as policy_obj from neutron.objects.qos import rule as rule_obj @@ -69,6 +73,7 @@ class TestFloatingIPQoSL3NatServicePlugin( qos_fip_apidef.ALIAS] +@ddt.ddt class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): CORE_PLUGIN_CLASS = 'neutron.plugins.ml2.plugin.Ml2Plugin' @@ -78,11 +83,20 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): 'test_qos.TestFloatingIPQoSL3NatServicePlugin') def setUp(self): + common_config.register_common_config_options() ovn_conf.register_opts() + driver_type.register_ml2_drivers_geneve_opts() + self.tenant_type = constants.TYPE_GENEVE cfg.CONF.set_override('extension_drivers', self._extension_drivers, group='ml2') cfg.CONF.set_override('enable_distributed_floating_ip', 'False', group='ovn') + cfg.CONF.set_override('external_network_type', 'vlan', + group='ml2') + cfg.CONF.set_override('tenant_network_types', [self.tenant_type], + group='ml2') + cfg.CONF.set_override('vni_ranges', ['1:200'], group='ml2_type_geneve') + cfg.CONF.set_override('max_header_size', 38, group='ml2_type_geneve') extensions.register_custom_supported_check(qos_api.ALIAS, lambda: True, plugin_agnostic=True) super().setUp() @@ -98,6 +112,10 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): self._mock_rules = mock.patch.object(self.qos_driver, '_update_port_qos_rules') self.mock_rules = self._mock_rules.start() + self.mock_lsp_get = mock.patch.object(self.qos_driver._driver._nb_idl, + 'ls_get').start() + self.mock_lsp_get.return_value.execute.return_value = mock.Mock( + external_ids={ovn_const.OVN_NETTYPE_EXT_ID_KEY: mock.ANY}) self.addCleanup(self._mock_rules.stop) self.ctx = context.get_admin_context() self.project_id = uuidutils.generate_uuid() @@ -108,6 +126,11 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): return obj_cls.modify_fields_to_db( self.get_random_object_fields(obj_cls)) + def _update_network(self, network_id, qos_policy_id): + data = {'network': {'qos_policy_id': qos_policy_id}} + return self._update('networks', network_id, data, + as_admin=True)['network'] + def _create_one_port(self, mac_address_int, network_id): mac_address = netaddr.EUI(mac_address_int) port = port_obj.Port( @@ -119,11 +142,11 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): return port def _create_one_router(self): - network = network_obj.Network( - self.ctx, id=uuidutils.generate_uuid(), project_id=self.project_id) - network.create() + kwargs = {enet_api.EXTERNAL: True} + network = self._make_network(self.fmt, 'fip_net', True, as_admin=True, + **kwargs)['network'] router_gw_port = self._create_one_port(random.randint(10**6, 10**7), - network.id) + network['id']) router = router_obj.Router(self.ctx, id=uuidutils.generate_uuid(), gw_port_id=router_gw_port.id) router.create() @@ -169,26 +192,26 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): qos_policy_id=qos_policy.id) qos_rule.create() - self.fips_ports.append(self._create_one_port(1000 + net_idx, - self.fips_network.id)) + self.fips_ports.append(self._create_one_port( + 1000 + net_idx, self.fips_network['id'])) fip_ip = str(netaddr.IPAddress(fip_cidr.ip + net_idx + 1)) fip = router_obj.FloatingIP( self.ctx, id=uuidutils.generate_uuid(), project_id=self.project_id, floating_ip_address=fip_ip, - floating_network_id=self.fips_network.id, + floating_network_id=self.fips_network['id'], floating_port_id=self.fips_ports[-1].id) fip.create() self.fips.append(fip) - network = network_obj.Network( - self.ctx, id=uuidutils.generate_uuid(), - project_id=self.project_id) - network.create() + network = self._make_network( + self.fmt, 'net_{}'.format(net_idx), True, + as_admin=True)['network'] self.networks.append(network) for port_idx in range(3): self.ports.append( - self._create_one_port(net_idx * 16 + port_idx, network.id)) + self._create_one_port(net_idx * 16 + port_idx, + network['id'])) router, router_network = self._create_one_router() self.routers.append(router) @@ -325,16 +348,15 @@ 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, lsp=None, - port_deleted=False) + mock.ANY, port.id, mock.ANY, port.network_id, None, None, lsp=None) # 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, - lsp=None, port_deleted=False) + mock.ANY, port.id, port.network_id, mock.ANY, + self.qos_policies[1].id, None, lsp=None) # No change (qos_policy0) self.mock_rules.reset_mock() @@ -355,8 +377,7 @@ 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, lsp=None, - port_deleted=False) + mock.ANY, port.id, port.network_id, mock.ANY, None, None, lsp=None) # Reset (qos_policy0, regardless of being the same a in the previous # state) @@ -365,8 +386,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, - lsp=None, port_deleted=False) + mock.ANY, port.id, port.network_id, mock.ANY, + self.qos_policies[0].id, None, lsp=None) # External port, OVN QoS extension does not apply. self.mock_rules.reset_mock() @@ -380,14 +401,17 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): self.qos_driver.update_port(mock.ANY, port, original_port) self.mock_rules.assert_not_called() - def test_delete_port(self): + @ddt.data(constants.TYPE_VLAN, constants.TYPE_GENEVE) + def test_delete_port(self, network_type): self.mock_rules.reset_mock() + self.mock_lsp_get.return_value.execute.return_value = mock.Mock( + external_ids={ovn_const.OVN_NETTYPE_EXT_ID_KEY: network_type}) self.qos_driver.delete_port(mock.ANY, self.ports[1]) # 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, - lsp=None, port_deleted=True) + mock.ANY, self.ports[1].id, self.ports[1].network_id, network_type, + None, None, lsp=None) def test_update_network(self): """Test update network (internal ports). @@ -407,15 +431,17 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): self.ports[2].qos_policy_id = self.qos_policies[1].id self.ports[2].update() for qos_policy_id, reference_ports in policies_ports: - self.networks[0].qos_policy_id = qos_policy_id - self.networks[0].update() - original_network = {'qos_policy_id': self.qos_policies[0]} + self.networks[0] = self._update_network(self.networks[0]['id'], + qos_policy_id) + original_network = {'qos_policy_id': self.qos_policies[0], + pnet_api.NETWORK_TYPE: mock.ANY, + } reviewed_port_ids, _, _ = self.qos_driver.update_network( mock.ANY, self.networks[0], original_network) self.assertEqual(reference_ports, reviewed_port_ids) calls = [mock.call(mock.ANY, self.ports[0].id, - self.ports[0].network_id, qos_policy_id, - None)] + self.ports[0].network_id, self.tenant_type, + qos_policy_id, None)] self.mock_rules.assert_has_calls(calls) self.mock_rules.reset_mock() @@ -436,9 +462,11 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): self.fips[0].qos_policy_id = self.qos_policies[0].id self.fips[0].update() for qos_policy_id, ref_fips, ref_routers in network_policies: - self.fips_network.qos_policy_id = qos_policy_id - self.fips_network.update() - original_network = {'qos_policy_id': self.qos_policies[0]} + self.fips_network = self._update_network(self.fips_network['id'], + qos_policy_id) + original_network = {'qos_policy_id': self.qos_policies[0], + pnet_api.NETWORK_TYPE: mock.ANY, + } _, reviewed_fips_ids, reviewed_router_ids = ( self.qos_driver.update_network( mock.Mock(), self.fips_network, original_network)) @@ -452,8 +480,8 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): (2) from no QoS policy to no QoS policy] """ for qos_policy_id in (self.qos_policies[0].id, None): - self.networks[0].qos_policy_id = qos_policy_id - self.networks[0].update() + self.networks[0] = self._update_network( + self.networks[0]['id'], qos_policy_id) original_network = {'qos_policy_id': qos_policy_id} port_ids, fip_ids, router_ids = self.qos_driver.update_network( mock.ANY, self.networks[0], original_network) @@ -480,14 +508,15 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): self.ports[2].qos_policy_id = self.qos_policies[1].id self.ports[2].update() for qos_policy_id, reference_ports in policies_ports: - self.networks[0].qos_policy_id = qos_policy_id - self.networks[0].update() + self.networks[0] = self._update_network( + self.networks[0]['id'], qos_policy_id) original_network = {'qos_policy_id': self.qos_policies[0]} reviewed_port_ids, _, _ = self.qos_driver.update_network( mock.ANY, self.networks[0], original_network, reset=True) self.assertEqual(reference_ports, reviewed_port_ids) calls = [mock.call(mock.ANY, self.ports[0].id, - self.ports[0].network_id, qos_policy_id, None)] + self.ports[0].network_id, self.tenant_type, + qos_policy_id, None)] self.mock_rules.assert_has_calls(calls) self.mock_rules.reset_mock() @@ -510,15 +539,15 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): mock.Mock(type=ovn_const.LSP_TYPE_LOCALNET), mock.Mock(type=ovn_const.LSP_TYPE_EXTERNAL)] for qos_policy_id, reference_ports in policies_ports: - self.networks[0].qos_policy_id = qos_policy_id - self.networks[0].update() + self.networks[0] = self._update_network(self.networks[0]['id'], + qos_policy_id) original_network = {'qos_policy_id': self.qos_policies[0]} reviewed_port_ids, _, _ = self.qos_driver.update_network( mock.ANY, self.networks[0], original_network, reset=True) self.assertEqual(reference_ports, reviewed_port_ids) calls = [mock.call( mock.ANY, self.ports[0].id, self.ports[0].network_id, - qos_policy_id, None)] + self.tenant_type, qos_policy_id, None)] self.mock_rules.assert_has_calls(calls) self.mock_rules.reset_mock() @@ -549,8 +578,8 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): self.ports[4].update() self.ports[5].qos_policy_id = self.qos_policies[1].id self.ports[5].update() - self.networks[1].qos_policy_id = self.qos_policies[0].id - self.networks[1].update() + self.networks[1] = self._update_network( + self.networks[1]['id'], self.qos_policies[0].id) self.fips[0].qos_policy_id = self.qos_policies[0].id self.fips[0].update() self.fips[1].qos_policy_id = self.qos_policies[1].id @@ -570,12 +599,12 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): # 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, - lsp=None, port_deleted=False) + self.tenant_type, self.qos_policies[0].id, + mock_qos_rules, lsp=None) 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.ports[3].network_id, self.tenant_type, self.qos_policies[0].id, mock_qos_rules)) # We can't ensure the call order because we are not enforcing any order @@ -715,8 +744,8 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): # Add network QoS policy ext_net = self.router_networks[0] - ext_net.qos_policy_id = self.qos_policies[1].id - ext_net.update() + self.networks[1] = self._update_network(ext_net['id'], + self.qos_policies[1].id) router = self._get_router(self.routers[0].id) self.qos_driver.update_router(txn, router) nb_idl.qos_add.assert_called_once() diff --git a/releasenotes/notes/ovn-qos-physical-networks-77af4a1c0795e73e.yaml b/releasenotes/notes/ovn-qos-physical-networks-77af4a1c0795e73e.yaml new file mode 100644 index 00000000000..fa30b781523 --- /dev/null +++ b/releasenotes/notes/ovn-qos-physical-networks-77af4a1c0795e73e.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + Since OVN 23.06.0 the QoS enforcement for maximum bandwidth and minimum + bandwidth rules on Logical_Switch_Ports connected to Logical_Switch with + localnet ports (that means ports connected to provider type networks) is + done in this localnet port connected to the physical bridge, via TC + commands. To meet that goal it is needed to define the maximum bandwidth + and minimum bandwidth rules in the Logical_Switch_Port ``options`` + dictionary field, using the keys ``qos_min_rate``, ``qos_max_rate`` and + ``qos_burst``. diff --git a/test-requirements.txt b/test-requirements.txt index a35a4a18250..6c61479911b 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -8,7 +8,7 @@ testscenarios>=0.4 # Apache-2.0/BSD WebTest>=2.0.27 # MIT oslotest>=3.2.0 # Apache-2.0 stestr>=1.0.0 # Apache-2.0 -ddt>=1.0.1 # MIT +ddt>=1.2.1 # MIT # Needed to run DB commands in virtualenvs PyMySQL>=0.7.6 # MIT License doc8>=0.6.0 # Apache-2.0