[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

[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