| Message ID | 20230406-mptcp-issue-368-selftests-old-kernels-v2-28-50313e4f83ab@tessares.net (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Paolo Abeni |
| Headers | show |
| Series | selftests: mptcp: skip tests when features are not supported | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/build | success | Build and static analysis OK |
| matttbe/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 35 lines checked |
| matttbe/KVM_Validation__normal__except_selftest_mptcp_join_ | warning | Unstable: 1 failed test(s): packetdrill_fastopen |
| matttbe/KVM_Validation__debug__except_selftest_mptcp_join_ | warning | Unstable: 2 failed test(s): packetdrill_fastopen selftest_diag |
| matttbe/KVM_Validation__debug__only_selftest_mptcp_join_ | success | Success! ✅ |
| matttbe/KVM_Validation__normal__only_selftest_mptcp_join_ | success | Success! ✅ |
On Mon, 2023-05-22 at 18:38 +0200, Matthieu Baerts wrote: > Selftests are supposed to run on any kernels, including the old ones not > supporting all MPTCP features. > > One of them is the support of MP_FASTCLOSE introduced in commit > f284c0c77321 ("mptcp: implement fastclose xmit path"). > > If the MIB counter is not available, the test cannot be verified and the > behaviour will not be the expected one. So we can skip the test if the > counter is missing. > > Link: https://github.com/multipath-tcp/mptcp_net-next/issues/368 > Fixes: 01542c9bf9ab ("selftests: mptcp: add fastclose testcase") > Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net> > --- > tools/testing/selftests/net/mptcp/mptcp_join.sh | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh > index 1f24495308f9..ccf52aba8a1c 100755 > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > @@ -263,6 +263,19 @@ reset() > return 0 > } > > +# $1: test name ; $2: counter to check > +reset_check_counter() > +{ > + reset "${1}" || return 1 > + > + local counter="${2}" > + > + if ! nstat -asz "${counter}" | grep -wq "${counter}"; then > + mark_as_skipped "counter '${counter}' is not available" > + return 1 > + fi > +} > + > # $1: test name > reset_with_cookies() > { > @@ -3179,14 +3192,14 @@ fullmesh_tests() > > fastclose_tests() > { > - if reset "fastclose test"; then > + if reset_check_counter "fastclose test" "MPTcpExtMPFastcloseTx"; then > run_tests $ns1 $ns2 10.0.1.1 1024 0 fastclose_client > chk_join_nr 0 0 0 > chk_fclose_nr 1 1 > chk_rst_nr 1 1 invert > fi > > - if reset "fastclose server test"; then > + if reset_check_counter "fastclose server test" "MPTcpExtMPFastcloseRx"; then > run_tests $ns1 $ns2 10.0.1.1 1024 0 fastclose_server > chk_join_nr 0 0 0 > chk_fclose_nr 1 1 invert > Here and in a few later patches, I would do the 'feature check' only once at the beginning of the functional block ('fastclose_test' in this case), in the most conservative way possible. The goal would be minimize the conflicts caused by present/absence of such patch. /P
Hi Paolo, On 22/05/2023 19:45, Paolo Abeni wrote: > On Mon, 2023-05-22 at 18:38 +0200, Matthieu Baerts wrote: >> Selftests are supposed to run on any kernels, including the old ones not >> supporting all MPTCP features. >> >> One of them is the support of MP_FASTCLOSE introduced in commit >> f284c0c77321 ("mptcp: implement fastclose xmit path"). >> >> If the MIB counter is not available, the test cannot be verified and the >> behaviour will not be the expected one. So we can skip the test if the >> counter is missing. >> >> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/368 >> Fixes: 01542c9bf9ab ("selftests: mptcp: add fastclose testcase") >> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net> >> --- >> tools/testing/selftests/net/mptcp/mptcp_join.sh | 17 +++++++++++++++-- >> 1 file changed, 15 insertions(+), 2 deletions(-) >> >> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh >> index 1f24495308f9..ccf52aba8a1c 100755 >> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh >> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh >> @@ -263,6 +263,19 @@ reset() >> return 0 >> } >> >> +# $1: test name ; $2: counter to check >> +reset_check_counter() >> +{ >> + reset "${1}" || return 1 >> + >> + local counter="${2}" >> + >> + if ! nstat -asz "${counter}" | grep -wq "${counter}"; then >> + mark_as_skipped "counter '${counter}' is not available" >> + return 1 >> + fi >> +} >> + >> # $1: test name >> reset_with_cookies() >> { >> @@ -3179,14 +3192,14 @@ fullmesh_tests() >> >> fastclose_tests() >> { >> - if reset "fastclose test"; then >> + if reset_check_counter "fastclose test" "MPTcpExtMPFastcloseTx"; then >> run_tests $ns1 $ns2 10.0.1.1 1024 0 fastclose_client >> chk_join_nr 0 0 0 >> chk_fclose_nr 1 1 >> chk_rst_nr 1 1 invert >> fi >> >> - if reset "fastclose server test"; then >> + if reset_check_counter "fastclose server test" "MPTcpExtMPFastcloseRx"; then >> run_tests $ns1 $ns2 10.0.1.1 1024 0 fastclose_server >> chk_join_nr 0 0 0 >> chk_fclose_nr 1 1 invert >> > > Here and in a few later patches, I would do the 'feature check' only > once at the beginning of the functional block ('fastclose_test' in this > case), in the most conservative way possible. The goal would be > minimize the conflicts caused by present/absence of such patch. I initially did that but then it breaks the feature to start a specific test. Also, the ID of the tests would be different on the different kernels which might make the tracking of the different issues more difficult. On older kernels, we already have conflicts because of all the "if reset (...)" I introduced one or two years ago :) But likely, we don't need to backport such checks to know if a test can be started or not too far: as long as we can backport such checks to the last stable kernels, that's enough for the different CIs using the last stable version of the kselftests on older kernels. Cheers, Matt
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index 1f24495308f9..ccf52aba8a1c 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -263,6 +263,19 @@ reset() return 0 } +# $1: test name ; $2: counter to check +reset_check_counter() +{ + reset "${1}" || return 1 + + local counter="${2}" + + if ! nstat -asz "${counter}" | grep -wq "${counter}"; then + mark_as_skipped "counter '${counter}' is not available" + return 1 + fi +} + # $1: test name reset_with_cookies() { @@ -3179,14 +3192,14 @@ fullmesh_tests() fastclose_tests() { - if reset "fastclose test"; then + if reset_check_counter "fastclose test" "MPTcpExtMPFastcloseTx"; then run_tests $ns1 $ns2 10.0.1.1 1024 0 fastclose_client chk_join_nr 0 0 0 chk_fclose_nr 1 1 chk_rst_nr 1 1 invert fi - if reset "fastclose server test"; then + if reset_check_counter "fastclose server test" "MPTcpExtMPFastcloseRx"; then run_tests $ns1 $ns2 10.0.1.1 1024 0 fastclose_server chk_join_nr 0 0 0 chk_fclose_nr 1 1 invert
Selftests are supposed to run on any kernels, including the old ones not supporting all MPTCP features. One of them is the support of MP_FASTCLOSE introduced in commit f284c0c77321 ("mptcp: implement fastclose xmit path"). If the MIB counter is not available, the test cannot be verified and the behaviour will not be the expected one. So we can skip the test if the counter is missing. Link: https://github.com/multipath-tcp/mptcp_net-next/issues/368 Fixes: 01542c9bf9ab ("selftests: mptcp: add fastclose testcase") Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net> --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)