diff --git a/neutron/agent/linux/tc_lib.py b/neutron/agent/linux/tc_lib.py index 48bff815953..6fad36debd6 100644 --- a/neutron/agent/linux/tc_lib.py +++ b/neutron/agent/linux/tc_lib.py @@ -82,10 +82,6 @@ class InvalidUnit(exceptions.NeutronException): message = _("Unit name '%(unit)s' is not valid.") -class TcLibPolicyClassInvalidMinKbpsValue(exceptions.NeutronException): - message = _("'min_kbps' is mandatory in a TC class and must be >= 1.") - - def convert_to_kilobits(value, base): value = value.lower() if "bit" in value: @@ -151,6 +147,21 @@ def _calc_burst(rate, buffer): (rtnl_common.TIME_UNITS_PER_SEC * rtnl_common.tick_in_usec))) +def _calc_min_rate(burst): + """Calculate minimum rate (bytes per second) accepted by Pyroute2 + + When creating a TC policy class, this function calculates the minimum + rate (bytes/sec) accepted by Pyroute2. This method is based on + pyroute2.netlink.rtnl.tcmsg.common.calc_xmittime + + :param rate: (int) rate in bytes per second. + :param burst: (int) burst in bytes. + :return: (int) minimum accepted rate in bytes per second. + """ + return max(8, math.ceil((rtnl_common.TIME_UNITS_PER_SEC * + rtnl_common.tick_in_usec * burst) / 2**32)) + + def _calc_latency_ms(limit, burst, rate): """Calculate latency value, in ms @@ -394,34 +405,45 @@ def delete_tc_qdisc(device, parent=None, is_ingress=False, raise_qdisc_not_found=raise_qdisc_not_found, namespace=namespace) -def add_tc_policy_class(device, parent, classid, min_kbps=1, max_kbps=None, +def add_tc_policy_class(device, parent, classid, max_kbps, min_kbps=None, burst_kb=None, namespace=None): """Add a TC policy class :param device: (string) device name :param parent: (string) qdisc parent class ('root', 'ingress', '2:10') :param classid: (string) major:minor handler identifier ('10:20') + :param max_kbps: (int) maximum bandwidth in kbps :param min_kbps: (int) (optional) minimum bandwidth in kbps - :param max_kbps: (int) (optional) maximum bandwidth in kbps :param burst_kb: (int) (optional) burst size in kb :param namespace: (string) (optional) namespace name :return: """ parent = TC_QDISC_PARENT.get(parent, parent) + if not burst_kb: + burst_kb = max_kbps * qos_consts.DEFAULT_BURST_RATE + # NOTE(ralonsoh): pyroute2 input parameters and units [1]: # - rate (min bw): bytes/second # - ceil (max bw): bytes/second # - burst: bytes # [1] https://www.systutorials.com/docs/linux/man/8-tc/ - if int(min_kbps) < 1: - raise TcLibPolicyClassInvalidMinKbpsValue() - args = {'rate': int(min_kbps * 1024 / 8)} - if max_kbps: - args['ceil'] = int(max_kbps * 1024 / 8) - if burst_kb: - args['burst'] = int(burst_kb * 1024 / 8) + kwargs = {'ceil': int(max_kbps * 1024 / 8), + 'burst': int(burst_kb * 1024 / 8)} + + rate = int((min_kbps or 0) * 1024 / 8) + min_rate = _calc_min_rate(kwargs['burst']) + if min_rate > rate: + LOG.warning('TC HTB class policy rate %(rate)s (bytes/second) is ' + 'lower than the minimum accepted %(min_rate)s ' + '(bytes/second), for device %(device)s, qdisc ' + '%(qdisc)s and classid %(classid)s', + {'rate': rate, 'min_rate': min_rate, 'device': device, + 'qdisc': parent, 'classid': classid}) + rate = min_rate + kwargs['rate'] = rate + priv_tc_lib.add_tc_policy_class(device, parent, classid, 'htb', - namespace=namespace, **args) + namespace=namespace, **kwargs) def list_tc_policy_class(device, namespace=None): diff --git a/neutron/tests/functional/agent/linux/test_tc_lib.py b/neutron/tests/functional/agent/linux/test_tc_lib.py index 11265066b2a..e4e882cd179 100644 --- a/neutron/tests/functional/agent/linux/test_tc_lib.py +++ b/neutron/tests/functional/agent/linux/test_tc_lib.py @@ -15,7 +15,9 @@ import random +import mock import netaddr +from neutron_lib.services.qos import constants as qos_consts from oslo_utils import uuidutils from neutron.agent.linux import bridge_lib @@ -139,6 +141,43 @@ class TcPolicyClassTestCase(functional_base.BaseSudoTestCase): self.assertGreater(tc_classes[0]['stats']['bytes'], bytes) self.assertGreater(tc_classes[0]['stats']['packets'], packets) + def test_add_tc_policy_class_check_min_kbps_values(self): + def warning_args(rate, min_rate): + return ('TC HTB class policy rate %(rate)s (bytes/second) is ' + 'lower than the minimum accepted %(min_rate)s ' + '(bytes/second), for device %(device)s, qdisc ' + '%(qdisc)s and classid %(classid)s', + {'rate': rate, 'min_rate': min_rate, 'classid': '1:10', + 'device': self.device[0], 'qdisc': '1:'}) + + self._create_two_namespaces() + tc_lib.add_tc_qdisc(self.device[0], 'htb', parent='root', handle='1:', + namespace=self.ns[0]) + + with mock.patch.object(tc_lib, 'LOG') as mock_log: + # rate > min_rate: OK + tc_lib.add_tc_policy_class(self.device[0], '1:', '1:10', + max_kbps=2000, burst_kb=1000, + min_kbps=4, namespace=self.ns[0]) + mock_log.warning.assert_not_called() + + # rate < min_rate: min_rate = 466 with burst = 128000 + tc_lib.add_tc_policy_class(self.device[0], '1:', '1:10', + max_kbps=2000, burst_kb=1000, + min_kbps=3, namespace=self.ns[0]) + mock_log.warning.assert_called_once_with( + *warning_args(3 * 128, tc_lib._calc_min_rate(1000 * 128))) + + # rate < min_rate: min_rate = 466 with burst = 0.8 ceil = 256000 + mock_log.reset_mock() + tc_lib.add_tc_policy_class(self.device[0], '1:', '1:10', + max_kbps=2000, burst_kb=None, + min_kbps=5, namespace=self.ns[0]) + min_rate = tc_lib._calc_min_rate(qos_consts.DEFAULT_BURST_RATE * + 2000 * 128) + mock_log.warning.assert_called_once_with( + *warning_args(5 * 128, min_rate)) + class TcFiltersTestCase(functional_base.BaseSudoTestCase):