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

[no-reply@sashiko.virtuozzo.com]

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