tools/testing/selftests/net/mptcp/mptcp_lib.sh | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
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")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
tools/testing/selftests/net/mptcp/mptcp_lib.sh | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 556a7d9784d7..e23fd85902d8 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -55,11 +55,13 @@ mptcp_lib_expect_all_features() {
[ "${SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES:-}" = "1" ]
}
-# $1: msg
+# $1: msg $2: continue or not
mptcp_lib_fail_if_expected_feature() {
+ local continue="${2:-0}"
+
if mptcp_lib_expect_all_features; then
- echo "ERROR: missing feature: ${*}"
- exit ${KSFT_FAIL}
+ echo "ERROR: missing feature: ${1}"
+ [ "${continue}" -eq 0 ] && exit ${KSFT_FAIL}
fi
return 1
@@ -107,7 +109,7 @@ mptcp_lib_kallsyms_has() {
return 0
fi
- mptcp_lib_fail_if_expected_feature "${sym} symbol not found"
+ mptcp_lib_fail_if_expected_feature "${sym} symbol not found" 1
}
# $1: part of a symbol to look at, add '$' at the end for full name
--
2.40.1Hi 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 :)
The fixes commit ('d' is missing at the beginning) here mentions:
> Note that this check can also
> mark the test as failed if 'SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES' env
> var is set to 1: by doing that, we can make sure a test is not being
> skipped by mistake.
So the goal is to fail when the env var is set. Otherwise, we would miss
tests that are skipped by accident.
If you use 'mptcp-upstream-virtme-docker' and you want to run recent
selftests on an older kernel, you can add this to the docker command not
to exit if a feature is missing:
-e INPUT_SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES=0
But clearly here, we don't want to continue if we expect all features to
work!
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
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.
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.
Thanks,
-Geliang
>
> The fixes commit ('d' is missing at the beginning) here mentions:
>
> > Note that this check can also
> > mark the test as failed if 'SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES' env
> > var is set to 1: by doing that, we can make sure a test is not being
> > skipped by mistake.
>
> So the goal is to fail when the env var is set. Otherwise, we would miss
> tests that are skipped by accident.
>
> If you use 'mptcp-upstream-virtme-docker' and you want to run recent
> selftests on an older kernel, you can add this to the docker command not
> to exit if a feature is missing:
>
> -e INPUT_SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES=0
>
> But clearly here, we don't want to continue if we expect all features to
> work!
>
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
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.
Hi Matt,
On Thu, Feb 22, 2024 at 11:49:18AM +0100, Matthieu Baerts wrote:
> 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
Thanks for your explanation, it took me some time to understand it. Yes, we
can use SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES to control whether exit or
not. Let's drop this patch.
-Geliang
>
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
© 2016 - 2024 Red Hat, Inc.