Re: [PATCH net 10/17] selftests: forwarding: ethtool_mm: Skip when using veth pairs

[Petr Machata <petrm@nvidia.com>]

Ido Schimmel <idosch@idosch.org> writes:

> On Wed, Aug 02, 2023 at 02:27:49PM +0200, Petr Machata wrote:
>> 
>> Vladimir Oltean <vladimir.oltean@nxp.com> writes:
>> 
>> > @@ -266,6 +278,14 @@ setup_prepare()
>> >  	h1=${NETIFS[p1]}
>> >  	h2=${NETIFS[p2]}
>> >
>> > +	for netif in ${NETIFS[@]}; do
>> > +		ethtool --show-mm $netif 2>&1 &> /dev/null
>> > +		if [[ $? -ne 0 ]]; then
>> > +			echo "SKIP: $netif does not support MAC Merge"
>> > +			exit $ksft_skip
>> > +		fi
>> > +	done
>> > +
>> 
>> Ido, if you decide to go this route, just hoist the loop to the global
>> scope before registering the trap, then you don't need the hX_created
>> business.
>
> I think the idea was to run this check after verifying that ethtool
> supports MAC Merge in setup_prepare(). How about moving all these checks

True, I missed that.

> before doing any configuration and registering a trap handler?
>
> diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
> index 4331e2161e8d..39e736f30322 100755
> --- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh
> +++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
> @@ -258,11 +258,6 @@ h2_destroy()
>  
>  setup_prepare()
>  {
> -       check_ethtool_mm_support
> -       check_tc_fp_support
> -       require_command lldptool
> -       bail_on_lldpad "autoconfigure the MAC Merge layer" "configure it manually"
> -
>         h1=${NETIFS[p1]}
>         h2=${NETIFS[p2]}
>  
> @@ -278,7 +273,18 @@ cleanup()
>         h1_destroy
>  }
>  
> -skip_on_veth
> +check_ethtool_mm_support
> +check_tc_fp_support
> +require_command lldptool
> +bail_on_lldpad "autoconfigure the MAC Merge layer" "configure it manually"
> +
> +for netif in ${NETIFS[@]}; do
> +       ethtool --show-mm $netif 2>&1 &> /dev/null
> +       if [[ $? -ne 0 ]]; then
> +               echo "SKIP: $netif does not support MAC Merge"
> +               exit $ksft_skip
> +       fi
> +done
>  
>  trap cleanup EXIT
>

Looks good. These checks are usually placed right after sourcing the
libraries, but I don't care much one way or another.