Re: [PATCH v3 2/2] selftests: mptcp: extend chk_rst_nr to validate per-event RST counters

[Matthieu Baerts <matttbe@kernel.org>]

Hi Shardul,

On 12/05/2026 20:33, Shardul Bankar wrote:
> Extend chk_rst_nr() with named env-var expectations for each
> per-event MPTCP_RST_EMPTCP counter, matching the pattern used by
> chk_join_nr().  Each counter defaults to 0 and is checked silently
> on success; mismatches print a check line and fail the test.
> Counters absent from the running kernel are skipped silently so
> older kernels do not false-fail.
> 
> Add a test at the end of signal_address_tests that triggers
> MPJoinSynAckNoMPJoin: ns1 signals an address that is already bound
> on the client (ns2), where a TCP-only mptcp_connect listener is
> started.  The client's MP_JOIN routes locally to the TCP listener,
> which responds with a plain SYN/ACK without the MP_JOIN option, and
> the new counter increments on the client side.
> 
> Other per-event counters (MD5SigReset, MPJoinAckNoMPJoin,
> MPJoinAckNoCtx, DssReset, MPJoinNotEstablished) are not currently
> reachable from mptcp_join.sh; the env-var hooks are in place for
> future tests to set expectations explicitly.
> 
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
> ---
> v3:
>   - Drop the third-netns approach for triggering
>     MPJoinSynAckNoMPJoin.  Use ns1 signalling an address that is
>     already bound on ns2 (10.0.2.2) with a TCP-only mptcp_connect
>     listener on ns2; the client's MP_JOIN routes locally to that
>     listener and the SYN/ACK arrives without MP_JOIN.
>   - Move the test into signal_address_tests; drop the standalone
>     rst_emptcp_tests group and its sorted-list entry.
>   - Drop speed=slow (not needed: only one ADD_ADDR, no in-flight
>     actions).
>   - The local-and-assign that tripped shellcheck SC2155 is gone
>     with the old test.
> v2: https://lore.kernel.org/all/20260509183335.969018-1-shardul.b@mpiricsoftware.com/
> 
>  .../testing/selftests/net/mptcp/mptcp_join.sh | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 5d4d0f127f795..c6ef7f785a558 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -76,6 +76,13 @@ unset join_create_err
>  unset join_bind_err
>  unset join_connect_err
>  
> +unset rst_md5sig_reset
> +unset rst_synack_no_mpjoin
> +unset rst_ack_no_mpjoin
> +unset rst_ack_no_ctx
> +unset rst_dss_reset
> +unset rst_not_established

See below: I think you can use the 'join_' prefix and attach them above,
except for md5sig and dss that could also simply be called rst_dss and
rst_md5sig.

> +
>  unset fb_ns1
>  unset fb_ns2
>  unset fb_infinite_map_tx
> @@ -1349,6 +1356,12 @@ chk_rst_nr()
>  	local rst_tx=$1
>  	local rst_rx=$2
>  	local ns_invert=${3:-""}
> +	local md5sig_reset=${rst_md5sig_reset:-0}
> +	local synack_no_mpjoin=${rst_synack_no_mpjoin:-0}
> +	local ack_no_mpjoin=${rst_ack_no_mpjoin:-0}
> +	local ack_no_ctx=${rst_ack_no_ctx:-0}
> +	local dss_reset=${rst_dss_reset:-0}
> +	local not_established=${rst_not_established:-0}
>  	local count
>  	local ns_tx=$ns1
>  	local ns_rx=$ns2
> @@ -1385,6 +1398,43 @@ chk_rst_nr()
>  	else
>  		print_ok
>  	fi
> +
> +	# Per-event MPTCP_RST_EMPTCP counters; default 0, gated on availability.
> +	count=$(mptcp_lib_get_counter ${ns_tx} "MPTcpExtMD5SigReset")
> +	if [ -n "$count" ] && [ "$count" != "$md5sig_reset" ]; then
> +		print_check "MD5SigReset ${tx}"
> +		fail_test "got $count MD5SigReset expected $md5sig_reset"
> +	fi
> +
> +	count=$(mptcp_lib_get_counter ${ns_rx} "MPTcpExtMPJoinSynAckNoMPJoin")

