Received-SPF: pass (google.com: domain of linux-kernel+bounces-214194-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 13 Jun 2024 18:49:38 -0700
From: Jakub Kicinski <kuba@kernel.org>
To: Abhinav Jain <jain.abhinav177@gmail.com>
Cc: horms@kernel.org, davem@davemloft.net, edumazet@google.com,
 pabeni@redhat.com, shuah@kernel.org, netdev@vger.kernel.org,
 linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
 skhan@linuxfoundation.org, javier.carrasco.cruz@gmail.com
Subject: Re: [PATCH v2] sefltests: net: Add on/off checks for network
 interface non fixed features
Message-ID: <20240613184938.0cc8cc56@kernel.org>
In-Reply-To: <20240609132124.51683-1-jain.abhinav177@gmail.com>
References: <20240609132124.51683-1-jain.abhinav177@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Sun,  9 Jun 2024 13:21:24 +0000 Abhinav Jain wrote:
> This patch addresses the TODO (add non fixed feature on/off check).
> I have tested it manually on my system and made changes as suggested in v1

>  	echo "PASS: $netdev: ethtool list features"
> -	#TODO for each non fixed features, try to turn them on/off
> +
> +	while read -r FEATURE VALUE FIXED; do
> +		[ "$FEATURE" != "Features" ] || continue # Skip "Features" line
> +		[ "$FIXED" != "[fixed]" ] || continue # Skip fixed features
> +		feature = "${FEATURE%:*}"
> +
> +		ethtool --offload "$netdev" "$feature" off
> +		if [ $? -eq 0 ]; then
> +			echo "PASS: $netdev: Turned off feature: $feature"
> +		else
> +			echo "FAIL: $netdev: Failed to turn off feature: $feature"
> +		fi
> +
> +		ethtool --offload "$netdev" "$feature" on

You do off then on, so you assume the feature was on to begin with.
Not all features will be on. You gotta change the order based on
the initial state so that the feature goes back to what it was.

> +		if [ $? -eq 0 ]; then
> +			echo "PASS: $netdev: Turned on feature: $feature"
> +		else
> +			echo "FAIL: $netdev: Failed to turn on feature: $feature"
> +		fi
> +	done < "$TMP_ETHTOOL_FEATURES"

More importantly, tho, currently in our CI this test passes:

https://netdev.bots.linux.dev/contest.html?test=netdevice-sh

But it doesn't do anything:

https://netdev-3.bots.linux.dev/vmksft-net/results/638724/98-netdevice-sh/stdout
https://netdev-3.bots.linux.dev/vmksft-net-dbg/results/638544/90-netdevice-sh/stdout

Because I'm guessing the VM does not have any interface to test.

Before we extend the test could you please make the test automatically
add a veth or netdevsim device if there is no testable device on the
system? Make sure you remove it before exiting.

You can look at tests in tools/testing/selftests/drivers/net/netdevsim
for inspiration and library scripts which can be reused.