netdev - Re: [PATCH net-next v2 14/14] selftests: forwarding: Add a test for verifying VXLAN MC underlay

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <426a2c83-38ca-4fa2-9270-b3e600e30d19@kernel.org>
Date: Mon, 16 Jun 2025 18:18:18 +0200
From: Matthieu Baerts <matttbe@...nel.org>
To: Petr Machata <petrm@...dia.com>
Cc: Jakub Kicinski <kuba@...nel.org>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Paolo Abeni <pabeni@...hat.com>, David Ahern <dsahern@...il.com>,
 netdev@...r.kernel.org, Simon Horman <horms@...nel.org>,
 Nikolay Aleksandrov <razor@...ckwall.org>, Ido Schimmel <idosch@...dia.com>,
 mlxsw@...dia.com, Shuah Khan <shuah@...nel.org>,
 linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next v2 14/14] selftests: forwarding: Add a test for
 verifying VXLAN MC underlay

Hi Petr,

On 16/06/2025 17:06, Petr Machata wrote:
> 
> Matthieu Baerts <matttbe@...nel.org> writes:
> 
>> Hi Jakub, Petr,
>>
>> On 13/06/2025 18:57, Jakub Kicinski wrote:
>>> On Thu, 12 Jun 2025 22:10:48 +0200 Petr Machata wrote:
>>>> Add tests for MC-routing underlay VXLAN traffic.
>>>>
>>>> Signed-off-by: Petr Machata <petrm@...dia.com>
>>>> ---
>>>>
>>>> Notes:
>>>>     v2:
>>>>     - Adjust as per shellcheck citations
>>>
>>> Noob question - would we also be able to squash the unreachable code
>>> warnings if we declared ALL_TESTS as an array instead of a string?
>>> IDK if there's any trick we could use to make shellcheck stop
>>> complaining. Not blocking the series, obviously.
>>>
>>> CC Matthieu, I presume you may have already investigated this :)
>>
>> Thank you for the Cc. Yes indeed, I already had this case.
>>
>> I don't think declaring ALL_TESTS as an array would help for this case
>> -- even if it looks clearer than a long string -- because I think
>> shellcheck will simply check if all the different functions are called
>> directly. As mentioned in Shellcheck wiki [1]: "ShellCheck may
>> incorrectly believe that code is unreachable if it's invoked by variable
>> name or in a trap. In such a case, please Ignore the message".
>>
>> That what we did with MPTCP, see the top of the mptcp_join.sh file for
>> example [2], where we have:
>>
>>> # ShellCheck incorrectly believes that most of the code here is unreachable
>>> # because it's invoked by variable name, see how the "tests" array is used
>>> #shellcheck disable=SC2317
>>
>> If you add this at the top of your new file, followed by an empty line,
>> shellcheck will ignore this issue for the whole file.
> 
> The ALL_TESTS issue is not the end of it either. We use helpers that
> call stuff indirectly all over the place. defer, in_ns... It would make
> sense to me to just disable SC2317 in NIPA runs. Or maybe even put it in
> net/forwarding/.shellcheckrc. Pretty much all those tests are written in
> a style that will hit these issues.

In this case, I think it would be better to add this .shellcheckrc file
in net/forwarding. If you modify NIPA, I don't think people will know
what is allowed or not, or what command line to use, no?

Note that NIPA's shellcheck reports are currently ignoring all "info"
and "style" severities -- so including SC2317 -- except for new files or
the ones that were previously shellcheck compliant.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.