From 18e05b5b2aad44cc53ea8b3d1f9c2c1c4bc08571 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Mon, 31 Jan 2022 15:42:04 +0100 Subject: [PATCH] Clean duplicated QoS bandwidth related methods in ovs_lib module This patch also some helper methods used in the minimum bandwidth qos methods as it seems that we had things almost duplicated in methods like _find/update/delete_{qos,queue} and find/update/delete_{qos,queue}. It also moves functional tests for the ingress bandwidth limit rules methods to more appropriate module. Related-bug: 1959567 Change-Id: I848af01c8fe3a08b26d05e37d225c944ea080f03 (cherry picked from commit 0255f41ad0405eac8b2a4e0870513846e2660f7f) --- neutron/agent/common/ovs_lib.py | 256 +++++++----------- .../agent/extension_drivers/qos_driver.py | 6 +- neutron/tests/fullstack/agents/ovs_agent.py | 2 +- neutron/tests/fullstack/test_qos.py | 12 +- .../functional/agent/common/test_ovs_lib.py | 142 ++++++++-- neutron/tests/functional/agent/l2/base.py | 2 +- .../tests/functional/agent/test_ovs_lib.py | 49 ---- .../extension_drivers/test_qos_driver.py | 11 +- 8 files changed, 230 insertions(+), 250 deletions(-) diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py index 471337cae20..b7d39177520 100644 --- a/neutron/agent/common/ovs_lib.py +++ b/neutron/agent/common/ovs_lib.py @@ -771,56 +771,6 @@ class OVSBridge(BaseOVS): return self._set_egress_bw_limit_for_port(port_name, 0, 0, check_error=False) - def find_qos(self, port_name): - qos = self.ovsdb.db_find( - 'QoS', - ('external_ids', '=', {'id': port_name}), - columns=['_uuid', 'other_config']).execute(check_error=True) - if qos: - return qos[0] - - def find_queue(self, port_name, queue_type): - queues = self.ovsdb.db_find( - 'Queue', - ('external_ids', '=', {'id': port_name, - 'queue_type': str(queue_type)}), - columns=['_uuid', 'other_config']).execute(check_error=True) - if queues: - return queues[0] - - def _update_bw_limit_queue(self, txn, port_name, queue_uuid, queue_type, - other_config): - if queue_uuid: - txn.add(self.ovsdb.db_set( - 'Queue', queue_uuid, - ('other_config', other_config))) - else: - external_ids = {'id': port_name, - 'queue_type': str(queue_type)} - queue_uuid = txn.add( - self.ovsdb.db_create( - 'Queue', external_ids=external_ids, - other_config=other_config)) - return queue_uuid - - def _update_bw_limit_profile(self, txn, port_name, qos_uuid, - queue_uuid, queue_type, qos_other_config): - queues = {queue_type: queue_uuid} - if qos_uuid: - txn.add(self.ovsdb.db_set( - 'QoS', qos_uuid, ('queues', queues))) - txn.add(self.ovsdb.db_set( - 'QoS', qos_uuid, ('other_config', qos_other_config))) - else: - external_ids = {'id': port_name} - qos_uuid = txn.add( - self.ovsdb.db_create( - 'QoS', external_ids=external_ids, - type='linux-htb', - queues=queues, - other_config=qos_other_config)) - return qos_uuid - def _update_bw_limit_profile_dpdk(self, txn, port_name, qos_uuid, other_config): if qos_uuid: @@ -835,120 +785,85 @@ class OVSBridge(BaseOVS): return qos_uuid def _update_ingress_bw_limit_for_port( - self, port_name, max_bw_in_bits, max_burst_in_bits): - qos_other_config = { - 'max-rate': str(int(max_bw_in_bits)) - } - queue_other_config = { - 'max-rate': str(int(max_bw_in_bits)), - 'burst': str(int(max_burst_in_bits)), - } - qos = self.find_qos(port_name) - queue = self.find_queue(port_name, QOS_DEFAULT_QUEUE) - qos_uuid = qos['_uuid'] if qos else None - queue_uuid = queue['_uuid'] if queue else None - with self.ovsdb.transaction(check_error=True) as txn: - queue_uuid = self._update_bw_limit_queue( - txn, port_name, queue_uuid, QOS_DEFAULT_QUEUE, - queue_other_config - ) - - qos_uuid = self._update_bw_limit_profile( - txn, port_name, qos_uuid, queue_uuid, QOS_DEFAULT_QUEUE, - qos_other_config - ) - - txn.add(self.ovsdb.db_set( - 'Port', port_name, ('qos', qos_uuid))) + self, port_name, max_kbps, max_burst_kbps): + queue_id = self._update_queue( + port_name, QOS_DEFAULT_QUEUE, + qos_constants.RULE_TYPE_BANDWIDTH_LIMIT, + max_kbps=max_kbps, max_burst_kbps=max_burst_kbps) + qos_id, qos_queues = self._find_qos( + port_name, + qos_constants.RULE_TYPE_BANDWIDTH_LIMIT) + if qos_queues: + qos_queues[QOS_DEFAULT_QUEUE] = queue_id + else: + qos_queues = {QOS_DEFAULT_QUEUE: queue_id} + qos_id = self._update_qos( + port_name, + qos_constants.RULE_TYPE_BANDWIDTH_LIMIT, + qos_id=qos_id, queues=qos_queues) + self._set_port_qos(port_name, qos_id=qos_id) def _update_ingress_bw_limit_for_dpdk_port( - self, port_name, max_bw_in_bits, max_burst_in_bits): + self, port_name, max_kbps, max_burst_kbps): # cir and cbs should be set in bytes instead of bits + max_bw_in_bits = max_kbps * p_const.SI_BASE + max_burst_in_bits = max_burst_kbps * p_const.SI_BASE qos_other_config = { 'cir': str(max_bw_in_bits // 8), 'cbs': str(max_burst_in_bits // 8) } - qos = self.find_qos(port_name) - qos_uuid = qos['_uuid'] if qos else None + qos_id, qos_queues = self._find_qos(port_name) with self.ovsdb.transaction(check_error=True) as txn: qos_uuid = self._update_bw_limit_profile_dpdk( - txn, port_name, qos_uuid, qos_other_config) + txn, port_name, qos_id, qos_other_config) txn.add(self.ovsdb.db_set( 'Port', port_name, ('qos', qos_uuid))) def update_ingress_bw_limit_for_port(self, port_name, max_kbps, max_burst_kbps): - max_bw_in_bits = max_kbps * p_const.SI_BASE - max_burst_in_bits = max_burst_kbps * p_const.SI_BASE port_type = self._get_port_val(port_name, "type") if port_type in constants.OVS_DPDK_PORT_TYPES: self._update_ingress_bw_limit_for_dpdk_port( - port_name, max_bw_in_bits, max_burst_in_bits) + port_name, max_kbps, max_burst_kbps) else: self._update_ingress_bw_limit_for_port( - port_name, max_bw_in_bits, max_burst_in_bits) + port_name, max_kbps, max_burst_kbps) def get_ingress_bw_limit_for_port(self, port_name): - max_kbps = None qos_max_kbps = None - queue_max_kbps = None max_burst_kbit = None - qos_res = self.find_qos(port_name) - if qos_res: - other_config = qos_res['other_config'] + queue = self._find_queue( + port_name, _type=qos_constants.RULE_TYPE_BANDWIDTH_LIMIT) + if queue: + other_config = queue['other_config'] max_bw_in_bits = other_config.get('max-rate') - if max_bw_in_bits is not None: - qos_max_kbps = int(max_bw_in_bits) / p_const.SI_BASE - - queue_res = self.find_queue(port_name, QOS_DEFAULT_QUEUE) - if queue_res: - other_config = queue_res['other_config'] - max_bw_in_bits = other_config.get('max-rate') - if max_bw_in_bits is not None: - queue_max_kbps = int(max_bw_in_bits) / p_const.SI_BASE + qos_max_kbps = int(int(max_bw_in_bits) / p_const.SI_BASE) max_burst_in_bits = other_config.get('burst') if max_burst_in_bits is not None: - max_burst_kbit = ( - int(max_burst_in_bits) / p_const.SI_BASE) + max_burst_kbit = int(int(max_burst_in_bits) / p_const.SI_BASE) - if qos_max_kbps == queue_max_kbps: - max_kbps = qos_max_kbps - else: - LOG.warning("qos max-rate %(qos_max_kbps)s is not equal to " - "queue max-rate %(queue_max_kbps)s", - {'qos_max_kbps': qos_max_kbps, - 'queue_max_kbps': queue_max_kbps}) - return max_kbps, max_burst_kbit - - def get_ingress_bw_limit_for_dpdk_port(self, port_name): - max_kbps = None - max_burst_kbit = None - res = self.find_qos(port_name) - if res: - other_config = res['other_config'] - max_bw_in_bytes = other_config.get("cir") - if max_bw_in_bytes is not None: - max_kbps = common_utils.bits_to_kilobits( - common_utils.bytes_to_bits(int(float(max_bw_in_bytes))), - p_const.SI_BASE) - max_burst_in_bytes = other_config.get("cbs") - if max_burst_in_bytes is not None: - max_burst_kbit = common_utils.bits_to_kilobits( - common_utils.bytes_to_bits(int(float(max_burst_in_bytes))), - p_const.SI_BASE) - return max_kbps, max_burst_kbit + return qos_max_kbps, max_burst_kbit def delete_ingress_bw_limit_for_port(self, port_name): - self.ovsdb.db_clear('Port', port_name, - 'qos').execute(check_error=False) - qos = self.find_qos(port_name) - queue = self.find_queue(port_name, QOS_DEFAULT_QUEUE) - with self.ovsdb.transaction(check_error=True) as txn: - if qos: - txn.add(self.ovsdb.db_destroy('QoS', qos['_uuid'])) - if queue: - txn.add(self.ovsdb.db_destroy('Queue', queue['_uuid'])) + qos_id, qos_queues = self._find_qos( + port_name, + qos_constants.RULE_TYPE_BANDWIDTH_LIMIT) + if not qos_queues: + return + if QOS_DEFAULT_QUEUE in qos_queues.keys(): + queue_uuid = qos_queues.pop(QOS_DEFAULT_QUEUE) + if qos_queues: + self._update_qos( + port_name, + qos_constants.RULE_TYPE_BANDWIDTH_LIMIT, + qos_id=qos_id, queues=qos_queues) + self.ovsdb.db_clear('Port', port_name, 'qos').execute( + check_error=False) + if not qos_queues: + self._delete_qos(qos_id) + self._delete_queue( + queue_uuid, qos_constants.RULE_TYPE_BANDWIDTH_LIMIT) def set_controller_field(self, field, value): attr = [(field, value)] @@ -1017,13 +932,19 @@ class OVSBridge(BaseOVS): def update_minimum_bandwidth_queue(self, port_id, egress_port_names, queue_num, min_kbps): queue_num = int(queue_num) - queue_id = self._update_queue(port_id, queue_num, min_kbps=min_kbps) - qos_id, qos_queues = self._find_qos() + queue_id = self._update_queue( + port_id, queue_num, qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH, + min_kbps=min_kbps) + qos_id, qos_queues = self._find_qos( + self._min_bw_qos_id, + qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH) if qos_queues: qos_queues[queue_num] = queue_id else: qos_queues = {queue_num: queue_id} qos_id = self._update_qos( + self._min_bw_qos_id, + qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH, qos_id=qos_id, queues=qos_queues) for egress_port_name in egress_port_names: self._set_port_qos(egress_port_name, qos_id=qos_id) @@ -1036,18 +957,29 @@ class OVSBridge(BaseOVS): return queue_num = int(queue['external_ids']['queue-num']) self._unset_queue_for_minimum_bandwidth(queue_num) - qos_id, qos_queues = self._find_qos() + qos_id, qos_queues = self._find_qos( + self._min_bw_qos_id, + qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH) if not qos_queues: return if queue_num in qos_queues.keys(): qos_queues.pop(queue_num) self._update_qos( + self._min_bw_qos_id, + qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH, qos_id=qos_id, queues=qos_queues) - self._delete_queue(queue['_uuid']) + self._delete_queue( + queue['_uuid'], qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH) - def clear_minimum_bandwidth_qos(self): - qoses = self._list_qos( - qos_type=qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH) + def clear_bandwidth_qos(self): + qoses = [] + qos_types = [ + (self._min_bw_qos_id, + qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH), + (None, + qos_constants.RULE_TYPE_BANDWIDTH_LIMIT)] + for rule_type_id, qos_type in qos_types: + qoses += self._list_qos(_id=rule_type_id, qos_type=qos_type) for qos in qoses: qos_id = qos['_uuid'] @@ -1059,21 +991,20 @@ class OVSBridge(BaseOVS): colmuns=['name']).execute(check_error=True) for port in ports: self._set_port_qos(port['name']) - self.ovsdb.db_destroy('QoS', qos_id).execute(check_error=True) + self._delete_qos(qos_id) for queue_uuid in queues.values(): self._delete_queue(queue_uuid) - def _update_queue(self, port_id, queue_num, max_kbps=None, + def _update_queue(self, port_id, queue_num, queue_type, max_kbps=None, max_burst_kbps=None, min_kbps=None): other_config = {} if max_kbps: - other_config['max-rate'] = str(max_kbps * 1000) - if max_burst_kbps: - other_config['burst'] = str(max_burst_kbps * 1000) + other_config['max-rate'] = str(int(max_kbps) * p_const.SI_BASE) + other_config['burst'] = str(int(max_burst_kbps) * p_const.SI_BASE) if min_kbps: - other_config['min-rate'] = str(min_kbps * 1000) + other_config['min-rate'] = str(min_kbps * p_const.SI_BASE) - queue = self._find_queue(port_id) + queue = self._find_queue(port_id, _type=queue_type) if queue and queue['_uuid']: if queue['other_config'] != other_config: self.set_db_attribute('Queue', queue['_uuid'], 'other_config', @@ -1082,12 +1013,12 @@ class OVSBridge(BaseOVS): # NOTE(ralonsoh): "external_ids" is a map of string-string pairs external_ids = { 'port': str(port_id), - 'type': str(qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH), + 'type': str(queue_type), 'queue-num': str(queue_num)} self.ovsdb.db_create( 'Queue', other_config=other_config, external_ids=external_ids).execute(check_error=True) - queue = self._find_queue(port_id) + queue = self._find_queue(port_id, _type=queue_type) return queue['_uuid'] def _find_queue(self, port_id, _type=None): @@ -1113,16 +1044,23 @@ class OVSBridge(BaseOVS): if queue['external_ids'].get('type') == str(_type)] return queues - def _delete_queue(self, queue_id): + def _delete_qos(self, qos_id): try: - self.ovsdb.db_destroy('Queue', queue_id).execute(check_error=True) + self.ovsdb.db_destroy('QoS', qos_id).execute(check_error=True) + except idlutils.RowNotFound: + LOG.info('OVS QoS %s was already deleted', str(qos_id)) + + def _delete_queue(self, queue_id, qos_type=None): + try: + self.ovsdb.db_destroy('Queue', queue_id).execute( + check_error=True) except idlutils.RowNotFound: LOG.info('OVS Queue %s was already deleted', queue_id) except RuntimeError as exc: with excutils.save_and_reraise_exception(): if 'referential integrity violation' not in str(exc): return - qos_regs = self._list_qos() + qos_regs = self._list_qos(qos_type=qos_type) qos_uuids = [] for qos_reg in qos_regs: queue_nums = [num for num, q in qos_reg['queues'].items() @@ -1134,17 +1072,17 @@ class OVSBridge(BaseOVS): {'queue': str(queue_id), 'qoses': ', '.join(sorted(qos_uuids))}) - def _update_qos(self, qos_id=None, queues=None): + def _update_qos(self, rule_type_id, rule_type, qos_id=None, queues=None): queues = queues or {} if not qos_id: - external_ids = {'id': self._min_bw_qos_id, - '_type': qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH} + external_ids = {'id': rule_type_id, + '_type': rule_type} self.ovsdb.db_create( 'QoS', type='linux-htb', queues=queues, external_ids=external_ids).execute(check_error=True) - qos_id, _ = self._find_qos() + qos_id, _ = self._find_qos(rule_type_id, rule_type) else: self.clear_db_attribute('QoS', qos_id, 'queues') if queues: @@ -1167,8 +1105,8 @@ class OVSBridge(BaseOVS): return self.ovsdb.db_find( 'QoS', colmuns=['_uuid', 'queues']).execute(check_error=True) - def _find_qos(self): - qos_regs = self._list_qos(_id=self._min_bw_qos_id) + def _find_qos(self, rule_type_id, qos_type=None): + qos_regs = self._list_qos(_id=rule_type_id, qos_type=qos_type) if qos_regs: queues = {num: queue.uuid for num, queue in qos_regs[0]['queues'].items()} diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py b/neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py index 7357e3186e2..14c8a8a3077 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py @@ -40,7 +40,7 @@ class QosOVSAgentDriver(qos.QosLinuxAgentDriver): def consume_api(self, agent_api): self.agent_api = agent_api - def _minimum_bandwidth_initialize(self): + def _qos_bandwidth_initialize(self): """Clear QoS setting at agent restart. This is for clearing stale settings (such as ports and QoS tables @@ -49,12 +49,12 @@ class QosOVSAgentDriver(qos.QosLinuxAgentDriver): rebuild. There is no performance impact however the QoS feature will be down until the QoS rules are rebuilt. """ - self.br_int.clear_minimum_bandwidth_qos() + self.br_int.clear_bandwidth_qos() def initialize(self): self.br_int = self.agent_api.request_int_br() self.cookie = self.br_int.default_cookie - self._minimum_bandwidth_initialize() + self._qos_bandwidth_initialize() def create_bandwidth_limit(self, port, rule): self.update_bandwidth_limit(port, rule) diff --git a/neutron/tests/fullstack/agents/ovs_agent.py b/neutron/tests/fullstack/agents/ovs_agent.py index 5ec8fe947fa..f10559ba963 100755 --- a/neutron/tests/fullstack/agents/ovs_agent.py +++ b/neutron/tests/fullstack/agents/ovs_agent.py @@ -41,7 +41,7 @@ def monkeypatch_init_handler(): def monkeypatch_qos(): - mock.patch.object(ovs_lib.OVSBridge, 'clear_minimum_bandwidth_qos').start() + mock.patch.object(ovs_lib.OVSBridge, 'clear_bandwidth_qos').start() if "qos" in cfg.CONF.service_plugins: mock.patch.object(qos_extension.QosAgentExtension, '_process_reset_port').start() diff --git a/neutron/tests/fullstack/test_qos.py b/neutron/tests/fullstack/test_qos.py index cfaae13e15e..ed9868d2f0d 100644 --- a/neutron/tests/fullstack/test_qos.py +++ b/neutron/tests/fullstack/test_qos.py @@ -19,7 +19,6 @@ from neutron_lib.services.qos import constants as qos_consts from neutronclient.common import exceptions from oslo_utils import uuidutils -from neutron.agent.common import ovs_lib from neutron.agent.linux import tc_lib from neutron.common import utils from neutron.tests.common.agents import l2_extensions @@ -364,7 +363,8 @@ class TestBwLimitQoSOvs(_TestBwLimitQoS, base.BaseFullStackTestCase): elif direction == constants.INGRESS_DIRECTION: utils.wait_until_true( lambda: vm.bridge.get_ingress_bw_limit_for_port( - vm.port.name) == (limit, burst)) + vm.port.name) == (limit, burst), + timeout=10) def test_bw_limit_qos_port_removed(self): """Test if rate limit config is properly removed when whole port is @@ -382,9 +382,11 @@ class TestBwLimitQoSOvs(_TestBwLimitQoS, base.BaseFullStackTestCase): # Delete port with qos policy attached vm.destroy(delete_port=True) self._wait_for_bw_rule_removed(vm, self.direction) - self.assertIsNone(vm.bridge.find_qos(vm.port.name)) - self.assertIsNone(vm.bridge.find_queue(vm.port.name, - ovs_lib.QOS_DEFAULT_QUEUE)) + qos_id, qos_queues = vm.bridge._find_qos( + vm.port.name, + qos_consts.RULE_TYPE_BANDWIDTH_LIMIT) + self.assertIsNone(qos_id) + self.assertIsNone(qos_queues) class TestBwLimitQoSLinuxbridge(_TestBwLimitQoS, base.BaseFullStackTestCase): diff --git a/neutron/tests/functional/agent/common/test_ovs_lib.py b/neutron/tests/functional/agent/common/test_ovs_lib.py index 1ef63156e85..d772f3dfa90 100644 --- a/neutron/tests/functional/agent/common/test_ovs_lib.py +++ b/neutron/tests/functional/agent/common/test_ovs_lib.py @@ -72,9 +72,9 @@ class BaseOVSTestCase(base.BaseSudoTestCase): for device in self.elements_to_clean['devices']: ip_lib.IPDevice(device).link.delete() for qos in self.elements_to_clean['qoses']: - self.ovs.ovsdb.db_destroy('QoS', qos).execute() + self.ovs.ovsdb.db_destroy('QoS', qos).execute(log_errors=False) for queue in self.elements_to_clean['queues']: - self.ovs.ovsdb.db_destroy('Queue', queue).execute() + self.ovs.ovsdb.db_destroy('Queue', queue).execute(log_errors=False) def _list_queues(self, queue_id=None): queues = self.ovs.ovsdb.db_list( @@ -88,7 +88,9 @@ class BaseOVSTestCase(base.BaseSudoTestCase): return None return queues - def _create_queue(self, max_kbps=int(MAX_RATE_DEFAULT / 1000), + def _create_queue(self, + queue_type=qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH, + max_kbps=int(MAX_RATE_DEFAULT / 1000), max_burst_kbps=int(BURST_DEFAULT / 1000), min_kbps=int(MIN_RATE_DEFAULT / 1000), neutron_port_id=None, queue_num=None): @@ -96,6 +98,7 @@ class BaseOVSTestCase(base.BaseSudoTestCase): if not neutron_port_id else neutron_port_id) queue_num = QUEUE_NUM_DEFAULT if not queue_num else queue_num queue_id = self.ovs._update_queue(neutron_port_id, queue_num, + queue_type, max_kbps=max_kbps, max_burst_kbps=max_burst_kbps, min_kbps=min_kbps) @@ -103,15 +106,19 @@ class BaseOVSTestCase(base.BaseSudoTestCase): self.elements_to_clean['queues'].append(queue_id) return queue_id, neutron_port_id - def _create_qos(self, qos_id=None, queues=None): - qos_id = self.ovs._update_qos(qos_id=qos_id, queues=queues) + def _create_qos(self, qos_id=None, queues=None, + rule_type_id=None, + rule_type=qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH): + qos_id = self.ovs._update_qos( + rule_type_id, rule_type, qos_id=qos_id, queues=queues) self.elements_to_clean['qoses'].append(qos_id) return qos_id def _list_qos(self, qos_id=None): qoses = self.ovs.ovsdb.db_list( 'QoS', - columns=('_uuid', 'queues', 'external_ids', 'type')).execute() + columns=('_uuid', 'queues', 'other_config', 'external_ids', 'type') + ).execute() if qos_id: for qos in (qos for qos in qoses if qos['_uuid'] == qos_id): return qos @@ -163,7 +170,9 @@ class BaseOVSTestCase(base.BaseSudoTestCase): def _check_no_minbw_qos(self): """Asserts that there are no min BW qos/queues for this OVS instance""" - qos_id, qos_queues = self.ovs._find_qos() + qos_id, qos_queues = self.ovs._find_qos( + self.ovs._min_bw_qos_id, + qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH) if not qos_id and not qos_queues: return @@ -259,9 +268,13 @@ class BaseOVSTestCase(base.BaseSudoTestCase): def test__delete_queue_still_used_in_a_qos(self): queue_id, port_id = self._create_queue() queues = {1: queue_id} - qos_id_1 = self._create_qos(queues=queues) + qos_id_1 = self._create_qos( + queues=queues, + rule_type_id=self.ovs._min_bw_qos_id) self.ovs._min_bw_qos_id = uuidutils.generate_uuid() - qos_id_2 = self._create_qos(queues=queues) + qos_id_2 = self._create_qos( + queues=queues, + rule_type_id=self.ovs._min_bw_qos_id) with mock.patch.object(ovs_lib.LOG, 'error') as mock_error: self.assertRaises(RuntimeError, self.ovs._delete_queue, queue_id) @@ -276,7 +289,9 @@ class BaseOVSTestCase(base.BaseSudoTestCase): queue_id, port_id = self._create_queue() queues = {1: queue_id} - qos_id = self._create_qos(queues=queues) + qos_id = self._create_qos( + queues=queues, + rule_type_id=self.ovs._min_bw_qos_id) external_ids = {'id': str(self.ovs._min_bw_qos_id), '_type': qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH} expected = {'_uuid': qos_id, @@ -291,7 +306,9 @@ class BaseOVSTestCase(base.BaseSudoTestCase): queue_id_1, _ = self._create_queue() queues = {1: queue_id_1} - qos_id = self._create_qos(queues=queues) + qos_id = self._create_qos( + queues=queues, + rule_type_id=self.ovs._min_bw_qos_id) external_ids = {'id': str(self.ovs._min_bw_qos_id), '_type': qos_constants.RULE_TYPE_MINIMUM_BANDWIDTH} expected = {'_uuid': qos_id, @@ -305,7 +322,10 @@ class BaseOVSTestCase(base.BaseSudoTestCase): queue_id_2, _ = self._create_queue() queues[2] = queue_id_2 - self._create_qos(qos_id=qos_id, queues=queues) + self._create_qos( + qos_id=qos_id, + queues=queues, + rule_type_id=self.ovs._min_bw_qos_id) self._check_value(expected, self._list_qos, qos_id, keys_to_check=['_uuid', 'type', 'external_ids']) qos = self._list_qos(qos_id) @@ -316,8 +336,11 @@ class BaseOVSTestCase(base.BaseSudoTestCase): def test__find_qos(self): queue_id, _ = self._create_queue() queues = {1: queue_id} - qos_id = self._create_qos(queues=queues) - self._check_value((qos_id, queues), self.ovs._find_qos) + qos_id = self._create_qos( + queues=queues, + rule_type_id=self.ovs._min_bw_qos_id) + self._check_value((qos_id, queues), self.ovs._find_qos, + self.ovs._min_bw_qos_id) def test__set_port_qos(self): port_name = ('port-' + uuidutils.generate_uuid())[:8] @@ -325,7 +348,8 @@ class BaseOVSTestCase(base.BaseSudoTestCase): self._create_port(port_name) self._check_value([], self._find_port_qos, port_name) - qos_id = self._create_qos() + qos_id = self._create_qos( + rule_type_id=self.ovs._min_bw_qos_id) self.ovs._set_port_qos(port_name, qos_id=qos_id) self._check_value(qos_id, self._find_port_qos, port_name) @@ -345,6 +369,75 @@ class BaseOVSTestCase(base.BaseSudoTestCase): bridge_ports.sort() self.assertEqual(device_names, bridge_ports) + def test_egress_bw_limit(self): + port_name = ('port-' + uuidutils.generate_uuid())[:8] + self._create_bridge() + self._create_port(port_name) + self.ovs.create_egress_bw_limit_for_port(port_name, 700, 70) + max_rate, burst = self.ovs.get_egress_bw_limit_for_port(port_name) + self.assertEqual(700, max_rate) + self.assertEqual(70, burst) + self.ovs.delete_egress_bw_limit_for_port(port_name) + max_rate, burst = self.ovs.get_egress_bw_limit_for_port(port_name) + self.assertIsNone(max_rate) + self.assertIsNone(burst) + + def test_ingress_bw_limit(self): + port_name = ('port-' + uuidutils.generate_uuid())[:8] + self._create_bridge() + self._create_port(port_name) + self.ovs.update_ingress_bw_limit_for_port(port_name, 700, 70) + qos_id = self._find_port_qos(port_name) + qos = self._list_qos(qos_id) + queue_id = qos['queues'][0].uuid + external_ids = {'port': str(port_name), + 'type': qos_constants.RULE_TYPE_BANDWIDTH_LIMIT, + 'queue-num': '0'} + other_config = {'burst': '70000', + 'max-rate': '700000'} + expected = {'_uuid': queue_id, + 'external_ids': external_ids, + 'other_config': other_config} + + self._check_value(expected, self._list_queues, queue_id) + self.elements_to_clean['qoses'].append(qos_id) + self.elements_to_clean['queues'].append(queue_id) + + self.ovs.update_ingress_bw_limit_for_port(port_name, 750, 100) + expected['other_config'] = {'burst': '100000', + 'max-rate': '750000'} + + self.ovs.delete_ingress_bw_limit_for_port(port_name) + self.assertIsNone(self._list_qos(qos_id)) + + def test_ingress_bw_limit_dpdk_port(self): + port_name = ('port-' + uuidutils.generate_uuid())[:8] + self._create_bridge() + self._create_port(port_name) + self.ovs.ovsdb.db_set( + 'Interface', port_name, + ('type', ovs_constants.OVS_DPDK_VHOST_USER)).execute() + self.ovs.update_ingress_bw_limit_for_port(port_name, 700, 70) + qos_id = self._find_port_qos(port_name) + external_ids = {'id': str(port_name)} + other_config = {'cir': str(700 * p_const.SI_BASE // 8), + 'cbs': str(70 * p_const.SI_BASE // 8)} + expected = {'_uuid': qos_id, + 'external_ids': external_ids, + 'other_config': other_config, + 'queues': {}, + 'type': 'egress-policer'} + self._check_value(expected, self._list_qos, qos_id) + + self.ovs.update_ingress_bw_limit_for_port(port_name, 750, 100) + expected['other_config'] = {'cir': str(750 * p_const.SI_BASE // 8), + 'cbs': str(100 * p_const.SI_BASE // 8)} + self._check_value(expected, self._list_qos, qos_id) + + self.ovs.delete_ingress_bw_limit_for_port(port_name) + qos = self._list_qos(qos_id) + self.assertEqual(0, len(qos['queues'])) + def test__set_queue_for_minimum_bandwidth(self): self._create_bridge() self.ovs._set_queue_for_minimum_bandwidth(1234) @@ -368,8 +461,9 @@ class BaseOVSTestCase(base.BaseSudoTestCase): queue_num = 1 queue_id, port_id = self._create_queue(neutron_port_id=self.port_id) queues = {queue_num: queue_id} - qos_id = self._create_qos(queues=queues) - + qos_id = self._create_qos( + queues=queues, + rule_type_id=self.ovs._min_bw_qos_id) self.ovs.update_minimum_bandwidth_queue(self.port_id, [port_name], queue_num, 1800) self._check_value(qos_id, self._find_port_qos, port_name) @@ -410,7 +504,9 @@ class BaseOVSTestCase(base.BaseSudoTestCase): queue_id_1, neutron_port_id_1 = self._create_queue(queue_num=1) queue_id_2, neutron_port_id_2 = self._create_queue(queue_num=2) queues = {1: queue_id_1, 2: queue_id_2} - qos_id = self._create_qos(queues=queues) + qos_id = self._create_qos( + queues=queues, + rule_type_id=self.ovs._min_bw_qos_id) self._check_value({'_uuid': qos_id}, self._list_qos, qos_id, keys_to_check=['_uuid']) qos = self._list_qos(qos_id) @@ -443,19 +539,21 @@ class BaseOVSTestCase(base.BaseSudoTestCase): queue = self._list_queues(queue_id) self.assertEqual(queue_id, queue['_uuid']) - def test_clear_minimum_bandwidth_qos(self): + def test_clear_bandwidth_qos(self): queue_id_1, _ = self._create_queue(queue_num=1) queue_id_2, _ = self._create_queue(queue_num=2) queue_id_3, port_id_3 = self._create_queue() queues = {1: queue_id_1, 2: queue_id_2} - qos_id = self._create_qos(queues=queues) + qos_id = self._create_qos( + queues=queues, + rule_type_id=self.ovs._min_bw_qos_id) # NOTE(ralonsoh): we need to clean only the QoS rule created in this # test in order to avoid any interference with other tests. qoses = self.ovs._list_qos(_id=self.ovs._min_bw_qos_id) with mock.patch.object(self.ovs, '_list_qos') as mock_list_qos: - mock_list_qos.return_value = qoses - self.ovs.clear_minimum_bandwidth_qos() + mock_list_qos.side_effect = [qoses, []] + self.ovs.clear_bandwidth_qos() self._check_value(None, self._list_qos, qos_id=qos_id) self._check_value(None, self._list_queues, queue_id=queue_id_1) self._check_value(None, self._list_queues, queue_id=queue_id_2) diff --git a/neutron/tests/functional/agent/l2/base.py b/neutron/tests/functional/agent/l2/base.py index 47bff66cce7..3a471b45375 100644 --- a/neutron/tests/functional/agent/l2/base.py +++ b/neutron/tests/functional/agent/l2/base.py @@ -176,7 +176,7 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase, OVSOFControllerHelper): self._bridge_classes()['br_phys'](self.br_phys).create() ext_mgr = ext_manager.L2AgentExtensionsManager(self.config) with mock.patch.object(ovs_qos_driver.QosOVSAgentDriver, - '_minimum_bandwidth_initialize'): + '_qos_bandwidth_initialize'): agent = ovs_agent.OVSNeutronAgent(self._bridge_classes(), ext_mgr, self.config) self.addCleanup(self.ovs.delete_bridge, self.br_int) diff --git a/neutron/tests/functional/agent/test_ovs_lib.py b/neutron/tests/functional/agent/test_ovs_lib.py index ecdfa996387..b34b45a657f 100644 --- a/neutron/tests/functional/agent/test_ovs_lib.py +++ b/neutron/tests/functional/agent/test_ovs_lib.py @@ -421,55 +421,6 @@ class OVSBridgeTestCase(OVSBridgeTestBase): controller, 'connection_mode')) - def test_egress_bw_limit(self): - port_name, _ = self.create_ovs_port() - self.br.create_egress_bw_limit_for_port(port_name, 700, 70) - max_rate, burst = self.br.get_egress_bw_limit_for_port(port_name) - self.assertEqual(700, max_rate) - self.assertEqual(70, burst) - self.br.delete_egress_bw_limit_for_port(port_name) - max_rate, burst = self.br.get_egress_bw_limit_for_port(port_name) - self.assertIsNone(max_rate) - self.assertIsNone(burst) - - def test_ingress_bw_limit(self): - port_name, _ = self.create_ovs_port() - self.br.update_ingress_bw_limit_for_port(port_name, 700, 70) - max_rate, burst = self.br.get_ingress_bw_limit_for_port(port_name) - self.assertEqual(700, max_rate) - self.assertEqual(70, burst) - - self.br.update_ingress_bw_limit_for_port(port_name, 750, 100) - max_rate, burst = self.br.get_ingress_bw_limit_for_port(port_name) - self.assertEqual(750, max_rate) - self.assertEqual(100, burst) - - self.br.delete_ingress_bw_limit_for_port(port_name) - max_rate, burst = self.br.get_ingress_bw_limit_for_port(port_name) - self.assertIsNone(max_rate) - self.assertIsNone(burst) - - def test_ingress_bw_limit_dpdk_port(self): - port_name, _ = self.create_ovs_port( - ('type', agent_const.OVS_DPDK_VHOST_USER)) - self.br.update_ingress_bw_limit_for_port(port_name, 700, 70) - max_rate, burst = self.br.get_ingress_bw_limit_for_dpdk_port( - port_name) - self.assertEqual(700, max_rate) - self.assertEqual(70, burst) - - self.br.update_ingress_bw_limit_for_port(port_name, 750, 100) - max_rate, burst = self.br.get_ingress_bw_limit_for_dpdk_port( - port_name) - self.assertEqual(750, max_rate) - self.assertEqual(100, burst) - - self.br.delete_ingress_bw_limit_for_port(port_name) - max_rate, burst = self.br.get_ingress_bw_limit_for_dpdk_port( - port_name) - self.assertIsNone(max_rate) - self.assertIsNone(burst) - def test_db_create_references(self): with self.ovs.ovsdb.transaction(check_error=True) as txn: queue = txn.add(self.ovs.ovsdb.db_create("Queue", diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/extension_drivers/test_qos_driver.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/extension_drivers/test_qos_driver.py index 714330863ad..2775cfa51ac 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/extension_drivers/test_qos_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/extension_drivers/test_qos_driver.py @@ -41,7 +41,7 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase): self.context = context.get_admin_context() self.qos_driver = qos_driver.QosOVSAgentDriver() self.mock_clear_minimum_bandwidth_qos = mock.patch.object( - self.qos_driver, '_minimum_bandwidth_initialize').start() + self.qos_driver, '_qos_bandwidth_initialize').start() os_ken_app = mock.Mock() self.agent_api = ovs_ext_api.OVSAgentExtensionAPI( ovs_bridge.OVSAgentBridge( @@ -59,7 +59,6 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase): self.qos_driver.br_int.get_egress_bw_limit_for_port = mock.Mock( return_value=(1000, 10)) self.get_egress = self.qos_driver.br_int.get_egress_bw_limit_for_port - self.get_ingress = self.qos_driver.br_int.get_ingress_bw_limit_for_port self.qos_driver.br_int.del_egress_bw_limit_for_port = mock.Mock() self.delete_egress = ( self.qos_driver.br_int.delete_egress_bw_limit_for_port) @@ -122,8 +121,6 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase): def test_create_new_rules(self): self.qos_driver.br_int.get_egress_bw_limit_for_port = mock.Mock( return_value=(None, None)) - self.qos_driver.br_int.get_ingress_bw_limit_for_port = mock.Mock( - return_value=(None, None)) self.qos_driver.create(self.port, self.qos_policy) self.assertEqual(0, self.delete_egress.call_count) self.assertEqual(0, self.delete_ingress.call_count) @@ -153,18 +150,12 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase): self.update_ingress.assert_not_called() def _test_delete_rules(self, qos_policy): - self.qos_driver.br_int.get_ingress_bw_limit_for_port = mock.Mock( - return_value=(self.rules[1].max_kbps, - self.rules[1].max_burst_kbps)) self.qos_driver.create(self.port, qos_policy) self.qos_driver.delete(self.port, qos_policy) self.delete_egress.assert_called_once_with(self.port_name) self.delete_ingress.assert_called_once_with(self.port_name) def _test_delete_rules_no_policy(self): - self.qos_driver.br_int.get_ingress_bw_limit_for_port = mock.Mock( - return_value=(self.rules[1].max_kbps, - self.rules[1].max_burst_kbps)) self.qos_driver.delete(self.port) self.delete_egress.assert_called_once_with(self.port_name) self.delete_ingress.assert_called_once_with(self.port_name)