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: <87wm9bu13q.fsf@nvidia.com>
Date: Mon, 16 Jun 2025 17:06:00 +0200
From: Petr Machata <petrm@...dia.com>
To: Matthieu Baerts <matttbe@...nel.org>
CC: Jakub Kicinski <kuba@...nel.org>, Petr Machata <petrm@...dia.com>, "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


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.

> Note: regarding the other issue you have:
>
>> In vxlan_bridge_1q_mc_ul.sh line 766:
>> setup_wait
>> ^--------^ SC2119 (info): Use setup_wait "$@" if function's $1 should mean script's $1.
>
> I guess you can also ignore it, or use "" as argument. If you ignore it
> -- which looks cleaner -- I think it is always good to add a comment, e.g.
>
>> # shellcheck disable=SC2119  # arguments are optional, not needed here.
>> setup_wait

I'll just pass "$NUM_NETIFS" to get rid of the warning. I really don't
like these shellcheck annotations.

I'll send a patch that changes the calling convention so that SC doesn't
get triggered, because I really don't want every new selftest to have to
deal with this.