| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c1890a67-6cc5-4f37-bc33-5fd49b6839c5@redhat.com>
Date: Tue, 22 Oct 2024 12:17:49 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Mohan Prasad J <mohan.prasad@...rochip.com>, netdev@...r.kernel.org,
davem@...emloft.net, kuba@...nel.org, andrew@...n.ch
Cc: edumazet@...gle.com, shuah@...nel.org, linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org, horms@...nel.org, brett.creeley@....com,
rosenp@...il.com, UNGLinuxDriver@...rochip.com, willemb@...gle.com,
petrm@...dia.com
Subject: Re: [PATCH net-next v3 1/3] selftests: nic_link_layer: Add link layer
selftest for NIC driver
On 10/16/24 23:50, Mohan Prasad J wrote:
> Add selftest file for the link layer tests of a NIC driver.
> Test for auto-negotiation is added.
> Add LinkConfig class for changing link layer configs.
> Selftest makes use of ksft modules and ethtool.
> Include selftest file in the Makefile.
>
> Signed-off-by: Mohan Prasad J <mohan.prasad@...rochip.com>
> ---
> .../testing/selftests/drivers/net/hw/Makefile | 1 +
> .../drivers/net/hw/lib/py/__init__.py | 1 +
> .../drivers/net/hw/lib/py/linkconfig.py | 220 ++++++++++++++++++
> .../drivers/net/hw/nic_link_layer.py | 84 +++++++
> 4 files changed, 306 insertions(+)
> create mode 100644 tools/testing/selftests/drivers/net/hw/lib/py/linkconfig.py
> create mode 100644 tools/testing/selftests/drivers/net/hw/nic_link_layer.py
>
> diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
> index c9f2f48fc..0dac40c4e 100644
> --- a/tools/testing/selftests/drivers/net/hw/Makefile
> +++ b/tools/testing/selftests/drivers/net/hw/Makefile
> @@ -10,6 +10,7 @@ TEST_PROGS = \
> hw_stats_l3.sh \
> hw_stats_l3_gre.sh \
> loopback.sh \
> + nic_link_layer.py \
> pp_alloc_fail.py \
> rss_ctx.py \
> #
> diff --git a/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py b/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py
> index b58288578..399789a96 100644
> --- a/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py
> +++ b/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py
> @@ -9,6 +9,7 @@ try:
> sys.path.append(KSFT_DIR.as_posix())
> from net.lib.py import *
> from drivers.net.lib.py import *
> + from .linkconfig import LinkConfig
> except ModuleNotFoundError as e:
> ksft_pr("Failed importing `net` library from kernel sources")
> ksft_pr(str(e))
> diff --git a/tools/testing/selftests/drivers/net/hw/lib/py/linkconfig.py b/tools/testing/selftests/drivers/net/hw/lib/py/linkconfig.py
> new file mode 100644
> index 000000000..86cbf10a3
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/hw/lib/py/linkconfig.py
> @@ -0,0 +1,220 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +from lib.py import cmd
> +from lib.py import ethtool
> +from lib.py import ksft_pr, ksft_eq
> +import re
> +import time
> +import json
> +
> +#The LinkConfig class is implemented to handle the link layer configurations.
> +#Required minimum ethtool version is 6.10
> +#The ethtool and ip would require authentication to make changes, so better
> +# to check them for sudo privileges for interruption free testing.
> +
> +class LinkConfig:
> + """Class for handling the link layer configurations"""
> + def __init__(self, cfg):
> + self.cfg = cfg
> + self.partner_netif = self.get_partner_netif_name()
> +
> + """Get the initial link configuration of local interface"""
> + self.common_link_modes = self.get_common_link_modes()
> +
> + def get_partner_netif_name(self):
You should use type hints for both the arguments and the return type.
> + partner_netif = None
> + try:
> + """Get partner interface name"""
> + partner_cmd = f"ip -o -f inet addr show | grep '{self.cfg.remote_addr}' " + "| awk '{print $2}'"
It's better if you use json output even here
[...]
> + def reset_interface(self, local=True, remote=True):
> + ksft_pr("Resetting interfaces in local and remote")
> + if remote:
> + if self.partner_netif is not None:
> + ifname = self.partner_netif
> + link_up_cmd = f"sudo ip link set up {ifname}"
> + link_down_cmd = f"sudo ip link set down {ifname}"
> + reset_cmd = f"{link_down_cmd} && sleep 5 && {link_up_cmd}"
> + try:
> + cmd(f"{reset_cmd}", host=self.cfg.remote)
> + except Exception as e:
> + ksft_pr("Check sudo permission for ip command")
> + ksft_pr(f"Unexpected error occurred: {e}")
Please, don't use sudo, just run the command directly. The selftests
should be executed only by privileged users.
[...]
> + def check_autoneg_supported(self, remote=False):
> + if remote==False:
> + local_autoneg = self.get_ethtool_field("supports-auto-negotiation")
> + if local_autoneg is None:
> + ksft_pr(f"Unable to fetch auto-negotiation status for interface {self.cfg.ifname}")
> + """Return autoneg status of the local interface"""
> + status = True if local_autoneg == True else False
> + return status
Out of sheer ignorance, in't
return local_autoneg
enough?
> + else:
> + """Check remote auto-negotiation support status"""
> + partner_autoneg = False
> + if self.partner_netif is not None:
> + partner_autoneg = self.get_ethtool_field("supports-auto-negotiation", remote=True)
> + if partner_autoneg is None:
> + ksft_pr(f"Unable to fetch auto-negotiation status for interface {partner_netif}")
> + status = True if partner_autoneg is True else False
> + return status
> +
> + def get_common_link_modes(self):
> + common_link_modes = None
> + """Populate common link modes"""
> + link_modes = self.get_ethtool_field("supported-link-modes")
> + partner_link_modes = self.get_ethtool_field("link-partner-advertised-link-modes")
> + if link_modes is None:
> + raise Exception(f"Link modes not available for {self.cfg.ifname}")
> + if partner_link_modes is None:
> + raise Exception(f"Partner link modes not available for {self.cfg.ifname}")
> + common_link_modes = set(link_modes) and set(partner_link_modes)
> + return common_link_modes
> +
> + def get_speed_duplex_values(self, link_modes):
> + speed = []
> + duplex = []
> + """Check the link modes"""
> + for data in link_modes:
> + parts = data.split('/')
> + speed_value = re.match(r'\d+', parts[0])
> + if speed_value:
> + speed.append(speed_value.group())
> + else:
> + ksft_pr(f"No speed value found for interface {self.ifname}")
> + return None, None
> + duplex.append(parts[1].lower())
> + return speed, duplex
> +
> + def get_ethtool_field(self, field: str, remote=False):
> + process = None
> + if remote == False:
> + """Get the ethtool field value for the local interface"""
> + ifname = self.cfg.ifname
> + try:
> + process = ethtool(f"--json {ifname}")
> + except Exception as e:
> + ksft_pr("Required minimum ethtool version is 6.10")
> + ksft_pr(f"Unexpected error occurred: {e}")
> + else:
> + """Get the ethtool field value for the remote interface"""
> + remote = True
> + ifname = self.partner_netif
> + self.cfg.require_cmd("ethtool", remote)
> + command = f"ethtool --json {ifname}"
> + try:
> + process = cmd(command, host=self.cfg.remote)
> + except Exception as e:
> + ksft_pr("Required minimum ethtool version is 6.10")
> + ksft_pr("Unexpected error occurred: {e}")
> + if process is None or process.ret != 0:
> + print(f"Error while getting the ethtool content for interface {ifname}. Required minimum ethtool version is 6.10")
> + return None
> + output = json.loads(process.stdout)
> + json_data = output[0]
> + """Check if the field exist in the json data"""
> + if field not in json_data:
> + raise Exception(f"Field {field} does not exist in the output of interface {json_data["ifname"]}")
> + return None
> + return json_data[field]
> diff --git a/tools/testing/selftests/drivers/net/hw/nic_link_layer.py b/tools/testing/selftests/drivers/net/hw/nic_link_layer.py
> new file mode 100644
> index 000000000..fb046efbe
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/hw/nic_link_layer.py
> @@ -0,0 +1,84 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0
> +
> +#Introduction:
> +#This file has basic link layer tests for generic NIC drivers.
> +#The test comprises of auto-negotiation, speed and duplex checks.
> +#
> +#Setup:
> +#Connect the DUT PC with NIC card to partner pc back via ethernet medium of your choice(RJ45, T1)
> +#
> +# DUT PC Partner PC
> +#┌───────────────────────┐ ┌──────────────────────────┐
> +#│ │ │ │
> +#│ │ │ │
> +#│ ┌───────────┐ │ │
> +#│ │DUT NIC │ Eth │ │
> +#│ │Interface ─┼─────────────────────────┼─ any eth Interface │
> +#│ └───────────┘ │ │
> +#│ │ │ │
> +#│ │ │ │
> +#└───────────────────────┘ └──────────────────────────┘
> +#
> +#Configurations:
> +#To prevent interruptions, Add ethtool, ip to the sudoers list in remote PC and get the ssh key from remote.
> +#Required minimum ethtool version is 6.10
> +#Change the below configuration based on your hw needs.
> +# """Default values"""
> +time_delay = 8 #time taken to wait for transitions to happen, in seconds.
> +test_duration = 10 #performance test duration for the throughput check, in seconds.
It would be probably useful to allow the user overriding the above
values via env variables and/or command line arguments.
'test_duration' declaration should probably moved to a later patch,
where it's used.
Thanks,
Paolo
Powered by blists - more mailing lists