From 1be857435288e4008a41fe67bbc8795dd3d8b134 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C5=82awek=20Kap=C5=82o=C5=84ski?= Date: Thu, 19 Oct 2017 14:13:41 +0000 Subject: [PATCH] Fix ingress bw limit for OVS DPDK ports For OVS based DPDK ports ingress bandwidth limit is now implemented using egress-policer qos type. Additionally limit values are set in other_config of QoS because there is no queue used in this case. This patch moves also helper methods used to conversion between bytes and bits and between bits and kilobits to neutron.common.utils to be able to use it also in ovs_lib module. Change-Id: I94d1e8dfb82df5c602476db8aaa884ae91fecd7f Closes-Bug: #1724729 --- neutron/agent/common/ovs_lib.py | 78 ++++++++++++++++--- neutron/agent/linux/tc_lib.py | 33 +++----- neutron/common/constants.py | 4 + neutron/common/utils.py | 9 +++ .../openvswitch/agent/common/constants.py | 2 + .../tests/functional/agent/test_ovs_lib.py | 23 ++++++ neutron/tests/unit/agent/linux/test_tc_lib.py | 50 ++---------- neutron/tests/unit/common/test_utils.py | 52 +++++++++++++ 8 files changed, 177 insertions(+), 74 deletions(-) diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py index d7a7d1942e2..20cdf8ccf70 100644 --- a/neutron/agent/common/ovs_lib.py +++ b/neutron/agent/common/ovs_lib.py @@ -31,6 +31,8 @@ from neutron._i18n import _ from neutron.agent.common import ip_lib from neutron.agent.common import utils from neutron.agent.ovsdb import api as ovsdb_api +from neutron.common import constants as common_constants +from neutron.common import utils as common_utils from neutron.conf.agent import ovs_conf from neutron.plugins.ml2.drivers.openvswitch.agent.common \ import constants @@ -732,16 +734,27 @@ class OVSBridge(BaseOVS): other_config=qos_other_config)) 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) + def _update_bw_limit_profile_dpdk(self, txn, port_name, qos_uuid, + other_config): + if qos_uuid: + txn.add(self.ovsdb.db_set( + 'QoS', qos_uuid, ('other_config', other_config))) + else: + external_ids = {'id': port_name} + qos_uuid = txn.add( + self.ovsdb.db_create( + 'QoS', external_ids=external_ids, type='egress-policer', + other_config=other_config)) + 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': max_bw_in_bits + 'max-rate': str(max_bw_in_bits) } queue_other_config = { - 'max-rate': max_bw_in_bits, - 'burst': max_burst_in_bits, + 'max-rate': str(max_bw_in_bits), + 'burst': str(max_burst_in_bits), } qos = self.find_qos(port_name) queue = self.find_queue(port_name, QOS_DEFAULT_QUEUE) @@ -761,6 +774,33 @@ class OVSBridge(BaseOVS): txn.add(self.ovsdb.db_set( 'Port', port_name, ('qos', qos_uuid))) + def _update_ingress_bw_limit_for_dpdk_port( + self, port_name, max_bw_in_bits, max_burst_in_bits): + # cir and cbs should be set in bytes instead of bits + 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 + 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.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 * common_constants.SI_BASE + max_burst_in_bits = max_burst_kbps * common_constants.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) + else: + self._update_ingress_bw_limit_for_port( + port_name, max_bw_in_bits, max_burst_in_bits) + def get_ingress_bw_limit_for_port(self, port_name): max_kbps = None qos_max_kbps = None @@ -772,17 +812,18 @@ class OVSBridge(BaseOVS): other_config = qos_res['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) / 1000 + qos_max_kbps = int(max_bw_in_bits) / common_constants.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) / 1000 + queue_max_kbps = int(max_bw_in_bits) / common_constants.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) / 1000 + max_burst_kbit = ( + int(max_burst_in_bits) / common_constants.SI_BASE) if qos_max_kbps == queue_max_kbps: max_kbps = qos_max_kbps @@ -791,7 +832,24 @@ class OVSBridge(BaseOVS): "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(max_bw_in_bytes)), + common_constants.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(max_burst_in_bytes)), + common_constants.SI_BASE) return max_kbps, max_burst_kbit def delete_ingress_bw_limit_for_port(self, port_name): diff --git a/neutron/agent/linux/tc_lib.py b/neutron/agent/linux/tc_lib.py index ab9d13e9c5b..6cd8a0803d3 100644 --- a/neutron/agent/linux/tc_lib.py +++ b/neutron/agent/linux/tc_lib.py @@ -20,14 +20,13 @@ from neutron_lib.services.qos import constants as qos_consts from neutron._i18n import _ from neutron.agent.linux import ip_lib +from neutron.common import constants +from neutron.common import utils INGRESS_QDISC_ID = "ffff:" MAX_MTU_VALUE = 65535 -SI_BASE = 1000 -IEC_BASE = 1024 - LATENCY_UNIT = "ms" BW_LIMIT_UNIT = "kbit" # kilobits per second in tc's notation BURST_UNIT = "kbit" # kilobits in tc's notation @@ -66,10 +65,10 @@ def convert_to_kilobits(value, base): if value.isdigit(): value = int(value) if input_in_bits: - return bits_to_kilobits(value, base) + return utils.bits_to_kilobits(value, base) else: - bits_value = bytes_to_bits(value) - return bits_to_kilobits(bits_value, base) + bits_value = utils.bytes_to_bits(value) + return utils.bits_to_kilobits(bits_value, base) unit = value[-1:] if unit not in UNITS.keys(): raise InvalidUnit(unit=unit) @@ -77,17 +76,8 @@ def convert_to_kilobits(value, base): if input_in_bits: bits_value = val * (base ** UNITS[unit]) else: - bits_value = bytes_to_bits(val * (base ** UNITS[unit])) - return bits_to_kilobits(bits_value, base) - - -def bytes_to_bits(value): - return value * 8 - - -def bits_to_kilobits(value, base): - #NOTE(slaweq): round up that even 1 bit will give 1 kbit as a result - return int((value + (base - 1)) / base) + bits_value = utils.bytes_to_bits(val * (base ** UNITS[unit])) + return utils.bits_to_kilobits(bits_value, base) class TcCommand(ip_lib.IPDevice): @@ -124,10 +114,11 @@ class TcCommand(ip_lib.IPDevice): if m: #NOTE(slaweq): because tc is giving bw limit in SI units # we need to calculate it as 1000bit = 1kbit: - bw_limit = convert_to_kilobits(m.group(1), SI_BASE) + bw_limit = convert_to_kilobits(m.group(1), constants.SI_BASE) #NOTE(slaweq): because tc is giving burst limit in IEC units # we need to calculate it as 1024bit = 1kbit: - burst_limit = convert_to_kilobits(m.group(2), IEC_BASE) + burst_limit = convert_to_kilobits( + m.group(2), constants.IEC_BASE) return bw_limit, burst_limit return None, None @@ -144,10 +135,10 @@ class TcCommand(ip_lib.IPDevice): return None, None #NOTE(slaweq): because tc is giving bw limit in SI units # we need to calculate it as 1000bit = 1kbit: - bw_limit = convert_to_kilobits(m.group(2), SI_BASE) + bw_limit = convert_to_kilobits(m.group(2), constants.SI_BASE) #NOTE(slaweq): because tc is giving burst limit in IEC units # we need to calculate it as 1024bit = 1kbit: - burst_limit = convert_to_kilobits(m.group(3), IEC_BASE) + burst_limit = convert_to_kilobits(m.group(3), constants.IEC_BASE) return bw_limit, burst_limit def set_filters_bw_limit(self, bw_limit, burst_limit): diff --git a/neutron/common/constants.py b/neutron/common/constants.py index d4cfb2bf11e..91d519174c7 100644 --- a/neutron/common/constants.py +++ b/neutron/common/constants.py @@ -222,3 +222,7 @@ FLOATING_IP_HOST_NEEDS_BINDING = "FLOATING_IP_HOST_NEEDS_BINDING" # Possible types of values (e.g. in QoS rule types) VALUES_TYPE_CHOICES = "choices" VALUES_TYPE_RANGE = "range" + +# Units base +SI_BASE = 1000 +IEC_BASE = 1024 diff --git a/neutron/common/utils.py b/neutron/common/utils.py index 71dfbab31dc..04b9e77aa56 100644 --- a/neutron/common/utils.py +++ b/neutron/common/utils.py @@ -801,3 +801,12 @@ def resolve_ref(ref): if isinstance(ref, weakref.ref): ref = ref() return ref + + +def bytes_to_bits(value): + return value * 8 + + +def bits_to_kilobits(value, base): + #NOTE(slaweq): round up that even 1 bit will give 1 kbit as a result + return int((value + (base - 1)) / base) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py b/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py index 949016bc023..3e0e3b18639 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py @@ -137,6 +137,8 @@ OVS_DATAPATH_NETDEV = 'netdev' OVS_DPDK_VHOST_USER = 'dpdkvhostuser' OVS_DPDK_VHOST_USER_CLIENT = 'dpdkvhostuserclient' +OVS_DPDK_PORT_TYPES = [OVS_DPDK_VHOST_USER, OVS_DPDK_VHOST_USER_CLIENT] + # default ovs vhost-user socket location VHOST_USER_SOCKET_DIR = '/var/run/openvswitch' diff --git a/neutron/tests/functional/agent/test_ovs_lib.py b/neutron/tests/functional/agent/test_ovs_lib.py index 8b8ffc2d6f5..bffc71ed47e 100644 --- a/neutron/tests/functional/agent/test_ovs_lib.py +++ b/neutron/tests/functional/agent/test_ovs_lib.py @@ -23,6 +23,8 @@ from neutron.agent.common import ovs_lib from neutron.agent.linux import ip_lib from neutron.agent.ovsdb.native import idlutils from neutron.common import utils +from neutron.plugins.ml2.drivers.openvswitch.agent.common import ( + constants as agent_const) from neutron.tests.common.exclusive_resources import port from neutron.tests.common import net_helpers from neutron.tests.functional.agent.linux import base @@ -392,6 +394,27 @@ class OVSBridgeTestCase(OVSBridgeTestBase): 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/agent/linux/test_tc_lib.py b/neutron/tests/unit/agent/linux/test_tc_lib.py index 2d04776409d..4aba2a5e3d7 100644 --- a/neutron/tests/unit/agent/linux/test_tc_lib.py +++ b/neutron/tests/unit/agent/linux/test_tc_lib.py @@ -17,6 +17,8 @@ import mock from neutron_lib.services.qos import constants as qos_consts from neutron.agent.linux import tc_lib +from neutron.common import constants +from neutron.common import utils from neutron.tests import base DEVICE_NAME = "tap_device" @@ -58,7 +60,7 @@ class BaseUnitConversionTest(object): def test_convert_to_kilobits_bits_value(self): value = "1000bit" - expected_value = tc_lib.bits_to_kilobits(1000, self.base_unit) + expected_value = utils.bits_to_kilobits(1000, self.base_unit) self.assertEqual( expected_value, tc_lib.convert_to_kilobits(value, self.base_unit) @@ -66,7 +68,7 @@ class BaseUnitConversionTest(object): def test_convert_to_kilobits_megabytes_value(self): value = "1m" - expected_value = tc_lib.bits_to_kilobits( + expected_value = utils.bits_to_kilobits( self.base_unit ** 2 * 8, self.base_unit) self.assertEqual( expected_value, @@ -75,7 +77,7 @@ class BaseUnitConversionTest(object): def test_convert_to_kilobits_megabits_value(self): value = "1mbit" - expected_value = tc_lib.bits_to_kilobits( + expected_value = utils.bits_to_kilobits( self.base_unit ** 2, self.base_unit) self.assertEqual( expected_value, @@ -89,53 +91,15 @@ class BaseUnitConversionTest(object): tc_lib.convert_to_kilobits, value, self.base_unit ) - def test_bytes_to_bits(self): - test_values = [ - (0, 0), # 0 bytes should be 0 bits - (1, 8) # 1 byte should be 8 bits - ] - for input_bytes, expected_bits in test_values: - self.assertEqual( - expected_bits, tc_lib.bytes_to_bits(input_bytes) - ) - class TestSIUnitConversions(BaseUnitConversionTest, base.BaseTestCase): - base_unit = tc_lib.SI_BASE - - def test_bits_to_kilobits(self): - test_values = [ - (0, 0), # 0 bites should be 0 kilobites - (1, 1), # 1 bit should be 1 kilobit - (999, 1), # 999 bits should be 1 kilobit - (1000, 1), # 1000 bits should be 1 kilobit - (1001, 2) # 1001 bits should be 2 kilobits - ] - for input_bits, expected_kilobits in test_values: - self.assertEqual( - expected_kilobits, - tc_lib.bits_to_kilobits(input_bits, self.base_unit) - ) + base_unit = constants.SI_BASE class TestIECUnitConversions(BaseUnitConversionTest, base.BaseTestCase): - base_unit = tc_lib.IEC_BASE - - def test_bits_to_kilobits(self): - test_values = [ - (0, 0), # 0 bites should be 0 kilobites - (1, 1), # 1 bit should be 1 kilobit - (1023, 1), # 1023 bits should be 1 kilobit - (1024, 1), # 1024 bits should be 1 kilobit - (1025, 2) # 1025 bits should be 2 kilobits - ] - for input_bits, expected_kilobits in test_values: - self.assertEqual( - expected_kilobits, - tc_lib.bits_to_kilobits(input_bits, self.base_unit) - ) + base_unit = constants.IEC_BASE class TestTcCommand(base.BaseTestCase): diff --git a/neutron/tests/unit/common/test_utils.py b/neutron/tests/unit/common/test_utils.py index f5616e737f6..b4c7fcc341b 100644 --- a/neutron/tests/unit/common/test_utils.py +++ b/neutron/tests/unit/common/test_utils.py @@ -28,6 +28,7 @@ import six import testscenarios import testtools +from neutron.common import constants as common_constants from neutron.common import exceptions as n_exc from neutron.common import utils from neutron.plugins.common import utils as plugin_utils @@ -775,3 +776,54 @@ class TestThrottler(base.BaseTestCase): obj = Klass() obj.method() + + +class BaseUnitConversionTest(object): + + def test_bytes_to_bits(self): + test_values = [ + (0, 0), # 0 bytes should be 0 bits + (1, 8) # 1 byte should be 8 bits + ] + for input_bytes, expected_bits in test_values: + self.assertEqual( + expected_bits, utils.bytes_to_bits(input_bytes) + ) + + +class TestSIUnitConversions(BaseUnitConversionTest, base.BaseTestCase): + + base_unit = common_constants.SI_BASE + + def test_bits_to_kilobits(self): + test_values = [ + (0, 0), # 0 bites should be 0 kilobites + (1, 1), # 1 bit should be 1 kilobit + (999, 1), # 999 bits should be 1 kilobit + (1000, 1), # 1000 bits should be 1 kilobit + (1001, 2) # 1001 bits should be 2 kilobits + ] + for input_bits, expected_kilobits in test_values: + self.assertEqual( + expected_kilobits, + utils.bits_to_kilobits(input_bits, self.base_unit) + ) + + +class TestIECUnitConversions(BaseUnitConversionTest, base.BaseTestCase): + + base_unit = common_constants.IEC_BASE + + def test_bits_to_kilobits(self): + test_values = [ + (0, 0), # 0 bites should be 0 kilobites + (1, 1), # 1 bit should be 1 kilobit + (1023, 1), # 1023 bits should be 1 kilobit + (1024, 1), # 1024 bits should be 1 kilobit + (1025, 2) # 1025 bits should be 2 kilobits + ] + for input_bits, expected_kilobits in test_values: + self.assertEqual( + expected_kilobits, + utils.bits_to_kilobits(input_bits, self.base_unit) + )