[PATCH net] selftests: rtnetlink: add 'ethtool' as a dependency

[PATCH net] selftests: rtnetlink: add 'ethtool' as a dependency

[Ales Nezbeda]

Introduction of `kci_test_macsec_offload()` in `rtnetlink.sh` created
a new dependency `ethtool` for the test suite. This dependency is not
reflected in checks that run before all the tests, so if the `ethtool`
is not present, all tests will start, but `macsec_offload` test will
fail with a misleading message. Message would say that MACsec offload is
not supported, yet it never actually managed to check this, since it
needs the `ethtool` to do so.

Fixes: 3b5222e2ac57 ("selftests: rtnetlink: add MACsec offload tests")
Signed-off-by: Ales Nezbeda <anezbeda@redhat.com>
---
 tools/testing/selftests/net/rtnetlink.sh | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index bdf6f10d0558..fdd116458222 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -1306,6 +1306,7 @@ if [ "$(id -u)" -ne 0 ];then
 	exit $ksft_skip
 fi
 
+#check for dependencies
 for x in ip tc;do
 	$x -Version 2>/dev/null >/dev/null
 	if [ $? -ne 0 ];then
@@ -1313,6 +1314,11 @@ for x in ip tc;do
 		exit $ksft_skip
 	fi
 done
+ethtool --version 2>/dev/null >/dev/null
+if [ $? -ne 0 ];then
+	end_test "SKIP: Could not run test without the ethtool tool"
+	exit $ksft_skip
+fi
 
 while getopts t:hvpP o; do
 	case $o in
-- 
2.46.0

Re: [PATCH net] selftests: rtnetlink: add 'ethtool' as a dependency

[Stanislav Fomichev]

On 09/03, Ales Nezbeda wrote:
> Introduction of `kci_test_macsec_offload()` in `rtnetlink.sh` created
> a new dependency `ethtool` for the test suite. This dependency is not
> reflected in checks that run before all the tests, so if the `ethtool`
> is not present, all tests will start, but `macsec_offload` test will
> fail with a misleading message. Message would say that MACsec offload is
> not supported, yet it never actually managed to check this, since it
> needs the `ethtool` to do so.
> 
> Fixes: 3b5222e2ac57 ("selftests: rtnetlink: add MACsec offload tests")
> Signed-off-by: Ales Nezbeda <anezbeda@redhat.com>
> ---
>  tools/testing/selftests/net/rtnetlink.sh | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
> index bdf6f10d0558..fdd116458222 100755
> --- a/tools/testing/selftests/net/rtnetlink.sh
> +++ b/tools/testing/selftests/net/rtnetlink.sh
> @@ -1306,6 +1306,7 @@ if [ "$(id -u)" -ne 0 ];then
>  	exit $ksft_skip
>  fi
>  
> +#check for dependencies
>  for x in ip tc;do
>  	$x -Version 2>/dev/null >/dev/null
>  	if [ $? -ne 0 ];then
> @@ -1313,6 +1314,11 @@ for x in ip tc;do
>  		exit $ksft_skip
>  	fi
>  done
> +ethtool --version 2>/dev/null >/dev/null
> +if [ $? -ne 0 ];then
> +	end_test "SKIP: Could not run test without the ethtool tool"
> +	exit $ksft_skip
> +fi

Can we use a 'require_command ethtool' (lib.sh helper) instead?

Re: [PATCH net] selftests: rtnetlink: add 'ethtool' as a dependency

[Ales Nezbeda]

On Tue, Sep 3, 2024 at 10:28 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> Can we use a 'require_command ethtool' (lib.sh helper) instead?

Hi, I apologize for not responding so long, I have looked at the
function from the `lib.sh` helper you mentioned. Thanks for pointing
it out, it is a much cleaner solution and would resolve the need to
test 'ip' and 'tc' differently from the 'ethtool'. It seems that it is
currently not present in the `lib.sh` file that is used in the
`rtnetlink.sh` that is modified by the patch.

There are multiple versions of `lib.sh` file, and the
`require_command` function is defined in only two of them. It looks
like some of the tests are reusing the `lib.sh` file that does contain
the `require_command` function from other tests (like
`selftests/drivers/net/netdevsim/psample.sh`), but I feel like adding
the `require_command` function to the currently used `lib.sh` file is
a cleaner solution.

I will create a new patch that will add the `require_command` to
`lib.sh` file and rewrite the check in `rtnetlink.sh` to use that
function.