[Devel] [PATCH hci-8.1 v3 0/1] selftests: drv-net-hw: skip the tests that need a real interface

[Devel] [PATCH hci-8.1 v3 0/1] selftests: drv-net-hw: skip the tests that need a real interface

[Eva Kurchatova]

Patchset for task https://virtuozzo.atlassian.net/browse/VSTOR-139651

Changes in v3:

- Check the kind of the first interface instead of NETIF_CREATE and
  NETIF_TYPE, which stay at their defaults when forwarding.config
  names real NICs; those ran through lib.sh and were skipped here.
- Take the interfaces from the command line too, as lib.sh does.
- Rebased.

Link to v2: http://lore.virtuozzo.com/kernel/20260928194332.551992-1-eva.kurchatova@virtuozzo.com/T/#t

Changes in v2:
- Read forwarding.config before deciding, the way lib.sh does.
- One helper, skip_on_veth.sh, instead of a copy in each test.
- Add ethtool_rmon.sh, it fails on veth the same way.
- One-line comments, fix the Feature tag.
- Sent alone, 1/4 to 3/4 are merged.

Link to v1: http://lore.virtuozzo.com/kernel/20260831225137.1642608-4-eva.kurchatova@virtuozzo.com/

Eva Kurchatova (1):
  selftests: drv-net-hw: skip the tests that need a real interface

 .../testing/selftests/drivers/net/hw/Makefile |  1 +
 .../selftests/drivers/net/hw/ethtool.sh       |  5 +++
 .../drivers/net/hw/ethtool_extended_state.sh  |  5 +++
 .../selftests/drivers/net/hw/ethtool_rmon.sh  |  5 +++
 .../selftests/drivers/net/hw/hw_stats_l3.sh   |  5 +++
 .../drivers/net/hw/hw_stats_l3_gre.sh         |  5 +++
 .../selftests/drivers/net/hw/skip_on_veth.sh  | 33 +++++++++++++++++++
 7 files changed, 59 insertions(+)
 create mode 100644 tools/testing/selftests/drivers/net/hw/skip_on_veth.sh

-- 
2.56.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v3 1/1] selftests: drv-net-hw: skip the tests that need a real interface

[Eva Kurchatova]

ethtool, ethtool_extended_state, ethtool_rmon, hw_stats_l3 and
hw_stats_l3_gre need hardware: link speed, extended link state, rmon
counters, L3 statistics offload. With no interfaces configured lib.sh
substitutes a veth pair, and every case fails on what veth does not
have.

