From d56fea0a39cbb53c36b0f7df3f7baef34588ec9a Mon Sep 17 00:00:00 2001 From: Miguel Angel Ajo Date: Tue, 11 Aug 2015 13:51:16 +0200 Subject: [PATCH] Fix the low level OVS driver to really do egress It seems that the Queue + QoS + linux-htb implementation was really limiting ingress by default. So this patch switches the implementation to the ovs ingress_policing_rate and ingress_policing_burst parameters of the Interface table. Later in time we may want to revise this, to make TC & queueing possible, but this is good enough for egress limiting. Also, removed the _update_bandwidth_limit del+set on OvS QoS driver for the bandwidth limit rule update, since that's not needed anymore. Change-Id: Ie802a235ae19bf679ba638563ac7377337448f2a Partially-Implements: ml2-qos --- neutron/agent/common/ovs_lib.py | 89 +++++-------------- .../agent/extension_drivers/qos_driver.py | 16 +--- neutron/tests/common/agents/l2_extensions.py | 2 +- .../test_ovs_agent_qos_extension.py | 4 +- .../tests/functional/agent/test_ovs_lib.py | 10 +-- .../extension_drivers/test_qos_driver.py | 20 ++--- 6 files changed, 43 insertions(+), 98 deletions(-) diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py index a4b22ea1278..9c23dd6ba61 100644 --- a/neutron/agent/common/ovs_lib.py +++ b/neutron/agent/common/ovs_lib.py @@ -489,80 +489,35 @@ class OVSBridge(BaseOVS): txn.add(self.ovsdb.db_set('Controller', controller_uuid, *attr)) - def _create_qos_bw_limit_queue(self, port_name, max_bw_in_bits, - max_burst_in_bits): - external_ids = {'id': port_name} - queue_other_config = {'min-rate': max_bw_in_bits, - 'max-rate': max_bw_in_bits, - 'burst': max_burst_in_bits} + def _set_egress_bw_limit_for_port(self, port_name, max_kbps, + max_burst_kbps): + with self.ovsdb.transaction(check_error=True) as txn: + txn.add(self.ovsdb.db_set('Interface', port_name, + ('ingress_policing_rate', max_kbps))) + txn.add(self.ovsdb.db_set('Interface', port_name, + ('ingress_policing_burst', + max_burst_kbps))) - self.ovsdb.db_create( - 'Queue', external_ids=external_ids, - other_config=queue_other_config).execute(check_error=True) + def create_egress_bw_limit_for_port(self, port_name, max_kbps, + max_burst_kbps): + self._set_egress_bw_limit_for_port( + port_name, max_kbps, max_burst_kbps) - def _create_qos_bw_limit_profile(self, port_name, max_bw_in_bits): - external_ids = {'id': port_name} - queue = self.ovsdb.db_find( - 'Queue', - ('external_ids', '=', {'id': port_name}), - columns=['_uuid']).execute( - check_error=True) - queues = {} - queues[0] = queue[0]['_uuid'] - qos_other_config = {'max-rate': max_bw_in_bits} - self.ovsdb.db_create('QoS', external_ids=external_ids, - other_config=qos_other_config, - type='linux-htb', - queues=queues).execute(check_error=True) + def get_egress_bw_limit_for_port(self, port_name): - def create_qos_bw_limit_for_port(self, port_name, max_kbps, - max_burst_kbps): - # TODO(QoS) implement this with transactions, - # or roll back on failure - max_bw_in_bits = str(max_kbps * 1000) - max_burst_in_bits = str(max_burst_kbps * 1000) + max_kbps = self.db_get_val('Interface', port_name, + 'ingress_policing_rate') + max_burst_kbps = self.db_get_val('Interface', port_name, + 'ingress_policing_burst') - self._create_qos_bw_limit_queue(port_name, max_bw_in_bits, - max_burst_in_bits) - self._create_qos_bw_limit_profile(port_name, max_bw_in_bits) + max_kbps = max_kbps or None + max_burst_kbps = max_burst_kbps or None - qos = self.ovsdb.db_find('QoS', - ('external_ids', '=', {'id': port_name}), - columns=['_uuid']).execute(check_error=True) - qos_profile = qos[0]['_uuid'] - self.set_db_attribute('Port', port_name, 'qos', qos_profile, - check_error=True) - - def get_qos_bw_limit_for_port(self, port_name): - - res = self.ovsdb.db_find( - 'Queue', - ('external_ids', '=', {'id': port_name}), - columns=['other_config']).execute(check_error=True) - - if res is None or len(res) == 0: - return None, None - - other_config = res[0]['other_config'] - max_kbps = int(other_config['max-rate']) / 1000 - max_burst_kbps = int(other_config['burst']) / 1000 return max_kbps, max_burst_kbps - def del_qos_bw_limit_for_port(self, port_name): - qos = self.ovsdb.db_find('QoS', - ('external_ids', '=', {'id': port_name}), - columns=['_uuid']).execute(check_error=True) - qos_row = qos[0]['_uuid'] - - queue = self.ovsdb.db_find('Queue', - ('external_ids', '=', {'id': port_name}), - columns=['_uuid']).execute(check_error=True) - queue_row = queue[0]['_uuid'] - - with self.ovsdb.transaction(check_error=True) as txn: - txn.add(self.ovsdb.db_set('Port', port_name, ('qos', []))) - txn.add(self.ovsdb.db_destroy('QoS', qos_row)) - txn.add(self.ovsdb.db_destroy('Queue', queue_row)) + def delete_egress_bw_limit_for_port(self, port_name): + self._set_egress_bw_limit_for_port( + port_name, 0, 0) def __enter__(self): self.create() 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 51c6564f58f..ce9f2868780 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 @@ -67,18 +67,10 @@ class QosOVSAgentDriver(qos.QosAgentDriver): max_kbps = rule.max_kbps max_burst_kbps = rule.max_burst_kbps - current_max_kbps, current_max_burst = ( - self.br_int.get_qos_bw_limit_for_port(port_name)) - if current_max_kbps is not None or current_max_burst is not None: - self.br_int.del_qos_bw_limit_for_port(port_name) - - self.br_int.create_qos_bw_limit_for_port(port_name, - max_kbps, - max_burst_kbps) + self.br_int.create_egress_bw_limit_for_port(port_name, + max_kbps, + max_burst_kbps) def _delete_bandwidth_limit(self, port): port_name = port['vif_port'].port_name - current_max_kbps, current_max_burst = ( - self.br_int.get_qos_bw_limit_for_port(port_name)) - if current_max_kbps is not None or current_max_burst is not None: - self.br_int.del_qos_bw_limit_for_port(port_name) + self.br_int.delete_egress_bw_limit_for_port(port_name) diff --git a/neutron/tests/common/agents/l2_extensions.py b/neutron/tests/common/agents/l2_extensions.py index 0d46d3676d4..11b354eeb3b 100644 --- a/neutron/tests/common/agents/l2_extensions.py +++ b/neutron/tests/common/agents/l2_extensions.py @@ -18,7 +18,7 @@ from neutron.agent.linux import utils as agent_utils def wait_until_bandwidth_limit_rule_applied(bridge, port_vif, rule): def _bandwidth_limit_rule_applied(): - bw_rule = bridge.get_qos_bw_limit_for_port(port_vif) + bw_rule = bridge.get_egress_bw_limit_for_port(port_vif) expected = None, None if rule: expected = rule.max_kbps, rule.max_burst_kbps 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 8fd8ee18b40..112f6fef789 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 @@ -90,13 +90,13 @@ class OVSAgentQoSExtensionTestFramework(base.OVSAgentTestFramework): def _assert_bandwidth_limit_rule_is_set(self, port, rule): max_rate, burst = ( - self.agent.int_br.get_qos_bw_limit_for_port(port['vif_name'])) + 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_qos_bw_limit_for_port(port['vif_name'])) + self.agent.int_br.get_egress_bw_limit_for_port(port['vif_name'])) self.assertIsNone(max_rate) self.assertIsNone(burst) diff --git a/neutron/tests/functional/agent/test_ovs_lib.py b/neutron/tests/functional/agent/test_ovs_lib.py index fee80d8d3c9..768209424ae 100644 --- a/neutron/tests/functional/agent/test_ovs_lib.py +++ b/neutron/tests/functional/agent/test_ovs_lib.py @@ -311,14 +311,14 @@ class OVSBridgeTestCase(OVSBridgeTestBase): controller, 'connection_mode')) - def test_qos_bw_limit(self): + def test_egress_bw_limit(self): port_name, _ = self.create_ovs_port() - self.br.create_qos_bw_limit_for_port(port_name, 700, 70) - max_rate, burst = self.br.get_qos_bw_limit_for_port(port_name) + 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.del_qos_bw_limit_for_port(port_name) - max_rate, burst = self.br.get_qos_bw_limit_for_port(port_name) + 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) 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 7b6c430b7f0..c9e276c72ab 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 @@ -30,13 +30,13 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase): self.qos_driver = qos_driver.QosOVSAgentDriver() self.qos_driver.initialize() self.qos_driver.br_int = mock.Mock() - self.qos_driver.br_int.get_qos_bw_limit_for_port = mock.Mock( + self.qos_driver.br_int.get_egress_bw_limit_for_port = mock.Mock( return_value=(1000, 10)) - self.get = self.qos_driver.br_int.get_qos_bw_limit_for_port - self.qos_driver.br_int.del_qos_bw_limit_for_port = mock.Mock() - self.delete = self.qos_driver.br_int.del_qos_bw_limit_for_port - self.qos_driver.br_int.create_qos_bw_limit_for_port = mock.Mock() - self.create = self.qos_driver.br_int.create_qos_bw_limit_for_port + 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.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.rule = self._create_bw_limit_rule_obj() self.qos_policy = self._create_qos_policy_obj([self.rule]) self.port = self._create_fake_port() @@ -69,12 +69,12 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase): return {'vif_port': FakeVifPort()} def test_create_new_rule(self): - self.qos_driver.br_int.get_qos_bw_limit_for_port = mock.Mock( + self.qos_driver.br_int.get_egress_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_qos_bw_limit_for_port', + 'create_egress_bw_limit_for_port', self.qos_driver.br_int.method_calls[-1][0]) self.assertEqual(0, self.delete.call_count) self.create.assert_called_once_with( @@ -96,11 +96,9 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase): def _assert_rule_create_updated(self): # Assert create is the last call self.assertEqual( - 'create_qos_bw_limit_for_port', + 'create_egress_bw_limit_for_port', self.qos_driver.br_int.method_calls[-1][0]) - self.delete.assert_called_once_with(self.port_name) - self.create.assert_called_once_with( self.port_name, self.rule.max_kbps, self.rule.max_burst_kbps)