Matthieu Baerts [off-list ref] writes:

Hi Jakub, Petr,

On 13/06/2025 18:57, Jakub Kicinski wrote:

quoted

On Thu, 12 Jun 2025 22:10:48 +0200 Petr Machata wrote:

quoted

Add tests for MC-routing underlay VXLAN traffic.

Signed-off-by: Petr Machata <petrm@nvidia.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:

quoted

# 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:

quoted

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.

quoted

# 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.