Here, the netns should be fixed: only the client should get it, and
using "chk_rst_nr ... invert" will not check at the right netns.

In fact, I think this counter and other MPJoin counters here below
should probably be added to the chk_join_nr() helper, no? Leaving here
only the MD5Sig and DSS related one. WDYT?

> +	if [ -n "$count" ] && [ "$count" != "$synack_no_mpjoin" ]; then
> +		print_check "MPJoinSynAckNoMPJoin ${rx}"
> +		fail_test "got $count MPJoinSynAckNoMPJoin expected $synack_no_mpjoin"
> +	fi
> +
> +	count=$(mptcp_lib_get_counter ${ns_tx} "MPTcpExtMPJoinAckNoMPJoin")
> +	if [ -n "$count" ] && [ "$count" != "$ack_no_mpjoin" ]; then
> +		print_check "MPJoinAckNoMPJoin ${tx}"
> +		fail_test "got $count MPJoinAckNoMPJoin expected $ack_no_mpjoin"
> +	fi
> +
> +	count=$(mptcp_lib_get_counter ${ns_tx} "MPTcpExtMPJoinAckNoCtx")
> +	if [ -n "$count" ] && [ "$count" != "$ack_no_ctx" ]; then
> +		print_check "MPJoinAckNoCtx ${tx}"
> +		fail_test "got $count MPJoinAckNoCtx expected $ack_no_ctx"
> +	fi
> +
> +	count=$(mptcp_lib_get_counter ${ns_rx} "MPTcpExtDssReset")
> +	if [ -n "$count" ] && [ "$count" != "$dss_reset" ]; then
> +		print_check "DssReset ${rx}"
> +		fail_test "got $count DssReset expected $dss_reset"
> +	fi
> +
> +	count=$(mptcp_lib_get_counter ${ns_rx} "MPTcpExtMPJoinNotEstablished")
> +	if [ -n "$count" ] && [ "$count" != "$not_established" ]; then
> +		print_check "MPJoinNotEstablished ${rx}"
> +		fail_test "got $count MPJoinNotEstablished expected $not_established"
> +	fi
>  }
>  
>  chk_infi_nr()
> @@ -2378,6 +2428,28 @@ signal_address_tests()
>  			chk_add_nr 4 4
>  		fi
>  	fi
> +
> +	# signalled address belongs to the client, where a TCP-only
> +	# listener is bound at it: the client's MP_JOIN routes locally
> +	# to the listener and receives a SYN/ACK without MP_JOIN.
> +	# MPJoinSynAckNoMPJoin increments on the client side.
> +	if reset "signal address, TCP-only listener on client"; then
> +		local extra_bind
> +
> +		pm_nl_set_limits $ns1 0 1
> +		pm_nl_set_limits $ns2 1 1
> +		pm_nl_add_endpoint $ns1 10.0.2.2 flags signal
> +
> +		ip netns exec ${ns2} ./mptcp_connect -l -t -1 -p "$(get_port)" \
> +			-s TCP 10.0.2.2 &
> +		extra_bind=$!

It would be better to call mptcp_lib_wait_local_port_listen(), just to
be sure it is listening. (Even if 'run_tests' will also call it on the
other netns)

On the other end, I see we didn't call it in similar other cases.

> +		run_tests $ns1 $ns2 10.0.1.1

See above: so here, call chk_join_nr() after, like in most tests, but
here check with join_synack_no_mpjoin=1.

> +		rst_synack_no_mpjoin=1 \
> +			chk_rst_nr 0 0
> +
> +		kill ${extra_bind} 2>/dev/null
> +	fi
>  }
>  
>  laminar_endp_tests()

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.