From 031817eecb14280e3f421cb9c391ab29dbc2fa60 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Tue, 31 Aug 2021 11:58:46 +0200 Subject: ethernet: T3514: bail out early on invalid adapter speed/duplex setting Ethernet adapters have a discrete set of available speed and duplex settings. Instead of passing every value down to ethtool and let it decide, we can do this early in the VyOS verify() function for ethernet interfaces. (cherry picked from commit 91892e431349ca0edb5e3e3023e4f340ab9b777f) --- src/conf_mode/interfaces-ethernet.py | 48 +++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 20 deletions(-) (limited to 'src/conf_mode/interfaces-ethernet.py') diff --git a/src/conf_mode/interfaces-ethernet.py b/src/conf_mode/interfaces-ethernet.py index 349b0e7a3..a7e01e279 100755 --- a/src/conf_mode/interfaces-ethernet.py +++ b/src/conf_mode/interfaces-ethernet.py @@ -63,32 +63,20 @@ def verify(ethernet): ifname = ethernet['ifname'] verify_interface_exists(ifname) + ethtool = Ethtool(ifname) # No need to check speed and duplex keys as both have default values. if ((ethernet['speed'] == 'auto' and ethernet['duplex'] != 'auto') or (ethernet['speed'] != 'auto' and ethernet['duplex'] == 'auto')): raise ConfigError('Speed/Duplex missmatch. Must be both auto or manually configured') - verify_mtu(ethernet) - verify_mtu_ipv6(ethernet) - verify_dhcpv6(ethernet) - verify_address(ethernet) - verify_vrf(ethernet) - verify_eapol(ethernet) - verify_mirror(ethernet) + if ethernet['speed'] != 'auto' and ethernet['duplex'] != 'auto': + # We need to verify if the requested speed and duplex setting is + # supported by the underlaying NIC. + speed = ethernet['speed'] + duplex = ethernet['duplex'] + if not ethtool.check_speed_duplex(speed, duplex): + raise ConfigError(f'Adapter does not support speed "{speed}" and duplex "{duplex}"!') - # verify offloading capabilities - if dict_search('offload.rps', ethernet) != None: - if not os.path.exists(f'/sys/class/net/{ifname}/queues/rx-0/rps_cpus'): - raise ConfigError('Interface does not suport RPS!') - - driver = EthernetIf(ifname).get_driver_name() - # T3342 - Xen driver requires special treatment - if driver == 'vif': - if int(ethernet['mtu']) > 1500 and dict_search('offload.sg', ethernet) == None: - raise ConfigError('Xen netback drivers requires scatter-gatter offloading '\ - 'for MTU size larger then 1500 bytes') - - ethtool = Ethtool(ifname) if 'ring_buffer' in ethernet: max_rx = ethtool.get_rx_buffer() if not max_rx: @@ -108,6 +96,26 @@ def verify(ethernet): raise ConfigError(f'Driver only supports a maximum TX ring-buffer '\ f'size of "{max_tx}" bytes!') + verify_mtu(ethernet) + verify_mtu_ipv6(ethernet) + verify_dhcpv6(ethernet) + verify_address(ethernet) + verify_vrf(ethernet) + verify_eapol(ethernet) + verify_mirror(ethernet) + + # verify offloading capabilities + if dict_search('offload.rps', ethernet) != None: + if not os.path.exists(f'/sys/class/net/{ifname}/queues/rx-0/rps_cpus'): + raise ConfigError('Interface does not suport RPS!') + + driver = EthernetIf(ifname).get_driver_name() + # T3342 - Xen driver requires special treatment + if driver == 'vif': + if int(ethernet['mtu']) > 1500 and dict_search('offload.sg', ethernet) == None: + raise ConfigError('Xen netback drivers requires scatter-gatter offloading '\ + 'for MTU size larger then 1500 bytes') + if {'is_bond_member', 'mac'} <= set(ethernet): print(f'WARNING: changing mac address "{mac}" will be ignored as "{ifname}" ' f'is a member of bond "{is_bond_member}"'.format(**ethernet)) -- cgit v1.2.3 From 2bfd809e9ae198d95b9fcb556440637fdcc4005c Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Tue, 31 Aug 2021 18:15:47 +0200 Subject: ethernet: T2241: check if interface supports changing speed/duplex settings Not all interface drivers have the ability to change the speed and duplex settings. Known drivers with this limitation are vmxnet3, virtio_net and xen_netfront. If this driver is detected, an error will be presented to the user. (cherry picked from commit cc742d48579e4f76e5d3230d87e22f71f76f9301) --- python/vyos/ethtool.py | 15 +++++++++++++++ src/conf_mode/interfaces-ethernet.py | 3 ++- src/migration-scripts/interfaces/20-to-21 | 15 +++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) (limited to 'src/conf_mode/interfaces-ethernet.py') diff --git a/python/vyos/ethtool.py b/python/vyos/ethtool.py index 55b7b776f..fb2e49c1d 100644 --- a/python/vyos/ethtool.py +++ b/python/vyos/ethtool.py @@ -13,7 +13,9 @@ # You should have received a copy of the GNU Lesser General Public # License along with this library. If not, see . +import os import re + from vyos.util import popen class Ethtool: @@ -41,8 +43,18 @@ class Ethtool: # } _speed_duplex = { } _ring_buffers = { } + _driver_name = None def __init__(self, ifname): + # Get driver used for interface + sysfs_file = f'/sys/class/net/{ifname}/device/driver/module' + if os.path.exists(sysfs_file): + link = os.readlink(sysfs_file) + self._driver_name = os.path.basename(link) + + if not self._driver_name: + raise ValueError(f'Could not determine driver for interface {ifname}!') + # Build a dictinary of supported link-speed and dupley settings. out, err = popen(f'ethtool {ifname}') reading = False @@ -142,6 +154,9 @@ class Ethtool: if duplex not in ['full', 'half']: raise ValueError(f'Value "{duplex}" for duplex is invalid!') + if self._driver_name in ['vmxnet3', 'virtio_net', 'xen_netfront']: + return False + if speed in self._speed_duplex: if duplex in self._speed_duplex[speed]: return True diff --git a/src/conf_mode/interfaces-ethernet.py b/src/conf_mode/interfaces-ethernet.py index a7e01e279..57e05d4ea 100755 --- a/src/conf_mode/interfaces-ethernet.py +++ b/src/conf_mode/interfaces-ethernet.py @@ -75,7 +75,8 @@ def verify(ethernet): speed = ethernet['speed'] duplex = ethernet['duplex'] if not ethtool.check_speed_duplex(speed, duplex): - raise ConfigError(f'Adapter does not support speed "{speed}" and duplex "{duplex}"!') + raise ConfigError(f'Adapter does not support changing speed and duplex '\ + f'settings to: {speed}/{duplex}!') if 'ring_buffer' in ethernet: max_rx = ethtool.get_rx_buffer() diff --git a/src/migration-scripts/interfaces/20-to-21 b/src/migration-scripts/interfaces/20-to-21 index 4b0e70d35..bd89dcdb4 100755 --- a/src/migration-scripts/interfaces/20-to-21 +++ b/src/migration-scripts/interfaces/20-to-21 @@ -89,6 +89,21 @@ for ifname in config.list_nodes(base): if config.exists(base + [ifname, 'offload', 'ufo']): config.delete(base + [ifname, 'offload', 'ufo']) + # Also while processing the interface configuration, not all adapters support + # changing the speed and duplex settings. If the desired speed and duplex + # values do not work for the NIC driver, we change them back to the default + # value of "auto" - which will be applied if the CLI node is deleted. + speed_path = base + [ifname, 'speed'] + duplex_path = base + [ifname, 'duplex'] + # speed and duplex must always be set at the same time if not set to "auto" + if config.exists(speed_path) and config.exists(duplex_path): + speed = config.return_value(speed_path) + duplex = config.return_value(duplex_path) + if speed != 'auto' and duplex != 'auto': + if not eth.check_speed_duplex(speed, duplex): + config.delete(speed_path) + config.delete(duplex_path) + try: with open(file_name, 'w') as f: f.write(config.to_string()) -- cgit v1.2.3 From de3c476ccb60d66a844f9e12a9ca2963ae2206e6 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Tue, 31 Aug 2021 21:50:05 +0200 Subject: ethernet: T3163: only change ring-buffer settings if required Only update the RX/TX ring-buffer settings if they are different from the ones currently programmed to the hardware. There is no need to write the same value to the hardware again - this could cause traffic disruption on some NICs. (cherry picked from commit 29082959e0efc02462fba8560d6726096e8743e9) --- python/vyos/ethtool.py | 29 ++++++++++++++++++++++------- python/vyos/ifconfig/ethernet.py | 15 ++++++++++----- src/conf_mode/interfaces-ethernet.py | 4 ++-- 3 files changed, 34 insertions(+), 14 deletions(-) (limited to 'src/conf_mode/interfaces-ethernet.py') diff --git a/python/vyos/ethtool.py b/python/vyos/ethtool.py index e803e28a1..f5796358d 100644 --- a/python/vyos/ethtool.py +++ b/python/vyos/ethtool.py @@ -43,6 +43,7 @@ class Ethtool: # } _speed_duplex = { } _ring_buffers = { } + _ring_buffers_max = { } _driver_name = None _auto_negotiation = None @@ -99,10 +100,20 @@ class Ethtool: 'fixed' : fixed } - out, err = popen(f'ethtool -g {ifname}') + out, err = popen(f'ethtool --show-ring {ifname}') # We are only interested in line 2-5 which contains the device maximum # ringbuffers for line in out.splitlines()[2:6]: + if ':' in line: + key, value = [s.strip() for s in line.strip().split(":", 1)] + key = key.lower().replace(' ', '_') + # T3645: ethtool version used on Debian Bullseye changed the + # output format from 0 -> n/a. As we are only interested in the + # tx/rx keys we do not care about RX Mini/Jumbo. + if value.isdigit(): + self._ring_buffers_max[key] = int(value) + # Now we wan't to get the current RX/TX ringbuffer values - used for + for line in out.splitlines()[7:11]: if ':' in line: key, value = [s.strip() for s in line.strip().split(":", 1)] key = key.lower().replace(' ', '_') @@ -143,15 +154,19 @@ class Ethtool: def get_tcp_segmentation_offload(self): return self._get_generic('tcp-segmentation-offload') - def get_rx_buffer(self): - # Configuration of RX ring-buffers is not supported on every device, + def get_ring_buffer_max(self, rx_tx): + # Configuration of RX/TX ring-buffers is not supported on every device, # thus when it's impossible return None - return self._ring_buffers.get('rx', None) + if rx_tx not in ['rx', 'tx']: + ValueError('Ring-buffer type must be either "rx" or "tx"') + return self._ring_buffers_max.get(rx_tx, None) - def get_tx_buffer(self): - # Configuration of TX ring-buffers is not supported on every device, + def get_ring_buffer(self, rx_tx): + # Configuration of RX/TX ring-buffers is not supported on every device, # thus when it's impossible return None - return self._ring_buffers.get('tx', None) + if rx_tx not in ['rx', 'tx']: + ValueError('Ring-buffer type must be either "rx" or "tx"') + return self._ring_buffers.get(rx_tx, None) def check_speed_duplex(self, speed, duplex): """ Check if the passed speed and duplex combination is supported by diff --git a/python/vyos/ifconfig/ethernet.py b/python/vyos/ifconfig/ethernet.py index d6e42db99..5974a3d8f 100644 --- a/python/vyos/ifconfig/ethernet.py +++ b/python/vyos/ifconfig/ethernet.py @@ -317,21 +317,26 @@ class EthernetIf(Interface): print('Adapter does not support changing tcp-segmentation-offload settings!') return False - def set_ring_buffer(self, b_type, b_size): + def set_ring_buffer(self, rx_tx, size): """ Example: >>> from vyos.ifconfig import EthernetIf >>> i = EthernetIf('eth0') >>> i.set_ring_buffer('rx', '4096') """ + current_size = self.ethtool.get_ring_buffer(rx_tx) + if current_size == size: + # bail out early if nothing is about to change + return None + ifname = self.config['ifname'] - cmd = f'ethtool -G {ifname} {b_type} {b_size}' + cmd = f'ethtool --set-ring {ifname} {rx_tx} {size}' output, code = self._popen(cmd) # ethtool error codes: # 80 - value already setted # 81 - does not possible to set value if code and code != 80: - print(f'could not set "{b_type}" ring-buffer for {ifname}') + print(f'could not set "{rx_tx}" ring-buffer for {ifname}') return output def update(self, config): @@ -370,8 +375,8 @@ class EthernetIf(Interface): # Set interface ring buffer if 'ring_buffer' in config: - for b_type in config['ring_buffer']: - self.set_ring_buffer(b_type, config['ring_buffer'][b_type]) + for rx_tx, size in config['ring_buffer'].items(): + self.set_ring_buffer(rx_tx, size) # call base class first super().update(config) diff --git a/src/conf_mode/interfaces-ethernet.py b/src/conf_mode/interfaces-ethernet.py index 57e05d4ea..f3f3fede8 100755 --- a/src/conf_mode/interfaces-ethernet.py +++ b/src/conf_mode/interfaces-ethernet.py @@ -79,11 +79,11 @@ def verify(ethernet): f'settings to: {speed}/{duplex}!') if 'ring_buffer' in ethernet: - max_rx = ethtool.get_rx_buffer() + max_rx = ethtool.get_ring_buffer_max('rx') if not max_rx: raise ConfigError('Driver does not support RX ring-buffer configuration!') - max_tx = ethtool.get_tx_buffer() + max_tx = ethtool.get_ring_buffer_max('tx') if not max_tx: raise ConfigError('Driver does not support TX ring-buffer configuration!') -- cgit v1.2.3 From b0d4112bd6073e4a947869c3bd80f8e87783fbfa Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Tue, 31 Aug 2021 23:03:01 +0200 Subject: vyos.ethtool: T3163: purify code to read and change flow-control settings It makes no sense to have a parser for the ethtool values in ethtool.py and ethernet.py - one instance ios more then enough! (cherry picked from commit 0229645c8248decb5664056df8aa5cd5dff41802) --- python/vyos/ethtool.py | 23 +++++++++++++++++ python/vyos/ifconfig/ethernet.py | 42 ++++++++----------------------- src/conf_mode/interfaces-ethernet.py | 4 +++ src/migration-scripts/interfaces/20-to-21 | 8 ++++++ 4 files changed, 45 insertions(+), 32 deletions(-) (limited to 'src/conf_mode/interfaces-ethernet.py') diff --git a/python/vyos/ethtool.py b/python/vyos/ethtool.py index f5796358d..87b9d7dd0 100644 --- a/python/vyos/ethtool.py +++ b/python/vyos/ethtool.py @@ -46,6 +46,8 @@ class Ethtool: _ring_buffers_max = { } _driver_name = None _auto_negotiation = None + _flow_control = None + _flow_control_enabled = None def __init__(self, ifname): # Get driver used for interface @@ -123,6 +125,15 @@ class Ethtool: if value.isdigit(): self._ring_buffers[key] = int(value) + # Get current flow control settings, but this is not supported by + # all NICs (e.g. vmxnet3 does not support is) + out, err = popen(f'ethtool --show-pause {ifname}') + if len(out.splitlines()) > 1: + self._flow_control = True + # read current flow control setting, this returns: + # ['Autonegotiate:', 'on'] + self._flow_control_enabled = out.splitlines()[1].split()[-1] + def _get_generic(self, feature): """ Generic method to read self._features and return a tuple for feature @@ -186,5 +197,17 @@ class Ethtool: return True return False + def check_flow_control(self): + """ Check if the NIC supports flow-control """ + if self._driver_name in ['vmxnet3', 'virtio_net', 'xen_netfront']: + return False + return self._flow_control + + def get_flow_control(self): + if self._flow_control_enabled == None: + raise ValueError('Interface does not support changing '\ + 'flow-control settings!') + return self._flow_control_enabled + def get_auto_negotiation(self): return self._auto_negotiation diff --git a/python/vyos/ifconfig/ethernet.py b/python/vyos/ifconfig/ethernet.py index 5974a3d8f..cb07693c3 100644 --- a/python/vyos/ifconfig/ethernet.py +++ b/python/vyos/ifconfig/ethernet.py @@ -122,38 +122,16 @@ class EthernetIf(Interface): 'flow control settings!') return - # Get current flow control settings: - cmd = f'ethtool --show-pause {ifname}' - output, code = self._popen(cmd) - if code == 76: - # the interface does not support it - return '' - if code: - # never fail here as it prevent vyos to boot - print(f'unexpected return code {code} from {cmd}') - return '' - - # The above command returns - with tabs: - # - # Pause parameters for eth0: - # Autonegotiate: on - # RX: off - # TX: off - if re.search("Autonegotiate:\ton", output): - if enable == "on": - # flowcontrol is already enabled - no need to re-enable it again - # this will prevent the interface from flapping as applying the - # flow-control settings will take the interface down and bring - # it back up every time. - return '' - - # Assemble command executed on system. Unfortunately there is no way - # to change this setting via sysfs - cmd = f'ethtool --pause {ifname} autoneg {enable} tx {enable} rx {enable}' - output, code = self._popen(cmd) - if code: - print(f'could not set flowcontrol for {ifname}') - return output + current = self.ethtool.get_flow_control() + if current != enable: + # Assemble command executed on system. Unfortunately there is no way + # to change this setting via sysfs + cmd = f'ethtool --pause {ifname} autoneg {enable} tx {enable} rx {enable}' + output, code = self._popen(cmd) + if code: + print(f'Could not set flowcontrol for {ifname}') + return output + return None def set_speed_duplex(self, speed, duplex): """ diff --git a/src/conf_mode/interfaces-ethernet.py b/src/conf_mode/interfaces-ethernet.py index f3f3fede8..6e0d8c4e8 100755 --- a/src/conf_mode/interfaces-ethernet.py +++ b/src/conf_mode/interfaces-ethernet.py @@ -78,6 +78,10 @@ def verify(ethernet): raise ConfigError(f'Adapter does not support changing speed and duplex '\ f'settings to: {speed}/{duplex}!') + if 'disable_flow_control' in ethernet: + if not ethtool.check_flow_control(): + raise ConfigError('Adapter does not support changing flow-control settings!') + if 'ring_buffer' in ethernet: max_rx = ethtool.get_ring_buffer_max('rx') if not max_rx: diff --git a/src/migration-scripts/interfaces/20-to-21 b/src/migration-scripts/interfaces/20-to-21 index bd89dcdb4..0bd858760 100755 --- a/src/migration-scripts/interfaces/20-to-21 +++ b/src/migration-scripts/interfaces/20-to-21 @@ -104,6 +104,14 @@ for ifname in config.list_nodes(base): config.delete(speed_path) config.delete(duplex_path) + # Also while processing the interface configuration, not all adapters support + # changing disabling flow-control - or change this setting. If disabling + # flow-control is not supported by the NIC, we remove the setting from CLI + flow_control_path = base + [ifname, 'disable-flow-control'] + if config.exists(flow_control_path): + if not eth.check_flow_control(): + config.delete(flow_control_path) + try: with open(file_name, 'w') as f: f.write(config.to_string()) -- cgit v1.2.3 From c6039b9a82fe8a1752dc82a9834faf3a85b5dd38 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Mon, 6 Sep 2021 21:17:42 +0200 Subject: ifconfig: T3806: "ipv6 address no_default_link_local" required for MTU < 1280 This commit also extends the smoketest to verify that the exception for this error is raised. (cherry picked from commit 84a429b41175b95634ec9492e0cf3a564a47abdd) --- python/vyos/configverify.py | 24 ++++++++++++------------ smoketest/scripts/cli/base_interfaces_test.py | 10 +++++++++- src/conf_mode/interfaces-ethernet.py | 15 +++++++-------- 3 files changed, 28 insertions(+), 21 deletions(-) (limited to 'src/conf_mode/interfaces-ethernet.py') diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py index cff673a6e..ce7e76eb4 100644 --- a/python/vyos/configverify.py +++ b/python/vyos/configverify.py @@ -67,22 +67,22 @@ def verify_mtu_ipv6(config): min_mtu = 1280 if int(config['mtu']) < min_mtu: interface = config['ifname'] - error_msg = f'IPv6 address will be configured on interface "{interface}" ' \ - f'thus the minimum MTU requirement is {min_mtu}!' + error_msg = f'IPv6 address will be configured on interface "{interface}",\n' \ + f'the required minimum MTU is {min_mtu}!' - for address in (dict_search('address', config) or []): - if address in ['dhcpv6'] or is_ipv6(address): - raise ConfigError(error_msg) + if 'address' in config: + for address in config['address']: + if address in ['dhcpv6'] or is_ipv6(address): + raise ConfigError(error_msg) - tmp = dict_search('ipv6.address', config) - if tmp and 'no_default_link_local' not in tmp: - raise ConfigError('link-local ' + error_msg) + tmp = dict_search('ipv6.address.no_default_link_local', config) + if tmp == None: raise ConfigError('link-local ' + error_msg) - if tmp and 'autoconf' in tmp: - raise ConfigError(error_msg) + tmp = dict_search('ipv6.address.autoconf', config) + if tmp != None: raise ConfigError(error_msg) - if tmp and 'eui64' in tmp: - raise ConfigError(error_msg) + tmp = dict_search('ipv6.address.eui64', config) + if tmp != None: raise ConfigError(error_msg) def verify_tunnel(config): """ diff --git a/smoketest/scripts/cli/base_interfaces_test.py b/smoketest/scripts/cli/base_interfaces_test.py index 947162889..4acde99d3 100644 --- a/smoketest/scripts/cli/base_interfaces_test.py +++ b/smoketest/scripts/cli/base_interfaces_test.py @@ -246,11 +246,19 @@ class BasicInterfaceTest: for intf in self._interfaces: base = self._base_path + [intf] self.cli_set(base + ['mtu', self._mtu]) - self.cli_set(base + ['ipv6', 'address', 'no-default-link-local']) for option in self._options.get(intf, []): self.cli_set(base + option.split()) + # check validate() - can not set low MTU if 'no-default-link-local' + # is not set on CLI + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + for intf in self._interfaces: + base = self._base_path + [intf] + self.cli_set(base + ['ipv6', 'address', 'no-default-link-local']) + # commit interface changes self.cli_commit() diff --git a/src/conf_mode/interfaces-ethernet.py b/src/conf_mode/interfaces-ethernet.py index 6e0d8c4e8..17f58b285 100755 --- a/src/conf_mode/interfaces-ethernet.py +++ b/src/conf_mode/interfaces-ethernet.py @@ -62,6 +62,13 @@ def verify(ethernet): ifname = ethernet['ifname'] verify_interface_exists(ifname) + verify_mtu(ethernet) + verify_mtu_ipv6(ethernet) + verify_dhcpv6(ethernet) + verify_address(ethernet) + verify_vrf(ethernet) + verify_eapol(ethernet) + verify_mirror(ethernet) ethtool = Ethtool(ifname) # No need to check speed and duplex keys as both have default values. @@ -101,14 +108,6 @@ def verify(ethernet): raise ConfigError(f'Driver only supports a maximum TX ring-buffer '\ f'size of "{max_tx}" bytes!') - verify_mtu(ethernet) - verify_mtu_ipv6(ethernet) - verify_dhcpv6(ethernet) - verify_address(ethernet) - verify_vrf(ethernet) - verify_eapol(ethernet) - verify_mirror(ethernet) - # verify offloading capabilities if dict_search('offload.rps', ethernet) != None: if not os.path.exists(f'/sys/class/net/{ifname}/queues/rx-0/rps_cpus'): -- cgit v1.2.3 From 07840977834816b69fa3b366817d90f44b5dc7a7 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Fri, 10 Sep 2021 16:46:55 +0200 Subject: ethernet: T3802: use only one implementation for get_driver_name() Move the two implementations to get the driver name of a NIC from ethernet.py and ethtool.py to only ethtool.py. --- python/vyos/ethtool.py | 13 ++++++++----- python/vyos/ifconfig/ethernet.py | 28 +++------------------------- src/conf_mode/interfaces-ethernet.py | 2 +- 3 files changed, 12 insertions(+), 31 deletions(-) (limited to 'src/conf_mode/interfaces-ethernet.py') diff --git a/python/vyos/ethtool.py b/python/vyos/ethtool.py index 7e46969cf..4efc3a234 100644 --- a/python/vyos/ethtool.py +++ b/python/vyos/ethtool.py @@ -134,6 +134,12 @@ class Ethtool: # ['Autonegotiate:', 'on'] self._flow_control_enabled = out.splitlines()[1].split()[-1] + def get_auto_negotiation(self): + return self._auto_negotiation + + def get_driver_name(self): + return self._driver_name + def _get_generic(self, feature): """ Generic method to read self._features and return a tuple for feature @@ -189,7 +195,7 @@ class Ethtool: if duplex not in ['full', 'half']: raise ValueError(f'Value "{duplex}" for duplex is invalid!') - if self._driver_name in ['vmxnet3', 'virtio_net', 'xen_netfront']: + if self.get_driver_name() in ['vmxnet3', 'virtio_net', 'xen_netfront']: return False if speed in self._speed_duplex: @@ -199,7 +205,7 @@ class Ethtool: def check_flow_control(self): """ Check if the NIC supports flow-control """ - if self._driver_name in ['vmxnet3', 'virtio_net', 'xen_netfront']: + if self.get_driver_name() in ['vmxnet3', 'virtio_net', 'xen_netfront']: return False return self._flow_control @@ -208,6 +214,3 @@ class Ethtool: raise ValueError('Interface does not support changing '\ 'flow-control settings!') return self._flow_control_enabled - - def get_auto_negotiation(self): - return self._auto_negotiation diff --git a/python/vyos/ifconfig/ethernet.py b/python/vyos/ifconfig/ethernet.py index 47d3b6b4d..50e865203 100644 --- a/python/vyos/ifconfig/ethernet.py +++ b/python/vyos/ifconfig/ethernet.py @@ -81,25 +81,6 @@ class EthernetIf(Interface): super().__init__(ifname, **kargs) self.ethtool = Ethtool(ifname) - def get_driver_name(self): - """ - Return the driver name used by NIC. Some NICs don't support all - features e.g. changing link-speed, duplex - - Example: - >>> from vyos.ifconfig import EthernetIf - >>> i = EthernetIf('eth0') - >>> i.get_driver_name() - 'vmxnet3' - """ - ifname = self.config['ifname'] - sysfs_file = f'/sys/class/net/{ifname}/device/driver/module' - if os.path.exists(sysfs_file): - link = os.readlink(sysfs_file) - return os.path.basename(link) - else: - return None - def set_flow_control(self, enable): """ Changes the pause parameters of the specified Ethernet device. @@ -117,8 +98,7 @@ class EthernetIf(Interface): raise ValueError("Value out of range") if not self.ethtool.check_flow_control(): - self._debug_msg(f'{driver_name} driver does not support changing '\ - 'flow control settings!') + self._debug_msg(f'NIC driver does not support changing flow control settings!') return False current = self.ethtool.get_flow_control() @@ -152,10 +132,8 @@ class EthernetIf(Interface): if duplex not in ['auto', 'full', 'half']: raise ValueError("Value out of range (duplex)") - driver_name = self.get_driver_name() - if driver_name in ['vmxnet3', 'virtio_net', 'xen_netfront']: - self._debug_msg(f'{driver_name} driver does not support changing '\ - 'speed/duplex settings!') + if not self.ethtool.check_speed_duplex(speed, duplex): + self._debug_msg(f'NIC driver does not support changing speed/duplex settings!') return # Get current speed and duplex settings: diff --git a/src/conf_mode/interfaces-ethernet.py b/src/conf_mode/interfaces-ethernet.py index 17f58b285..de851262b 100755 --- a/src/conf_mode/interfaces-ethernet.py +++ b/src/conf_mode/interfaces-ethernet.py @@ -113,7 +113,7 @@ def verify(ethernet): if not os.path.exists(f'/sys/class/net/{ifname}/queues/rx-0/rps_cpus'): raise ConfigError('Interface does not suport RPS!') - driver = EthernetIf(ifname).get_driver_name() + driver = ethtool.get_driver_name() # T3342 - Xen driver requires special treatment if driver == 'vif': if int(ethernet['mtu']) > 1500 and dict_search('offload.sg', ethernet) == None: -- cgit v1.2.3