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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAL_-bo0QJJJootMQysNSLNmu0Fps3dqjPE0F0_=R23h7GqAkfQ@mail.gmail.com>
Date: Mon, 16 Sep 2024 11:18:13 +0200
From: Ales Nezbeda <anezbeda@...hat.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org, sdf@...ichev.me, sd@...asysnail.net, 
	davem@...emloft.net
Subject: Re: [PATCH net v2] selftests: rtnetlink: add 'ethtool' as a dependency

On Tue, Sep 10, 2024 at 2:17 AM Jakub Kicinski <kuba@...nel.org> wrote:
> Don't think it qualifies as a fix, it's an improvement.

Well, if the `ethtool` is not present the test will fail with dubious
results that would indicate that the test failed due to the system not
supporting MACsec offload, which is not true. The error message
doesn't reflect what went wrong, so I thought about it as an issue
that is being fixed. If this is not the case please let me know,
because based on the docs 'Fixes: tag indicates that the patch fixes
an issue', which I would think that wrong error message is an issue. I
might be wrong here though, so if the definition of 'issue' is more
restrictive I can remove the 'Fixes:' tag.

> You can use net/forwarding's lib.sh in net/, altnames.sh already
> uses it.

I see, the problem (and probably should have mentioned it in the patch
itself) is that `rtnetlink.sh` is using one of the variables defined
in the `net/lib.sh` - specifically `ksft_skip`. Furthermore, I felt
like a more clean approach would be to add the `require_command()` to
the `net/lib.sh` so that other tests down the road could potentially
use it as well. Picking a different `lib.sh` would mean either
modifying the `rtnetlink.sh` around it (removing the `ksft_skip` - lot
of changes in unrelated parts and harder for review) or adding it to
the new `lib.sh` and at that point we are adding stuff to the lib
anyway.