summaryrefslogtreecommitdiff
path: root/python
diff options
context:
space:
mode:
authorViacheslav Hletenko <v.gletenko@vyos.io>2023-06-16 13:29:41 +0000
committerViacheslav Hletenko <v.gletenko@vyos.io>2023-06-16 13:38:26 +0000
commit08b333eac3c274a9111e68823e5c9de585add5e4 (patch)
tree97a2d82d228e5b4fc0b0d7633d85afe0971994a5 /python
parentc67c51e0202d9eafed6a719a49dd6885132c76ec (diff)
downloadvyos-1x-08b333eac3c274a9111e68823e5c9de585add5e4.tar.gz
vyos-1x-08b333eac3c274a9111e68823e5c9de585add5e4.zip
T5295: Fix QoS shaper rate limit
Do not handle rate via 'tc filter' directly but rather set the 'tc filter' to direct traffic to the correct tc class flow. As it in 1.3. It fixes random unexpected shapes, when you set for example 300mbit but get 3-11mbit Current implementation seems not correct as it uses rate limits two times (in class and in filter): tc class replace dev eth0 parent 1:1 classid 1:17 htb rate 250000000 \ burst 15k quantum 1514 tc filter replace dev eth0 parent 1: protocol all u32 match \ ip dst 192.168.122.11 action police rate 250000000 burst 15k flowid 1:17 The correct way after fix: tc class replace dev eth0 parent 1:1 classid 1:17 htb rate 250000000 \ burst 15k quantum 1514 tc filter replace dev eth0 parent 1: protocol all u32 match \ ip dst 192.168.122.11 flowid 1:17
Diffstat (limited to 'python')
-rw-r--r--python/vyos/qos/base.py114
1 files changed, 62 insertions, 52 deletions
diff --git a/python/vyos/qos/base.py b/python/vyos/qos/base.py
index 33bb8ae28..bd1676107 100644
--- a/python/vyos/qos/base.py
+++ b/python/vyos/qos/base.py
@@ -1,4 +1,4 @@
-# Copyright 2022 VyOS maintainers and contributors <maintainers@vyos.io>
+# Copyright 2022-2023 VyOS maintainers and contributors <maintainers@vyos.io>
#
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
@@ -243,60 +243,70 @@ class QoSBase:
# 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 cls_config):
- filter_cmd += f' action police'
- if 'exceed' in cls_config:
- action = cls_config['exceed']
- filter_cmd += f' conform-exceed {action}'
- if 'not_exceed' in cls_config:
- action = cls_config['not_exceed']
- filter_cmd += f'/{action}'
-
- if 'bandwidth' in cls_config:
- rate = self._rate_convert(cls_config['bandwidth'])
- filter_cmd += f' rate {rate}'
-
- if 'burst' in cls_config:
- burst = cls_config['burst']
- filter_cmd += f' burst {burst}'
+ # T5295: We do not handle rate via tc filter directly,
+ # but rather set the tc filter to direct traffic to the correct tc class flow.
+ #
+ # if any(tmp in ['exceed', 'bandwidth', 'burst'] for tmp in cls_config):
+ # filter_cmd += f' action police'
+ #
+ # if 'exceed' in cls_config:
+ # action = cls_config['exceed']
+ # filter_cmd += f' conform-exceed {action}'
+ # if 'not_exceed' in cls_config:
+ # action = cls_config['not_exceed']
+ # filter_cmd += f'/{action}'
+ #
+ # if 'bandwidth' in cls_config:
+ # rate = self._rate_convert(cls_config['bandwidth'])
+ # filter_cmd += f' rate {rate}'
+ #
+ # if 'burst' in cls_config:
+ # 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 '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)
-
+ # 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)