Re: [PATCH mptcp-next v3] selftests: net: mptcp: add checks for fallback counters

[Matthieu Baerts <matttbe@kernel.org>]

Hi Gang,

15 Aug 2025 09:32:11 Gang Yan <yangang@kylinos.cn>:

> Recently, some mib counters about fallback has been added, this patch
> provides a method 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>
>
> ------
> Changelog:
>   v3:
>     - Squash into a single patch.
>     - Using 'fb_' as a prefix instead of the suffix for variables.
>     - Adjust the order of mib counters to check, ensuring it matches the
>       variable declaration.
>     - The namespace('ns1'/'ns2') is included in the output of
>       'chk_fallback_nr', like:
>       "ns2 infinite map tx fallback        [IGNO] (flaky) got 1 infinite map tx fallback[s] in ns2 expected 0"
>     - Add a new helper named 'chk_fallback_nr_all', which prints only
>       when an error occurs.
>     - Fix some code style issues.

Thank you for the new version!

>   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.
>
> Notes:
>
> Hi Matt,
>
> I wanted to apologize for splitting the changes into multiple commits,
> my intention was to ease the review process, but it unfortunately
> backfired.

That's OK, no need to apologize, I was just trying to explain when
splitting makes sense or not.

> Thanks for your valuable suggestions, I've made some improvements based
> on your feedback.
>
> Please take another look at the updated changes.
>
> Best regards,
> Gang
>
> ---
> .../testing/selftests/net/mptcp/mptcp_join.sh | 123 ++++++++++++++++++
> 1 file changed, 123 insertions(+)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index b8af65373b3a..898779a9af58 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -74,6 +74,17 @@ unset join_create_err
> unset join_bind_err
> unset join_connect_err
>
> +unset fb_ns1
> +unset fb_ns2
> +unset fb_infinite_map_tx
> +unset fb_dss_corruption
> +unset fb_simult_conn
> +unset fb_mpc_passive
> +unset fb_mpc_active
> +unset fb_mpc_data
> +unset fb_md5_sig
> +unset fb_dss
> +
> # generated using "nfbpf_compile '(ip && (ip[54] & 0xf0) == 0x30) ||
> #                (ip6 && (ip6[74] & 0xf0) == 0x30)'"
> CBPF_MPTCP_SUBOPTION_ADD_ADDR="14,
> @@ -1399,6 +1410,115 @@ chk_join_tx_nr()
>     print_results "join Tx" ${rc}
> }
>
> +chk_fallback_nr()
> +{
> +   local infinite_map_tx=${fb_infinite_map_tx:-0}
> +   local dss_corruption=${fb_dss_corruption:-0}
> +   local simult_conn=${fb_simult_conn:-0}
> +   local mpc_passive=${fb_mpc_passive:-0}
> +   local mpc_active=${fb_mpc_active:-0}
> +   local mpc_data=${fb_mpc_data:-0}
> +   local md5_sig=${fb_md5_sig:-0}
> +   local dss=${fb_dss:-0}
> +   local rc=${KSFT_PASS}
> +   local ns=$1
> +   local count
> +
> +   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 "$ns infinite map tx fallback"
> +       fail_test "got $count infinite map tx fallback[s] in $ns expected $infinite_map_tx"
> +   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 "$ns dss corruption fallback"
> +       fail_test "got $count dss corruption fallback[s] in $ns expected $dss_corruption"
> +   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 "$ns simult conn fallback"
> +       fail_test "got $count simult conn fallback[s] in $ns expected $simult_conn"
> +   fi
> +
> +   count=$(mptcp_lib_get_counter ${!ns} "MPTcpExtMPCapableFallbackACK")
> +   if [ -z "$count" ]; then
> +       rc=${KSFT_SKIP}
> +   elif [ "$count" != "$mpc_passive" ]; then
> +       rc=${KSFT_FAIL}
> +       print_check "$ns mpc passive fallback"
> +       fail_test "got $count mpc passive fallback[s] in $ns 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 "$ns mpc active fallback"
> +       fail_test "got $count mpc active fallback[s] in $ns expected $mpc_active"
> +   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 "$ns mpc data fallback"
> +       fail_test "got $count mpc data fallback[s] in $ns 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 "$ns MD5 Sig fallback"
> +       fail_test "got $count MD5 Sig fallback[s] in $ns expected $MD5_Sig"

Should be $md5_sig without capital letters. I wonder if shellcheck
will detect this.

If the CI is happy with this version, I can do the modification when
applying the patch.

Cheers,
Matt

> +   fi
> +
> +   count=$(mptcp_lib_get_counter ${!ns} "MPTcpExtDssFallback")
> +   if [ -z "$count" ]; then
> +       rc=${KSFT_SKIP}
> +   elif [ "$count" != "$dss" ]; then
> +       rc=${KSFT_FAIL}
> +       print_check "$ns dss fallback"
> +       fail_test "got $count dss fallback[s] in $ns expected $dss"
> +   fi
> +
> +   return $rc
> +}
> +
> +chk_fallback_nr_all()
> +{
> +   local netns=("ns1" "ns2")
> +   local fb_ns=("fb_ns1" "fb_ns2")
> +   local rc=${KSFT_PASS}
> +
> +   for i in 0 1; do
> +       if [ -n "${!fb_ns[i]}" ]; then
> +           eval "${!fb_ns[i]}" \
> +               chk_fallback_nr ${netns[i]} || rc={?}
> +       else
> +           chk_fallback_nr ${netns[i]} || rc={?}
> +       fi
> +   done
> +
> +   if [ "$rc" != "${KSFT_PASS}" ]; then
> +       print_results "fallback" $rc
> +   fi
> +}
> +
> chk_join_nr()
> {
>     local syn_nr=$1
> @@ -1484,6 +1604,8 @@ chk_join_nr()
>     join_syn_tx="${join_syn_tx:-${syn_nr}}" \
>         chk_join_tx_nr
>
> +   chk_fallback_nr_all
> +
>     if $validate_checksum; then
>         chk_csum_nr $csum_ns1 $csum_ns2
>         chk_fail_nr $fail_nr $fail_nr
> @@ -3337,6 +3459,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)" \
> +           fb_ns1="fb_dss=1" fb_ns2="fb_infinite_map_tx=1" \
>             chk_join_nr 0 0 0
>         chk_fail_nr 1 -1 invert
>     fi
> --
> 2.43.0