* [PATCH mptcp-next v2 0/2] selftests: check fallback mib counter @ 2025-08-14 3:16 Gang Yan 2025-08-14 3:16 ` [PATCH mptcp-next v2 1/2] selftests: net: mptcp: add checks for fallback counters Gang Yan ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Gang Yan @ 2025-08-14 3:16 UTC (permalink / raw) To: mptcp; +Cc: Gang Yan These patches can add a verification step to check the expected behavior of these MIB counters during the test execution. Link: https://github.com/multipath-tcp/mptcp_net-next/issues/571 ------ Changelog v2: - add a helper which can check all the fallback mib counters. - put the 'chk_fallback_nr' in 'chk_join_nr' like chk_join_tx_nr. Gang Yan (2): selftests: net: mptcp: add checks for fallback counters selftests: net: mptcp: add fallback checks in chk_join_nr .../testing/selftests/net/mptcp/mptcp_join.sh | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) -- 2.43.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH mptcp-next v2 1/2] selftests: net: mptcp: add checks for fallback counters 2025-08-14 3:16 [PATCH mptcp-next v2 0/2] selftests: check fallback mib counter Gang Yan @ 2025-08-14 3:16 ` Gang Yan 2025-08-14 14:35 ` Matthieu Baerts 2025-08-14 3:16 ` [PATCH mptcp-next v2 2/2] selftests: net: mptcp: add fallback checks in chk_join_nr Gang Yan 2025-08-14 5:09 ` [PATCH mptcp-next v2 0/2] selftests: check fallback mib counter MPTCP CI 2 siblings, 1 reply; 6+ messages in thread From: Gang Yan @ 2025-08-14 3:16 UTC (permalink / raw) To: mptcp; +Cc: Gang Yan Recently, some mib counters about fallback has been added, this patch provides a helper to check the expected behavior of these mib counters during the test execution. Link: https://github.com/multipath-tcp/mptcp_net-next/issues/571 Signed-off-by: Gang Yan <yangang@kylinos.cn> --- .../testing/selftests/net/mptcp/mptcp_join.sh | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index b8af65373b3a..2c1f0704bc17 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -74,6 +74,15 @@ unset join_create_err unset join_bind_err unset join_connect_err +unset infinite_map_tx_fb +unset dss_corruption_fb +unset simult_conn_fb +unset mpc_passive_fb +unset mpc_active_fb +unset mpc_data_fb +unset MD5_Sig_fb +unset dss_fb + # generated using "nfbpf_compile '(ip && (ip[54] & 0xf0) == 0x30) || # (ip6 && (ip6[74] & 0xf0) == 0x30)'" CBPF_MPTCP_SUBOPTION_ADD_ADDR="14, @@ -1399,6 +1408,95 @@ chk_join_tx_nr() print_results "join Tx" ${rc} } +chk_fallback_nr() +{ + local infinite_map_tx=${infinite_map_tx_fb:-0} + local dss_corruption=${dss_corruption_fb:-0} + local simult_conn=${simult_conn_fb:-0} + local mpc_passive=${mpc_passive_fb:-0} + local mpc_active=${mpc_active_fb:-0} + local mpc_data=${mpc_data_fb:-0} + local MD5_Sig=${MD5_Sig_fb:-0} + local dss=${dss_fb:-0} + local rc=${KSFT_PASS} + local ns=$1 + local count + + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtMPCapableFallbackACK") + if [ -z "$count" ]; then + rc=${KSFT_SKIP} + elif [ "$count" != "$mpc_passive" ]; then + rc=${KSFT_FAIL} + print_check "mpc passive fallback " + fail_test "got $count mpc passive fallback[s] expected $mpc_passive" + fi + + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtMPCapableFallbackSYNACK") + if [ -z "$count" ]; then + rc=${KSFT_SKIP} + elif [ "$count" != "$mpc_active" ]; then + rc=${KSFT_FAIL} + print_check "mpc active fallback " + fail_test "got $count mpc active fallback[s] expected $mpc_active" + fi + + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtDSSCorruptionFallback") + if [ -z "$count" ]; then + rc=${KSFT_SKIP} + elif [ "$count" != "$dss_corruption" ]; then + rc=${KSFT_FAIL} + print_check "dss corruption fallback " + fail_test "got $count dss corruption fallback[s] expected $dss_corruption" + fi + + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtInfiniteMapTx") + if [ -z "$count" ]; then + rc=${KSFT_SKIP} + elif [ "$count" != "$infinite_map_tx" ]; then + rc=${KSFT_FAIL} + print_check "infinite map tx fallback " + fail_test "got $count infinite map tx fallback[s] expected $infinite_map_tx" + fi + + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtMPCapableDataFallback") + if [ -z "$count" ]; then + rc=${KSFT_SKIP} + elif [ "$count" != "$mpc_data" ]; then + rc=${KSFT_FAIL} + print_check "mpc data fallback " + fail_test "got $count mpc data fallback[s] expected $mpc_data" + fi + + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtMD5SigFallback") + if [ -z "$count" ]; then + rc=${KSFT_SKIP} + elif [ "$count" != "$MD5_Sig" ]; then + rc=${KSFT_FAIL} + print_check "MD5 Sig fallback " + fail_test "got $count MD5 Sig fallback[s] expected $MD5_Sig" + fi + + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtDssFallback") + if [ -z "$count" ]; then + rc=${KSFT_SKIP} + elif [ "$count" != "$dss" ]; then + rc=${KSFT_FAIL} + print_check "dss fallback " + fail_test "got $count dss fallback[s] expected $dss" + fi + + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtSimultConnectFallback") + if [ -z "$count" ]; then + rc=${KSFT_SKIP} + elif [ "$count" != "$simult_conn" ]; then + rc=${KSFT_FAIL} + print_check "simult conn fallback " + fail_test "got $count simult conn fallback[s] expected $simult_conn" + fi + + print_results "check fallback nr" ${rc} +} + chk_join_nr() { local syn_nr=$1 -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH mptcp-next v2 1/2] selftests: net: mptcp: add checks for fallback counters 2025-08-14 3:16 ` [PATCH mptcp-next v2 1/2] selftests: net: mptcp: add checks for fallback counters Gang Yan @ 2025-08-14 14:35 ` Matthieu Baerts 0 siblings, 0 replies; 6+ messages in thread From: Matthieu Baerts @ 2025-08-14 14:35 UTC (permalink / raw) To: Gang Yan, mptcp Hi Gang, On 14/08/2025 05:16, Gang Yan wrote: > Recently, some mib counters about fallback has been added, this patch > provides a helper to check the expected behavior of these mib counters > during the test execution. Thank you for the patch. But alone, it doesn't make sense, and it is harder to review without knowing how the helper is going to be used. Better to squash the two patches together. I think having patches dedicated to the introduction of new helpers only make sense when such helpers will be used on another subsystem (different maintainers) or when moving existing code to a new helper to be re-used elsewhere. But here, I think it causes more troubles. > > Link: https://github.com/multipath-tcp/mptcp_net-next/issues/571 > Signed-off-by: Gang Yan <yangang@kylinos.cn> > --- > .../testing/selftests/net/mptcp/mptcp_join.sh | 98 +++++++++++++++++++ > 1 file changed, 98 insertions(+) > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh > index b8af65373b3a..2c1f0704bc17 100755 > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > @@ -74,6 +74,15 @@ unset join_create_err > unset join_bind_err > unset join_connect_err > > +unset infinite_map_tx_fb > +unset dss_corruption_fb > +unset simult_conn_fb > +unset mpc_passive_fb > +unset mpc_active_fb > +unset mpc_data_fb > +unset MD5_Sig_fb No need to use capital letters here. > +unset dss_fb It might make more sense to use the "fb_" prefix, instead of the suffix, similar to what has been done with "join_" above. > + > # generated using "nfbpf_compile '(ip && (ip[54] & 0xf0) == 0x30) || > # (ip6 && (ip6[74] & 0xf0) == 0x30)'" > CBPF_MPTCP_SUBOPTION_ADD_ADDR="14, > @@ -1399,6 +1408,95 @@ chk_join_tx_nr() > print_results "join Tx" ${rc} > } > > +chk_fallback_nr() > +{ > + local infinite_map_tx=${infinite_map_tx_fb:-0} > + local dss_corruption=${dss_corruption_fb:-0} > + local simult_conn=${simult_conn_fb:-0} > + local mpc_passive=${mpc_passive_fb:-0} > + local mpc_active=${mpc_active_fb:-0} > + local mpc_data=${mpc_data_fb:-0} > + local MD5_Sig=${MD5_Sig_fb:-0} > + local dss=${dss_fb:-0} No need to respect the reverse Xmas tree declaration for the variables here in bash. I think it is better here (and above with the unset) to follow the same order as below, when they are used, so it is easy to check if they are all used correctly. > + local rc=${KSFT_PASS} > + local ns=$1 > + local count > + > + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtMPCapableFallbackACK") > + if [ -z "$count" ]; then > + rc=${KSFT_SKIP} > + elif [ "$count" != "$mpc_passive" ]; then > + rc=${KSFT_FAIL} > + print_check "mpc passive fallback " All your "print_check" here and below have an extra whitespace before the last double quote: why? I don't think it is needed. > + fail_test "got $count mpc passive fallback[s] expected $mpc_passive" Because this helper is used twice, we need to know which direction had an issue: can you pass that info (connector/listener or ns2/ns1) as argument to this helper, and adding it to "print_check" and "fail_test"? Same below. > + fi > + > + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtMPCapableFallbackSYNACK") > + if [ -z "$count" ]; then > + rc=${KSFT_SKIP} > + elif [ "$count" != "$mpc_active" ]; then > + rc=${KSFT_FAIL} > + print_check "mpc active fallback " > + fail_test "got $count mpc active fallback[s] expected $mpc_active" > + fi > + > + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtDSSCorruptionFallback") > + if [ -z "$count" ]; then > + rc=${KSFT_SKIP} > + elif [ "$count" != "$dss_corruption" ]; then > + rc=${KSFT_FAIL} > + print_check "dss corruption fallback " > + fail_test "got $count dss corruption fallback[s] expected $dss_corruption" > + fi > + > + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtInfiniteMapTx") > + if [ -z "$count" ]; then > + rc=${KSFT_SKIP} > + elif [ "$count" != "$infinite_map_tx" ]; then > + rc=${KSFT_FAIL} > + print_check "infinite map tx fallback " > + fail_test "got $count infinite map tx fallback[s] expected $infinite_map_tx" > + fi > + > + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtMPCapableDataFallback") > + if [ -z "$count" ]; then > + rc=${KSFT_SKIP} > + elif [ "$count" != "$mpc_data" ]; then > + rc=${KSFT_FAIL} > + print_check "mpc data fallback " > + fail_test "got $count mpc data fallback[s] expected $mpc_data" > + fi > + > + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtMD5SigFallback") > + if [ -z "$count" ]; then > + rc=${KSFT_SKIP} > + elif [ "$count" != "$MD5_Sig" ]; then > + rc=${KSFT_FAIL} > + print_check "MD5 Sig fallback " > + fail_test "got $count MD5 Sig fallback[s] expected $MD5_Sig" > + fi > + > + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtDssFallback") > + if [ -z "$count" ]; then > + rc=${KSFT_SKIP} > + elif [ "$count" != "$dss" ]; then > + rc=${KSFT_FAIL} > + print_check "dss fallback " > + fail_test "got $count dss fallback[s] expected $dss" > + fi > + > + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtSimultConnectFallback") > + if [ -z "$count" ]; then > + rc=${KSFT_SKIP} > + elif [ "$count" != "$simult_conn" ]; then > + rc=${KSFT_FAIL} > + print_check "simult conn fallback " > + fail_test "got $count simult conn fallback[s] expected $simult_conn" > + fi > + > + print_results "check fallback nr" ${rc} Just "fallback"? EDIT: see the next patch: you should have a different message per netns. > +} > + > chk_join_nr() > { > local syn_nr=$1 Cheers, Matt -- Sponsored by the NGI0 Core fund. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH mptcp-next v2 2/2] selftests: net: mptcp: add fallback checks in chk_join_nr 2025-08-14 3:16 [PATCH mptcp-next v2 0/2] selftests: check fallback mib counter Gang Yan 2025-08-14 3:16 ` [PATCH mptcp-next v2 1/2] selftests: net: mptcp: add checks for fallback counters Gang Yan @ 2025-08-14 3:16 ` Gang Yan 2025-08-14 14:36 ` Matthieu Baerts 2025-08-14 5:09 ` [PATCH mptcp-next v2 0/2] selftests: check fallback mib counter MPTCP CI 2 siblings, 1 reply; 6+ messages in thread From: Gang Yan @ 2025-08-14 3:16 UTC (permalink / raw) To: mptcp; +Cc: Gang Yan This patch can track these counters in each namespace and check if they don't have unexpected values in tests. Signed-off-by: Gang Yan <yangang@kylinos.cn> --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index 2c1f0704bc17..ac9e9d4fed59 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -74,6 +74,8 @@ unset join_create_err unset join_bind_err unset join_connect_err +unset ns1_fb +unset ns2_fb unset infinite_map_tx_fb unset dss_corruption_fb unset simult_conn_fb @@ -1582,6 +1584,20 @@ chk_join_nr() join_syn_tx="${join_syn_tx:-${syn_nr}}" \ chk_join_tx_nr + if [[ -n "$ns1_fb" ]]; then + eval "$ns1_fb" \ + chk_fallback_nr ${ns1} + else + chk_fallback_nr ${ns1} + fi + + if [[ -n "$ns2_fb" ]]; then + eval "$ns2_fb" \ + chk_fallback_nr ${ns2} + else + chk_fallback_nr ${ns2} + fi + if $validate_checksum; then chk_csum_nr $csum_ns1 $csum_ns2 chk_fail_nr $fail_nr $fail_nr @@ -3435,6 +3451,7 @@ fail_tests() join_csum_ns1=+1 join_csum_ns2=+0 \ join_fail_nr=1 join_rst_nr=0 join_infi_nr=1 \ join_corrupted_pkts="$(pedit_action_pkts)" \ + ns1_fb="dss_fb=1" ns2_fb="infinite_map_tx_fb=1" \ chk_join_nr 0 0 0 chk_fail_nr 1 -1 invert fi -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH mptcp-next v2 2/2] selftests: net: mptcp: add fallback checks in chk_join_nr 2025-08-14 3:16 ` [PATCH mptcp-next v2 2/2] selftests: net: mptcp: add fallback checks in chk_join_nr Gang Yan @ 2025-08-14 14:36 ` Matthieu Baerts 0 siblings, 0 replies; 6+ messages in thread From: Matthieu Baerts @ 2025-08-14 14:36 UTC (permalink / raw) To: Gang Yan, mptcp On 14/08/2025 05:16, Gang Yan wrote: > This patch can track these counters in each namespace and check if they > don't have unexpected values in tests. > > Signed-off-by: Gang Yan <yangang@kylinos.cn> > --- > tools/testing/selftests/net/mptcp/mptcp_join.sh | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh > index 2c1f0704bc17..ac9e9d4fed59 100755 > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > @@ -74,6 +74,8 @@ unset join_create_err > unset join_bind_err > unset join_connect_err > > +unset ns1_fb > +unset ns2_fb > unset infinite_map_tx_fb > unset dss_corruption_fb > unset simult_conn_fb > @@ -1582,6 +1584,20 @@ chk_join_nr() > join_syn_tx="${join_syn_tx:-${syn_nr}}" \ > chk_join_tx_nr > > + if [[ -n "$ns1_fb" ]]; then Detail: you can use simple [], like in most places in this file. > + eval "$ns1_fb" \ > + chk_fallback_nr ${ns1} > + else > + chk_fallback_nr ${ns1} > + fi > + > + if [[ -n "$ns2_fb" ]]; then Same here for simple []. > + eval "$ns2_fb" \ > + chk_fallback_nr ${ns2} > + else > + chk_fallback_nr ${ns2} > + fi This will print this for each test: # check fallback nr [ OK ] # check fallback nr [ OK ] Not pretty... Also, it might be confusing to have all these "fallback" lines when we don't expect any fallbacks. What about printing it only in case of error? (or only once? but still confusing / not worth it when printed for all tests?) You could have another helper containing the code here above, and instead: rc=${KSFT_PASS} chk_fallback_nr_all || rc=${?} if [ "$rc" != "${KSFT_PASS}" ]; then print_results "fallback" ${rc} fi > + > if $validate_checksum; then > chk_csum_nr $csum_ns1 $csum_ns2 > chk_fail_nr $fail_nr $fail_nr > @@ -3435,6 +3451,7 @@ fail_tests() > join_csum_ns1=+1 join_csum_ns2=+0 \ > join_fail_nr=1 join_rst_nr=0 join_infi_nr=1 \ > join_corrupted_pkts="$(pedit_action_pkts)" \ > + ns1_fb="dss_fb=1" ns2_fb="infinite_map_tx_fb=1" \ > chk_join_nr 0 0 0 > chk_fail_nr 1 -1 invert > fi Cheers, Matt -- Sponsored by the NGI0 Core fund. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH mptcp-next v2 0/2] selftests: check fallback mib counter 2025-08-14 3:16 [PATCH mptcp-next v2 0/2] selftests: check fallback mib counter Gang Yan 2025-08-14 3:16 ` [PATCH mptcp-next v2 1/2] selftests: net: mptcp: add checks for fallback counters Gang Yan 2025-08-14 3:16 ` [PATCH mptcp-next v2 2/2] selftests: net: mptcp: add fallback checks in chk_join_nr Gang Yan @ 2025-08-14 5:09 ` MPTCP CI 2 siblings, 0 replies; 6+ messages in thread From: MPTCP CI @ 2025-08-14 5:09 UTC (permalink / raw) To: Gang Yan; +Cc: mptcp Hi Gang, Thank you for your modifications, that's great! Our CI did some validations and here is its report: - KVM Validation: normal: Success! ✅ - KVM Validation: debug: Unstable: 1 failed test(s): packetdrill_syscalls 🔴 - KVM Validation: btf-normal (only bpftest_all): Success! ✅ - KVM Validation: btf-debug (only bpftest_all): Success! ✅ - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/16955181183 Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/464ef2c0d83c Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=991281 If there are some issues, you can reproduce them using the same environment as the one used by the CI thanks to a docker image, e.g.: $ cd [kernel source code] $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \ --pull always mptcp/mptcp-upstream-virtme-docker:latest \ auto-normal For more details: https://github.com/multipath-tcp/mptcp-upstream-virtme-docker Please note that despite all the efforts that have been already done to have a stable tests suite when executed on a public CI like here, it is possible some reported issues are not due to your modifications. Still, do not hesitate to help us improve that ;-) Cheers, MPTCP GH Action bot Bot operated by Matthieu Baerts (NGI0 Core) ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-08-14 14:36 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-14 3:16 [PATCH mptcp-next v2 0/2] selftests: check fallback mib counter Gang Yan 2025-08-14 3:16 ` [PATCH mptcp-next v2 1/2] selftests: net: mptcp: add checks for fallback counters Gang Yan 2025-08-14 14:35 ` Matthieu Baerts 2025-08-14 3:16 ` [PATCH mptcp-next v2 2/2] selftests: net: mptcp: add fallback checks in chk_join_nr Gang Yan 2025-08-14 14:36 ` Matthieu Baerts 2025-08-14 5:09 ` [PATCH mptcp-next v2 0/2] selftests: check fallback mib counter MPTCP CI
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).