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

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)