From 562f7131d0c822a94c3b2bdb1f5fd70bfe8b3036 Mon Sep 17 00:00:00 2001 From: Nataliia Solomko Date: Thu, 30 Jul 2026 11:10:24 +0300 Subject: qos: T9134: Fix commit crash when classes match different protocols Every class match is installed as its own tc filter, and tc binds a filter priority ("prio"/"pref") to a single protocol. Two filters with the same priority but different protocols are rejected by the kernel, so the commit crashes - e.g. one class matching "ether protocol arp" and another matching IP. The filter priority came from the class id, the class "priority" (which the limiter defaults to 20 for every class), or the per-class match index - none of which is unique across classes. Assign every match a unique tc filter priority by ranking the matches in evaluation order (class id, an explicit or default class "priority", or the per-class match index) and numbering them 1, 2, 3, ... This keeps the match evaluation order unchanged across all class-based policies (shaper, shaper-hfsc, limiter, round-robin, priority-queue). The class "priority" still drives the HTB class scheduling priority on the shaper; the tc filter priority is an internal evaluation-order rank, not the CLI "priority" value. Add smoketests for mixed-protocol classes (shaper and limiter) and for match evaluation order. --- python/vyos/qos/base.py | 49 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 8 deletions(-) (limited to 'python') diff --git a/python/vyos/qos/base.py b/python/vyos/qos/base.py index 65991704d..08b42b839 100644 --- a/python/vyos/qos/base.py +++ b/python/vyos/qos/base.py @@ -238,6 +238,46 @@ class QoSBase: pprint.pprint(config) if 'class' in config: + # T9134: every class match is installed as its own tc filter, and + # tc ties a filter priority ("prio"/"pref") to a single protocol - + # two filters with the same priority but different protocols (e.g. + # the default "all" and an explicit "arp") are rejected by the + # kernel and the commit fails. So every filter needs a priority + # that is unique across all classes. + # + # Rank the matches in evaluation order, then number them 1, 2, 3...: + # - by class id for policies that key the filter on it + # (round-robin, priority-queue), else + # - by an explicit or default class "priority", else + # - by the per-class match index (a shaper class with no priority) + # The last fallback orders matches by declaration position, not by + # match specificity, and it shares the number space with explicit + # priorities. This preserves the historical ordering, but it means + # overlapping matches whose relative order matters (e.g. a specific + # /32 vs a broad /24 in another class) must be given an explicit + # "priority" to be ordered deterministically. + # + # NOTE: this number is an internal evaluation-order rank, not the + # CLI "priority" value. The CLI "priority" still decides the order + # (and, on the shaper, the HTB class scheduling priority set in + # trafficshaper.py); it is not reused verbatim as the tc priority + # because it is not unique across classes. + filter_pref = {} + ranked = [] + for cls, cls_config in config['class'].items(): + for index, match in enumerate(cls_config.get('match', {}), start=1): + if priority: + key = int(cls) + elif 'priority' in cls_config: + key = int(cls_config['priority']) + else: + key = index + ranked.append((key, (int(cls), match))) + # stable sort keeps declaration order among matches with equal keys + ranked.sort(key=lambda entry: entry[0]) + for pref, (_, ident) in enumerate(ranked, start=1): + filter_pref[ident] = pref + for cls, cls_config in config['class'].items(): self._build_base_qdisc(cls_config, int(cls)) @@ -245,12 +285,6 @@ class QoSBase: filter_cmd_base = ['tc', 'filter', 'add', 'dev', self._interface, 'parent', f'{self._parent:x}:'] - if priority: - filter_cmd_base += ['prio', str(cls)] - elif 'priority' in cls_config: - prio = cls_config['priority'] - filter_cmd_base += ['prio', str(prio)] - if 'match' in cls_config: has_filter = False has_action_policy = any(tmp in ['exceed', 'bandwidth', 'burst'] for tmp in cls_config) @@ -266,8 +300,7 @@ class QoSBase: tmp = dict_search(f'ether.protocol', match_config) or 'all' filter_cmd += ['protocol', str(tmp)] - if self.qostype in ['shaper', 'shaper_hfsc'] and 'prio' not in filter_cmd: - filter_cmd += ['prio', str(index)] + filter_cmd += ['prio', str(filter_pref[(int(cls), match)])] if 'mark' in match_config: mark = match_config['mark'] -- cgit v1.2.3