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: <be7149b5-0286-4b39-b6ce-809618354b13@kernel.org>
Date: Mon, 16 Jun 2025 22:40:49 +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

On 16/06/2025 21:53, Petr Machata wrote:
> 
> Matthieu Baerts <matttbe@...nel.org> writes:
> 
>> Hi Petr,
>>
>> On 16/06/2025 17:06, Petr Machata wrote:
>>>
>>> Matthieu Baerts <matttbe@...nel.org> writes:
> 
>>>> 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?
> 
> Good point. The question then is whether to put it to forwarding/ or
> directly net/, which is has seen more use of lib.sh and therefore the
> same sort of coding style. I'll experiment with it and should be able to
> send it later in the week. I don't want to add it to the MC patchset.

Since Jakub disabled SC2317 in NIPA [1], then I guess we can put this
.shellcheckrc file in net/, 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.
> 
> Yeah, I know, but the result is still very noisy, if you want to verify
> it prior to sending the patchset. It's a bit annoying to have to scroll
> through the report trying to find relevant stuff. I could add
> .shellcheckrc in my own clone, but everybody is going to hit these.

When Shellcheck got introduced, there were some discussions about SC2317
and SC2086 [2]. I don't think they are useless -- maybe a function is
not used by mistake, maybe double quotes are really needed for some
cases, etc. -- but I agree with you: they create a lot of noise.

[1] https://github.com/linux-netdev/nipa/commit/23b74dd
[2] https://github.com/linux-netdev/nipa/pull/51#issuecomment-2939556291

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