linux-kernel - Re: [PATCH net-next v2 1/3] selftests: nic_basic_tests: Add selftest file for basic tests of NIC

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1ad4656f-eb3f-4ecb-9e5d-5846f4f6c3e6@lunn.ch>
Date: Tue, 17 Sep 2024 17:56:17 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Mohan Prasad J <mohan.prasad@...rochip.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
	edumazet@...gle.com, pabeni@...hat.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
Subject: Re: [PATCH net-next v2 1/3] selftests: nic_basic_tests: Add selftest
 file for basic tests of NIC

On Tue, Sep 17, 2024 at 08:04:07AM +0530, Mohan Prasad J wrote:
> Add selftest file to test basic features of a NIC driver.
> Tests for link modes, auto-negotiation are placed.
> Selftest makes use of ksft modules and ethtool.
> Add 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/nic_basic_tests.py         | 145 ++++++++++++++++++
>  2 files changed, 146 insertions(+)
>  create mode 100644 tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> 
> diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
> index c9f2f48fc..9f105227c 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_basic_tests.py \
>  	pp_alloc_fail.py \
>  	rss_ctx.py \
>  	#
> diff --git a/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py b/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> new file mode 100644
> index 000000000..27f780032
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/hw/nic_basic_tests.py
> @@ -0,0 +1,145 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0
> +
> +#Introduction:
> +#This file has basic tests for generic NIC drivers.
> +#The test comprises of auto-negotiation, speed and duplex checks.
> +#Also has tests to check the throughput
> +#
> +#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:
> +# Change the below configuration based on your hw needs.
> +# """Default values"""
> +sleep_time = 5 #time taken to wait for transitions to happen, in seconds.
> +test_duration = 5  #performance test duration for the throughput check, in seconds.
> +throughput_threshold = 0.8 #percentage of throughput required to pass the throughput
> +
> +import time
> +import os
> +import re
> +import configparser
> +import json
> +from lib.py import ksft_run, ksft_exit, ksft_pr, ksft_eq
> +from lib.py import KsftFailEx, KsftSkipEx
> +from lib.py import NetDrvEpEnv
> +from lib.py import cmd
> +from lib.py import ethtool
> +
> +"""Global variables"""
> +common_link_modes = []

Software engineers have a dislike for global variables. In this patch,
it is not even used. Please consider replacing it by passing it as a
parameter, or turn the code into a class and make it part of self.

> +def test_link_modes(cfg) -> None:
> +    global common_link_modes
> +    link_modes = get_ethtool_content(cfg.ifname, "Supported link modes:")
> +    partner_link_modes = get_ethtool_content(cfg.ifname, "Link partner advertised link modes:")
> +
> +    if link_modes and partner_link_modes:
> +        for idx1 in range(len(link_modes)):
> +            for idx2 in range(len(partner_link_modes)):
> +                if link_modes[idx1] == partner_link_modes[idx2]:
> +                    common_link_modes.append(link_modes[idx1])

You can use the power of python here.

        "supported-link-modes": [ "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000baseT/Full" ],
	"link-partner-modes": [ "10baseT/Half","10baseT/Full","100baseT/Half","100baseT/Full","1000baseT/Full" ],

convert the list into a set, and then use 'and'.

	common_modes = set(josn['supported-link-modes']) and set(json['link-partner-modes'])

	Andrew