Fix the wrong value for QoS rate conversion to bytes/s

The rate value is converted to bytes per second before being
sent to Pyroute2, but it used the wrong value for the calculations.
This resulted in incorrect rates.
It should be multiplied by 1000 (kbit), not 1024 (Kibit).
The same applies to the burst value (kb).

Change-Id: I70cb1fe651a50b2f6495d7a365a6beb2ba111c6d
Closes-Bug: #1884273
This commit is contained in:
Weronika Sikora 2020-06-19 17:16:29 +02:00
parent 0580d03a2b
commit 94d6e38fa0
4 changed files with 31 additions and 31 deletions

View File

@ -345,8 +345,8 @@ def add_tc_qdisc(device, qdisc_type, parent=None, handle=None, latency_ms=None,
qdisc_type=qdisc_type,
needed_arguments=['latency_ms', 'max_kbps', 'kernel_hz'])
args['burst'] = int(
_get_tbf_burst_value(max_kbps, burst_kb, kernel_hz) * 1024 / 8)
args['rate'] = int(max_kbps * 1024 / 8)
_get_tbf_burst_value(max_kbps, burst_kb, kernel_hz) * 1000 / 8)
args['rate'] = int(max_kbps * 1000 / 8)
args['latency'] = latency_ms * 1000
if parent:
args['parent'] = rtnl.TC_H_ROOT if parent == 'root' else parent
@ -371,10 +371,10 @@ def list_tc_qdiscs(device, namespace=None):
if qdisc_attrs['qdisc_type'] == 'tbf':
tca_options = _get_attr(qdisc, 'TCA_OPTIONS')
tca_tbf_parms = _get_attr(tca_options, 'TCA_TBF_PARMS')
qdisc_attrs['max_kbps'] = int(tca_tbf_parms['rate'] * 8 / 1024)
qdisc_attrs['max_kbps'] = int(tca_tbf_parms['rate'] * 8 / 1000)
burst_bytes = _calc_burst(tca_tbf_parms['rate'],
tca_tbf_parms['buffer'])
qdisc_attrs['burst_kb'] = int(burst_bytes * 8 / 1024)
qdisc_attrs['burst_kb'] = int(burst_bytes * 8 / 1000)
qdisc_attrs['latency_ms'] = _calc_latency_ms(
tca_tbf_parms['limit'], burst_bytes, tca_tbf_parms['rate'])
retval.append(qdisc_attrs)
@ -427,10 +427,10 @@ def add_tc_policy_class(device, parent, classid, max_kbps, min_kbps=None,
# - ceil (max bw): bytes/second
# - burst: bytes
# [1] https://www.systutorials.com/docs/linux/man/8-tc/
kwargs = {'ceil': int(max_kbps * 1024 / 8),
'burst': int(burst_kb * 1024 / 8)}
kwargs = {'ceil': int(max_kbps * 1000 / 8),
'burst': int(burst_kb * 1000 / 8)}
rate = int((min_kbps or 0) * 1024 / 8)
rate = int((min_kbps or 0) * 1000 / 8)
min_rate = _calc_min_rate(kwargs['burst'])
if min_rate > rate:
LOG.warning('TC HTB class policy rate %(rate)s (bytes/second) is '
@ -460,9 +460,9 @@ def list_tc_policy_class(device, namespace=None):
tca_params = _get_attr(tca_options,
'TCA_' + qdisc_type.upper() + '_PARMS')
burst_kb = int(
_calc_burst(tca_params['rate'], tca_params['buffer']) * 8 / 1024)
max_kbps = int(tca_params['ceil'] * 8 / 1024)
min_kbps = int(tca_params['rate'] * 8 / 1024)
_calc_burst(tca_params['rate'], tca_params['buffer']) * 8 / 1000)
max_kbps = int(tca_params['ceil'] * 8 / 1000)
min_kbps = int(tca_params['rate'] * 8 / 1000)
return max_kbps, min_kbps, burst_kb
tc_classes = priv_tc_lib.list_tc_policy_classes(device,
@ -562,8 +562,8 @@ def add_tc_filter_policy(device, parent, rate_kbps, burst_kb, mtu, action,
:param namespace: (string) (optional) namespace name
"""
rate = int(rate_kbps * 1024 / 8)
burst = int(burst_kb * 1024 / 8)
rate = int(rate_kbps * 1000 / 8)
burst = int(burst_kb * 1000 / 8)
priv_tc_lib.add_tc_filter_policy(device, parent, priority, rate, burst,
mtu, action, protocol=protocol,
namespace=namespace)
@ -604,10 +604,10 @@ def list_tc_filters(device, parent, namespace=None):
if tca_u32_police:
tca_police_tbf = _get_attr(tca_u32_police, 'TCA_POLICE_TBF')
if tca_police_tbf:
value['rate_kbps'] = int(tca_police_tbf['rate'] * 8 / 1024)
value['rate_kbps'] = int(tca_police_tbf['rate'] * 8 / 1000)
value['burst_kb'] = int(
_calc_burst(tca_police_tbf['rate'],
tca_police_tbf['burst']) * 8 / 1024)
tca_police_tbf['burst']) * 8 / 1000)
value['mtu'] = tca_police_tbf['mtu']
retval.append(value)

View File

@ -166,17 +166,17 @@ class TcPolicyClassTestCase(functional_base.BaseSudoTestCase):
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)))
*warning_args(3 * 125, tc_lib._calc_min_rate(1000 * 125)))
# rate < min_rate: min_rate = 466 with burst = 0.8 ceil = 256000
# rate < min_rate: min_rate = 455 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)
2000 * 125)
mock_log.warning.assert_called_once_with(
*warning_args(5 * 128, min_rate))
*warning_args(5 * 125, min_rate))
class TcFiltersTestCase(functional_base.BaseSudoTestCase):

View File

@ -279,8 +279,8 @@ class TcFilterClassTestCase(functional_base.BaseSudoTestCase):
namespace=self.namespace)
# NOTE(ralonsoh):
# - rate: 320000 bytes/sec (pyroute2 units) = 2500 kbits/sec (OS units)
# - burst: 192000 bytes/sec = 1500 kbits/sec
# - rate: 320000 bytes/sec (pyroute2 units) = 2560 kbits/sec (OS units)
# - burst: 192000 bytes/sec = 1536 kbits/sec
priv_tc_lib.add_tc_filter_policy(
self.device, 'ffff:', 49, 320000, 192000, 1200, 'drop',
namespace=self.namespace)
@ -288,6 +288,6 @@ class TcFilterClassTestCase(functional_base.BaseSudoTestCase):
filters = tc_lib.list_tc_filters(
self.device, 'ffff:', namespace=self.namespace)
self.assertEqual(1, len(filters))
self.assertEqual(2500, filters[0]['rate_kbps'])
self.assertEqual(1500, filters[0]['burst_kb'])
self.assertEqual(2560, filters[0]['rate_kbps'])
self.assertEqual(1536, filters[0]['burst_kb'])
self.assertEqual(1200, filters[0]['mtu'])

View File

@ -271,9 +271,9 @@ class TcTestCase(base.BaseTestCase):
tc_lib.add_tc_qdisc('device', 'tbf', parent='root', max_kbps=10000,
burst_kb=1500, latency_ms=70, kernel_hz=250,
namespace=self.namespace)
burst = tc_lib._get_tbf_burst_value(10000, 1500, 70) * 1024 / 8
burst = tc_lib._get_tbf_burst_value(10000, 1500, 70) * 1000 / 8
self.mock_add_tc_qdisc.assert_called_once_with(
'device', parent=rtnl.TC_H_ROOT, kind='tbf', rate=10000 * 128,
'device', parent=rtnl.TC_H_ROOT, kind='tbf', rate=10000 * 125,
burst=burst, latency=70000, namespace=self.namespace)
def test_add_tc_qdisc_tbf_missing_arguments(self):
@ -318,8 +318,8 @@ class TcTestCase(base.BaseTestCase):
self.assertEqual('root', qdiscs[0]['parent'])
self.assertEqual('5:1', qdiscs[0]['handle'])
self.assertEqual('tbf', qdiscs[0]['qdisc_type'])
self.assertEqual(2500, qdiscs[0]['max_kbps'])
self.assertEqual(1500, qdiscs[0]['burst_kb'])
self.assertEqual(2560, qdiscs[0]['max_kbps'])
self.assertEqual(1536, qdiscs[0]['burst_kb'])
self.assertEqual(50, qdiscs[0]['latency_ms'])
def test__get_tbf_burst_value_when_burst_bigger_then_minimal(self):
@ -346,8 +346,8 @@ class TcPolicyClassTestCase(base.BaseTestCase):
'device', 'root', '1:10', min_kbps=1000, max_kbps=2000,
burst_kb=1600, namespace=self.namespace)
self.mock_add_tc_policy_class.assert_called_once_with(
'device', rtnl.TC_H_ROOT, '1:10', 'htb', rate=1000 * 128,
ceil=2000 * 128, burst=1600 * 128, namespace=self.namespace)
'device', rtnl.TC_H_ROOT, '1:10', 'htb', rate=1000 * 125,
ceil=2000 * 125, burst=1600 * 125, namespace=self.namespace)
@mock.patch('pyroute2.netlink.rtnl.tcmsg.common.tick_in_usec', 15.625)
def test_list_tc_policy_classes(self):
@ -367,9 +367,9 @@ class TcPolicyClassTestCase(base.BaseTestCase):
'parent': 'root',
'classid': '1:1',
'qdisc_type': 'htb',
'min_kbps': 1500,
'max_kbps': 2000,
'burst_kb': 1200}
'min_kbps': 1536,
'max_kbps': 2048,
'burst_kb': 1228}
self.assertEqual(reference, _class)