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

[Konstantin Khorenko <khorenko@virtuozzo.com>]

> 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 on veth before sourcing lib.sh, which creates the pair as it is
> sourced and would leave it behind. The check reads forwarding.config
> the way lib.sh does, so real interfaces configured there are honoured.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-139651
> Feature: fix selftests
> Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
>
> diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
> index e556b5150a51b..39aece5e27767 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 fa6953de6b6dd..b9eeeb56589ff 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 a7584448416ea..fd3ecd9623420 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 8f60c1685ad4b..91445a379face 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 67fafefc80bea..13104dc7d8190 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 a94d92e1abce6..b94b2338c853b 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 0000000000000..845cc65b1df7c
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/hw/skip_on_veth.sh
> @@ -0,0 +1,22 @@
> +#!/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
> +
> +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()
> +{
> +	if [[ "$NETIF_CREATE" = yes && "$NETIF_TYPE" = veth ]]; then
> +		echo "SKIP: needs a real interface, set NETIFS in forwarding.config"
> +		exit 4 # ksft_skip, lib.sh is not sourced yet

NAK

This skips a valid hardware setup. create_netif_veth() only creates
the interfaces that do not exist, so NETIFS naming real NICs with
NETIF_CREATE/NETIF_TYPE left at the defaults works in lib.sh - and
that is what you get by editing forwarding.config.sample, which does
not mention either variable. Such a host skips all five tests, and
the message advises what the user has already done.

Check the device kind instead; that works before lib.sh too,
forwarding.config is already read here:

        local p1=${NETIFS[p1]}

        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: ..."
        exit 4

On a bare host NETIFS is unset, the elif still skips.

-- 
Konstantin Khorenko <khorenko@virtuozzo.com>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel