| Message ID | 20240903151524.586614-1-anezbeda@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] selftests: rtnetlink: add 'ethtool' as a dependency | expand |
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?
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.
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
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(+)