T6806: Rework QoS Policy for HFSC Shaper (#4181)

- Removed default `m1` and `m2` values from interface definitions
- Adjusted filter priorities for shapers
- Fixed SFQ qdisc and HFSC class creation to fully support `m1`, `d`, and `m2` parameters
- Added validation logic similar to VyOS 1.3 to improve error handling and user experience

2 files changed, 49 insertions, 69 deletions

diff --git a/python/vyos/qos/base.py b/python/vyos/qos/base.py
index 35cc5be18..12d940e3c 100644
--- a/python/vyos/qos/base.py
+++ b/python/vyos/qos/base.py
@@ -259,7 +259,7 @@ class QoSBase:
                                     has_filter = True
                                     break
 
-                        if self.qostype == 'shaper' and 'prio ' not in filter_cmd:
+                        if self.qostype in ['shaper', 'shaper_hfsc'] and 'prio ' not in filter_cmd:
                             filter_cmd += f' prio {index}'
                         if 'mark' in match_config:
                             mark = match_config['mark']
diff --git a/python/vyos/qos/trafficshaper.py b/python/vyos/qos/trafficshaper.py
index 8b0333c21..9f92ccd8b 100644
--- a/python/vyos/qos/trafficshaper.py
+++ b/python/vyos/qos/trafficshaper.py
@@ -126,91 +126,71 @@ class TrafficShaper(QoSBase):
         # call base class
         super().update(config, direction)
 
+
 class TrafficShaperHFSC(QoSBase):
+    """
+    Traffic shaper using Hierarchical Fair Service Curve (HFSC).
+    Documentation: https://man7.org/linux/man-pages/man8/tc-hfsc.8.html
+    """
+
     _parent = 1
     qostype = 'shaper_hfsc'
 
-    # https://man7.org/linux/man-pages/man8/tc-hfsc.8.html
-    def update(self, config, direction):
-        class_id_max = 0
-        if 'class' in config:
-            tmp = list(config['class'])
-            tmp.sort()
-            class_id_max = tmp[-1]
+    criteria = ['linkshare', 'realtime', 'upperlimit']
+    short_criterion = {
+        'linkshare': 'ls',
+        'realtime': 'rt',
+        'upperlimit': 'ul',
+    }
+
+    def _gen_class(self, cls: int, cls_config: dict):
+        """
+        Generate HFSC class and add Stochastic Fair Queueing (SFQ) qdisc.
+
+        Args:
+            cls (int): Class ID
+            cls_config (dict): Configuration for the class
+        """
+        tmp = f'tc class replace dev {self._interface} parent {self._parent:x}:1 classid {self._parent:x}:{cls:x} hfsc'
+
+        for crit in self.criteria:
+            param = cls_config.get(crit)
+            if param:
+                tmp += (
+                    f' {self.short_criterion[crit]}'
+                    f' m1 {self._rate_convert(param["m1"]) if param.get("m1") else 0}'
+                    f' d {param.get("d", 0)}ms'
+                    f' m2 {self._rate_convert(param["m2"])}'
+                )
 
-        r2q = 10
-        # bandwidth is a mandatory CLI node
-        speed = self._rate_convert(config['bandwidth'])
-        speed_bps = int(speed) // 8
+        self._cmd(tmp)
 
-        # need a bigger r2q if going fast than 16 mbits/sec
-        if (speed_bps // r2q) >= MAXQUANTUM: # integer division
-            r2q = ceil(speed_bps // MAXQUANTUM)
-        else:
-            # if there is a slow class then may need smaller value
-            if 'class' in config:
-                min_speed = speed_bps
-                for cls, cls_options in config['class'].items():
-                    # find class with the lowest bandwidth used
-                    if 'bandwidth' in cls_options:
-                        bw_bps = int(self._rate_convert(cls_options['bandwidth'])) // 8 # bandwidth in bytes per second
-                        if bw_bps < min_speed:
-                            min_speed = bw_bps
+        tmp = f'tc qdisc replace dev {self._interface} parent {self._parent:x}:{cls:x} sfq perturb 10'
+        self._cmd(tmp)
 
-                while (r2q > 1) and (min_speed // r2q) < MINQUANTUM:
-                    tmp = r2q -1
-                    if (speed_bps // tmp) >= MAXQUANTUM:
-                        break
-                    r2q = tmp
+    def update(self, config, direction):
+        class_id_max = self._get_class_max_id(config)
+        default_cls_id = int(class_id_max) + 1 if class_id_max else 2
 
-        default_minor_id = int(class_id_max) +1
-        tmp = f'tc qdisc replace dev {self._interface} root handle {self._parent:x}: hfsc default {default_minor_id:x}' # default is in hex
+        speed = self._rate_convert(config['bandwidth'])
+
+        tmp = f'tc qdisc replace dev {self._interface} root handle {self._parent:x}: hfsc default {default_cls_id:x}'  # default is in hex
         self._cmd(tmp)
 
         tmp = f'tc class replace dev {self._interface} parent {self._parent:x}: classid {self._parent:x}:1 hfsc sc rate {speed} ul rate {speed}'
         self._cmd(tmp)
 
+        # tmp = f'tc qdisc add dev {self._interface} parent {self._parent:x}:1 handle f1: sfq perturb 10'
+        # self._cmd(tmp)
+
         if 'class' in config:
             for cls, cls_config in config['class'].items():
-                # class id is used later on and passed as hex, thus this needs to be an int
-                cls = int(cls)
-                # ls m1
-                if cls_config.get('linkshare', {}).get('m1').endswith('%'):
-                    percent = cls_config['linkshare']['m1'].rstrip('%')
-                    m_one_rate = self._rate_convert(config['bandwidth']) * int(percent) // 100
-                else:
-                    m_one_rate = cls_config['linkshare']['m1']
-                # ls m2
-                if cls_config.get('linkshare', {}).get('m2').endswith('%'):
-                    percent = cls_config['linkshare']['m2'].rstrip('%')
-                    m_two_rate = self._rate_convert(config['bandwidth']) * int(percent) // 100
-                else:
-                    m_two_rate = self._rate_convert(cls_config['linkshare']['m2'])
-
-                tmp = f'tc class replace dev {self._interface} parent {self._parent:x}:1 classid {self._parent:x}:{cls:x} hfsc ls m1 {m_one_rate} m2 {m_two_rate} '
-                self._cmd(tmp)
-
-                tmp = f'tc qdisc replace dev {self._interface} parent {self._parent:x}:{cls:x} sfq perturb 10'
-                self._cmd(tmp)
+                self._gen_class(cls=int(cls), cls_config=cls_config)
 
         if 'default' in config:
-            # ls m1
-            if config.get('default', {}).get('linkshare', {}).get('m1').endswith('%'):
-                percent = config['default']['linkshare']['m1'].rstrip('%')
-                m_one_rate = self._rate_convert(config['default']['linkshare']['m1']) * int(percent) // 100
-            else:
-                m_one_rate = config['default']['linkshare']['m1']
-            # ls m2
-            if config.get('default', {}).get('linkshare', {}).get('m2').endswith('%'):
-                percent = config['default']['linkshare']['m2'].rstrip('%')
-                m_two_rate = self._rate_convert(config['default']['linkshare']['m2']) * int(percent) // 100
-            else:
-                m_two_rate = self._rate_convert(config['default']['linkshare']['m2'])
-            tmp = f'tc class replace dev {self._interface} parent {self._parent:x}:1 classid {self._parent:x}:{default_minor_id:x} hfsc ls m1 {m_one_rate} m2 {m_two_rate} '
-            self._cmd(tmp)
-
-            tmp = f'tc qdisc replace dev {self._interface} parent {self._parent:x}:{default_minor_id:x} sfq perturb 10'
-            self._cmd(tmp)
+            self._gen_class(
+                cls=int(default_cls_id), cls_config=config.get('default', {})
+            )
 
         # call base class
         super().update(config, direction)