Re: [PATCH mptcp-next RFC 2/3] mptcp: reject MP_JOIN earlier

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The selftest checks for a non-existent MIB counter `MPTcpExtMPJoinDisallowed` due to a typo in the counter name.
--

--- Patch [2]: [PATCH mptcp-next RFC 2/3] mptcp: reject MP_JOIN earlier ---
commit 70d57e8c1f9f68365a9aab7308b481ca3d231b17
Author: Matthieu Baerts (NGI0) <[email protected]>

mptcp: reject MP_JOIN earlier

This commit checks if an MP_JOIN is allowed earlier when a SYN + MP_JOIN is
received, rather than waiting for subflow establishment. It also adds a new
MIB counter to track this behavior and updates the relevant selftests to
verify it.

> diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
> index 608cb568897cf..988755b41a5ab 100644
> --- a/net/mptcp/mib.c
> +++ b/net/mptcp/mib.c
> @@ -34,6 +34,7 @@ static const struct snmp_mib mptcp_snmp_list[] = {
>  	SNMP_MIB_ITEM("MPJoinAckNoCtx", MPTCP_MIB_MPJOINACKNOCTX),
>  	SNMP_MIB_ITEM("MPJoinRejected", MPTCP_MIB_JOINREJECTED),
>  	SNMP_MIB_ITEM("MPJoinNotEstablished", MPTCP_MIB_MPJOINNOTESTABLISHED),
> +	SNMP_MIB_ITEM("MPJoinDisallow", MPTCP_MIB_MPJOINDISALLOWED),
>  	SNMP_MIB_ITEM("MPJoinSynTx", MPTCP_MIB_JOINSYNTX),

[ ... ]

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 18ce7136a2b0e..1924ff488c8dd 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -1723,6 +1725,13 @@ chk_join_nr()
>  		fail_test "got $count JOIN[s] no id found expected $no_id_found"
>  	fi
>  
> +	count=$(mptcp_lib_get_counter ${ns1} "MPTcpExtMPJoinDisallowed")

[Severity: Medium]
Does this selftest query the correct MIB counter name?

The kernel code in mptcp_snmp_list[] exports this counter as "MPJoinDisallow",
but the test script checks for "MPTcpExtMPJoinDisallowed". 

Because the names don't match, mptcp_lib_get_counter will return an empty
string. The subsequent [ -n "$count" ] check will then evaluate to false,
causing the test to silently skip validating this new behavior altogether.

> +	if [ -n "$count" ] && [ "$count" != "$disallowed" ]; then
> +		rc=${KSFT_FAIL}
> +		print_check "join disallowed"
> +		fail_test "got $count JOIN[s] disallowed expected $disallowed"
> +	fi
> +
>  	print_results "join Rx" ${rc}
>  
>  	join_syn_tx="${join_syn_tx:-${syn_nr}}" \

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2