Check tc_lib.add_tc_policy_class input parameters
All input rate parameters (ceil, burst and rate) are mandatory in Pyroute "tc" class [1]. "tc_lib.add_tc_policy" should provide them. max_kbps: Now "max_kbps" is mandatory. burst_kb: In case this parameter does not exist, the library uses the method previously defined in Neutron, consisting of asigning the 80% of "max_kbps". min_kbps: In Pyroute2 [2], the value of "burst" depends on the "rate" value. If the "rate" value is too small, the "burst" value will exceed the 2^32 limit defined in the iproute2 structure. In order to avoid the error described in c#4 of the related bug, the method checks if the provided "rate" value is greater than the minimum possible value. If not, the minimum value is set and a warning message is logged. [1]fb2453c999/neutron/privileged/agent/linux/tc_lib.py (L107-L108)
[2]943502a95c/pyroute2/netlink/rtnl/tcmsg/sched_htb.py (L88-L91)
Change-Id: If60f4ea9793b0872e348a6319f2c24127e93f540 Related-Bug: #1826565 (cherry picked from commit1e69279817
)
This commit is contained in:
parent
0471f2c2e2
commit
62cbdfbaa2
|
@ -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):
|
||||
|
|
|
@ -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):
|
||||
|
||||
|
|
Loading…
Reference in New Issue