Upstream runs these tests on real hardware only and dropped the veth
check in commit 0c499a351777 ("selftests: forwarding: Ditch
skip_on_veth()"), so this patch is not for upstream.

Skip before sourcing lib.sh, which creates the pair as it is sourced
and would leave it behind. The check takes the interfaces the way
lib.sh does, from the command line or forwarding.config, and skips when
the first one is a veth, or does not exist and would be created as one.
Real interfaces named there run with NETIF_CREATE and NETIF_TYPE left
at their defaults, as lib.sh only creates what is missing.

https://virtuozzo.atlassian.net/browse/VSTOR-139651
Feature: fix selftests
Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 .../testing/selftests/drivers/net/hw/Makefile |  1 +
 .../selftests/drivers/net/hw/ethtool.sh       |  5 +++
 .../drivers/net/hw/ethtool_extended_state.sh  |  5 +++
 .../selftests/drivers/net/hw/ethtool_rmon.sh  |  5 +++
 .../selftests/drivers/net/hw/hw_stats_l3.sh   |  5 +++
 .../drivers/net/hw/hw_stats_l3_gre.sh         |  5 +++
 .../selftests/drivers/net/hw/skip_on_veth.sh  | 33 +++++++++++++++++++
 7 files changed, 59 insertions(+)
 create mode 100644 tools/testing/selftests/drivers/net/hw/skip_on_veth.sh

diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
index e556b5150a51..39aece5e2776 100644
--- a/tools/testing/selftests/drivers/net/hw/Makefile
+++ b/tools/testing/selftests/drivers/net/hw/Makefile
@@ -19,6 +19,7 @@ TEST_PROGS = \
 
 TEST_FILES := \
 	ethtool_lib.sh \
+	skip_on_veth.sh \
 	#
 
 TEST_INCLUDES := \
diff --git a/tools/testing/selftests/drivers/net/hw/ethtool.sh b/tools/testing/selftests/drivers/net/hw/ethtool.sh
index fa6953de6b6d..e1fbe37a71fc 100755
--- a/tools/testing/selftests/drivers/net/hw/ethtool.sh
+++ b/tools/testing/selftests/drivers/net/hw/ethtool.sh
@@ -11,6 +11,11 @@ ALL_TESTS="
 "
 NUM_NETIFS=2
 lib_dir=$(dirname "$0")
+
+# Needs a device that can set link speed and autonegotiation
+source "$lib_dir"/skip_on_veth.sh
+skip_on_veth "$@"
+
 source "$lib_dir"/../../../net/forwarding/lib.sh
 source ethtool_lib.sh
 
diff --git a/tools/testing/selftests/drivers/net/hw/ethtool_extended_state.sh b/tools/testing/selftests/drivers/net/hw/ethtool_extended_state.sh
index a7584448416e..1201130cae67 100755
--- a/tools/testing/selftests/drivers/net/hw/ethtool_extended_state.sh
+++ b/tools/testing/selftests/drivers/net/hw/ethtool_extended_state.sh
@@ -9,6 +9,11 @@ ALL_TESTS="
 
 NUM_NETIFS=2
 lib_dir=$(dirname "$0")
+
+# Needs a device that reports an extended link state
+source "$lib_dir"/skip_on_veth.sh
+skip_on_veth "$@"
+
 source "$lib_dir"/../../../net/forwarding/lib.sh
 source ethtool_lib.sh
 
diff --git a/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh b/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh
index 8f60c1685ad4..1745afe671d4 100755
--- a/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh
+++ b/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh
@@ -8,6 +8,11 @@ ALL_TESTS="
 
 NUM_NETIFS=2
 lib_dir=$(dirname "$0")
+
+# Needs a device with rmon histogram counters
+source "$lib_dir"/skip_on_veth.sh
+skip_on_veth "$@"
+
 source "$lib_dir"/../../../net/forwarding/lib.sh
 
 ETH_FCS_LEN=4
diff --git a/tools/testing/selftests/drivers/net/hw/hw_stats_l3.sh b/tools/testing/selftests/drivers/net/hw/hw_stats_l3.sh
index 67fafefc80be..460633f27f40 100755
--- a/tools/testing/selftests/drivers/net/hw/hw_stats_l3.sh
+++ b/tools/testing/selftests/drivers/net/hw/hw_stats_l3.sh
@@ -49,6 +49,11 @@ ALL_TESTS="
 "
 NUM_NETIFS=4
 lib_dir=$(dirname "$0")
+
+# Needs a device that offloads L3 statistics
+source "$lib_dir"/skip_on_veth.sh
+skip_on_veth "$@"
+
 source "$lib_dir"/../../../net/forwarding/lib.sh
 source "$lib_dir"/../../../net/forwarding/tc_common.sh
 
diff --git a/tools/testing/selftests/drivers/net/hw/hw_stats_l3_gre.sh b/tools/testing/selftests/drivers/net/hw/hw_stats_l3_gre.sh
index a94d92e1abce..abe8c09700b4 100755
--- a/tools/testing/selftests/drivers/net/hw/hw_stats_l3_gre.sh
+++ b/tools/testing/selftests/drivers/net/hw/hw_stats_l3_gre.sh
@@ -13,6 +13,11 @@ ALL_TESTS="
 "
 NUM_NETIFS=6
 lib_dir=$(dirname "$0")
+
+# Needs a device that offloads L3 statistics
+source "$lib_dir"/skip_on_veth.sh
+skip_on_veth "$@"
+
 source "$lib_dir"/../../../net/forwarding/lib.sh
 source "$lib_dir"/../../../net/forwarding/ipip_lib.sh
 source "$lib_dir"/../../../net/forwarding/tc_common.sh
diff --git a/tools/testing/selftests/drivers/net/hw/skip_on_veth.sh b/tools/testing/selftests/drivers/net/hw/skip_on_veth.sh
new file mode 100644
index 000000000000..aa2bb98dc278
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/hw/skip_on_veth.sh
@@ -0,0 +1,33 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# Source with lib_dir set, ahead of lib.sh, which creates the interfaces.
+
+forwarding_dir="$lib_dir"/../../../net/forwarding
+
+# forwarding.config assigns by name into the array lib.sh declares
+declare -A NETIFS
+
+if [[ -f $forwarding_dir/forwarding.config ]]; then
+	source "$forwarding_dir/forwarding.config"
+fi
+
+: "${NETIF_CREATE:=yes}"
+: "${NETIF_TYPE:=veth}"
+
+# Call before sourcing lib.sh, exiting after it leaves the veth pair behind
+skip_on_veth()
+{
+	# lib.sh takes the interfaces from the command line ahead of the config
+	local p1=${1:-${NETIFS[p1]-}}
+	local kind
+
+	if [[ -n $p1 ]] && ip link show dev "$p1" &> /dev/null; then
+		kind=$(ip -j -d link show dev "$p1" | jq -r '.[].linkinfo.info_kind')
+		[[ $kind == veth ]] || return
+	elif [[ $NETIF_CREATE != yes || $NETIF_TYPE != veth ]]; then
+		return
+	fi
+	echo "SKIP: needs a real interface, set NETIFS in forwarding.config"
+	exit 4 # ksft_skip, lib.sh is not sourced yet
+}
-- 
2.56.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v3 1/1] selftests: drv-net-hw: skip the tests that need a real interface

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests: skip_on_veth() ignores NETIF_FIND_DRIVER and falsely skips on real NICs
--

