From 113ed87c0aa9f6eebbc65545b336469b1ebcea84 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Thu, 17 Aug 2023 21:06:01 +0200 Subject: wireguard: T5409: rename threaded CLI not to per-client-thread Using threaded as CLI node is a very deep term used by kernel threads. To make this more understandable to users, rename the node to per-client-thread. It's also not necessary to test if any one peer is configured and probing if the option is set. There is a base test which requires at least one peer to be configured. --- .../include/interface/per-client-thread.xml.i | 8 +++++ interface-definitions/interfaces-wireguard.xml.in | 7 +--- python/vyos/ifconfig/interface.py | 39 +++++++++++++++++++++- python/vyos/ifconfig/wireguard.py | 8 ----- smoketest/scripts/cli/test_interfaces_wireguard.py | 11 ++---- src/conf_mode/interfaces-wireguard.py | 11 ++---- 6 files changed, 51 insertions(+), 33 deletions(-) create mode 100644 interface-definitions/include/interface/per-client-thread.xml.i diff --git a/interface-definitions/include/interface/per-client-thread.xml.i b/interface-definitions/include/interface/per-client-thread.xml.i new file mode 100644 index 000000000..2fd19b5ce --- /dev/null +++ b/interface-definitions/include/interface/per-client-thread.xml.i @@ -0,0 +1,8 @@ + + + + Process traffic from each client in a dedicated thread + + + + diff --git a/interface-definitions/interfaces-wireguard.xml.in b/interface-definitions/interfaces-wireguard.xml.in index 75db9f617..2e238a9bd 100644 --- a/interface-definitions/interfaces-wireguard.xml.in +++ b/interface-definitions/interfaces-wireguard.xml.in @@ -119,12 +119,7 @@ #include - - - Process traffic from each peer in a dedicated thread - - - + #include #include diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index 99ddb2021..efacad902 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -1,4 +1,4 @@ -# Copyright 2019-2022 VyOS maintainers and contributors +# Copyright 2019-2023 VyOS maintainers and contributors # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -219,6 +219,10 @@ class Interface(Control): 'validate': lambda link: assert_range(link,0,3), 'location': '/proc/sys/net/ipv4/conf/{ifname}/link_filter', }, + 'per_client_thread': { + 'validate': assert_boolean, + 'location': '/sys/class/net/{ifname}/threaded', + }, } _sysfs_get = { @@ -267,6 +271,10 @@ class Interface(Control): 'link_detect': { 'location': '/proc/sys/net/ipv4/conf/{ifname}/link_filter', }, + 'per_client_thread': { + 'validate': assert_boolean, + 'location': '/sys/class/net/{ifname}/threaded', + }, } @classmethod @@ -1357,6 +1365,30 @@ class Interface(Control): f'egress redirect dev {target_if}') if err: print('tc filter add for redirect failed') + def set_per_client_thread(self, enable): + """ + Per-device control to enable/disable the threaded mode for all the napi + instances of the given network device, without the need for a device up/down. + + User sets it to 1 or 0 to enable or disable threaded mode. + + Example: + >>> from vyos.ifconfig import Interface + >>> Interface('wg1').set_per_client_thread(1) + """ + # In the case of a "virtual" interface like wireguard, the sysfs + # node is only created once there is a peer configured. We can now + # add a verify() code-path for this or make this dynamic without + # nagging the user + tmp = self._sysfs_get['per_client_thread']['location'] + if not os.path.exists(tmp): + return None + + tmp = self.get_interface('per_client_thread') + if tmp == enable: + return None + self.set_interface('per_client_thread', enable) + def update(self, config): """ General helper function which works on a dictionary retrived by get_config_dict(). It's main intention is to consolidate the scattered @@ -1565,6 +1597,11 @@ class Interface(Control): # configure interface mirror or redirection target self.set_mirror_redirect() + # enable/disable NAPI threading mode + tmp = dict_search('per_client_thread', config) + value = '1' if (tmp != None) else '0' + self.set_per_client_thread(value) + # Enable/Disable of an interface must always be done at the end of the # derived class to make use of the ref-counting set_admin_state() # function. We will only enable the interface if 'up' was called as diff --git a/python/vyos/ifconfig/wireguard.py b/python/vyos/ifconfig/wireguard.py index 58613813f..4aac103ec 100644 --- a/python/vyos/ifconfig/wireguard.py +++ b/python/vyos/ifconfig/wireguard.py @@ -27,7 +27,6 @@ from vyos.ifconfig import Operational from vyos.template import is_ipv6 from vyos.base import Warning - class WireGuardOperational(Operational): def _dump(self): """Dump wireguard data in a python friendly way.""" @@ -230,12 +229,5 @@ class WireGuardIf(Interface): if psk_file != no_psk_file and os.path.exists(psk_file): os.remove(psk_file) - try: - self._write_sysfs(f'/sys/devices/virtual/net/{self.ifname}/threaded', - '1' if 'threaded' in config else '0') - except Exception: - Warning(f'Update threaded status on interface "{config["ifname"]}" FAILED.\n' - f'An unexpected error occurred.') - # call base class super().update(config) diff --git a/smoketest/scripts/cli/test_interfaces_wireguard.py b/smoketest/scripts/cli/test_interfaces_wireguard.py index f6f2499a6..48c7cb6a1 100755 --- a/smoketest/scripts/cli/test_interfaces_wireguard.py +++ b/smoketest/scripts/cli/test_interfaces_wireguard.py @@ -145,18 +145,11 @@ class WireGuardInterfaceTest(VyOSUnitTestSHIM.TestCase): self.cli_set(base_path + [interface, 'peer', 'PEER01', 'public-key', pubkey]) self.cli_set(base_path + [interface, 'peer', 'PEER01', 'allowed-ips', '10.205.212.10/32']) self.cli_set(base_path + [interface, 'peer', 'PEER01', 'address', '192.0.2.1']) - self.cli_set(base_path + [interface, 'peer', 'PEER01', 'disable']) - self.cli_set(base_path + [interface, 'threaded']) - - # Threaded is set and no enabled peer is configured - with self.assertRaises(ConfigSessionError): - self.cli_commit() - - self.cli_delete(base_path + [interface, 'peer', 'PEER01', 'disable']) + self.cli_set(base_path + [interface, 'per-client-thread']) # Commit peers self.cli_commit() - tmp = read_file(f'/sys/devices/virtual/net/{interface}/threaded') + tmp = read_file(f'/sys/class/net/{interface}/threaded') self.assertTrue(tmp, "1") if __name__ == '__main__': diff --git a/src/conf_mode/interfaces-wireguard.py b/src/conf_mode/interfaces-wireguard.py index ef0fdae15..c0f3f4d6e 100755 --- a/src/conf_mode/interfaces-wireguard.py +++ b/src/conf_mode/interfaces-wireguard.py @@ -90,7 +90,6 @@ def verify(wireguard): # run checks on individual configured WireGuard peer public_keys = [] - peer_enabled = False for tmp in wireguard['peer']: peer = wireguard['peer'][tmp] @@ -107,18 +106,12 @@ def verify(wireguard): if peer['public_key'] in public_keys: raise ConfigError(f'Duplicate public-key defined on peer "{tmp}"') - if 'disable' not in peer and is_wireguard_key_pair(wireguard['private_key'], peer['public_key']): - raise ConfigError(f'Peer "{tmp}" has the same public key as the interface "{wireguard["ifname"]}"') - if 'disable' not in peer: - peer_enabled = True + if is_wireguard_key_pair(wireguard['private_key'], peer['public_key']): + raise ConfigError(f'Peer "{tmp}" has the same public key as the interface "{wireguard["ifname"]}"') public_keys.append(peer['public_key']) - #Threaded can be enabled only if one enabled peer exists. - if not peer_enabled and 'threaded' in wireguard: - raise ConfigError(f'Set threaded on interface "{wireguard["ifname"]}" FAILED.\nNo enabled peers are configured') - def apply(wireguard): tmp = WireGuardIf(wireguard['ifname']) if 'deleted' in wireguard: -- cgit v1.2.3