From 6eafdbf1a8e195c14607447492d29f6209aa87c0 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sat, 7 Mar 2026 22:25:08 +0100 Subject: configdict: T8358: rename internal representation from traffic_policy to qos Commit 0bf386cee9b0 ('qos: T4284: rename "traffic-policy" node to "qos policy"') already renamed the CLI for QoS. The internal data representation kept the old name which is now corrected. --- python/vyos/configdict.py | 8 ++++---- python/vyos/configverify.py | 4 ++-- python/vyos/ifconfig/interface.py | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) (limited to 'python') diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py index e1d05c384..b16c3cf1d 100644 --- a/python/vyos/configdict.py +++ b/python/vyos/configdict.py @@ -472,7 +472,7 @@ def get_interface_dict(config, base, ifname='', recursive_defaults=True, with_pk # Check if QoS policy applied on this interface - See ifconfig.interface.set_mirror_redirect() if config.exists(['qos', 'interface', ifname]): - dict.update({'traffic_policy': {}}) + dict.update({'qos': {}}) address = leaf_node_changed(config, base + [ifname, 'address']) if address: dict.update({'address_old' : address}) @@ -531,7 +531,7 @@ def get_interface_dict(config, base, ifname='', recursive_defaults=True, with_pk dict['vif'][vif].update({'ifname' : f'{ifname}.{vif}'}) if config.exists(['qos', 'interface', f'{ifname}.{vif}']): - dict['vif'][vif].update({'traffic_policy': {}}) + dict['vif'][vif].update({'qos': {}}) if 'deleted' not in dict: address = leaf_node_changed(config, base + [ifname, 'vif', vif, 'address']) @@ -558,7 +558,7 @@ def get_interface_dict(config, base, ifname='', recursive_defaults=True, with_pk dict['vif_s'][vif_s].update({'ifname' : f'{ifname}.{vif_s}'}) if config.exists(['qos', 'interface', f'{ifname}.{vif_s}']): - dict['vif_s'][vif_s].update({'traffic_policy': {}}) + dict['vif_s'][vif_s].update({'qos': {}}) if 'deleted' not in dict: address = leaf_node_changed(config, base + [ifname, 'vif-s', vif_s, 'address']) @@ -586,7 +586,7 @@ def get_interface_dict(config, base, ifname='', recursive_defaults=True, with_pk dict['vif_s'][vif_s]['vif_c'][vif_c].update({'ifname' : f'{ifname}.{vif_s}.{vif_c}'}) if config.exists(['qos', 'interface', f'{ifname}.{vif_s}.{vif_c}']): - dict['vif_s'][vif_s]['vif_c'][vif_c].update({'traffic_policy': {}}) + dict['vif_s'][vif_s]['vif_c'][vif_c].update({'qos': {}}) if 'deleted' not in dict: address = leaf_node_changed(config, base + [ifname, 'vif-s', vif_s, 'vif-c', vif_c, 'address']) diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py index cc4419913..1094288c5 100644 --- a/python/vyos/configverify.py +++ b/python/vyos/configverify.py @@ -197,10 +197,10 @@ def verify_mirror_redirect(config): raise ConfigError(f'Requested redirect interface "{redirect_ifname}" '\ 'does not exist!') - if ('mirror' in config or 'redirect' in config) and dict_search('traffic_policy.in', config) is not None: + if 'qos' in config and ('mirror' in config or 'redirect' in config): # XXX: support combination of limiting and redirect/mirror - this is an # artificial limitation - raise ConfigError('Can not use ingress policy together with mirror or redirect!') + raise ConfigError('Can not use QoS together with mirror/redirect!') def verify_authentication(config): """ diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index f03d0cb68..33b643e7e 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -1630,7 +1630,7 @@ class Interface(Control): # clear existing ingess - ignore errors (e.g. "Error: Cannot find specified # qdisc on specified device") - we simply cleanup all stuff here - if not 'traffic_policy' in self.config: + if not 'qos' in self.config: self._popen(f'tc qdisc del dev {source_if} parent ffff: 2>/dev/null'); self._popen(f'tc qdisc del dev {source_if} parent 1: 2>/dev/null'); -- cgit v1.2.3 From 87465bd3709aa963d60d593a17f76d4bbd024183 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sat, 7 Mar 2026 22:30:40 +0100 Subject: vyos.utils: T8358: remove duplicated imports for cmd() and loads() Multiple helper functions imported json.loads() or vyos.utils.process.cmd(), this has been unified in one place. --- python/vyos/utils/network.py | 42 +++++++----------------------------------- 1 file changed, 7 insertions(+), 35 deletions(-) (limited to 'python') diff --git a/python/vyos/utils/network.py b/python/vyos/utils/network.py index 54dc0b2d9..217428a21 100644 --- a/python/vyos/utils/network.py +++ b/python/vyos/utils/network.py @@ -14,6 +14,8 @@ # License along with this library. If not, see . import hashlib + +from json import loads from socket import AF_INET from socket import AF_INET6 from vyos.utils.process import cmd @@ -107,8 +109,6 @@ def gen_mac(name: str, addr: str, ident: str) -> str: return ":".join(f"{x:02x}" for x in b) def get_netns_all() -> list: - from json import loads - from vyos.utils.process import cmd tmp = loads(cmd('ip --json netns ls')) return [ netns['name'] for netns in tmp ] @@ -118,14 +118,12 @@ def get_vrf_members(vrf: str) -> list: :param vrf: str :return: list """ - import json - from vyos.utils.process import cmd interfaces = [] try: if not interface_exists(vrf): raise ValueError(f'VRF "{vrf}" does not exist!') output = cmd(f'ip --json --brief link show vrf {vrf}') - answer = json.loads(output) + answer = loads(output) for data in answer: if 'ifname' in data: # Skip PIM interfaces which appears in VRF @@ -166,8 +164,6 @@ def get_interface_config(interface): """ if not interface_exists(interface): return None - from json import loads - from vyos.utils.process import cmd tmp = loads(cmd(f'ip --detail --json link show dev {interface}'))[0] return tmp @@ -177,8 +173,6 @@ def get_interface_address(interface): """ if not interface_exists(interface): return None - from json import loads - from vyos.utils.process import cmd tmp = loads(cmd(f'ip --detail --json addr show dev {interface}'))[0] return tmp @@ -186,9 +180,6 @@ def get_interface_namespace(interface: str): """ Returns wich netns the interface belongs to """ - from json import loads - from vyos.utils.process import cmd - # Bail out early if netns does not exist tmp = cmd(f'ip --json netns ls') if not tmp: return None @@ -216,14 +207,13 @@ def is_ipv6_tentative(iface: str, ipv6_address: str) -> bool: Returns: bool: True if the IPv6 address is tentative, False otherwise. """ - import json from vyos.utils.process import rc_cmd rc, out = rc_cmd(f'ip -6 --json address show dev {iface}') if rc: return False - data = json.loads(out) + data = loads(out) for addr_info in data[0]['addr_info']: if ( addr_info.get('local') == ipv6_address and @@ -235,9 +225,7 @@ def is_ipv6_tentative(iface: str, ipv6_address: str) -> bool: def is_wwan_connected(interface): """ Determine if a given WWAN interface, e.g. wwan0 is connected to the carrier network or not """ - import json from vyos.utils.dict import dict_search - from vyos.utils.process import cmd from vyos.utils.process import is_systemd_service_active if not interface.startswith('wwan'): @@ -251,7 +239,7 @@ def is_wwan_connected(interface): modem = interface.lstrip('wwan') tmp = cmd(f'mmcli --modem {modem} --output-json') - tmp = json.loads(tmp) + tmp = loads(tmp) # return True/False if interface is in connected state return dict_search('modem.generic.state', tmp) == 'connected' @@ -260,15 +248,11 @@ def get_bridge_fdb(interface): """ Returns the forwarding database entries for a given interface """ if not interface_exists(interface): return None - from json import loads - from vyos.utils.process import cmd tmp = loads(cmd(f'bridge -j fdb show dev {interface}')) return tmp def get_all_vrfs(): """ Return a dictionary of all system wide known VRF instances """ - from json import loads - from vyos.utils.process import cmd tmp = loads(cmd('ip --json vrf list')) # Result is of type [{"name":"red","table":1000},{"name":"blue","table":2000}] # so we will re-arrange it to a more nicer representation: @@ -287,7 +271,6 @@ def interface_list() -> list: """ return Section.interfaces() - def vrf_list() -> list: """ Get list of VRFs in system @@ -432,7 +415,6 @@ def is_intf_addr_assigned(ifname: str, addr: str, netns: str=None) -> bool: It can check both a single IP address (e.g. 192.0.2.1 or a assigned CIDR address 192.0.2.1/24. """ - import json import jmespath from vyos.utils.process import rc_cmd @@ -441,7 +423,7 @@ def is_intf_addr_assigned(ifname: str, addr: str, netns: str=None) -> bool: netns_cmd = f'ip netns exec {netns}' if netns else '' rc, out = rc_cmd(f'{netns_cmd} ip --json address show dev {ifname}') if rc == 0: - json_out = json.loads(out) + json_out = loads(out) addresses = jmespath.search("[].addr_info[].{family: family, address: local, prefixlen: prefixlen}", json_out) for address_info in addresses: family = address_info['family'] @@ -471,7 +453,6 @@ def is_wireguard_key_pair(private_key: str, public_key:str) -> bool: :return: If public/private keys are keypair returns True else False :rtype: bool """ - from vyos.utils.process import cmd gen_public_key = cmd('wg pubkey', input=private_key) if gen_public_key == public_key: return True @@ -488,8 +469,6 @@ def get_wireguard_peers(ifname: str) -> list: """ if not interface_exists(ifname): return [] - - from vyos.utils.process import cmd peers = cmd(f'wg show {ifname} peers') return peers.splitlines() @@ -557,9 +536,6 @@ def is_afi_configured(interface: str, afi): def get_vxlan_vlan_tunnels(interface: str) -> list: """ Return a list of strings with VLAN IDs configured in the Kernel """ - from json import loads - from vyos.utils.process import cmd - if not interface.startswith('vxlan'): raise ValueError('Only applicable for VXLAN interfaces!') @@ -600,9 +576,6 @@ def get_vxlan_vlan_tunnels(interface: str) -> list: def get_vxlan_vni_filter(interface: str) -> list: """ Return a list of strings with VNIs configured in the Kernel""" - from json import loads - from vyos.utils.process import cmd - if not interface.startswith('vxlan'): raise ValueError('Only applicable for VXLAN interfaces!') @@ -673,9 +646,8 @@ def get_nft_vrf_zone_mapping() -> dict: {'interface': 'eth2', 'vrf_tableid': 1000}, {'interface': 'blue', 'vrf_tableid': 2000}] """ - from json import loads from jmespath import search - from vyos.utils.process import cmd + output = [] tmp = loads(cmd('sudo nft -j list table inet vrf_zones')) # {'nftables': [{'metainfo': {'json_schema_version': 1, -- cgit v1.2.3 From abc7060ddcca0315682427a1e38faed8cfa01796 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sat, 7 Mar 2026 22:33:02 +0100 Subject: vyos.ifconfig: T8358: clear qdiscs when deleting mirror CLI node When removing the mirror CLI node to stop mirroring or redirecting traffic to another interface, the egress configuration was not cleared. This caused traffic to continue being sent out the SPAN port even after the node was removed. Fix by properly clearing all tc(8) qdiscs. Update smoketests to verify nothing remains after mirror deletion. --- python/vyos/ifconfig/interface.py | 12 ++++---- smoketest/scripts/cli/base_interfaces_test.py | 37 +++++++++++++++++------ smoketest/scripts/cli/test_interfaces_bonding.py | 1 - smoketest/scripts/cli/test_interfaces_bridge.py | 1 - smoketest/scripts/cli/test_interfaces_dummy.py | 1 + smoketest/scripts/cli/test_interfaces_ethernet.py | 1 - 6 files changed, 34 insertions(+), 19 deletions(-) (limited to 'python') diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index 33b643e7e..a416de1ab 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -1617,7 +1617,7 @@ class Interface(Control): if 'netns' in self.config: return None - source_if = self.config['ifname'] + source_if = self.ifname mirror_config = None if 'mirror' in self.config: @@ -1631,8 +1631,8 @@ class Interface(Control): # clear existing ingess - ignore errors (e.g. "Error: Cannot find specified # qdisc on specified device") - we simply cleanup all stuff here if not 'qos' in self.config: - self._popen(f'tc qdisc del dev {source_if} parent ffff: 2>/dev/null'); - self._popen(f'tc qdisc del dev {source_if} parent 1: 2>/dev/null'); + self._popen(f'tc qdisc del dev {source_if} root 2>/dev/null') + self._popen(f'tc qdisc del dev {source_if} ingress 2>/dev/null') # Apply interface mirror policy if mirror_config: @@ -1644,14 +1644,14 @@ class Interface(Control): handle = '1: root prio' parent = '1:' - # Mirror egress traffic + # Mirror traffic mirror_cmd = f'tc qdisc add dev {source_if} handle {handle}; ' # Export the mirrored traffic to the interface mirror_cmd += f'tc filter add dev {source_if} parent {parent} protocol '\ f'all prio 10 u32 match u32 0 0 flowid 1:1 action mirred '\ f'egress mirror dev {target_if}' _, err = self._popen(mirror_cmd) - if err: print('tc qdisc(filter for mirror port failed') + if err: print('tc filter for mirror port failed') # Apply interface traffic redirection policy elif 'redirect' in self.config: @@ -1662,7 +1662,7 @@ class Interface(Control): _, err = self._popen(f'tc filter add dev {source_if} parent ffff: protocol '\ f'all prio 10 u32 match u32 0 0 flowid 1:1 action mirred '\ f'egress redirect dev {target_if}') - if err: print('tc filter add for redirect failed') + if err: print('tc filter for redirect failed') def set_per_client_thread(self, enable): """ diff --git a/smoketest/scripts/cli/base_interfaces_test.py b/smoketest/scripts/cli/base_interfaces_test.py index 7e9c5b6a4..b2f700fce 100644 --- a/smoketest/scripts/cli/base_interfaces_test.py +++ b/smoketest/scripts/cli/base_interfaces_test.py @@ -13,6 +13,7 @@ # along with this program. If not, see . import re +import jmespath from json import loads from netifaces import ifaddresses # pylint: disable = no-name-in-module @@ -121,9 +122,9 @@ def get_certificate_count(interface, cert_type): tmp = read_file(f'/run/wpa_supplicant/{interface}_{cert_type}.pem') return tmp.count(CERT_BEGIN) -def is_mirrored_to(interface, mirror_if, qdisc): +def is_mirrored_to(interface, mirror_if, qdisc) -> bool: """ - Ask TC if we are mirroring traffic to a discrete interface. + Ask tc(8) if we are mirroring traffic to a specific interface. interface: source interface mirror_if: destination where we mirror our data to @@ -132,12 +133,11 @@ def is_mirrored_to(interface, mirror_if, qdisc): if qdisc not in ['ffff', '1']: raise ValueError() - ret_val = False - tmp = cmd(f'tc -s -p filter ls dev {interface} parent {qdisc}: | grep mirred') - tmp = tmp.lower() - if mirror_if in tmp: - ret_val = True - return ret_val + tmp = loads(cmd(f'tc -json filter ls dev {interface} parent {qdisc}:')) + # the following syntax looks odd but we need to filter out the first + # result sets from tc which do not have "options.actions...". + tmp = jmespath.search("[?options.actions[0].kind=='mirred'].options.actions[0].{mirred_action: mirred_action, to_dev: to_dev} | [0]", tmp) + return bool(dict_search('mirred_action', tmp) == 'mirror' and dict_search('to_dev', tmp) == mirror_if) class BasicInterfaceTest: class TestCase(VyOSUnitTestSHIM.TestCase): @@ -161,7 +161,7 @@ class BasicInterfaceTest: _test_addr = ['192.0.2.1/26', '192.0.2.255/31', '192.0.2.64/32', '2001:db8:1::ffff/64', '2001:db8:101::1/112'] - _mirror_interfaces = [] + _mirror_interfaces = ['dum21354'] # choose IPv6 minimum MTU value for tests - this must always work _mtu = '1280' @@ -181,6 +181,7 @@ class BasicInterfaceTest: cls._test_ipv6_pd = cli_defined(cls._base_path + ['dhcpv6-options'], 'pd') cls._test_mtu = cli_defined(cls._base_path, 'mtu') cls._test_vrf = cli_defined(cls._base_path, 'vrf') + cls._test_mirror = cli_defined(cls._base_path, 'mirror') # Setup mirror interfaces for SPAN (Switch Port Analyzer) for span in cls._mirror_interfaces: @@ -456,9 +457,13 @@ class BasicInterfaceTest: self.cli_set(self._base_path + [interface, 'description', 'test_add_to_invalid_vrf']) def test_span_mirror(self): - if not self._mirror_interfaces: + if not self._test_mirror: self.skipTest(MSG_TESTCASE_UNSUPPORTED) + for interface in self._interfaces: + for option in self._options.get(interface, []): + self.cli_set(self._base_path + [interface] + option.split()) + # Check the two-way mirror rules of ingress and egress for mirror in self._mirror_interfaces: for interface in self._interfaces: @@ -473,6 +478,18 @@ class BasicInterfaceTest: self.assertTrue(is_mirrored_to(interface, mirror, 'ffff')) self.assertTrue(is_mirrored_to(interface, mirror, '1')) + # delete interface mirror - check that configuration from tc is removed + for mirror in self._mirror_interfaces: + for interface in self._interfaces: + self.cli_delete(self._base_path + [interface, 'mirror']) + self.cli_commit() + + # Verify config + for mirror in self._mirror_interfaces: + for interface in self._interfaces: + self.assertFalse(is_mirrored_to(interface, mirror, 'ffff')) + self.assertFalse(is_mirrored_to(interface, mirror, '1')) + def test_interface_disable(self): # Check if description can be added to interface and # can be read back diff --git a/smoketest/scripts/cli/test_interfaces_bonding.py b/smoketest/scripts/cli/test_interfaces_bonding.py index a565168cf..a4afae5da 100755 --- a/smoketest/scripts/cli/test_interfaces_bonding.py +++ b/smoketest/scripts/cli/test_interfaces_bonding.py @@ -30,7 +30,6 @@ class BondingInterfaceTest(BasicInterfaceTest.TestCase): @classmethod def setUpClass(cls): cls._base_path = ['interfaces', 'bonding'] - cls._mirror_interfaces = ['dum21354'] cls._members = [] # we need to filter out VLAN interfaces identified by a dot (.) diff --git a/smoketest/scripts/cli/test_interfaces_bridge.py b/smoketest/scripts/cli/test_interfaces_bridge.py index 1a67a9165..a41ad1482 100755 --- a/smoketest/scripts/cli/test_interfaces_bridge.py +++ b/smoketest/scripts/cli/test_interfaces_bridge.py @@ -35,7 +35,6 @@ class BridgeInterfaceTest(BasicInterfaceTest.TestCase): @classmethod def setUpClass(cls): cls._base_path = ['interfaces', 'bridge'] - cls._mirror_interfaces = ['dum21354'] cls._members = [] # we need to filter out VLAN interfaces identified by a dot (.) diff --git a/smoketest/scripts/cli/test_interfaces_dummy.py b/smoketest/scripts/cli/test_interfaces_dummy.py index b19ab02dd..c011a411b 100755 --- a/smoketest/scripts/cli/test_interfaces_dummy.py +++ b/smoketest/scripts/cli/test_interfaces_dummy.py @@ -24,6 +24,7 @@ class DummyInterfaceTest(BasicInterfaceTest.TestCase): def setUpClass(cls): cls._base_path = ['interfaces', 'dummy'] cls._interfaces = ['dum435', 'dum8677', 'dum0931', 'dum089'] + cls._mirror_interfaces = ['eth0'] # call base-classes classmethod super(DummyInterfaceTest, cls).setUpClass() diff --git a/smoketest/scripts/cli/test_interfaces_ethernet.py b/smoketest/scripts/cli/test_interfaces_ethernet.py index 7c8106521..7e0da2079 100755 --- a/smoketest/scripts/cli/test_interfaces_ethernet.py +++ b/smoketest/scripts/cli/test_interfaces_ethernet.py @@ -43,7 +43,6 @@ class EthernetInterfaceTest(BasicInterfaceTest.TestCase): @classmethod def setUpClass(cls): cls._base_path = ['interfaces', 'ethernet'] - cls._mirror_interfaces = ['dum21354'] # We only test on physical interfaces and not VLAN (sub-)interfaces if 'TEST_ETH' in os.environ: -- cgit v1.2.3