From e3361366038fb0acb8c00d54ac30fb9b95debf52 Mon Sep 17 00:00:00 2001 From: Viacheslav Hletenko Date: Sat, 1 Jul 2023 12:38:26 +0000 Subject: T5295: QoS fix policy limiter tc filter rate limit tc filter rate limit should be used only if qostype is 'limiter' and not 'shaper' --- python/vyos/qos/base.py | 78 ++++++++++++++++++++++------------------------ python/vyos/qos/limiter.py | 1 + 2 files changed, 38 insertions(+), 41 deletions(-) (limited to 'python/vyos/qos') diff --git a/python/vyos/qos/base.py b/python/vyos/qos/base.py index 26ec65535..226773c4f 100644 --- a/python/vyos/qos/base.py +++ b/python/vyos/qos/base.py @@ -61,6 +61,7 @@ class QoSBase: "CS7": 0xE0, "EF": 0xB8 } + qostype = None def __init__(self, interface): if os.path.exists('/tmp/vyos.qos.debug'): @@ -322,44 +323,39 @@ class QoSBase: filter_cmd += f' flowid {self._parent:x}:{cls:x}' self._cmd(filter_cmd) - # T5295: Do not do any tc filter action for 'default' - # In VyOS 1.4, we have the following configuration: - # tc filter replace dev eth0 parent 1: prio 255 protocol all basic action police rate 300000000 burst 15k - # However, this caused unexpected random speeds. - # In VyOS 1.3, we do not use any 'tc filter' for rate limits, - # It gets rate from tc class classid 1:1 - # - # if 'default' in config: - # if 'class' in config: - # class_id_max = self._get_class_max_id(config) - # default_cls_id = int(class_id_max) +1 - # self._build_base_qdisc(config['default'], default_cls_id) - # - # filter_cmd = f'tc filter replace dev {self._interface} parent {self._parent:x}: ' - # filter_cmd += 'prio 255 protocol all basic' - # - # # The police block allows limiting of the byte or packet rate of - # # traffic matched by the filter it is attached to. - # # https://man7.org/linux/man-pages/man8/tc-police.8.html - # if any(tmp in ['exceed', 'bandwidth', 'burst'] for tmp in config['default']): - # filter_cmd += f' action police' - # - # if 'exceed' in config['default']: - # action = config['default']['exceed'] - # filter_cmd += f' conform-exceed {action}' - # if 'not_exceed' in config['default']: - # action = config['default']['not_exceed'] - # filter_cmd += f'/{action}' - # - # if 'bandwidth' in config['default']: - # rate = self._rate_convert(config['default']['bandwidth']) - # filter_cmd += f' rate {rate}' - # - # if 'burst' in config['default']: - # burst = config['default']['burst'] - # filter_cmd += f' burst {burst}' - # - # if 'class' in config: - # filter_cmd += f' flowid {self._parent:x}:{default_cls_id:x}' - # - # self._cmd(filter_cmd) + if self.qostype == 'limiter': + if 'default' in config: + if 'class' in config: + class_id_max = self._get_class_max_id(config) + default_cls_id = int(class_id_max) + 1 + self._build_base_qdisc(config['default'], default_cls_id) + + filter_cmd = f'tc filter replace dev {self._interface} parent {self._parent:x}: ' + filter_cmd += 'prio 255 protocol all basic' + + # The police block allows limiting of the byte or packet rate of + # traffic matched by the filter it is attached to. + # https://man7.org/linux/man-pages/man8/tc-police.8.html + if any(tmp in ['exceed', 'bandwidth', 'burst'] for tmp in + config['default']): + filter_cmd += f' action police' + + if 'exceed' in config['default']: + action = config['default']['exceed'] + filter_cmd += f' conform-exceed {action}' + if 'not_exceed' in config['default']: + action = config['default']['not_exceed'] + filter_cmd += f'/{action}' + + if 'bandwidth' in config['default']: + rate = self._rate_convert(config['default']['bandwidth']) + filter_cmd += f' rate {rate}' + + if 'burst' in config['default']: + burst = config['default']['burst'] + filter_cmd += f' burst {burst}' + + if 'class' in config: + filter_cmd += f' flowid {self._parent:x}:{default_cls_id:x}' + + self._cmd(filter_cmd) diff --git a/python/vyos/qos/limiter.py b/python/vyos/qos/limiter.py index ace0c0b6c..3f5c11112 100644 --- a/python/vyos/qos/limiter.py +++ b/python/vyos/qos/limiter.py @@ -17,6 +17,7 @@ from vyos.qos.base import QoSBase class Limiter(QoSBase): _direction = ['ingress'] + qostype = 'limiter' def update(self, config, direction): tmp = f'tc qdisc add dev {self._interface} handle {self._parent:x}: {direction}' -- cgit v1.2.3 From 4917d7ab4601904bb19e9e2c8ea3e4bf7bc7521d Mon Sep 17 00:00:00 2001 From: Viacheslav Hletenko Date: Sat, 1 Jul 2023 17:30:55 +0000 Subject: T5302: QoS fix class with multiple matches generate one rule Fix QoS tc class with multiple matches generates one rule but expects multiple filter rules: set qos policy shaper test class 23 match one ip protocol 'tcp' set qos policy shaper test class 23 match two ip protocol 'udp' tc filter add dev eth0 parent 1: protocol all prio 1 u32 match ip protocol 6 0xff flowid 1:17 tc filter add dev eth0 parent 1: protocol all prio 2 u32 match ip protocol 17 0xff flowid 1:17 --- python/vyos/qos/base.py | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) (limited to 'python/vyos/qos') diff --git a/python/vyos/qos/base.py b/python/vyos/qos/base.py index 226773c4f..b992fe904 100644 --- a/python/vyos/qos/base.py +++ b/python/vyos/qos/base.py @@ -204,18 +204,20 @@ class QoSBase: self._build_base_qdisc(cls_config, int(cls)) # every match criteria has it's tc instance - filter_cmd = f'tc filter replace dev {self._interface} parent {self._parent:x}:' + filter_cmd_base = f'tc filter add dev {self._interface} parent {self._parent:x}:' if priority: - filter_cmd += f' prio {cls}' + filter_cmd_base += f' prio {cls}' elif 'priority' in cls_config: prio = cls_config['priority'] - filter_cmd += f' prio {prio}' + filter_cmd_base += f' prio {prio}' - filter_cmd += ' protocol all' + filter_cmd_base += ' protocol all' if 'match' in cls_config: - for match, match_config in cls_config['match'].items(): + for index, (match, match_config) in enumerate(cls_config['match'].items(), start=1): + filter_cmd = filter_cmd_base + filter_cmd += f' prio {index}' if 'mark' in match_config: mark = match_config['mark'] filter_cmd += f' handle {mark} fw' @@ -290,10 +292,19 @@ class QoSBase: elif af == 'ipv6': filter_cmd += f' match u8 {mask} {mask} at 53' + cls = int(cls) + filter_cmd += f' flowid {self._parent:x}:{cls:x}' + self._cmd(filter_cmd) + else: filter_cmd += ' basic' + cls = int(cls) + filter_cmd += f' flowid {self._parent:x}:{cls:x}' + self._cmd(filter_cmd) + + # The police block allows limiting of the byte or packet rate of # traffic matched by the filter it is attached to. # https://man7.org/linux/man-pages/man8/tc-police.8.html @@ -319,10 +330,6 @@ class QoSBase: # burst = cls_config['burst'] # filter_cmd += f' burst {burst}' - cls = int(cls) - filter_cmd += f' flowid {self._parent:x}:{cls:x}' - self._cmd(filter_cmd) - if self.qostype == 'limiter': if 'default' in config: if 'class' in config: -- cgit v1.2.3