[PATCH mptcp-next RFC 0/3] mptcp: reset fully_estab after fallback & reject MPJ earlier

[PATCH mptcp-next RFC 0/3] mptcp: reset fully_estab after fallback & reject MPJ earlier

[Matthieu Baerts (NGI0)]

Here is a small RFC series to present my idea: it seems better with some
code than describing it.

I suggest bailing out early when we can: reset the fully established
when a fallback is done, then reject the MPJ earlier, and finally stop
processing connections early with the in-kernel PM.

I didn't check if any of those should be backported, probably not.

Only very lightly tested.

@Chenguang / @Gang: feel free to take over this series if you want to
extend it, or if fixes are needed.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Matthieu Baerts (NGI0) (3):
      mptcp: no longer fully_established after a fallback
      mptcp: reject MP_JOIN earlier
      mptcp: pm: kernel: skip operating on closing connections

 net/mptcp/mib.c                                 |  1 +
 net/mptcp/mib.h                                 |  1 +
 net/mptcp/pm_kernel.c                           | 16 +++++++++++-----
 net/mptcp/protocol.c                            |  1 +
 net/mptcp/subflow.c                             | 19 ++++++++++++++++---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 25 +++++++++++++++++--------
 6 files changed, 47 insertions(+), 16 deletions(-)
---
base-commit: d536472a82d18356d323227213acb5603bb9050c
change-id: 20260814-mptcp-reset-mpj-early-dcc7d4d92df9

Best regards,
--  
Matthieu Baerts (NGI0) <matttbe@kernel.org>

[PATCH mptcp-next RFC 1/3] mptcp: no longer fully_established after a fallback

[Matthieu Baerts (NGI0)]

When a fallback is done, the MPTCP_FALLBACK_DONE bit is set, which is
protected by the fallback lock. That's good, but while at it, the msk
fully established state can also be reset, to easily prevent some
operations.

This is just an optimisation to bail early in some situations: with the
PM, or when receiving an MP_JOIN. This is not a protection that requires
locks to avoid races: the fallback lock and other variables are there
for that.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/protocol.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index c2762d74f29d..a3982e4ed553 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -94,6 +94,7 @@ bool __mptcp_try_fallback(struct mptcp_sock *msk, int fb_mib)
 		return false;
 	}
 
+	WRITE_ONCE(msk->fully_established, false);
 	msk->allow_subflows = false;
 	set_bit(MPTCP_FALLBACK_DONE, &msk->flags);
 	__MPTCP_INC_STATS(net, fb_mib);

-- 
2.53.0

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

[Matthieu Baerts (NGI0)]

When a SYN + MP_JOIN is received, it is good to check directly if the
MP_JOIN is allowed, then checking that later at the establishment of the
new subflow.

This avoids situations where the 3WHS is done, then the joined subflow
is rejected.

This changes the behaviour, and the new behaviour is tracked with the
MPJoinDisallow MIB counter.

TODO: adapt the selftests to be able to run on older kernels: if the new
counter is not available, increment syn_ack and syn_rej instead.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/mib.c                                 |  1 +
 net/mptcp/mib.h                                 |  1 +
 net/mptcp/subflow.c                             | 19 ++++++++++++++++---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 25 +++++++++++++++++--------
 4 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
index 608cb568897c..988755b41a5a 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),
 	SNMP_MIB_ITEM("MPJoinSynTxCreatSkErr", MPTCP_MIB_JOINSYNTXCREATSKERR),
 	SNMP_MIB_ITEM("MPJoinSynTxBindErr", MPTCP_MIB_JOINSYNTXBINDERR),
diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
index 1ebdb55e9534..e97ee2d952a7 100644
--- a/net/mptcp/mib.h
+++ b/net/mptcp/mib.h
@@ -29,6 +29,7 @@ enum linux_mptcp_mib_field {
 	MPTCP_MIB_MPJOINACKNOCTX,	/* MP_RST: no subflow context on ACK */
 	MPTCP_MIB_JOINREJECTED,		/* The PM rejected the JOIN request */
 	MPTCP_MIB_MPJOINNOTESTABLISHED,	/* MP_RST: JOIN on not-fully-established msk */
+	MPTCP_MIB_MPJOINDISALLOWED,	/* RST JOIN early if disallowed: fallback/PM */
 	MPTCP_MIB_JOINSYNTX,		/* Sending a SYN + MP_JOIN */
 	MPTCP_MIB_JOINSYNTXCREATSKERR,	/* Not able to create a socket when sending a SYN + MP_JOIN */
 	MPTCP_MIB_JOINSYNTXBINDERR,	/* Not able to bind() the address when sending a SYN + MP_JOIN */
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index af81ad5e699d..7552abf4c333 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -82,7 +82,8 @@ static void subflow_req_create_thmac(struct mptcp_subflow_request_sock *subflow_
 	subflow_req->thmac = get_unaligned_be64(hmac);
 }
 
-static struct mptcp_sock *subflow_token_join_request(struct request_sock *req)
+static struct mptcp_sock *subflow_token_join_request(struct request_sock *req,
+						     u8 *reason)
 {
 	struct mptcp_subflow_request_sock *subflow_req = mptcp_subflow_rsk(req);
 	struct mptcp_sock *msk;
@@ -91,12 +92,22 @@ static struct mptcp_sock *subflow_token_join_request(struct request_sock *req)
 	msk = mptcp_token_get_sock(sock_net(req_to_sk(req)), subflow_req->token);
 	if (!msk) {
 		SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINNOTOKEN);
+		*reason = MPTCP_RST_EMPTCP;
+		return NULL;
+	}
+
+	/* Stop it early if the subflow cannot be accepted */
+	if (!mptcp_can_accept_new_subflow(msk)) {
+		SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_MPJOINDISALLOWED);
+		*reason = MPTCP_RST_EPROHIBIT;
+		sock_put((struct sock *)msk);
 		return NULL;
 	}
 
 	local_id = mptcp_pm_get_local_id(msk, (struct sock_common *)req);
 	if (local_id < 0) {
 		SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_MPJOINNOIDFOUND);
+		*reason = MPTCP_RST_EMPTCP;
 		sock_put((struct sock *)msk);
 		return NULL;
 	}
@@ -217,17 +228,19 @@ static int subflow_check_req(struct request_sock *req,
 			SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_TOKENFALLBACKINIT);
 
 	} else if (opt_mp_join && listener->request_mptcp) {
+		u8 reason;
+
 		subflow_req->ssn_offset = TCP_SKB_CB(skb)->seq;
 		subflow_req->mp_join = 1;
 		subflow_req->backup = mp_opt.backup;
 		subflow_req->remote_id = mp_opt.join_id;
 		subflow_req->token = mp_opt.token;
 		subflow_req->remote_nonce = mp_opt.nonce;
-		subflow_req->msk = subflow_token_join_request(req);
+		subflow_req->msk = subflow_token_join_request(req, &reason);
 
 		/* Can't fall back to TCP in this case. */
 		if (!subflow_req->msk) {
-			subflow_add_reset_reason(skb, MPTCP_RST_EMPTCP);
+			subflow_add_reset_reason(skb, reason);
 			return -EPERM;
 		}
 
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 18ce7136a2b0..1924ff488c8d 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -80,6 +80,7 @@ unset join_ack_no_mpjoin
 unset join_ack_no_ctx
 unset join_not_established
 unset join_no_id_found
+unset join_disallowed
 
 unset rst_md5sig
 unset rst_dss
@@ -1617,6 +1618,7 @@ chk_join_nr()
 	local ack_no_ctx=${join_ack_no_ctx:-0}
 	local not_established=${join_not_established:-0}
 	local no_id_found=${join_no_id_found:-0}
+	local disallowed=${join_disallowed:-0}
 	local rc=${KSFT_PASS}
 	local count
 	local with_cookie
@@ -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")
+	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}}" \
@@ -2202,8 +2211,8 @@ subflows_tests()
 		pm_nl_set_limits $ns2 0 1
 		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow
 		run_tests $ns1 $ns2 10.0.1.1
-		join_syn_rej=1 \
-			chk_join_nr 1 1 0
+		join_disallowed=1 \
+			chk_join_nr 1 0 0
 	fi
 
 	# subflow
@@ -2232,8 +2241,8 @@ subflows_tests()
 		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow
 		pm_nl_add_endpoint $ns2 10.0.2.2 flags subflow
 		run_tests $ns1 $ns2 10.0.1.1
-		join_syn_rej=1 \
-			chk_join_nr 2 2 1
+		join_disallowed=1 \
+			chk_join_nr 2 1 1
 	fi
 
 	# single subflow, dev
@@ -4114,8 +4123,8 @@ userspace_tests()
 		pm_nl_set_limits $ns2 1 1
 		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow
 		run_tests $ns1 $ns2 10.0.1.1
-		join_syn_rej=1 \
-			chk_join_nr 1 1 0
+		join_disallowed=1 \
+			chk_join_nr 1 0 0
 	fi
 
 	# userspace pm type does not send join
@@ -4138,8 +4147,8 @@ userspace_tests()
 		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow
 		sflags=backup speed=slow \
 			run_tests $ns1 $ns2 10.0.1.1
-		join_syn_rej=1 \
-			chk_join_nr 1 1 0
+		join_disallowed=1 \
+			chk_join_nr 1 0 0
 		chk_prio_nr 0 0 0 0
 	fi
 

