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

diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py
index a4b22ea12784a7609c695c15a5f04a9fa99690d1..9c23dd6ba618e771e4653d723fcd8abfd11c2d86 100644 (file)
--- 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}
-
-        self.ovsdb.db_create(
-            'Queue', external_ids=external_ids,
-            other_config=queue_other_config).execute(check_error=True)
-
-    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 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)
-
-        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)
-
-        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 _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)))
 
-    def get_qos_bw_limit_for_port(self, port_name):
+    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)
 
-        res = self.ovsdb.db_find(
-                'Queue',
-                ('external_ids', '=', {'id': port_name}),
-                columns=['other_config']).execute(check_error=True)
+    def get_egress_bw_limit_for_port(self, port_name):
 
-        if res is None or len(res) == 0:
-            return None, None
+        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')
 
-        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
+        max_kbps = max_kbps or None
+        max_burst_kbps = max_burst_kbps or None
 
-    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']
+        return max_kbps, max_burst_kbps
 
-        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 51c6564f58f871ce43bb0e472d7d5f0efbcff185..ce9f28687808b2048b37545f93eedccd3847d228 100644 (file)
--- 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 0d46d3676d438bcf068a21349c7bf0dce36124c2..11b354eeb3bd9d0c416689f5d43f474b5beb7707 100644 (file)
--- 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 8fd8ee18b40ee1e104ff0123e7c1ee1a3b6e805a..112f6fef789c6f1e2bacfd518d91fe14be091463 100644 (file)
--- 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 fee80d8d3c93eca83bbea3e4c1c833fca0971f7c..768209424ae30d0208a8d99ce729847e06308f03 100644 (file)
--- 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 7b6c430b7f026a05e58c4127ece45f1897de1845..c9e276c72abe8a48772c888908a67e0874df1f64 100644 (file)
--- 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)