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

From: Petr Machata <[email-masked]>
Date: Wed, 2 Aug 2023 14:27:49 +0200

Vladimir Oltean <[email-masked]> writes:

> Hi Ido,
>
> On Wed, Aug 02, 2023 at 10:51:11AM +0300, Ido Schimmel wrote:
>> MAC Merge cannot be tested with veth pairs, resulting in failures:
>> 
>>  # ./ethtool_mm.sh
>>  [...]
>>  TEST: Manual configuration with verification: swp1 to swp2          [FAIL]
>>          Verification did not succeed
>> 
>> Fix by skipping the test when used with veth pairs.
>> 
>> Fixes: e6991384ace5 ("selftests: forwarding: add a test for MAC Merge layer")
>> Reported-by: Mirsad Todorovac <[email-masked]>
>> Closes: https://lore.kernel.org/netdev/[email-masked]/
>> Signed-off-by: Ido Schimmel <[email-masked]>
>> Reviewed-by: Petr Machata <[email-masked]>
>> Tested-by: Mirsad Todorovac <[email-masked]>
>> ---
>
> That will skip the selftest just for veth pairs. This will skip it for
> any device that doesn't support the MAC Merge layer:
>
> diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
> index c580ad623848..5432848a3c59 100755
> --- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh
> +++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
> @@ -224,6 +224,8 @@ h1_create()
>  		hw 1
>
>  	ethtool --set-mm $h1 pmac-enabled on tx-enabled off verify-enabled off
> +
> +	h1_created=yes
>  }
>
>  h2_create()
> @@ -236,10 +238,16 @@ h2_create()
>  		queues 1@0 1@1 1@2 1@3 \
>  		fp P E E E \
>  		hw 1
> +
> +	h2_created=yes
>  }
>
>  h1_destroy()
>  {
> +	if ! [[ $h1_created = yes ]]; then
> +		return
> +	fi
> +
>  	ethtool --set-mm $h1 pmac-enabled off tx-enabled off verify-enabled off
>
>  	tc qdisc del dev $h1 root
> @@ -249,6 +257,10 @@ h1_destroy()
>
>  h2_destroy()
>  {
> +	if ! [[ $h2_created = yes ]]; then
> +		return
> +	fi
> +
>  	tc qdisc del dev $h2 root
>
>  	ethtool --set-mm $h2 pmac-enabled off tx-enabled off verify-enabled off
> @@ -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 tho hX_created
business.

>  	h1_create
>  	h2_create
>  }