-- 
2.53.0

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

[sashiko-bot]

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) <matttbe@kernel.org>

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/20260814-mptcp-reset-mpj-early-v1-0-3ca3260f9474@kernel.org?part=2

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

[Matthieu Baerts]

Hello,

On 14/08/2026 20:31, Matthieu Baerts (NGI0) wrote:
> When a SYN + MP_JOIN is received, it is good to check directly if the
> MP_JOIN is allowed, then checking that later at the establishment of the
> new subflow.
> 
> This avoids situations where the 3WHS is done, then the joined subflow
> is rejected.
> 
> This changes the behaviour, and the new behaviour is tracked with the
> MPJoinDisallow MIB counter.
> 
> TODO: adapt the selftests to be able to run on older kernels: if the new
> counter is not available, increment syn_ack and syn_rej instead.

(...)

> @@ -217,17 +228,19 @@ static int subflow_check_req(struct request_sock *req,
>  			SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_TOKENFALLBACKINIT);
>  
>  	} else if (opt_mp_join && listener->request_mptcp) {
> +		u8 reason;
> +
>  		subflow_req->ssn_offset = TCP_SKB_CB(skb)->seq;
>  		subflow_req->mp_join = 1;
>  		subflow_req->backup = mp_opt.backup;
>  		subflow_req->remote_id = mp_opt.join_id;
>  		subflow_req->token = mp_opt.token;
>  		subflow_req->remote_nonce = mp_opt.nonce;
> -		subflow_req->msk = subflow_token_join_request(req);
> +		subflow_req->msk = subflow_token_join_request(req, &reason);
>  
>  		/* Can't fall back to TCP in this case. */
>  		if (!subflow_req->msk) {
> -			subflow_add_reset_reason(skb, MPTCP_RST_EMPTCP);
> +			subflow_add_reset_reason(skb, reason);
>  			return -EPERM;
>  		}

Note: the call to mptcp_can_accept_new_subflow() here below, when
handling req->syncookies, is no longer needed as it is now done earlier
for all cases, in subflow_token_join_request().

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

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

[Matthieu Baerts]

On 16/08/2026 18:12, Matthieu Baerts wrote:
> Hello,
> 
> On 14/08/2026 20:31, Matthieu Baerts (NGI0) wrote:
>> When a SYN + MP_JOIN is received, it is good to check directly if the
>> MP_JOIN is allowed, then checking that later at the establishment of the
>> new subflow.
>>
>> This avoids situations where the 3WHS is done, then the joined subflow
>> is rejected.
>>
>> This changes the behaviour, and the new behaviour is tracked with the
>> MPJoinDisallow MIB counter.
>>
>> TODO: adapt the selftests to be able to run on older kernels: if the new
>> counter is not available, increment syn_ack and syn_rej instead.
> 
> (...)
> 
>> @@ -217,17 +228,19 @@ static int subflow_check_req(struct request_sock *req,
>>  			SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_TOKENFALLBACKINIT);
>>  
>>  	} else if (opt_mp_join && listener->request_mptcp) {
>> +		u8 reason;
>> +
>>  		subflow_req->ssn_offset = TCP_SKB_CB(skb)->seq;
>>  		subflow_req->mp_join = 1;
>>  		subflow_req->backup = mp_opt.backup;
>>  		subflow_req->remote_id = mp_opt.join_id;
>>  		subflow_req->token = mp_opt.token;
>>  		subflow_req->remote_nonce = mp_opt.nonce;
>> -		subflow_req->msk = subflow_token_join_request(req);
>> +		subflow_req->msk = subflow_token_join_request(req, &reason);
>>  
>>  		/* Can't fall back to TCP in this case. */
>>  		if (!subflow_req->msk) {
>> -			subflow_add_reset_reason(skb, MPTCP_RST_EMPTCP);
>> +			subflow_add_reset_reason(skb, reason);
>>  			return -EPERM;
>>  		}
> 
> Note: the call to mptcp_can_accept_new_subflow() here below, when
> handling req->syncookies, is no longer needed as it is now done earlier
> for all cases, in subflow_token_join_request().
I meant to add the diff:

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 7552abf4c333..c629bcce6ecc 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -258,15 +258,8 @@ static int subflow_check_req(struct request_sock *req,
 
 		subflow_req_create_thmac(subflow_req);
 
-		if (unlikely(req->syncookie)) {
-			if (!mptcp_can_accept_new_subflow(subflow_req->msk)) {
-				SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINREJECTED);
-				subflow_add_reset_reason(skb, MPTCP_RST_EPROHIBIT);
-				return -EPERM;
-			}
-
+		if (unlikely(req->syncookie))
 			subflow_init_req_cookie_join_save(subflow_req, skb);
-		}
 
 		pr_debug("token=%u, remote_nonce=%u msk=%p\n", subflow_req->token,
 			 subflow_req->remote_nonce, subflow_req->msk);


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

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

[Paolo Abeni]

On 8/14/26 8:31 PM, Matthieu Baerts (NGI0) wrote:
> @@ -91,12 +92,22 @@ static struct mptcp_sock *subflow_token_join_request(struct request_sock *req)
>  	msk = mptcp_token_get_sock(sock_net(req_to_sk(req)), subflow_req->token);
>  	if (!msk) {
>  		SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINNOTOKEN);
> +		*reason = MPTCP_RST_EMPTCP;
> +		return NULL;
> +	}
> +
> +	/* Stop it early if the subflow cannot be accepted */
> +	if (!mptcp_can_accept_new_subflow(msk)) {

Side note: the above is inherently racy, the consistency safeguard is
given by the fallback check at finish_join time.

Minor nit: I'm not sure this check belongs here or in
subflow_check_req(). The latter option would avoid the additional
argument and possibly produce a smaller diff (and smaller code).

/P

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

[Matthieu Baerts]

Hi Paolo,

Thank you for having looked at this!

On 17/08/2026 09:27, Paolo Abeni wrote:
> On 8/14/26 8:31 PM, Matthieu Baerts (NGI0) wrote:
>> @@ -91,12 +92,22 @@ static struct mptcp_sock *subflow_token_join_request(struct request_sock *req)
>>  	msk = mptcp_token_get_sock(sock_net(req_to_sk(req)), subflow_req->token);
>>  	if (!msk) {
>>  		SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINNOTOKEN);
>> +		*reason = MPTCP_RST_EMPTCP;
>> +		return NULL;
>> +	}
>> +
>> +	/* Stop it early if the subflow cannot be accepted */
>> +	if (!mptcp_can_accept_new_subflow(msk)) {
> 
> Side note: the above is inherently racy, the consistency safeguard is
> given by the fallback check at finish_join time.

Just to be sure: even if it is racy, is it OK to use it here but
continuing having the safe check at finish_join time?

In other words, having such a comment is enough?

  /* Stop it early if the subflow cannot be accepted.
   * This check is inherently racy, the consistency safeguard is
   * given by the fallback check at finish_join time.
   */

> Minor nit: I'm not sure this check belongs here or in
> subflow_check_req(). The latter option would avoid the additional
> argument and possibly produce a smaller diff (and smaller code).

After re-looking at this code yesterday, I agree with you. Also, I think
subflow_token_join_request() should only return the msk, and not
changing anything else from subflow_req.

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

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

[Paolo Abeni]

On 8/17/26 11:49 AM, Matthieu Baerts wrote:
> On 17/08/2026 09:27, Paolo Abeni wrote:
>> On 8/14/26 8:31 PM, Matthieu Baerts (NGI0) wrote:
>>> @@ -91,12 +92,22 @@ static struct mptcp_sock *subflow_token_join_request(struct request_sock *req)
>>>  	msk = mptcp_token_get_sock(sock_net(req_to_sk(req)), subflow_req->token);
>>>  	if (!msk) {
>>>  		SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINNOTOKEN);
>>> +		*reason = MPTCP_RST_EMPTCP;
>>> +		return NULL;
>>> +	}
>>> +
>>> +	/* Stop it early if the subflow cannot be accepted */
>>> +	if (!mptcp_can_accept_new_subflow(msk)) {
>>
>> Side note: the above is inherently racy, the consistency safeguard is
>> given by the fallback check at finish_join time.
> 
> Just to be sure: even if it is racy, is it OK to use it here but
> continuing having the safe check at finish_join time?
> 
> In other words, having such a comment is enough?
> 
>   /* Stop it early if the subflow cannot be accepted.
>    * This check is inherently racy, the consistency safeguard is
>    * given by the fallback check at finish_join time.
>    */

I think so.

>> Minor nit: I'm not sure this check belongs here or in
>> subflow_check_req(). The latter option would avoid the additional
>> argument and possibly produce a smaller diff (and smaller code).
> 
> After re-looking at this code yesterday, I agree with you. Also, I think
> subflow_token_join_request() should only return the msk, and not
> changing anything else from subflow_req.
Makes sense.

Somewhat related: to avoid subflow_check_req() growing too much, it
would possibly make sense move the final mp_join related bits in a
specific helper, and have just:

	} else if (opt_mp_join && listener->request_mptcp)
		return subflow_mpj_check_req(req, sk_listener);