commit 6f44dc091649c1067eaf65154f6f0a7b79b13a25
Author: Eva Kurchatova <eva.kurchatova@virtuozzo.com>

selftests: drv-net-hw: skip the tests that need a real interface

This adds a skip_on_veth() helper that runs before lib.sh is sourced. It is
called from the ethtool, ethtool_extended_state, ethtool_rmon, hw_stats_l3
and hw_stats_l3_gre tests. It exits with ksft_skip when the first interface
is a veth, or when it does not exist and lib.sh would create it as a veth.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-139651

> diff --git a/tools/testing/selftests/drivers/net/hw/skip_on_veth.sh b/tools/testing/selftests/drivers/net/hw/skip_on_veth.sh
> new file mode 100644
> index 0000000000000..aa2bb98dc2786
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/hw/skip_on_veth.sh

[ ... ]

> +# Call before sourcing lib.sh, exiting after it leaves the veth pair behind
> +skip_on_veth()
> +{
> +	# lib.sh takes the interfaces from the command line ahead of the config
> +	local p1=${1:-${NETIFS[p1]-}}
> +	local kind
> +
> +	if [[ -n $p1 ]] && ip link show dev "$p1" &> /dev/null; then
> +		kind=$(ip -j -d link show dev "$p1" | jq -r '.[].linkinfo.info_kind')
> +		[[ $kind == veth ]] || return
> +	elif [[ $NETIF_CREATE != yes || $NETIF_TYPE != veth ]]; then
> +		return
> +	fi
> +	echo "SKIP: needs a real interface, set NETIFS in forwarding.config"
> +	exit 4 # ksft_skip, lib.sh is not sourced yet
> +}

[Severity: Medium]
Can skip_on_veth() skip a test that would have run on real interfaces?

When NETIF_FIND_DRIVER is set in the environment and neither a command
line interface nor NETIFS[p1] is given, p1 is empty in skip_on_veth().
The first branch is not taken. NETIF_CREATE and NETIF_TYPE are still at
their defaults (yes and veth), so the elif condition is false.  The
function then falls through to the echo and exit 4.

lib.sh handles this case later, in net/forwarding/lib.sh:

if [[ $NETIF_FIND_DRIVER ]]; then
	unset NETIFS
	declare -A NETIFS
	netif_find_driver
fi

netif_find_driver() fills NETIFS from real devices matched by driver name.
skip_on_veth() runs before lib.sh is sourced and never looks at
NETIF_FIND_DRIVER.  Other selftests already set it, for example
drivers/net/virtio_net/virtio_net_common.sh sets
NETIF_FIND_DRIVER="virtio_net".

Without this patch the test would have run on the discovered hardware.
With it, the test is reported as SKIP.

The commit message says the check takes the interfaces "the way lib.sh
does, from the command line or forwarding.config".  Is that accurate,
given that the NETIF_FIND_DRIVER path is not covered?

Also, only p1 is inspected.  If p1 is a real interface and p2 is missing,
lib.sh would still create a veth for p2.  Can skip_on_veth() handle that
case too?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261006222651.2510417-1-eva.kurchatova@virtuozzo.com?part=1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel