From 2d0d1a2d76c3383bd1e2d14e8860824d843f5047 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C5=82awek=20Kap=C5=82o=C5=84ski?= Date: Wed, 19 Apr 2017 00:40:38 +0200 Subject: [PATCH] Add support for ingress bandwidth limit rules in ovs agent Add support for QoS ingress bandwidth limiting in openvswitch agent. It uses default ovs QoS policies on bandwidth limiting mechanism. DocImpact: Ingress bandwidth limit in QoS supported by Openvswitch agent Change-Id: I9d94e27db5d574b61061689dc99f12f095625ca0 Partial-Bug: #1560961 --- doc/source/devref/quality_of_service.rst | 13 +- neutron/agent/common/ovs_lib.py | 99 ++++++++++ .../agent/extension_drivers/qos_driver.py | 46 ++++- .../qos/drivers/openvswitch/driver.py | 2 +- neutron/tests/common/agents/l2_extensions.py | 14 +- neutron/tests/fullstack/resources/client.py | 4 +- neutron/tests/fullstack/test_qos.py | 100 +++++++--- .../test_ovs_agent_qos_extension.py | 182 +++++++++++++----- .../tests/functional/agent/test_ovs_lib.py | 17 ++ .../extension_drivers/test_qos_driver.py | 75 +++++--- ...in-openvswitch-agent-51cda9bb6b511885.yaml | 5 + 11 files changed, 448 insertions(+), 109 deletions(-) create mode 100644 releasenotes/notes/Ingress-bandwidth-limit-in-openvswitch-agent-51cda9bb6b511885.yaml diff --git a/doc/source/devref/quality_of_service.rst b/doc/source/devref/quality_of_service.rst index b79dc212c32..c0342c308a4 100644 --- a/doc/source/devref/quality_of_service.rst +++ b/doc/source/devref/quality_of_service.rst @@ -154,10 +154,9 @@ QoS versioned objects For QoS, the following neutron objects are implemented: * QosPolicy: directly maps to the conceptual policy resource, as defined above. -* QosBandwidthLimitRule: defines the bandwidth limit rule, characterized by a - max_kbps parameter and a max_burst_kbits parameter. This rule also has a - direction parameter to set the traffic direction, from the instance point of - view. +* QosBandwidthLimitRule: defines the instance bandwidth limit rule type, + characterized by a max kbps and a max burst kbits. This rule has also a + direction parameter to set the traffic direction, from the instance's point of view. * QosDscpMarkingRule: defines the DSCP rule type, characterized by an even integer between 0 and 56. These integers are the result of the bits in the DiffServ section of the IP header, and only certain configurations are valid. As a result, the list @@ -296,6 +295,9 @@ Open vSwitch implementation relies on the new ovs_lib OVSBridge functions: * get_egress_bw_limit_for_port * create_egress_bw_limit_for_port * delete_egress_bw_limit_for_port +* get_ingress_bw_limit_for_port +* update_ingress_bw_limit_for_port +* delete_ingress_bw_limit_for_port An egress bandwidth limit is effectively configured on the port by setting the port Interface parameters ingress_policing_rate and @@ -305,6 +307,9 @@ That approach is less flexible than linux-htb, Queues and OvS QoS profiles, which we may explore in the future, but which will need to be used in combination with openflow rules. +An ingress bandwidth limit is effectively configured on the port by setting +Queue and OvS QoS profile with linux-htb type for port. + The Open vSwitch DSCP marking implementation relies on the recent addition of the ovs_agent_extension_api OVSAgentExtensionAPI to request access to the integration bridge functions: diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py index 25959632065..cce168f6cb3 100644 --- a/neutron/agent/common/ovs_lib.py +++ b/neutron/agent/common/ovs_lib.py @@ -58,6 +58,10 @@ OVS_DEFAULT_CAPS = { 'iface_types': [], } +# It's default queue, all packets not tagged with 'set_queue' will go through +# this one +QOS_DEFAULT_QUEUE = 0 + _SENTINEL = object() @@ -663,6 +667,101 @@ class OVSBridge(BaseOVS): self._set_egress_bw_limit_for_port( port_name, 0, 0) + 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): + queues = {queue_type: queue_uuid} + if qos_uuid: + txn.add(self.ovsdb.db_set( + 'QoS', qos_uuid, ('queues', queues))) + else: + external_ids = {'id': port_name} + qos_uuid = txn.add( + self.ovsdb.db_create( + 'QoS', external_ids=external_ids, type='linux-htb', + queues=queues)) + return qos_uuid + + def update_ingress_bw_limit_for_port(self, port_name, max_kbps, + max_burst_kbps): + max_bw_in_bits = str(max_kbps * 1000) + max_burst_in_bits = str(max_burst_kbps * 1000) + queue_other_config = { + 'max-rate': max_bw_in_bits, + 'burst': 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 + ) + + txn.add(self.ovsdb.db_set( + 'Port', port_name, ('qos', qos_uuid))) + + def get_ingress_bw_limit_for_port(self, port_name): + max_kbps = None + max_burst_kbit = None + res = self._find_queue(port_name, QOS_DEFAULT_QUEUE) + if res: + other_config = res['other_config'] + max_bw_in_bits = other_config.get('max-rate') + if max_bw_in_bits is not None: + max_kbps = int(max_bw_in_bits) / 1000 + max_burst_in_bits = other_config.get('burst') + if max_burst_in_bits is not None: + max_burst_kbit = int(max_burst_in_bits) / 1000 + return max_kbps, max_burst_kbit + + def delete_ingress_bw_limit_for_port(self, port_name): + qos = self._find_qos(port_name) + queue = self._find_queue(port_name, QOS_DEFAULT_QUEUE) + with self.ovsdb.transaction(check_error=True) as txn: + txn.add(self.ovsdb.db_clear("Port", port_name, 'qos')) + if qos: + txn.add(self.ovsdb.db_destroy('QoS', qos['_uuid'])) + if queue: + txn.add(self.ovsdb.db_destroy('Queue', queue['_uuid'])) + def __enter__(self): self.create() return self 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 d43e1eff8a3..7dff5e98670 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 @@ -18,6 +18,7 @@ from oslo_config import cfg from oslo_log import log as logging from neutron.agent.l2.extensions import qos_linux as qos +from neutron.common import constants from neutron.services.qos.drivers.openvswitch import driver from neutron.services.qos import qos_consts @@ -54,15 +55,10 @@ class QosOVSAgentDriver(qos.QosLinuxAgentDriver): "vif_port was not found. It seems that port is already " "deleted", port_id) return - max_kbps = rule.max_kbps - # NOTE(slaweq): According to ovs docs: - # http://openvswitch.org/support/dist-docs/ovs-vswitchd.conf.db.5.html - # ovs accepts only integer values of burst: - max_burst_kbps = int(self._get_egress_burst_value(rule)) - - self.br_int.create_egress_bw_limit_for_port(vif_port.port_name, - max_kbps, - max_burst_kbps) + if rule.direction == constants.INGRESS_DIRECTION: + self._update_ingress_bandwidth_limit(vif_port, rule) + else: + self._update_egress_bandwidth_limit(vif_port, rule) def delete_bandwidth_limit(self, port): vif_port = port.get('vif_port') @@ -74,6 +70,16 @@ class QosOVSAgentDriver(qos.QosLinuxAgentDriver): return self.br_int.delete_egress_bw_limit_for_port(vif_port.port_name) + def delete_bandwidth_limit_ingress(self, port): + vif_port = port.get('vif_port') + if not vif_port: + port_id = port.get('port_id') + LOG.debug("delete_bandwidth_limit_ingress was received " + "for port %s but vif_port was not found. " + "It seems that port is already deleted", port_id) + return + self.br_int.delete_ingress_bw_limit_for_port(vif_port.port_name) + def create_dscp_marking(self, port, rule): self.update_dscp_marking(port, rule) @@ -128,3 +134,25 @@ class QosOVSAgentDriver(qos.QosLinuxAgentDriver): LOG.debug("delete_dscp_marking was received for port %s but " "no port information was stored to be deleted", port['port_id']) + + def _update_egress_bandwidth_limit(self, vif_port, rule): + max_kbps = rule.max_kbps + # NOTE(slaweq): According to ovs docs: + # http://openvswitch.org/support/dist-docs/ovs-vswitchd.conf.db.5.html + # ovs accepts only integer values of burst: + max_burst_kbps = int(self._get_egress_burst_value(rule)) + + self.br_int.create_egress_bw_limit_for_port(vif_port.port_name, + max_kbps, + max_burst_kbps) + + def _update_ingress_bandwidth_limit(self, vif_port, rule): + port_name = vif_port.port_name + max_kbps = rule.max_kbps or 0 + max_burst_kbps = rule.max_burst_kbps or 0 + + self.br_int.update_ingress_bw_limit_for_port( + port_name, + max_kbps, + max_burst_kbps + ) diff --git a/neutron/services/qos/drivers/openvswitch/driver.py b/neutron/services/qos/drivers/openvswitch/driver.py index c54da7188cb..2600b9d546c 100644 --- a/neutron/services/qos/drivers/openvswitch/driver.py +++ b/neutron/services/qos/drivers/openvswitch/driver.py @@ -31,7 +31,7 @@ SUPPORTED_RULES = { qos_consts.MAX_BURST: { 'type:range': [0, constants.DB_INTEGER_MAX_VALUE]}, qos_consts.DIRECTION: { - 'type:values': [constants.EGRESS_DIRECTION]} + 'type:values': constants.VALID_DIRECTIONS} }, qos_consts.RULE_TYPE_DSCP_MARKING: { qos_consts.DSCP_MARK: {'type:values': constants.VALID_DSCP_MARKS} diff --git a/neutron/tests/common/agents/l2_extensions.py b/neutron/tests/common/agents/l2_extensions.py index 92a7f82810f..91bb05113f6 100644 --- a/neutron/tests/common/agents/l2_extensions.py +++ b/neutron/tests/common/agents/l2_extensions.py @@ -48,9 +48,9 @@ def extract_dscp_value_from_iptables_rules(rules): return int(m.group("dscp_value"), 16) -def wait_until_bandwidth_limit_rule_applied(bridge, port_vif, rule): +def wait_until_bandwidth_limit_rule_applied(check_function, port_vif, rule): def _bandwidth_limit_rule_applied(): - bw_rule = bridge.get_egress_bw_limit_for_port(port_vif) + bw_rule = check_function(port_vif) expected = None, None if rule: expected = rule.max_kbps, rule.max_burst_kbps @@ -59,6 +59,16 @@ def wait_until_bandwidth_limit_rule_applied(bridge, port_vif, rule): common_utils.wait_until_true(_bandwidth_limit_rule_applied) +def wait_until_egress_bandwidth_limit_rule_applied(bridge, port_vif, rule): + wait_until_bandwidth_limit_rule_applied( + bridge.get_egress_bw_limit_for_port, port_vif, rule) + + +def wait_until_ingress_bandwidth_limit_rule_applied(bridge, port_vif, rule): + wait_until_bandwidth_limit_rule_applied( + bridge.get_ingress_bw_limit_for_port, port_vif, rule) + + def wait_until_dscp_marking_rule_applied_ovs(bridge, port_vif, rule): def _dscp_marking_rule_applied(): port_num = bridge.get_port_ofport(port_vif) diff --git a/neutron/tests/fullstack/resources/client.py b/neutron/tests/fullstack/resources/client.py index b2ffb4dedeb..427bd291862 100644 --- a/neutron/tests/fullstack/resources/client.py +++ b/neutron/tests/fullstack/resources/client.py @@ -158,12 +158,14 @@ class ClientFixture(fixtures.Fixture): return policy['policy'] def create_bandwidth_limit_rule(self, tenant_id, qos_policy_id, limit=None, - burst=None): + burst=None, direction=None): rule = {'tenant_id': tenant_id} if limit: rule['max_kbps'] = limit if burst: rule['max_burst_kbps'] = burst + if direction: + rule['direction'] = direction rule = self.client.create_bandwidth_limit_rule( policy=qos_policy_id, body={'bandwidth_limit_rule': rule}) diff --git a/neutron/tests/fullstack/test_qos.py b/neutron/tests/fullstack/test_qos.py index 039d4945be7..64e84fd4460 100644 --- a/neutron/tests/fullstack/test_qos.py +++ b/neutron/tests/fullstack/test_qos.py @@ -17,8 +17,10 @@ import functools from neutron_lib import constants from neutronclient.common import exceptions from oslo_utils import uuidutils +import testscenarios from neutron.agent.linux import tc_lib +from neutron.common import constants as common_constants from neutron.common import utils from neutron.services.qos import qos_consts from neutron.tests.common.agents import l2_extensions @@ -47,6 +49,13 @@ class BaseQoSRuleTestCase(object): ovsdb_interface = None number_of_hosts = 1 + @property + def reverse_direction(self): + if self.direction == common_constants.INGRESS_DIRECTION: + return common_constants.EGRESS_DIRECTION + elif self.direction == common_constants.EGRESS_DIRECTION: + return common_constants.INGRESS_DIRECTION + def setUp(self): host_desc = [ environment.HostDescription( @@ -102,14 +111,25 @@ class _TestBwLimitQoS(BaseQoSRuleTestCase): number_of_hosts = 1 - def _wait_for_bw_rule_removed(self, vm): - # No values are provided when port doesn't have qos policy - self._wait_for_bw_rule_applied(vm, None, None) + @staticmethod + def _get_expected_burst_value(limit, direction): + # For egress bandwidth limit this value should be calculated as + # bandwidth_limit * qos_consts.DEFAULT_BURST_RATE + if direction == common_constants.EGRESS_DIRECTION: + return int( + limit * qos_consts.DEFAULT_BURST_RATE + ) + else: + return 0 - def _add_bw_limit_rule(self, limit, burst, qos_policy): + def _wait_for_bw_rule_removed(self, vm, direction): + # No values are provided when port doesn't have qos policy + self._wait_for_bw_rule_applied(vm, None, None, direction) + + def _add_bw_limit_rule(self, limit, burst, direction, qos_policy): qos_policy_id = qos_policy['id'] rule = self.safe_client.create_bandwidth_limit_rule( - self.tenant_id, qos_policy_id, limit, burst) + self.tenant_id, qos_policy_id, limit, burst, direction) # Make it consistent with GET reply rule['type'] = qos_consts.RULE_TYPE_BANDWIDTH_LIMIT rule['qos_policy_id'] = qos_policy_id @@ -120,54 +140,90 @@ class _TestBwLimitQoS(BaseQoSRuleTestCase): # Create port with qos policy attached vm, qos_policy = self._prepare_vm_with_qos_policy( - [functools.partial(self._add_bw_limit_rule, - BANDWIDTH_LIMIT, BANDWIDTH_BURST)]) + [functools.partial( + self._add_bw_limit_rule, + BANDWIDTH_LIMIT, BANDWIDTH_BURST, self.direction)]) bw_rule = qos_policy['rules'][0] - self._wait_for_bw_rule_applied(vm, BANDWIDTH_LIMIT, BANDWIDTH_BURST) + self._wait_for_bw_rule_applied( + vm, BANDWIDTH_LIMIT, BANDWIDTH_BURST, self.direction) qos_policy_id = qos_policy['id'] self.client.delete_bandwidth_limit_rule(bw_rule['id'], qos_policy_id) - self._wait_for_bw_rule_removed(vm) + self._wait_for_bw_rule_removed(vm, self.direction) # Create new rule with no given burst value, in such case ovs and lb # agent should apply burst value as # bandwidth_limit * qos_consts.DEFAULT_BURST_RATE - new_expected_burst = int( - new_limit * qos_consts.DEFAULT_BURST_RATE - ) + new_expected_burst = self._get_expected_burst_value(new_limit, + self.direction) new_rule = self.safe_client.create_bandwidth_limit_rule( - self.tenant_id, qos_policy_id, new_limit) - self._wait_for_bw_rule_applied(vm, new_limit, new_expected_burst) + self.tenant_id, qos_policy_id, new_limit, direction=self.direction) + self._wait_for_bw_rule_applied( + vm, new_limit, new_expected_burst, self.direction) # Update qos policy rule id self.client.update_bandwidth_limit_rule( new_rule['id'], qos_policy_id, body={'bandwidth_limit_rule': {'max_kbps': BANDWIDTH_LIMIT, 'max_burst_kbps': BANDWIDTH_BURST}}) - self._wait_for_bw_rule_applied(vm, BANDWIDTH_LIMIT, BANDWIDTH_BURST) + self._wait_for_bw_rule_applied( + vm, BANDWIDTH_LIMIT, BANDWIDTH_BURST, self.direction) # Remove qos policy from port self.client.update_port( vm.neutron_port['id'], body={'port': {'qos_policy_id': None}}) - self._wait_for_bw_rule_removed(vm) + self._wait_for_bw_rule_removed(vm, self.direction) class TestBwLimitQoSOvs(_TestBwLimitQoS, base.BaseFullStackTestCase): l2_agent_type = constants.AGENT_TYPE_OVS - scenarios = fullstack_utils.get_ovs_interface_scenarios() + direction_scenarios = [ + ('ingress', {'direction': common_constants.INGRESS_DIRECTION}), + ('egress', {'direction': common_constants.EGRESS_DIRECTION}) + ] + scenarios = testscenarios.multiply_scenarios( + direction_scenarios, fullstack_utils.get_ovs_interface_scenarios()) - def _wait_for_bw_rule_applied(self, vm, limit, burst): - utils.wait_until_true( - lambda: vm.bridge.get_egress_bw_limit_for_port( - vm.port.name) == (limit, burst)) + def _wait_for_bw_rule_applied(self, vm, limit, burst, direction): + if direction == common_constants.EGRESS_DIRECTION: + utils.wait_until_true( + lambda: vm.bridge.get_egress_bw_limit_for_port( + vm.port.name) == (limit, burst)) + elif direction == common_constants.INGRESS_DIRECTION: + utils.wait_until_true( + lambda: vm.bridge.get_ingress_bw_limit_for_port( + vm.port.name) == (limit, burst)) + + def test_bw_limit_direction_change(self): + # Create port with qos policy attached, with rule self.direction + vm, qos_policy = self._prepare_vm_with_qos_policy( + [functools.partial( + self._add_bw_limit_rule, + BANDWIDTH_LIMIT, BANDWIDTH_BURST, self.direction)]) + bw_rule = qos_policy['rules'][0] + + self._wait_for_bw_rule_applied( + vm, BANDWIDTH_LIMIT, BANDWIDTH_BURST, self.direction) + + # Update rule by changing direction to opposite then it was before + self.client.update_bandwidth_limit_rule( + bw_rule['id'], qos_policy['id'], + body={'bandwidth_limit_rule': { + 'direction': self.reverse_direction}}) + self._wait_for_bw_rule_removed(vm, self.direction) + self._wait_for_bw_rule_applied( + vm, BANDWIDTH_LIMIT, BANDWIDTH_BURST, self.reverse_direction) class TestBwLimitQoSLinuxbridge(_TestBwLimitQoS, base.BaseFullStackTestCase): l2_agent_type = constants.AGENT_TYPE_LINUXBRIDGE + scenarios = [ + ('egress', {'direction': common_constants.EGRESS_DIRECTION}) + ] - def _wait_for_bw_rule_applied(self, vm, limit, burst): + def _wait_for_bw_rule_applied(self, vm, limit, burst, direction): port_name = linuxbridge_agent.LinuxBridgeManager.get_tap_device_name( vm.neutron_port['id']) tc = tc_lib.TcCommand( diff --git a/neutron/tests/functional/agent/l2/extensions/test_ovs_agent_qos_extension.py b/neutron/tests/functional/agent/l2/extensions/test_ovs_agent_qos_extension.py index 59e8c20991f..68dea5eeaac 100644 --- a/neutron/tests/functional/agent/l2/extensions/test_ovs_agent_qos_extension.py +++ b/neutron/tests/functional/agent/l2/extensions/test_ovs_agent_qos_extension.py @@ -17,55 +17,62 @@ import copy import mock from oslo_utils import uuidutils +import testscenarios from neutron.api.rpc.callbacks.consumer import registry as consumer_reg from neutron.api.rpc.callbacks import events from neutron.api.rpc.callbacks import resources +from neutron.common import constants from neutron.objects.qos import policy from neutron.objects.qos import rule from neutron.tests.common.agents import l2_extensions from neutron.tests.functional.agent.l2 import base +from neutron.tests.functional.agent.linux import base as linux_base +load_tests = testscenarios.load_tests_apply_scenarios + TEST_POLICY_ID1 = "a2d72369-4246-4f19-bd3c-af51ec8d70cd" TEST_POLICY_ID2 = "46ebaec0-0570-43ac-82f6-60d2b03168c5" TEST_DSCP_MARK_1 = 14 TEST_DSCP_MARK_2 = 30 -TEST_DSCP_MARKING_RULE_1 = rule.QosDscpMarkingRule( - context=None, - qos_policy_id=TEST_POLICY_ID1, - id="9f126d84-551a-4dcf-bb01-0e9c0df0c793", - dscp_mark=TEST_DSCP_MARK_1) -TEST_DSCP_MARKING_RULE_2 = rule.QosDscpMarkingRule( - context=None, - qos_policy_id=TEST_POLICY_ID2, - id="7f126d84-551a-4dcf-bb01-0e9c0df0c793", - dscp_mark=TEST_DSCP_MARK_2) -TEST_BW_LIMIT_RULE_1 = rule.QosBandwidthLimitRule( - context=None, - qos_policy_id=TEST_POLICY_ID1, - id="5f126d84-551a-4dcf-bb01-0e9c0df0c793", - max_kbps=1000, - max_burst_kbps=10) -TEST_BW_LIMIT_RULE_2 = rule.QosBandwidthLimitRule( - context=None, - qos_policy_id=TEST_POLICY_ID2, - id="fa9128d9-44af-49b2-99bb-96548378ad42", - max_kbps=900, - max_burst_kbps=9) class OVSAgentQoSExtensionTestFramework(base.OVSAgentTestFramework): + + test_dscp_marking_rule_1 = rule.QosDscpMarkingRule( + context=None, + qos_policy_id=TEST_POLICY_ID1, + id="9f126d84-551a-4dcf-bb01-0e9c0df0c793", + dscp_mark=TEST_DSCP_MARK_1) + test_dscp_marking_rule_2 = rule.QosDscpMarkingRule( + context=None, + qos_policy_id=TEST_POLICY_ID2, + id="7f126d84-551a-4dcf-bb01-0e9c0df0c793", + dscp_mark=TEST_DSCP_MARK_2) + test_bw_limit_rule_1 = rule.QosBandwidthLimitRule( + context=None, + qos_policy_id=TEST_POLICY_ID1, + id="5f126d84-551a-4dcf-bb01-0e9c0df0c793", + max_kbps=1000, + max_burst_kbps=10) + test_bw_limit_rule_2 = rule.QosBandwidthLimitRule( + context=None, + qos_policy_id=TEST_POLICY_ID2, + id="fa9128d9-44af-49b2-99bb-96548378ad42", + max_kbps=900, + max_burst_kbps=9) + def setUp(self): super(OVSAgentQoSExtensionTestFramework, self).setUp() self.config.set_override('extensions', ['qos'], 'agent') self._set_pull_mock() self.set_test_qos_rules(TEST_POLICY_ID1, - [TEST_BW_LIMIT_RULE_1, - TEST_DSCP_MARKING_RULE_1]) + [self.test_bw_limit_rule_1, + self.test_dscp_marking_rule_1]) self.set_test_qos_rules(TEST_POLICY_ID2, - [TEST_BW_LIMIT_RULE_2, - TEST_DSCP_MARKING_RULE_2]) + [self.test_bw_limit_rule_2, + self.test_dscp_marking_rule_2]) def _set_pull_mock(self): @@ -108,20 +115,36 @@ class OVSAgentQoSExtensionTestFramework(base.OVSAgentTestFramework): return dev def _assert_bandwidth_limit_rule_is_set(self, port, rule): - max_rate, burst = ( - self.agent.int_br.get_egress_bw_limit_for_port(port['vif_name'])) + if rule.direction == constants.INGRESS_DIRECTION: + max_rate, burst = ( + self.agent.int_br.get_ingress_bw_limit_for_port( + port['vif_name'])) + else: + max_rate, burst = ( + self.agent.int_br.get_egress_bw_limit_for_port( + port['vif_name'])) self.assertEqual(max_rate, rule.max_kbps) self.assertEqual(burst, rule.max_burst_kbps) - def _assert_bandwidth_limit_rule_not_set(self, port): - max_rate, burst = ( - self.agent.int_br.get_egress_bw_limit_for_port(port['vif_name'])) + def _assert_bandwidth_limit_rule_not_set(self, port, rule_direction): + if rule_direction == constants.INGRESS_DIRECTION: + max_rate, burst = ( + self.agent.int_br.get_ingress_bw_limit_for_port( + port['vif_name'])) + else: + max_rate, burst = ( + self.agent.int_br.get_egress_bw_limit_for_port( + port['vif_name'])) self.assertIsNone(max_rate) self.assertIsNone(burst) def wait_until_bandwidth_limit_rule_applied(self, port, rule): - l2_extensions.wait_until_bandwidth_limit_rule_applied( - self.agent.int_br, port['vif_name'], rule) + if rule and rule.direction == constants.INGRESS_DIRECTION: + l2_extensions.wait_until_ingress_bandwidth_limit_rule_applied( + self.agent.int_br, port['vif_name'], rule) + else: + l2_extensions.wait_until_egress_bandwidth_limit_rule_applied( + self.agent.int_br, port['vif_name'], rule) def _assert_dscp_marking_rule_is_set(self, port, dscp_rule): port_num = self.agent.int_br._get_port_val(port['vif_name'], 'ofport') @@ -150,12 +173,34 @@ class OVSAgentQoSExtensionTestFramework(base.OVSAgentTestFramework): self.setup_agent_and_ports([port_dict]) self.wait_until_ports_state(self.ports, up=True) self.wait_until_bandwidth_limit_rule_applied(port_dict, - TEST_BW_LIMIT_RULE_1) + self.test_bw_limit_rule_1) return port_dict class TestOVSAgentQosExtension(OVSAgentQoSExtensionTestFramework): + interface_scenarios = linux_base.BaseOVSLinuxTestCase.scenarios + + direction_scenarios = [ + ('ingress', {'direction': constants.INGRESS_DIRECTION}), + ('egress', {'direction': constants.EGRESS_DIRECTION}) + ] + + scenarios = testscenarios.multiply_scenarios( + interface_scenarios, direction_scenarios) + + def setUp(self): + super(TestOVSAgentQosExtension, self).setUp() + self.test_bw_limit_rule_1.direction = self.direction + self.test_bw_limit_rule_2.direction = self.direction + + @property + def reverse_direction(self): + if self.direction == constants.INGRESS_DIRECTION: + return constants.EGRESS_DIRECTION + elif self.direction == constants.EGRESS_DIRECTION: + return constants.INGRESS_DIRECTION + def test_port_creation_with_bandwidth_limit(self): """Make sure bandwidth limit rules are set in low level to ports.""" @@ -166,7 +211,31 @@ class TestOVSAgentQosExtension(OVSAgentQoSExtensionTestFramework): for port in self.ports: self._assert_bandwidth_limit_rule_is_set( - port, TEST_BW_LIMIT_RULE_1) + port, self.test_bw_limit_rule_1) + + def test_port_creation_with_bandwidth_limits_both_directions(self): + """Make sure bandwidth limit rules are set in low level to ports. + + This test is checking applying rules for both possible + directions at once + """ + + reverse_direction_bw_limit_rule = copy.deepcopy( + self.test_bw_limit_rule_1) + reverse_direction_bw_limit_rule.direction = self.reverse_direction + self.qos_policies[TEST_POLICY_ID1].rules.append( + reverse_direction_bw_limit_rule) + + self.setup_agent_and_ports( + port_dicts=self.create_test_ports(amount=1, + policy_id=TEST_POLICY_ID1)) + self.wait_until_ports_state(self.ports, up=True) + + for port in self.ports: + self._assert_bandwidth_limit_rule_is_set( + port, self.test_bw_limit_rule_1) + self._assert_bandwidth_limit_rule_is_set( + port, reverse_direction_bw_limit_rule) def test_port_creation_with_different_bandwidth_limits(self): """Make sure different types of policies end on the right ports.""" @@ -180,12 +249,13 @@ class TestOVSAgentQosExtension(OVSAgentQoSExtensionTestFramework): self.wait_until_ports_state(self.ports, up=True) self._assert_bandwidth_limit_rule_is_set(self.ports[0], - TEST_BW_LIMIT_RULE_1) + self.test_bw_limit_rule_1) self._assert_bandwidth_limit_rule_is_set(self.ports[1], - TEST_BW_LIMIT_RULE_2) + self.test_bw_limit_rule_2) - self._assert_bandwidth_limit_rule_not_set(self.ports[2]) + self._assert_bandwidth_limit_rule_not_set(self.ports[2], + self.direction) def test_port_creation_with_dscp_marking(self): """Make sure dscp marking rules are set in low level to ports.""" @@ -197,7 +267,7 @@ class TestOVSAgentQosExtension(OVSAgentQoSExtensionTestFramework): for port in self.ports: self._assert_dscp_marking_rule_is_set( - port, TEST_DSCP_MARKING_RULE_1) + port, self.test_dscp_marking_rule_1) def test_port_creation_with_different_dscp_markings(self): """Make sure different types of policies end on the right ports.""" @@ -211,10 +281,10 @@ class TestOVSAgentQosExtension(OVSAgentQoSExtensionTestFramework): self.wait_until_ports_state(self.ports, up=True) self._assert_dscp_marking_rule_is_set(self.ports[0], - TEST_DSCP_MARKING_RULE_1) + self.test_dscp_marking_rule_1) self._assert_dscp_marking_rule_is_set(self.ports[1], - TEST_DSCP_MARKING_RULE_2) + self.test_dscp_marking_rule_2) self._assert_dscp_marking_rule_not_set(self.ports[2]) @@ -224,7 +294,7 @@ class TestOVSAgentQosExtension(OVSAgentQoSExtensionTestFramework): policy_id=TEST_POLICY_ID1)) self.wait_until_ports_state(self.ports, up=True) self._assert_dscp_marking_rule_is_set(self.ports[0], - TEST_DSCP_MARKING_RULE_1) + self.test_dscp_marking_rule_1) policy_copy = copy.deepcopy(self.qos_policies[TEST_POLICY_ID1]) policy_copy.rules[0].max_kbps = 500 policy_copy.rules[0].max_burst_kbps = 5 @@ -237,7 +307,31 @@ class TestOVSAgentQosExtension(OVSAgentQoSExtensionTestFramework): self._assert_bandwidth_limit_rule_is_set(self.ports[0], policy_copy.rules[0]) self._assert_dscp_marking_rule_is_set(self.ports[0], - TEST_DSCP_MARKING_RULE_2) + self.test_dscp_marking_rule_2) + + def test_simple_port_policy_update_change_bw_limit_direction(self): + self.setup_agent_and_ports( + port_dicts=self.create_test_ports(amount=1, + policy_id=TEST_POLICY_ID1)) + self.wait_until_ports_state(self.ports, up=True) + + self._assert_bandwidth_limit_rule_is_set(self.ports[0], + self.test_bw_limit_rule_1) + self._assert_bandwidth_limit_rule_not_set(self.ports[0], + self.reverse_direction) + + policy_copy = copy.deepcopy(self.qos_policies[TEST_POLICY_ID1]) + policy_copy.rules[0].direction = self.reverse_direction + context = mock.Mock() + consumer_reg.push(context, resources.QOS_POLICY, + [policy_copy], events.UPDATED) + self.wait_until_bandwidth_limit_rule_applied(self.ports[0], + policy_copy.rules[0]) + + self._assert_bandwidth_limit_rule_not_set(self.ports[0], + self.direction) + self._assert_bandwidth_limit_rule_is_set(self.ports[0], + policy_copy.rules[0]) def test_port_qos_disassociation(self): """Test that qos_policy_id set to None will remove all qos rules from @@ -260,7 +354,7 @@ class TestOVSAgentQosExtension(OVSAgentQoSExtensionTestFramework): self.agent.port_update(None, port=port_dict) self.wait_until_bandwidth_limit_rule_applied(port_dict, - TEST_BW_LIMIT_RULE_2) + self.test_bw_limit_rule_2) def test_policy_rule_delete(self): port_dict = self._create_port_with_qos() diff --git a/neutron/tests/functional/agent/test_ovs_lib.py b/neutron/tests/functional/agent/test_ovs_lib.py index dd7b4add99a..32df6964596 100644 --- a/neutron/tests/functional/agent/test_ovs_lib.py +++ b/neutron/tests/functional/agent/test_ovs_lib.py @@ -381,6 +381,23 @@ class OVSBridgeTestCase(OVSBridgeTestBase): 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_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 498cdde439f..dba464b3ddb 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 @@ -16,6 +16,7 @@ import mock from neutron_lib import context from oslo_utils import uuidutils +from neutron.common import constants from neutron.objects.qos import policy from neutron.objects.qos import rule from neutron.plugins.ml2.drivers.openvswitch.agent import ( @@ -46,22 +47,32 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase): self.qos_driver.br_int = mock.Mock() 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.dump_flows_for = mock.Mock(return_value=None) - self.get = self.qos_driver.br_int.get_egress_bw_limit_for_port self.qos_driver.br_int.del_egress_bw_limit_for_port = mock.Mock() - self.delete = self.qos_driver.br_int.delete_egress_bw_limit_for_port + self.delete_egress = ( + self.qos_driver.br_int.delete_egress_bw_limit_for_port) + self.delete_ingress = ( + self.qos_driver.br_int.delete_ingress_bw_limit_for_port) self.qos_driver.br_int.create_egress_bw_limit_for_port = mock.Mock() - self.create = self.qos_driver.br_int.create_egress_bw_limit_for_port - self.rules = [self._create_bw_limit_rule_obj(), - self._create_dscp_marking_rule_obj()] + self.create_egress = ( + self.qos_driver.br_int.create_egress_bw_limit_for_port) + self.update_ingress = ( + self.qos_driver.br_int.update_ingress_bw_limit_for_port) + self.rules = [ + self._create_bw_limit_rule_obj(constants.EGRESS_DIRECTION), + self._create_bw_limit_rule_obj(constants.INGRESS_DIRECTION), + self._create_dscp_marking_rule_obj()] self.qos_policy = self._create_qos_policy_obj(self.rules) self.port = self._create_fake_port(self.qos_policy.id) - def _create_bw_limit_rule_obj(self): + def _create_bw_limit_rule_obj(self, direction): rule_obj = rule.QosBandwidthLimitRule() rule_obj.id = uuidutils.generate_uuid() rule_obj.max_kbps = 2 rule_obj.max_burst_kbps = 200 + rule_obj.direction = direction rule_obj.obj_reset_changes() return rule_obj @@ -98,55 +109,67 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase): 'port_id': uuidutils.generate_uuid(), 'device_owner': uuidutils.generate_uuid()} - def test_create_new_rule(self): + 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) - # Assert create is the last call - self.assertEqual( - 'create_egress_bw_limit_for_port', - self.qos_driver.br_int.method_calls[0][0]) - self.assertEqual(0, self.delete.call_count) - self.create.assert_called_once_with( + self.assertEqual(0, self.delete_egress.call_count) + self.assertEqual(0, self.delete_ingress.call_count) + self.create_egress.assert_called_once_with( self.port_name, self.rules[0].max_kbps, self.rules[0].max_burst_kbps) + self.update_ingress.assert_called_once_with( + self.port_name, self.rules[1].max_kbps, + self.rules[1].max_burst_kbps) self._assert_dscp_rule_create_updated() def test_create_existing_rules(self): self.qos_driver.create(self.port, self.qos_policy) - self._assert_bw_rule_create_updated() + self._assert_rules_create_updated() self._assert_dscp_rule_create_updated() def test_update_rules(self): self.qos_driver.update(self.port, self.qos_policy) - self._assert_bw_rule_create_updated() + self._assert_rules_create_updated() self._assert_dscp_rule_create_updated() def test_update_rules_no_vif_port(self): port = copy.copy(self.port) port.pop("vif_port") self.qos_driver.update(port, self.qos_policy) - self.create.assert_not_called() + self.create_egress.assert_not_called() + self.update_ingress.assert_not_called() + + def _test_delete_rules(self, 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.delete(self.port) + self.delete_egress.assert_called_once_with(self.port_name) + self.delete_ingress.assert_called_once_with(self.port_name) def test_delete_rules(self): - self.qos_driver.delete(self.port, self.qos_policy) - self.delete.assert_called_once_with(self.port_name) + self._test_delete_rules(self.qos_policy) + + def test_delete_rules_no_policy(self): + self._test_delete_rules(None) def test_delete_rules_no_vif_port(self): port = copy.copy(self.port) port.pop("vif_port") self.qos_driver.delete(port, self.qos_policy) - self.delete.assert_not_called() + self.delete_egress.assert_not_called() + self.delete_ingress.assert_not_called() - def _assert_bw_rule_create_updated(self): - # Assert create is the last call - self.assertEqual( - 'create_egress_bw_limit_for_port', - self.qos_driver.br_int.method_calls[0][0]) - - self.create.assert_called_once_with( + def _assert_rules_create_updated(self): + self.create_egress.assert_called_once_with( self.port_name, self.rules[0].max_kbps, self.rules[0].max_burst_kbps) + self.update_ingress.assert_called_once_with( + self.port_name, self.rules[1].max_kbps, + self.rules[1].max_burst_kbps) def _assert_dscp_rule_create_updated(self): # Assert add_flow is the last call diff --git a/releasenotes/notes/Ingress-bandwidth-limit-in-openvswitch-agent-51cda9bb6b511885.yaml b/releasenotes/notes/Ingress-bandwidth-limit-in-openvswitch-agent-51cda9bb6b511885.yaml new file mode 100644 index 00000000000..dc2e8487b86 --- /dev/null +++ b/releasenotes/notes/Ingress-bandwidth-limit-in-openvswitch-agent-51cda9bb6b511885.yaml @@ -0,0 +1,5 @@ +--- +prelude: > + Openvswitch L2 agent supports ingress bandwidth limit. +features: + - The openvswitch L2 agent now supports bi-directional bandwidth limiting.