/P

[PATCH mptcp-next RFC 3/3] mptcp: pm: kernel: skip operating on closing connections

[Matthieu Baerts (NGI0)]

When iterating over each MPTCP connection after having manipulated MPTCP
endpoints, there is no need to operate on closing (or not ready)
connections.

We can then safely skip those.

Note that mptcp_nl_add_subflow_or_signal_addr() was already checking the
fully_established state, but it is better to check the connection state,
which is what mptcp_is_fully_established() is doing.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_kernel.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index d3014bf57bf3..c185e53fe42b 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -969,7 +969,7 @@ static int mptcp_nl_add_subflow_or_signal_addr(struct net *net,
 		struct sock *sk = (struct sock *)msk;
 		struct mptcp_addr_info mpc_addr;
 
-		if (!READ_ONCE(msk->fully_established) ||
+		if (!mptcp_is_fully_established(sk) ||
 		    mptcp_pm_is_userspace(msk))
 			goto next;
 
@@ -1095,7 +1095,8 @@ static int mptcp_nl_remove_subflow_and_signal_addr(struct net *net,
 		struct sock *sk = (struct sock *)msk;
 		bool remove_subflow;
 
-		if (mptcp_pm_is_userspace(msk))
+		if (!mptcp_is_fully_established(sk) ||
+		    mptcp_pm_is_userspace(msk))
 			goto next;
 
 		lock_sock(sk);
@@ -1141,7 +1142,9 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
 		struct mptcp_addr_info anno_addr;
 		bool announced;
 
-		if (list_empty(&msk->conn_list) || mptcp_pm_is_userspace(msk))
+		if (list_empty(&msk->conn_list) ||
+		    !mptcp_is_fully_established(sk) ||
+		    mptcp_pm_is_userspace(msk))
 			goto next;
 
 		mptcp_local_address((struct sock_common *)msk, &msk_local);
@@ -1288,7 +1291,8 @@ static void mptcp_nl_flush_addrs_list(struct net *net,
 	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
 		struct sock *sk = (struct sock *)msk;
 
-		if (!mptcp_pm_is_userspace(msk)) {
+		if (mptcp_is_fully_established(sk) &&
+		    !mptcp_pm_is_userspace(msk)) {
 			lock_sock(sk);
 			mptcp_pm_flush_addrs_and_subflows(msk, rm_list, NULL);
 			release_sock(sk);
@@ -1502,7 +1506,9 @@ static void mptcp_pm_nl_set_flags_all(struct net *net,
 	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
 		struct sock *sk = (struct sock *)msk;
 
-		if (list_empty(&msk->conn_list) || mptcp_pm_is_userspace(msk))
+		if (list_empty(&msk->conn_list) ||
+		    !mptcp_is_fully_established(sk) ||
+		    mptcp_pm_is_userspace(msk))
 			goto next;
 
 		lock_sock(sk);

-- 
2.53.0

Re: [PATCH mptcp-next RFC 3/3] mptcp: pm: kernel: skip operating on closing connections

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] PM endpoint operations silently ignore half-closed connections, leaving stale PM state and breaking MPTCP resilience.
- [High] Lockless connection state check introduces a Time-of-Check to Time-of-Use race that causes transitioning sockets to permanently miss endpoint removals.
--

--- Patch [3]: [PATCH mptcp-next RFC 3/3] mptcp: pm: kernel: skip operating on closing connections ---
commit d802951fdbb5f62840e58ea428dab6858edf0731
Author: Matthieu Baerts (NGI0) <matttbe@kernel.org>

mptcp: pm: kernel: skip operating on closing connections

When iterating over each MPTCP connection after having manipulated MPTCP
endpoints, there is no need to operate on closing (or not ready)
connections. We can then safely skip those.

> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index d3014bf57bf3c..c185e53fe42b1 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c

[ ... ]

> @@ -1095,7 +1095,8 @@ static int mptcp_nl_remove_subflow_and_signal_addr(struct net *net,
>  		struct sock *sk = (struct sock *)msk;
>  		bool remove_subflow;
>  
> -		if (mptcp_pm_is_userspace(msk))
> +		if (!mptcp_is_fully_established(sk) ||
> +		    mptcp_pm_is_userspace(msk))
>  			goto next;
>  
>  		lock_sock(sk);

[Severity: High]
Are half-closed connections skipped by this check?

mptcp_is_fully_established() requires the socket to be in the TCP_ESTABLISHED
state. If a connection is transferring data while in a half-closed state
like TCP_CLOSE_WAIT or TCP_FIN_WAIT1, the PM endpoint operations will silently
skip it.

If an interface goes down, could this cause dead subflows to remain alive,
breaking failover because data continues to stall over removed interfaces
instead of seamlessly switching to active paths?

[Severity: High]
Can this lockless state check cause transitioning connections to miss
endpoint removals?

Because the check happens before lock_sock(sk) is called, a connection
transitioning from TCP_SYN_RECV to TCP_ESTABLISHED could be skipped.

If the socket transitions to TCP_ESTABLISHED immediately after being skipped
here, the state machine triggers mptcp_pm_nl_fully_established(). However,
that function only synchronizes additions from the PM database, not removals.

Will this result in the endpoint removal event being lost permanently,
leaving stale subflows active indefinitely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814-mptcp-reset-mpj-early-v1-0-3ca3260f9474@kernel.org?part=3

Re: [PATCH mptcp-next RFC 3/3] mptcp: pm: kernel: skip operating on closing connections

[Paolo Abeni]

On 8/14/26 8:31 PM, Matthieu Baerts (NGI0) wrote:
> When iterating over each MPTCP connection after having manipulated MPTCP
> endpoints, there is no need to operate on closing (or not ready)
> connections.
> 
> We can then safely skip those.
> 
> Note that mptcp_nl_add_subflow_or_signal_addr() was already checking the
> fully_established state, but it is better to check the connection state,
> which is what mptcp_is_fully_established() is doing.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  net/mptcp/pm_kernel.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index d3014bf57bf3..c185e53fe42b 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -969,7 +969,7 @@ static int mptcp_nl_add_subflow_or_signal_addr(struct net *net,
>  		struct sock *sk = (struct sock *)msk;
>  		struct mptcp_addr_info mpc_addr;
>  
> -		if (!READ_ONCE(msk->fully_established) ||
> +		if (!mptcp_is_fully_established(sk) ||
>  		    mptcp_pm_is_userspace(msk))
>  			goto next;

I'm a little double-minded here.

The add/removal operations are issued by the user space to update
subflows, as a consequence of endpoint addition/removal.

Possibly removing the subflow even for not fully established connections
could be the right thing to do, I'm not 110% sure either way. Possibly
worth to call this out with a comment?

Also the asymmetry with the user-space PM is a bit strange. I can't
recall why it was needed in the first place?!?

/P

Re: [PATCH mptcp-next RFC 3/3] mptcp: pm: kernel: skip operating on closing connections

[Matthieu Baerts]

Hi Paolo,

On 17/08/2026 09:39, Paolo Abeni wrote:
> On 8/14/26 8:31 PM, Matthieu Baerts (NGI0) wrote:
>> When iterating over each MPTCP connection after having manipulated MPTCP
>> endpoints, there is no need to operate on closing (or not ready)
>> connections.
>>
>> We can then safely skip those.
>>
>> Note that mptcp_nl_add_subflow_or_signal_addr() was already checking the
>> fully_established state, but it is better to check the connection state,
>> which is what mptcp_is_fully_established() is doing.
>>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  net/mptcp/pm_kernel.c | 16 +++++++++++-----
>>  1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
>> index d3014bf57bf3..c185e53fe42b 100644
>> --- a/net/mptcp/pm_kernel.c
>> +++ b/net/mptcp/pm_kernel.c
>> @@ -969,7 +969,7 @@ static int mptcp_nl_add_subflow_or_signal_addr(struct net *net,
>>  		struct sock *sk = (struct sock *)msk;
>>  		struct mptcp_addr_info mpc_addr;
>>  
>> -		if (!READ_ONCE(msk->fully_established) ||
>> +		if (!mptcp_is_fully_established(sk) ||
>>  		    mptcp_pm_is_userspace(msk))
>>  			goto next;
> 
> I'm a little double-minded here.
> 
> The add/removal operations are issued by the user space to update
> subflows, as a consequence of endpoint addition/removal.
> 
> Possibly removing the subflow even for not fully established connections
> could be the right thing to do, I'm not 110% sure either way. Possibly
> worth to call this out with a comment?

Indeed, I'm also not sure. Initially, I only converted this
READ_ONCE(msk->fully_established), also because later in
mptcp_pm_create_subflow_or_signal_addr(), mptcp_is_fully_established()
is used.

Probably best to stick to this one.

On the other hand, I just noticed that there is no check for "fallback
done" here: so the operations leading to a notification (ADD_ADDR,
RM_ADDR) will send a dup-ACK (pure ACK without MPTCP options), and
adding an MPTCP endpoint will lead to... the creation of a new subflow,
even after a fallback, e.g.

-------------------------- 8< --------------------------
C > S: Flags [S], seq 0, win 65535, options [mss 1460,sackOK,nop,nop,nop,wscale 6,mptcp 4 capable v1], length 0
S > C: Flags [S.], seq 2252073282, ack 1, win 65535, options [mss 1460,nop,nop,sackOK,nop,wscale 8,mptcp 12 capable v1 {0xb80fe7957527e1f7}], length 0
C > S: Flags [.], ack 1, win 2048, options [mptcp 20 capable v1 {0x200000000000000,0xb80fe7957527e1f7}], length 0
C > S: Flags [P.], seq 1:501, ack 1, win 2048, options [mptcp 22 capable v1 {0xb80fe7957527e1f7,0x200000000000000},nop,nop], length 500: HTTP
S > C: Flags [.], ack 501, win 1049, options [mptcp 12 dss ack 13263177308786789273], length 0

// Here, the client receives the ACK without MPTCP options due to middleboxes → fallback

S > C: Flags [P.], seq 1:101, ack 501, win 1049, options [mptcp 26 dss ack 13263177308786789273 seq 14591534807969507511 subseq 1 len 100,nop,nop], length 100: HTTP
C > S: Flags [.], ack 101, win 2048, length 0

// fallback on the server side

C > S: Flags [P.], seq 501:601, ack 101, win 2048, length 100: HTTP
S > C: Flags [.], ack 601, win 1049, length 0

// Data OK
// Adding a new subflow endpoint now:

S2 > C: Flags [S], seq 618398320, win 65535, options [mss 1460,sackOK,TS val 3041525989 ecr 0,nop,wscale 8,mptcp 12 join id 1 token 0xd86e8112 nonce 0x6cca79f5], length 0
-------------------------- 8< --------------------------

Oops. Still, not a big deal, but probably better and safer to avoid
that, no?

Maybe patch 1 (reset fully_estab after a fallback) and this patch here
but only looking at READ_ONCE(msk->fully_established) is enough?
Patch 2 would be for -next.

> Also the asymmetry with the user-space PM is a bit strange. I can't
> recall why it was needed in the first place?!?

Are you talking about all the mptcp_pm_is_userspace() checks? If yes,
they are required because the pm_type sysctl could be changed, and we
could end-up with a mix of msk handled by different PMs in the same
netns.

It would be better to do the opposite I guess:

  if (!mptcp_pm_is_kernel(msk))
      goto next;

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

Re: [PATCH mptcp-next RFC 3/3] mptcp: pm: kernel: skip operating on closing connections

[Paolo Abeni]

On 8/17/26 1:07 PM, Matthieu Baerts wrote:
> On 17/08/2026 09:39, Paolo Abeni wrote:
>> On 8/14/26 8:31 PM, Matthieu Baerts (NGI0) wrote:
>>> When iterating over each MPTCP connection after having manipulated MPTCP
>>> endpoints, there is no need to operate on closing (or not ready)
>>> connections.
>>>
>>> We can then safely skip those.
>>>
>>> Note that mptcp_nl_add_subflow_or_signal_addr() was already checking the
>>> fully_established state, but it is better to check the connection state,
>>> which is what mptcp_is_fully_established() is doing.
>>>
>>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>> ---
>>>  net/mptcp/pm_kernel.c | 16 +++++++++++-----
>>>  1 file changed, 11 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
>>> index d3014bf57bf3..c185e53fe42b 100644
>>> --- a/net/mptcp/pm_kernel.c
>>> +++ b/net/mptcp/pm_kernel.c
>>> @@ -969,7 +969,7 @@ static int mptcp_nl_add_subflow_or_signal_addr(struct net *net,
>>>  		struct sock *sk = (struct sock *)msk;
>>>  		struct mptcp_addr_info mpc_addr;
>>>  
>>> -		if (!READ_ONCE(msk->fully_established) ||
>>> +		if (!mptcp_is_fully_established(sk) ||
>>>  		    mptcp_pm_is_userspace(msk))
>>>  			goto next;
>>
>> I'm a little double-minded here.
>>
>> The add/removal operations are issued by the user space to update
>> subflows, as a consequence of endpoint addition/removal.
>>
>> Possibly removing the subflow even for not fully established connections
>> could be the right thing to do, I'm not 110% sure either way. Possibly
>> worth to call this out with a comment?
> 
> Indeed, I'm also not sure. Initially, I only converted this
> READ_ONCE(msk->fully_established), also because later in
> mptcp_pm_create_subflow_or_signal_addr(), mptcp_is_fully_established()
> is used.
> 
> Probably best to stick to this one.
> 
> On the other hand, I just noticed that there is no check for "fallback
> done" here: so the operations leading to a notification (ADD_ADDR,
> RM_ADDR) will send a dup-ACK (pure ACK without MPTCP options), and
> adding an MPTCP endpoint will lead to... the creation of a new subflow,
> even after a fallback, e.g.

Sorry, I was not clear enough. I'm specifically doubtful WRT the remove
operations (i.e. mptcp_nl_flush_addrs_list,
mptcp_nl_remove_id_zero_address,
mptcp_nl_remove_subflow_and_signal_addr) and _not_ vs the 'add' one
(mptcp_nl_add_subflow_or_signal_addr). I'm fine with the check change there.

>> Also the asymmetry with the user-space PM is a bit strange. I can't
>> recall why it was needed in the first place?!?
> 
> Are you talking about all the mptcp_pm_is_userspace() checks? If yes,
> they are required because the pm_type sysctl could be changed, and we
> could end-up with a mix of msk handled by different PMs in the same
> netns.
> 
> It would be better to do the opposite I guess:
> 
>   if (!mptcp_pm_is_kernel(msk))
>       goto next;
Yup, I prefer the check this way more.

My doubt is that user-space will allow creating subflow even for not
established msk.

/P

Re: [PATCH mptcp-next RFC 3/3] mptcp: pm: kernel: skip operating on closing connections

[Matthieu Baerts]

On 17/08/2026 15:09, Paolo Abeni wrote:
> On 8/17/26 1:07 PM, Matthieu Baerts wrote:
>> On 17/08/2026 09:39, Paolo Abeni wrote:
>>> On 8/14/26 8:31 PM, Matthieu Baerts (NGI0) wrote:
>>>> When iterating over each MPTCP connection after having manipulated MPTCP
>>>> endpoints, there is no need to operate on closing (or not ready)
>>>> connections.
>>>>
>>>> We can then safely skip those.
>>>>
>>>> Note that mptcp_nl_add_subflow_or_signal_addr() was already checking the
>>>> fully_established state, but it is better to check the connection state,
>>>> which is what mptcp_is_fully_established() is doing.
>>>>
>>>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>>> ---
>>>>  net/mptcp/pm_kernel.c | 16 +++++++++++-----
>>>>  1 file changed, 11 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
>>>> index d3014bf57bf3..c185e53fe42b 100644
>>>> --- a/net/mptcp/pm_kernel.c
>>>> +++ b/net/mptcp/pm_kernel.c
>>>> @@ -969,7 +969,7 @@ static int mptcp_nl_add_subflow_or_signal_addr(struct net *net,
>>>>  		struct sock *sk = (struct sock *)msk;
>>>>  		struct mptcp_addr_info mpc_addr;
>>>>  
>>>> -		if (!READ_ONCE(msk->fully_established) ||
>>>> +		if (!mptcp_is_fully_established(sk) ||
>>>>  		    mptcp_pm_is_userspace(msk))
>>>>  			goto next;
>>>
>>> I'm a little double-minded here.
>>>
>>> The add/removal operations are issued by the user space to update
>>> subflows, as a consequence of endpoint addition/removal.
>>>
>>> Possibly removing the subflow even for not fully established connections
>>> could be the right thing to do, I'm not 110% sure either way. Possibly
>>> worth to call this out with a comment?
>>
>> Indeed, I'm also not sure. Initially, I only converted this
>> READ_ONCE(msk->fully_established), also because later in
>> mptcp_pm_create_subflow_or_signal_addr(), mptcp_is_fully_established()
>> is used.
>>
>> Probably best to stick to this one.
>>
>> On the other hand, I just noticed that there is no check for "fallback
>> done" here: so the operations leading to a notification (ADD_ADDR,
>> RM_ADDR) will send a dup-ACK (pure ACK without MPTCP options), and
>> adding an MPTCP endpoint will lead to... the creation of a new subflow,
>> even after a fallback, e.g.
> 
> Sorry, I was not clear enough. I'm specifically doubtful WRT the remove
> operations (i.e. mptcp_nl_flush_addrs_list,
> mptcp_nl_remove_id_zero_address,
> mptcp_nl_remove_subflow_and_signal_addr) and _not_ vs the 'add' one
> (mptcp_nl_add_subflow_or_signal_addr). I'm fine with the check change there.

Sorry, I think it was me not being clear enough:

- In mptcp_nl_add_subflow_or_signal_addr: use mptcp_is_fully_established
- In the others (flush, 2 remove): check only msk->fully_established

To avoid acting on fallback MPTCP connections, should we target patch 1
(reset fully_estab after a fallback) and this one here for -net?
Or only patch 1?

>>> Also the asymmetry with the user-space PM is a bit strange. I can't
>>> recall why it was needed in the first place?!?
>>
>> Are you talking about all the mptcp_pm_is_userspace() checks? If yes,
>> they are required because the pm_type sysctl could be changed, and we
>> could end-up with a mix of msk handled by different PMs in the same
>> netns.
>>
>> It would be better to do the opposite I guess:
>>
>>   if (!mptcp_pm_is_kernel(msk))
>>       goto next;
> Yup, I prefer the check this way more.
> 
> My doubt is that user-space will allow creating subflow even for not
> established msk.

It shouldn't: in __mptcp_subflow_connect(), it will return early if
mptcp_is_fully_established() is false.

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