linux-kernel - Re: [PATCH RFC net-next 1/2] net: selftest: add net_selftest

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] [day] [month] [year] [list]

Message-ID: <31a7c481-0b0c-4a46-9eb9-983f88ca137e@engleder-embedded.com>
Date: Fri, 25 Apr 2025 22:15:38 +0200
From: Gerhard Engleder <gerhard@...leder-embedded.com>
To: Jijie Shao <shaojijie@...wei.com>
Cc: shenjian15@...wei.com, wangpeiyang1@...wei.com, liuyonglong@...wei.com,
 chenhao418@...wei.com, jonathan.cameron@...wei.com,
 shameerali.kolothum.thodi@...wei.com, salil.mehta@...wei.com,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
 andrew+netdev@...n.ch, horms@...nel.org
Subject: Re: [PATCH RFC net-next 1/2] net: selftest: add net_selftest_custom()
 interface

On 21.04.25 15:43, Jijie Shao wrote:
> In net/core/selftests.c,
> net_selftest() supports loopback tests.
> However, the loopback content of this interface is a fixed common test
> and cannot be expanded to add the driver's own test.
> 
> In this patch, the net_selftest_custom() interface is added
> to support driver customized loopback tests and
> extra common loopback tests.
> 
> Signed-off-by: Jijie Shao <shaojijie@...wei.com>
> ---
>   include/net/selftests.h |  61 +++++++++++++
>   net/core/selftests.c    | 188 +++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 245 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/selftests.h b/include/net/selftests.h
> index e65e8d230d33..a36e6ee0a41f 100644
> --- a/include/net/selftests.h
> +++ b/include/net/selftests.h
> @@ -4,6 +4,48 @@
>   
>   #include <linux/ethtool.h>
>   
> +#define NET_TEST_NETIF_CARRIER		BIT(0)
> +#define NET_TEST_FULL_DUPLEX		BIT(1)
> +#define NET_TEST_TCP			BIT(2)
> +#define NET_TEST_UDP			BIT(3)
> +#define NET_TEST_UDP_MAX_MTU		BIT(4)
> +
> +#define NET_EXTRA_CARRIER_TEST		BIT(0)
> +#define NET_EXTRA_FULL_DUPLEX_TEST	BIT(1)
> +#define NET_EXTRA_PHY_TEST		BIT(2)

What is the difference between NET_TEST_NETIF_CARRIER and
NET_EXTRA_CARRIER_TEST? Aren't these the same tests?

> +struct net_test_entry {
> +	char name[ETH_GSTRING_LEN];
> +
> +	/* can set to NULL */
> +	int (*enable)(struct net_device *ndev, bool enable);
> +
> +	/* can set to NULL */
> +	int (*fn)(struct net_device *ndev);
> +
> +	/* if flag is set, fn() will be ignored,
> +	 * and will do test according to the flag,
> +	 * such as NET_TEST_UDP...
> +	 */
> +	unsigned long flags;

Looks limited, this interface does not scale as the bits in flags are
limited.

> +};
> +
> +#define NET_TEST_E(_name, _enable, _flags) { \
> +	.name = _name, \
> +	.enable = _enable, \
> +	.fn = NULL, \
> +	.flags = _flags }
> +
> +#define NET_TEST_ENTRY_MAX_COUNT	10

I expect that you have to eliminate this limitation too.

> +struct net_test {
> +	/* extra tests will be added based on this flag */
> +	unsigned long extra_flags;

Why this extra_flags to trigger tests? AFAIU the same tests can be
triggered with entries.

> +
> +	struct net_test_entry entries[NET_TEST_ENTRY_MAX_COUNT];
> +	/* the count of entries, must <= NET_TEST_ENTRY_MAX_COUNT */
> +	u32 count;
> +};

You try to make net selftests more usable for drivers. I also tried
that, but Andrew Lunn argumented for controlling the selftests some
user space interface is expected. IMO the situation has not changed.

https://lore.kernel.org/netdev/20250227203138.60420-3-gerhard@engleder-embedded.com/T/#md5e4ac1ca41adbdb43755d3c189aa8b8228bf8c9

Gerhard