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
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.
© 2016 - 2025 Red Hat, Inc.