Merge pull request #5305 from natali-rs1985/T8996

qos: T8996: Implement set-dscp packet remarking for shaper policy

Author: dmbaturin

python/vyos/qos/base.py

@@ -241,6 +241,13 @@ def update(self, config, direction, priority=None):
             for cls, cls_config in config['class'].items():
                 self._build_base_qdisc(cls_config, int(cls))
 
+
+                # Get DSCP value for packet remarking via tc pedit action
+                set_dscp = dict_search('set_dscp', cls_config)
+                dscp_value = None
+                if set_dscp:
+                    dscp_value = str(self._get_dsfield(set_dscp))
+
                 # every match criteria has it's tc instance
                 filter_cmd_base = ['tc', 'filter', 'add', 'dev', self._interface,
                                     'parent', f'{self._parent:x}:']
@@ -263,8 +270,10 @@ def update(self, config, direction, priority=None):
                                     has_filter = True
                                     break
 
-                        tmp = dict_search(f'ether.protocol', match_config) or 'all'
-                        filter_cmd += ['protocol', str(tmp)]
+                        filter_protocol = (
+                            dict_search(f'ether.protocol', match_config) or 'all'
+                        )
+                        filter_cmd += ['protocol', filter_protocol]
 
                         if self.qostype in ['shaper', 'shaper_hfsc'] and 'prio' not in filter_cmd:
                             filter_cmd += ['prio', str(index)]
@@ -361,9 +370,29 @@ def update(self, config, direction, priority=None):
                                         elif af == 'ipv6':
                                             filter_cmd += ['match', 'u8', str(mask), str(mask), 'at', '53']
 
+                        # Build pedit action to rewrite DSCP on matched packets.
+                        # retain 0xfc preserves ECN bits (bottom 2 bits of TOS/Traffic Class).
+                        # Non-IP match types skip pedit to avoid corrupting
+                        # non-IP packets, unless ether protocol is ip or ipv6.
+                        dscp_action = []
+                        if dscp_value is not None:
+                            proto = str(filter_protocol).lower()
+                            is_ipv4 = 'ip' in match_config or proto in ('ip', '0x0800', '2048')
+                            is_ipv6 = 'ipv6' in match_config or proto in ('ipv6', '0x86dd', '34525')
+                            if is_ipv4:
+                                dscp_action = ['action', 'pedit', 'ex', 'munge', 'ip',
+                                               'dsfield', 'set', dscp_value, 'retain', '0xfc',
+                                               'pipe', 'action', 'csum', 'ip4h']
+                            elif is_ipv6:
+                                dscp_action = ['action', 'pedit', 'ex', 'munge',
+                                               'ip6', 'traffic_class', 'set', dscp_value, 'retain', '0xfc']
+
                         if index != max_index or not has_action_policy:
                             # avoid duplicate last match rule
                             cls = int(cls)
+                            # add pedit before flowid for filters without police
+                            if dscp_action:
+                                filter_cmd += dscp_action
                             filter_cmd += ['flowid', f'{self._parent:x}:{cls:x}']
                             self._cmdl(filter_cmd)
 
@@ -373,6 +402,9 @@ def update(self, config, direction, priority=None):
                     if has_action_policy and has_filter:
                         # For "vif" "basic match" is used instead of "action police" T5961
                         if not match_vlan:
+                            # chain pedit before police with pipe
+                            if dscp_action:
+                                filter_cmd += dscp_action + ['pipe']
                             filter_cmd += ['action', 'police']
 
                             if 'exceed' in cls_config:
@@ -393,6 +425,9 @@ def update(self, config, direction, priority=None):
                             if 'mtu' in cls_config:
                                 mtu = cls_config['mtu']
                                 filter_cmd += ['mtu', str(mtu)]
+                        elif dscp_action:
+                            # vlan match skips police (T5961) but still needs pedit
+                            filter_cmd += dscp_action
 
                         cls = int(cls)
                         filter_cmd += ['flowid', f'{self._parent:x}:{cls:x}']
@@ -430,6 +465,32 @@ def update(self, config, direction, priority=None):
                 default_cls_id = int(class_id_max) +1
             self._build_base_qdisc(config['default'], default_cls_id)
 
