Re: [PATCH mptcp-net] selftests: mptcp: don't exit when a symbol not found

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 22/02/2024 11:31 am, Geliang Tang wrote:
> On Thu, Feb 22, 2024 at 09:58:39AM +0100, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 22/02/2024 8:56 am, Geliang Tang wrote:
>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>
>>> mptcp_lib_kallsyms_has() will always exit when a symbol has not found, it
>>> breaks the test itself. Unexpected errors occur:
>>>
>>>  007 userspace pm add & remove address
>>>        syn                                 [ OK ]
>>>        synack                              [ OK ]
>>>        ack                                 [ OK ]
>>>        add                                 [ OK ]
>>>        echo                                [ OK ]
>>>        mptcp_info subflows=2:2             [ OK ]
>>>        mptcp_info subflows_total=3:3       [ OK ]
>>>        mptcp_info add_addr_signal=2:2      [ OK ]
>>>        mptcp_info last_data_sent=191:18    [ OK ]
>>>        mptcp_info last_data_recv=40:68     [ OK ]
>>>        mptcp_info last_ack_recv=93:74      [ OK ]
>>>        dump addrs signal                   ERROR: missing feature: \
>>> 			mptcp_userspace_pm_dump_addr$ symbol not found
>>>  Cannot open network namespace "ns1-65d6fb5a-5FviVK": \
>>> 			No such file or directory
>>>  Cannot open network namespace "ns2-65d6fb5a-5FviVK": \
>>> 			No such file or directory
>>>  cmp: /tmp/tmp.a7osJs7Nj0: No such file or directory
>>>  cmp: /tmp/tmp.f02z6brCQu: No such file or directory
>>>  cat: /tmp/tmp.27TzxD2efV: No such file or directory
>>> not ok 1 test: selftest_mptcp_join # FAIL
>>>
>>> To fix this, this patch adds a new argument 'continue' for the helper
>>> mptcp_lib_fail_if_expected_feature() to control whether exit or not.
>>>
>>> Always set this argument to 1 in mptcp_lib_kallsyms_has().
>>>
>>> Fixes: 83013bdf90a ("selftests: mptcp: connect: skip if MPTCP is not supported")
>>
>> I'm not sure to understand why you sent this :)
> 
> Say in the test "signal addresses race test":
> 
>                 if ! mptcp_lib_kallsyms_has "mptcp_pm_subflow_check_next$"; then
>                         chk_join_nr 3 3 2
>                         chk_add_nr 4 4
>                 else
>                         chk_join_nr 3 3 3
>                         # the server will not signal the address terminating
>                         # the MPC subflow
>                         chk_add_nr 3 3
>                 fi
> 
> This code will never run to this block:
> 
>                         chk_join_nr 3 3 2
>                         chk_add_nr 4 4
> 
> Since if no symbal of mptcp_pm_subflow_check_next, mptcp_lib_kallsyms_has exit.

This block will never be executed in our CI, because our CI always run
the selftests linked to the same version of the kernel. In this case,
that would not be normal to execute this block in our CI, it has to
complain because we expect to have this symbol in the latest version.

Other CIs, like LKFT, which are validating stable kernels (e.g. 5.15.x)
using the selftests from the last stable kernel (e.g. 6.7.x) will not
have SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES env var being set to 1 as
we did in our CI. Then it is fine, tests and checks will be skipped if
some features are missing:

https://tuxapi.tuxsuite.com/v1/groups/linaro/projects/lkft/tests/2cJxC6Sua3VVzRZmpQYKK2jzzNN/logs?format=html

> You can trigger this by renaming "mptcp_pm_subflow_check_next$" to
> "no_mptcp_pm_subflow_check_next$", an non-exist function name, for test.

I confirm that if you use 'mptcp-upstream-virtme-docker', this env var
will be set to 1 by default:

  SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES=1

You can unset it on your side, but it only makes sense to do that if you
work on stable kernels (and you use selftests from a more recent
version, not the ones attached to the kernel you are validating).

So yes, in this environment (where we force setting the env var to 1,
not the "default" behaviour), picking a non-existing function name will
cause the test to fail: but that's what we want → we want to be notified
if we picked a non-existing function, like it happened recently:

  https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7988706773

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