+            # Default class has no match filters, so catch-all filters
+            # are needed to attach the pedit action for DSCP remarking.
+            # Separate filters per protocol to avoid corrupting non-IP packets (e.g. ARP).
+            # IPv4 uses u32 catch-all, IPv6 uses basic classifier (u32 doesn't support protocol ipv6).
+            # prio 255/256 ensures class filters match first.
+            set_dscp = dict_search('set_dscp', config['default'])
+            if set_dscp and self.qostype == 'shaper':
+                dscp_value = str(self._get_dsfield(set_dscp))
+                filter_cmd = ['tc', 'filter', 'replace', 'dev', self._interface,
+                              'parent', f'{self._parent:x}:']
+                filter_cmd += ['prio', '255', 'protocol', 'ip', 'u32',
+                               'match', 'u32', '0', '0']
+                filter_cmd += ['action', 'pedit', 'ex', 'munge',
+                               'ip', 'dsfield', 'set', dscp_value, 'retain', '0xfc',
+                               'pipe', 'action', 'csum', 'ip4h']
+                filter_cmd += ['flowid', f'{self._parent:x}:{default_cls_id:x}']
+                self._cmdl(filter_cmd)
+
+                filter_cmd = ['tc', 'filter', 'replace', 'dev', self._interface,
+                              'parent', f'{self._parent:x}:']
+                filter_cmd += ['prio', '256', 'protocol', 'ipv6', 'basic']
+                filter_cmd += ['action', 'pedit', 'ex', 'munge', 'ip6',
+                               'traffic_class', 'set', dscp_value, 'retain', '0xfc']
+                filter_cmd += ['flowid', f'{self._parent:x}:{default_cls_id:x}']
+                self._cmdl(filter_cmd)
+
         if self.qostype == 'limiter':
             if 'default' in config:
                 filter_cmd = ['tc', 'filter', 'replace', 'dev', self._interface,

smoketest/scripts/cli/test_qos.py

@@ -614,15 +614,23 @@ def test_11_shaper(self):
 
         for interface in self._interfaces:
             shaper_name = f'qos-shaper-{interface}'
+            shaper_path = base_path + ['policy', 'shaper', shaper_name]
+            class_path = shaper_path + ['class', '23']
 
             self.cli_set(base_path + ['interface', interface, 'egress', shaper_name])
-            self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'bandwidth', f'{bandwidth}mbit'])
-            self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'default', 'bandwidth', f'{default_bandwidth}mbit'])
-            self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'default', 'ceiling', f'{default_ceil}mbit'])
-            self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'default', 'queue-type', 'fair-queue'])
-            self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'class', '23', 'bandwidth', f'{class_bandwidth}mbit'])
-            self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'class', '23', 'ceiling', f'{class_ceil}mbit'])
-            self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'class', '23', 'match', '10', 'ip', 'destination', 'address', dst_address])
+            self.cli_set(shaper_path + ['bandwidth', f'{bandwidth}mbit'])
+            self.cli_set(
+                shaper_path + ['default', 'bandwidth', f'{default_bandwidth}mbit']
+            )
+            self.cli_set(shaper_path + ['default', 'ceiling', f'{default_ceil}mbit'])
+            self.cli_set(shaper_path + ['default', 'queue-type', 'fair-queue'])
+            self.cli_set(shaper_path + ['default', 'set-dscp', 'AF11'])
+            self.cli_set(class_path + ['bandwidth', f'{class_bandwidth}mbit'])
+            self.cli_set(class_path + ['ceiling', f'{class_ceil}mbit'])
+            self.cli_set(
+                class_path
+                + ['match', '10', 'ip', 'destination', 'address', dst_address]
+            )
 
             bandwidth += 1
             default_bandwidth += 1
@@ -651,6 +659,16 @@ def test_11_shaper(self):
             for config_entry in config_entries:
                 self.assertIn(config_entry, output)
 
+            # set-dscp on default class: catch-all filters with pedit
+            # AF11 = DSCP 10 << 2 = 0x28
+            filter_output = get_tc_filter_details(interface)
+            self.assertIn('pedit', filter_output)
+            self.assertIn('protocol ip pref 255', filter_output)
+            self.assertIn('at ipv4+0: val 00280000', filter_output)
+            self.assertIn('csum (iph)', filter_output)
+            self.assertIn('protocol ipv6 pref 256', filter_output)
+            self.assertIn('at ipv6+0: val 02800000', filter_output)
+
             bandwidth += 1
             default_bandwidth += 1
             default_ceil += 1
@@ -1325,5 +1343,73 @@ def test_24_policy_shaper_match_ether(self):
                               get_tc_filter_details(interface))
 
 
+    def test_25_shaper_set_dscp(self):
+        interface = self._interfaces[0]
+        shaper_name = f'qos-shaper-{interface}'
+        shaper_path = base_path + ['policy', 'shaper', shaper_name]
+        class_path = shaper_path + ['class', '10']
+
+        self.cli_set(base_path + ['interface', interface, 'egress', shaper_name])
+        self.cli_set(shaper_path + ['bandwidth', '100mbit'])
+        self.cli_set(shaper_path + ['default', 'bandwidth', '10mbit'])
+        self.cli_set(class_path + ['bandwidth', '50mbit'])
+        self.cli_set(class_path + ['set-dscp', 'CS4'])
+
+        # IPv4 match: pedit should target ipv4+0
+        self.cli_set(
+            class_path + ['match', 'RULE', 'ip', 'source', 'address', '172.17.1.2/32']
+        )
+        self.cli_commit()
+
+        # CS4 = DSCP 32 << 2 = 0x80
+        filter_output = get_tc_filter_details(interface)
+        self.assertIn('action order 1:  pedit', filter_output)
+        self.assertIn('at ipv4+0: val 00800000', filter_output)
+        self.assertIn('csum (iph)', filter_output)
+        self.assertNotIn('at ipv6+0', filter_output)
+
+        # IPv6 match: pedit should target ipv6+0
+        self.cli_delete(class_path + ['match', 'RULE'])
+        self.cli_set(
+            class_path + ['match', 'RULE', 'ipv6', 'source', 'address', '2001:db8::/32']
+        )
+        self.cli_set(class_path + ['set-dscp', 'AF21'])
+        self.cli_commit()
+
+        # AF21 = DSCP 18 << 2 = 0x48
+        filter_output = get_tc_filter_details(interface)
+        self.assertIn('action order 1:  pedit', filter_output)
+        self.assertIn('at ipv6+0: val 04800000', filter_output)
+        self.assertNotIn('at ipv4+0', filter_output)
+
+        # Ether match with protocol all: pedit is skipped to avoid
+        # corrupting non-IP packets
+        self.cli_delete(class_path + ['match', 'RULE'])
+        self.cli_set(class_path + ['match', 'RULE', 'ether', 'protocol', 'all'])
+        self.cli_set(class_path + ['set-dscp', '46'])
+        self.cli_commit()
+
+        filter_output = get_tc_filter_details(interface)
+        self.assertNotIn('pedit', filter_output)
+
+        # Ether match with protocol ip: pedit targets ipv4+0 only
+        self.cli_set(class_path + ['match', 'RULE', 'ether', 'protocol', 'ip'])
+        self.cli_commit()
+
+        # numeric 46 << 2 = 0xb8
+        filter_output = get_tc_filter_details(interface)
+        self.assertIn('action order 1:  pedit', filter_output)
+        self.assertIn('at ipv4+0: val 00b80000', filter_output)
+        self.assertIn('csum (iph)', filter_output)
+        self.assertNotIn('at ipv6+0', filter_output)
+
+        # Removing set-dscp: no pedit should remain
+        self.cli_delete(class_path + ['set-dscp'])
+        self.cli_commit()
+
+        filter_output = get_tc_filter_details(interface)
+        self.assertNotIn('pedit', filter_output)
+
+
 if __name__ == '__main__':
     unittest.main(verbosity=2, failfast=VyOSUnitTestSHIM.TestCase.debug_on())