[RFC PATCH mptcp-next v3 0/3] mptcp: pm: kernel: retry subflows closed after an error

[RFC PATCH mptcp-next v3 0/3] mptcp: pm: kernel: retry subflows closed after an error

[Anil Kaushik]

A subflow closed after an error is never re-created, even once the path
is back (#440).

When TCP closes a subflow we created because of an error, the PM opens
a new one to the same addresses, and if it fails, the same socket is
used for the next try. A new sysctl, net.mptcp.subflow_retries, sets the
max tries.

Open question: an ICMP unreachable aborts the SYN right away, so the
tries can be used up in a few RTTs. Adding a delay would need a timer
again. Is that acceptable, or is there a better way?

Changes in v3 (Sashiko and CI reviews):
 - no retry after MP_RST without the T flag, local abort or EBADMSG
 - no retry on a removed endpoint or address
 - count the tries from scratch only after a minute of uptime
 - use only request_bkup for the backup flag
 - reused socket: SUB_CLOSED event, MIB counter and BPF pre_connect
 - lock_sock_nested() when reconnecting
 - patch 1: don't wait forever on EAGAIN
 - patch 3: check the subflow address, create the files before the
   background job, bigger file for slow debug kernels
 - wrap lines over 80 columns

Changes in v2:
 - use the subflow sockets, no list or timer
 - sysctl is now the max number of tries
 - code moved to pm_kernel.c
 - Fixes tag on patch 1
 - one selftest with netfilter
 - Assisted-by on all patches

v2: https://lore.kernel.org/mptcp/20261013025653.523755-1-anilkaushikwireless@gmail.com/
v1: https://lore.kernel.org/mptcp/20261012171740.464517-1-anilkaushikwireless@gmail.com/

Anil Kaushik (3):
  selftests: mptcp: connect: retry write() on EAGAIN
  mptcp: pm: kernel: retry subflows closed after an error
  selftests: mptcp: join: validate subflow retry after link failure

 Documentation/networking/mptcp-sysctl.rst     |  18 +++
 net/mptcp/ctrl.c                              |  13 ++
 net/mptcp/mib.c                               |   1 +
 net/mptcp/mib.h                               |   1 +
 net/mptcp/pm.c                                |   8 +
 net/mptcp/pm_kernel.c                         | 145 ++++++++++++++++++
 net/mptcp/protocol.c                          |   9 ++
 net/mptcp/protocol.h                          |  24 +++
 net/mptcp/subflow.c                           |  63 ++++++++
 .../selftests/net/mptcp/mptcp_connect.c       |  16 ++
 .../testing/selftests/net/mptcp/mptcp_join.sh | 106 +++++++++++++
 11 files changed, 404 insertions(+)

-- 
2.25.1

[RFC PATCH mptcp-next v3 1/3] selftests: mptcp: connect: retry write() on EAGAIN

[Anil Kaushik]

mptcp_connect treats any write() error as fatal. But write() can fail
with EAGAIN even after poll() said the socket was writable: the MPTCP
send buffer shrinks when a subflow is closed.

The tool then exits and the transfer fails on both sides. Instead, go
back to poll() and wait for more space. If EAGAIN lasts longer than the
poll timeout, still fail: poll() does not check every cause of EAGAIN.

Fixes: 048d19d444be ("mptcp: add basic kselftest for mptcp")
Assisted-by: LLM
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---
 .../testing/selftests/net/mptcp/mptcp_connect.c  | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index 178d98d91fea..03fff859e4d1 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -638,6 +638,7 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd,
 		.events = POLLIN | POLLOUT,
 	};
 	unsigned int total_wlen = 0, total_rlen = 0;
+	int eagain_ms = 0;
 
 	set_nonblock(peerfd, true);
 
@@ -711,6 +712,20 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd,
 
 				bw = do_rnd_write(peerfd, winfo->buf + winfo->off, winfo->len);
 				if (bw < 0) {
+					/* POLLOUT does not guarantee that
+					 * the next write will not block:
+					 * the MPTCP send buffer can shrink
+					 * in between, e.g. when a subflow
+					 * is closed. Wait for more space,
+					 * but not forever: poll() does not
+					 * check every cause of EAGAIN.
+					 */
+					if (errno == EAGAIN &&
+					    ++eagain_ms <= poll_timeout) {
+						usleep(1000);
+						continue;
+					}
+
 					/* expected reset, continue to read */
 					if (cfg_rcv_trunc &&
 					    (errno == ECONNRESET ||
@@ -723,6 +738,7 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd,
 					return 111;
 				}
 
+				eagain_ms = 0;
 				winfo->off += bw;
 				winfo->len -= bw;
 				total_wlen += bw;
-- 
2.25.1

[RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[Anil Kaushik]

With the in-kernel PM, a subflow closed after an error, e.g. a timeout
when a path silently drops packets, is never re-created, even once the
path works again. For example, with a main link and a backup link,
connections move to the backup after an outage but never come back.

Add an opt-in sysctl, net.mptcp.subflow_retries (0 by default): the
maximum number of attempts to re-create such a subflow. When TCP closes
a subflow created by this side after an error, the PM creates a new one
with the same addresses right away. Its SYN retransmissions space out
the attempts, so no extra state or timer is needed. If it cannot
connect, the same socket is used for the next attempt. The number of
attempts is kept in the subflow context, and counted from scratch once
a re-created subflow stayed established for a minute. Re-creating the
initial subflow uses one of the additional subflows allowed by the
limits.

Subflows refused by the peer, reset by the peer without the transient
flag (RFC 8684 section 3.6), aborted locally, or using a removed address
are not retried.

A new counter, SubflowRetry, counts the attempts.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/440
Assisted-by: LLM
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---
 Documentation/networking/mptcp-sysctl.rst |  18 +++
 net/mptcp/ctrl.c                          |  13 ++
 net/mptcp/mib.c                           |   1 +
 net/mptcp/mib.h                           |   1 +
 net/mptcp/pm.c                            |   8 ++
 net/mptcp/pm_kernel.c                     | 145 ++++++++++++++++++++++
 net/mptcp/protocol.c                      |   9 ++
 net/mptcp/protocol.h                      |  24 ++++
 net/mptcp/subflow.c                       |  63 ++++++++++
 9 files changed, 282 insertions(+)

diff --git a/Documentation/networking/mptcp-sysctl.rst b/Documentation/networking/mptcp-sysctl.rst
index b9b5f58e0625..544f6b84d4af 100644
--- a/Documentation/networking/mptcp-sysctl.rst
+++ b/Documentation/networking/mptcp-sysctl.rst
@@ -149,6 +149,24 @@ stale_loss_cnt - INTEGER
 
 	Default: 4
 
+subflow_retries - INTEGER
+	Maximum number of attempts to re-create a subflow closed by TCP after
+	an error, e.g. a timeout or an unreachable path, while the MPTCP
+	connection is still established. The in-kernel path manager creates a
+	new subflow with the same local and remote addresses: its SYN
+	retransmissions, see tcp_syn_retries, space out the attempts. If it
+	cannot connect, the same socket is used for the next attempt. The
+	count starts again once a re-created subflow stayed established for a
+	minute. Only subflows initiated by this side are re-created, within
+	the limits set for the path manager: re-creating the initial subflow
+	uses one of the additional subflows. Subflows refused by the peer,
+	reset by the peer without the transient flag, aborted locally, or
+	using a removed address are not retried.
+
+	0 disables the retries. This is a per-namespace sysctl.
+
+	Default: 0
+
 syn_retrans_before_tcp_fallback - INTEGER
 	The number of SYN + MP_CAPABLE retransmissions before falling back to
 	TCP, i.e. dropping the MPTCP options. In other words, if all the packets
diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 63c5747f0f63..37d96f89c4b1 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -34,6 +34,7 @@ struct mptcp_pernet {
 	atomic_t active_disable_times;
 	unsigned long active_disable_stamp;
 	u8 syn_retrans_before_tcp_fallback;
+	u8 subflow_retries;
 	u8 mptcp_enabled;
 	u8 checksum_enabled;
 	u8 allow_join_initial_addr_port;
@@ -73,6 +74,11 @@ unsigned int mptcp_stale_loss_cnt(const struct net *net)
 	return mptcp_get_pernet(net)->stale_loss_cnt;
 }
 
+u8 mptcp_subflow_retries(const struct net *net)
+{
+	return READ_ONCE(mptcp_get_pernet(net)->subflow_retries);
+}
+
 unsigned int mptcp_close_timeout(const struct sock *sk)
 {
 	if (sock_flag(sk, SOCK_DEAD))
@@ -377,6 +383,12 @@ static struct ctl_table mptcp_sysctl_table[] = {
 		.extra1       = SYSCTL_ZERO,
 		.extra2       = SYSCTL_ONE
 	},
+	{
+		.procname = "subflow_retries",
+		.maxlen = sizeof(u8),
+		.mode = 0644,
+		.proc_handler = proc_dou8vec_minmax,
+	},
 };
 
 static int mptcp_pernet_new_table(struct net *net, struct mptcp_pernet *pernet)
@@ -405,6 +417,7 @@ static int mptcp_pernet_new_table(struct net *net, struct mptcp_pernet *pernet)
 	table[11].data = &pernet->path_manager;
 	/* table[12] is for available_path_managers which is read-only info */
 	table[13].data = &pernet->add_addr_v6_port_drop_ts;
+	table[14].data = &pernet->subflow_retries;
 
 	hdr = register_net_sysctl_sz(net, MPTCP_SYSCTL_PATH, table,
 				     ARRAY_SIZE(mptcp_sysctl_table));
diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
index 608cb568897c..5263ac6f2c43 100644
--- a/net/mptcp/mib.c
+++ b/net/mptcp/mib.c
@@ -95,6 +95,7 @@ static const struct snmp_mib mptcp_snmp_list[] = {
 	SNMP_MIB_ITEM("BacklogDrop", MPTCP_MIB_BACKLOGDROP),
 	SNMP_MIB_ITEM("RcvPruned", MPTCP_MIB_RCVPRUNED),
 	SNMP_MIB_ITEM("OFOPruned", MPTCP_MIB_OFOPRUNED),
+	SNMP_MIB_ITEM("SubflowRetry", MPTCP_MIB_SUBFLOWRETRY),
 };
 
 /* mptcp_mib_alloc - allocate percpu mib counters
diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
index 1ebdb55e9534..1359839f4bbe 100644
--- a/net/mptcp/mib.h
+++ b/net/mptcp/mib.h
@@ -98,6 +98,7 @@ enum linux_mptcp_mib_field {
 	MPTCP_MIB_BACKLOGDROP,		/* Backlog over memory limit */
 	MPTCP_MIB_RCVPRUNED,		/* Dropped due to memory constraints */
 	MPTCP_MIB_OFOPRUNED,		/* MPTCP-level OoO queue pruned */
+	MPTCP_MIB_SUBFLOWRETRY,		/* Retried a subflow closed on error */
 	__MPTCP_MIB_MAX
 };
 
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index d7c5b50b34cc..b3f7f7b2ef84 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -794,6 +794,14 @@ static void mptcp_pm_rm_addr_or_subflow(struct mptcp_sock *msk,
 			int how = RCV_SHUTDOWN | SEND_SHUTDOWN;
 			u8 id = subflow_get_local_id(subflow);
 
+			/* a dead subflow still waiting to be closed must not
+			 * be re-created on a removed address
+			 */
+			if ((rm_type == MPTCP_MIB_RMADDR &&
+			     remote_id == rm_id) ||
+			    (rm_type == MPTCP_MIB_RMSUBFLOW && id == rm_id))
+				subflow->retries = MPTCP_SUBFLOW_NO_RETRY;
+
 			if ((1 << inet_sk_state_load(ssk)) &
 			    (TCPF_FIN_WAIT1 | TCPF_FIN_WAIT2 | TCPF_CLOSING | TCPF_CLOSE))
 				continue;
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index d3014bf57bf3..d020e5f69c01 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -1591,6 +1591,151 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk)
 	return true;
 }
 
+/* a re-created subflow reset sooner counts as a failed attempt */
+#define MPTCP_PM_RETRY_STABLE	(60 * HZ)
+
+static bool mptcp_pm_retry_path_error(int err)
+{
+	switch (err) {
+	case ETIMEDOUT:
+	case EHOSTUNREACH:
+	case ENETUNREACH:
+	case EHOSTDOWN:
+	case ENETDOWN:
+		return true;
+	}
+
+	return false;
+}
+
+/* A subflow initiated by this side and closed by TCP after an error can be
+ * re-created with the same addresses, see the 'subflow_retries' sysctl. The
+ * SYN retransmissions of the new subflow space out the attempts. Called with
+ * the msk socket lock held, before the subflow is closed.
+ */
+bool mptcp_pm_kernel_retry_check(struct mptcp_sock *msk,
+				 const struct mptcp_subflow_context *subflow,
+				 struct mptcp_pm_retry *retry)
+{
+	struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+	bool established = READ_ONCE(subflow->fully_established);
+	struct sock *sk = (struct sock *)msk;
+	int err = READ_ONCE(ssk->sk_err);
+
+	if (!mptcp_pm_is_kernel(msk) || !err ||
+	    sk->sk_state != TCP_ESTABLISHED || __mptcp_check_fallback(msk))
+		return false;
+
+	/* mptcp_close_ssk() will not handle it again, e.g. the initial
+	 * subflow, kept in the list once closed
+	 */
+	if (subflow->close_event_done || READ_ONCE(subflow->local_id) < 0)
+		return false;
+
+	/* removed address, see mptcp_pm_rm_addr_or_subflow() */
+	if (subflow->retries == MPTCP_SUBFLOW_NO_RETRY)
+		return false;
+
+	/* The peer asked not to use this path again (MP_RST without the T
+	 * flag), the subflow has been aborted locally, or it hit an MPTCP
+	 * protocol error.
+	 */
+	if ((subflow->reset_seen && !subflow->reset_transient) ||
+	    err == ECONNABORTED || err == EBADMSG)
+		return false;
+
+	/* A subflow that never got established is retried only after network
+	 * errors: if the peer rejected it, trying again will not help.
+	 */
+	if (!established && !mptcp_pm_retry_path_error(err))
+		return false;
+
+	/* Only re-create subflows initiated by this side: MP_JOIN ones, or the
+	 * initial one on the client side.
+	 */
+	if (!subflow->request_join &&
+	    (subflow->mp_join || READ_ONCE(msk->pm.server_side)))
+		return false;
+
+	/* Count the attempts from scratch only if the path worked for a
+	 * while: a re-created subflow reset soon after its creation counts
+	 * as a failed attempt.
+	 */
+	retry->retries = subflow->retries;
+	if (established &&
+	    (!subflow->retries ||
+	     (u32)(tcp_jiffies32 - subflow->retry_stamp) >=
+	     MPTCP_PM_RETRY_STABLE))
+		retry->retries = 0;
+	/* a failed attempt is retried with the same socket */
+	retry->reuse = !established && subflow->retries;
+	if (retry->retries >= mptcp_subflow_retries(sock_net(sk)))
+		return false;
+
+	mptcp_remote_address((struct sock_common *)ssk, &retry->remote);
+	retry->remote.id = READ_ONCE(subflow->remote_id);
+	if (!retry->remote.id && READ_ONCE(msk->pm.remote_deny_join_id0))
+		return false;
+
+	mptcp_local_address((struct sock_common *)ssk, &retry->local.addr);
+	retry->local.addr.port = 0;
+	retry->local.addr.id = subflow_get_local_id(subflow);
+	retry->local.flags = subflow->request_bkup ?
+			     MPTCP_PM_ADDR_FLAG_BACKUP : 0;
+	retry->local.ifindex = ssk->sk_bound_dev_if;
+
+	return true;
+}
+
+/* Called with the msk socket lock held, instead of closing the subflow */
+bool mptcp_pm_kernel_retry_reconnect(struct mptcp_sock *msk, struct sock *ssk,
+				     const struct mptcp_pm_retry *retry)
+{
+	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
+	struct sock *sk = (struct sock *)msk;
+
+	/* like mptcp_close_ssk() does for the failed attempt */
+	mptcp_event(MPTCP_EVENT_SUB_CLOSED, msk, ssk, GFP_KERNEL);
+
+	MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_SUBFLOWRETRY);
+	if (mptcp_subflow_reconnect(sk, ssk, &retry->local, &retry->remote))
+		return false;
+
+	/* the reset cleared them */
+	subflow->retries = retry->retries + 1;
+	subflow->retry_stamp = tcp_jiffies32;
+	return true;
+}
+
+/* Called with the msk socket lock held, after the subflow has been closed */
+void mptcp_pm_kernel_retry(struct mptcp_sock *msk,
+			   const struct mptcp_pm_retry *retry)
+{
+	struct mptcp_subflow_context *subflow;
+	struct sock *sk = (struct sock *)msk;
+	bool allowed;
+
+	spin_lock_bh(&msk->pm.lock);
+	allowed = msk->pm.extra_subflows <
+		  mptcp_pm_get_limit_extra_subflows(msk);
+	if (allowed)
+		msk->pm.extra_subflows++;
+	spin_unlock_bh(&msk->pm.lock);
+
+	if (!allowed)
+		return;
+
+	MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_SUBFLOWRETRY);
+	if (__mptcp_subflow_connect(sk, &retry->local, &retry->remote))
+		return;
+
+	/* the new subflow is the last one: it carries the number of attempts */
+	subflow = list_last_entry(&msk->conn_list, struct mptcp_subflow_context,
+				  node);
+	subflow->retries = retry->retries + 1;
+	subflow->retry_stamp = tcp_jiffies32;
+}
+
 /* Called under PM lock */
 void __mptcp_pm_kernel_worker(struct mptcp_sock *msk)
 {
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 60cbd91be632..f29cfd2ad69b 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2816,6 +2816,8 @@ static void __mptcp_close_subflow(struct sock *sk)
 	mptcp_for_each_subflow_safe(msk, subflow, tmp) {
 		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
 		int ssk_state = inet_sk_state_load(ssk);
+		struct mptcp_pm_retry retry;
+		bool do_retry;
 
 		if (ssk_state != TCP_CLOSE &&
 		    (ssk_state != TCP_CLOSE_WAIT ||
@@ -2827,7 +2829,14 @@ static void __mptcp_close_subflow(struct sock *sk)
 		if (!skb_queue_empty_lockless(&ssk->sk_receive_queue))
 			continue;
 
+		do_retry = mptcp_pm_kernel_retry_check(msk, subflow, &retry);
+		if (do_retry && retry.reuse &&
+		    mptcp_pm_kernel_retry_reconnect(msk, ssk, &retry))
+			continue;
+
 		mptcp_close_ssk(sk, ssk, subflow);
+		if (do_retry && !retry.reuse)
+			mptcp_pm_kernel_retry(msk, &retry);
 	}
 
 }
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index ed2f363bb915..1aa92c9614cd 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -622,6 +622,8 @@ struct mptcp_subflow_context {
 	u8	reset_transient:1;
 	u8	reset_reason:4;
 	u8	stale_count;	    /* Protected by the msk socket lock */
+	u8	retries;	    /* re-creation attempts, msk socket lock */
+	u32	retry_stamp;	    /* creation of a re-created subflow */
 	u8	stale;		    /* Protected by the msk socket lock,
 				     * if set the subflow is unable to snd/rcv
 				     * data, the schedule should skip it
@@ -818,6 +820,7 @@ int mptcp_is_checksum_enabled(const struct net *net);
 int mptcp_allow_join_id0(const struct net *net);
 unsigned int mptcp_stale_loss_cnt(const struct net *net);
 unsigned int mptcp_close_timeout(const struct sock *sk);
+u8 mptcp_subflow_retries(const struct net *net);
 int mptcp_get_pm_type(const struct net *net);
 const char *mptcp_get_path_manager(const struct net *net);
 const char *mptcp_get_scheduler(const struct net *net);
@@ -859,6 +862,9 @@ void mptcp_remote_address(const struct sock_common *skc,
 			  struct mptcp_addr_info *addr);
 
 /* called with sk socket lock held */
+int mptcp_subflow_reconnect(struct sock *sk, struct sock *ssk,
+			    const struct mptcp_pm_local *local,
+			    const struct mptcp_addr_info *remote);
 int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_local *local,
 			    const struct mptcp_addr_info *remote);
 int mptcp_subflow_create_socket(struct sock *sk, unsigned short family,
@@ -1258,6 +1264,24 @@ void __init mptcp_pm_userspace_register(void);
 void __init mptcp_pm_nl_init(void);
 void mptcp_pm_worker(struct mptcp_sock *msk);
 void __mptcp_pm_kernel_worker(struct mptcp_sock *msk);
+
+/* subflow->retries value preventing any re-creation */
+#define MPTCP_SUBFLOW_NO_RETRY	U8_MAX
+
+struct mptcp_pm_retry {
+	struct mptcp_pm_local	local;
+	struct mptcp_addr_info	remote;
+	u8			retries;
+	bool			reuse;	/* failed attempt: same socket */
+};
+
+bool mptcp_pm_kernel_retry_check(struct mptcp_sock *msk,
+				 const struct mptcp_subflow_context *subflow,
+				 struct mptcp_pm_retry *retry);
+void mptcp_pm_kernel_retry(struct mptcp_sock *msk,
+			   const struct mptcp_pm_retry *retry);
+bool mptcp_pm_kernel_retry_reconnect(struct mptcp_sock *msk, struct sock *ssk,
+				     const struct mptcp_pm_retry *retry);
 u8 mptcp_pm_get_endp_signal_max(const struct mptcp_sock *msk);
 u8 mptcp_pm_get_endp_subflow_max(const struct mptcp_sock *msk);
 u8 mptcp_pm_get_endp_laminar_max(const struct mptcp_sock *msk);
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 78e886b20258..0cdeae4b45fa 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1766,6 +1766,69 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_local *local,
 	return err;
 }
 
+/* Connect again an MPJ subflow that failed to connect, with the same socket.
+ * The subflow never carried data: like for the initial subflow in
+ * __mptcp_subflow_disconnect(), resetting the TCP and the subflow states is
+ * enough. Called with the msk socket lock held.
+ */
+int mptcp_subflow_reconnect(struct sock *sk, struct sock *ssk,
+			    const struct mptcp_pm_local *local,
+			    const struct mptcp_addr_info *remote)
+{
+	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
+	struct sockaddr_unsized *uaddr;
+	struct mptcp_sock *msk = mptcp_sk(sk);
+	struct sockaddr_storage addr;
+	u32 remote_token;
+	int addrlen;
+	int err;
+
+	lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
+	err = tcp_disconnect(ssk, 0);
+	if (err)
+		goto out;
+
+	mptcp_subflow_ctx_reset(subflow);
+	subflow->request_mptcp = 0;
+	do {
+		subflow->local_nonce = get_random_u32();
+	} while (!subflow->local_nonce);
+	subflow_set_local_id(subflow, local->addr.id);
+	subflow->remote_key_valid = 1;
+	subflow->remote_key = READ_ONCE(msk->remote_key);
+	subflow->local_key = READ_ONCE(msk->local_key);
+	subflow->token = msk->token;
+	mptcp_crypto_key_sha(subflow->remote_key, &remote_token, NULL);
+	subflow->remote_token = remote_token;
+	WRITE_ONCE(subflow->remote_id, remote->id);
+	subflow->request_join = 1;
+	subflow->request_bkup = !!(local->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
+	subflow->subflow_id = msk->subflow_id++;
+
+	mptcp_info2sockaddr(remote, &addr, ssk->sk_family);
+	addrlen = sizeof(struct sockaddr_in);
+#if IS_ENABLED(CONFIG_MPTCP_IPV6)
+	if (addr.ss_family == AF_INET6)
+		addrlen = sizeof(struct sockaddr_in6);
+#endif
+	uaddr = (struct sockaddr_unsized *)&addr;
+
+	/* like __inet_stream_connect() for __mptcp_subflow_connect() */
+	if (BPF_CGROUP_PRE_CONNECT_ENABLED(ssk)) {
+		err = ssk->sk_prot->pre_connect(ssk, uaddr, addrlen);
+		if (err)
+			goto out;
+	}
+
+	err = ssk->sk_prot->connect(ssk, uaddr, addrlen);
+
+out:
+	release_sock(ssk);
+	MPTCP_INC_STATS(sock_net(sk), err ? MPTCP_MIB_JOINSYNTXCONNECTERR :
+					    MPTCP_MIB_JOINSYNTX);
+	return err;
+}
+
 void __mptcp_inherit_memcg(struct sock *sk, struct sock *ssk, gfp_t gfp)
 {
 	/* Only if the msk has been accepted already (and not orphaned).*/
-- 
2.25.1

[RFC PATCH mptcp-next v3 3/3] selftests: mptcp: join: validate subflow retry after link failure

[Anil Kaushik]

Test the new sysctl with a long connection in the background: drop the
packets the client sends from the additional subflow address with
netfilter, without bringing the link down, until the subflow is closed
and a retry happens. Then let them through again, and check the subflow
comes back, from the same address. Then do the same with the initial
subflow.

Assisted-by: LLM
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 106 ++++++++++++++++++
 1 file changed, 106 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index f4d8afaeda4f..1048c9cfceb7 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -2151,6 +2151,76 @@ wait_attempt_fail()
 	return 1
 }
 
+# $1: ns ; $2: minimum number of retries
+wait_sf_retry()
+{
+	local ns="${1}"
+	local retry_min="${2}"
+	local cnt
+
+	for _ in $(seq $((timeout_poll * 10))); do
+		cnt=$(mptcp_lib_get_counter "${ns}" "MPTcpExtSubflowRetry")
+		[ "${cnt:-0}" -ge "${retry_min}" ] && return 0
+		sleep 0.1
+	done
+
+	return 1
+}
+
+# $1: expected number of MPJ ACK received by the server
+wait_mpj_ack()
+{
+	local exp_cnt="${1}"
+	local cnt
+
+	for _ in $(seq $((timeout_poll * 10))); do
+		cnt=$(mptcp_lib_get_counter "${ns1}" "MPTcpExtMPJoinAckRx")
+		[ "${cnt:-0}" -ge "${exp_cnt}" ] && return 0
+		sleep 0.1
+	done
+
+	return 1
+}
+
+# $1: ns ; $2: minimum number of retries
+chk_sf_retry_nr()
+{
+	local ns="${1}"
+	local retry_min="${2}"
+	local count
+
+	print_check "subflow retry"
+	count=$(mptcp_lib_get_counter "${ns}" "MPTcpExtSubflowRetry")
+	if [ -z "${count}" ]; then
+		print_skip
+	elif [ "${count}" -lt "${retry_min}" ]; then
+		fail_test "got ${count} retry[s] expected at least ${retry_min}"
+	else
+		print_ok
+	fi
+}
+
+# $1: client address to block ; $2: retries ; $3: MPJ, expected after it
+#
+# The link stays up, but the client packets from this address are dropped:
+# the subflow using it is closed after a few retransmissions, then re-created
+# once the packets go through again.
+sf_retry_after_block()
+{
+	ip netns exec $ns2 nft add rule ip filter OUTPUT ip saddr "${1}" drop
+	wait_sf_retry $ns2 "${2}"
+	ip netns exec $ns2 nft flush chain ip filter OUTPUT
+	wait_mpj_ack "${3}"
+
+	print_check "subflow from ${1}"
+	if ip netns exec $ns2 ss -tnH state established src "${1}" |
+	   grep -q .; then
+		print_ok
+	else
+		fail_test "no established subflow from ${1}"
+	fi
+}
+
 set_userspace_pm()
 {
 	local ns=$1
@@ -2585,6 +2655,42 @@ link_failure_tests()
 		chk_stale_nr $ns2 1 -1 2
 		chk_link_usage $ns2 ns2eth3 $cinsent 50
 	fi
+
+	# subflows closed after a silent link failure, then re-created: the
+	# additional one, then the initial one, with an MPJ using ID 0
+	if reset "subflow retry after link failure" &&
+	   continue_if mptcp_lib_has_file \
+			'/proc/sys/net/mptcp/subflow_retries'; then
+		init_nftables
+		init_shapers
+		ip netns exec $ns2 sysctl -q net.mptcp.subflow_retries=15
+		ip netns exec $ns2 sysctl -q net.ipv4.tcp_retries2=3
+		ip netns exec $ns2 sysctl -q net.ipv4.tcp_syn_retries=1
+		pm_nl_set_limits $ns1 0 4
+		pm_nl_set_limits $ns2 0 2
+		pm_nl_add_endpoint $ns2 10.0.3.2 dev ns2eth3 flags subflow
+		# created here, not in the background job, for cleanup()
+		if [ -z "$cinfail" ]; then
+			cinfail=$(mktemp)
+		fi
+		if [ -z "$sinfail" ]; then
+			sinfail=$(mktemp)
+		fi
+		{ timeout_test=120 test_linkfail=131072 \
+			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
+		local tests_pid=$!
+
+		wait_mpj_ack 1
+		sf_retry_after_block 10.0.3.2 2 2
+		sf_retry_after_block 10.0.1.2 3 3
+		mptcp_lib_kill_group_wait $tests_pid
+
+		local retry_nr
+		retry_nr=$(mptcp_lib_get_counter $ns2 "MPTcpExtSubflowRetry")
+		join_syn_tx=$((1 + ${retry_nr:-0})) \
+			chk_join_nr 3 3 3
+		chk_sf_retry_nr $ns2 3
+	fi
 }
 
 add_addr_timeout_tests()
-- 
2.25.1

Re: [RFC PATCH mptcp-next v3 1/3] selftests: mptcp: connect: retry write() on EAGAIN

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests: mptcp_connect fails on EAGAIN when poll_timeout is infinite
--

commit c4cb29705a0b8f55f650eb00f7f218f1708de879
Author: Anil Kaushik <anilkaushikwireless@gmail.com>

selftests: mptcp: connect: retry write() on EAGAIN

mptcp_connect treats any write() error as fatal. But write() can fail
with EAGAIN even after poll() said the socket was writable: the MPTCP
send buffer shrinks when a subflow is closed.

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index 178d98d91fea2..03fff859e4d1d 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
[ ... ]
> @@ -711,6 +712,20 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd,
>  
>  				bw = do_rnd_write(peerfd, winfo->buf + winfo->off, winfo->len);
>  				if (bw < 0) {
> +					/* POLLOUT does not guarantee that
> +					 * the next write will not block:
> +					 * the MPTCP send buffer can shrink
> +					 * in between, e.g. when a subflow
> +					 * is closed. Wait for more space,
> +					 * but not forever: poll() does not
> +					 * check every cause of EAGAIN.
> +					 */
> +					if (errno == EAGAIN &&
> +					    ++eagain_ms <= poll_timeout) {
                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: Medium]
Will this condition incorrectly abort the test immediately if poll_timeout
is set to -1 (infinite timeout)?

If the tool is invoked with a timeout of 0 or less (e.g., -t 0), poll_timeout
is set to -1. When write() returns EAGAIN, the new condition evaluates to
1 <= -1, which is false. This bypasses the intended retry logic, immediately
failing the transfer instead of waiting.

I noticed a similar issue in the preexisting code in xdisconnect():

tools/testing/selftests/net/mptcp/mptcp_connect.c:xdisconnect() {
	...
	for (i = 0; ; i += msec_sleep) {
		/* closed socket are not listed by 'ss' */
		if (system(cmd) != 0)
			break;

		if (i > poll_timeout)
			xerror("timeout while waiting for spool to complete");
		usleep(msec_sleep * 1000);
	}
	...
}

Does this code also incorrectly assume poll_timeout is positive, causing an
immediate crash if poll_timeout is -1?

> +						usleep(1000);
> +						continue;
> +					}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014143349.768138-1-anilkaushikwireless@gmail.com?part=1

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mptcp: duplicate MPTCP_EVENT_SUB_CLOSED event emitted on reconnect failure
--

commit 66071f5b3caa5390ad8bce4ae347cb2285123fa3
Author: Anil Kaushik <anilkaushikwireless@gmail.com>

mptcp: pm: kernel: retry subflows closed after an error

This commit adds an opt-in sysctl to retry creating subflows closed by
TCP after an error. The PM immediately creates a new subflow using the
same addresses, and spaces out attempts via SYN retransmissions. A new
SubflowRetry counter tracks these attempts.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/440

> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index d3014bf57bf3c..d020e5f69c013 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
[ ... ]
> @@ -1591,6 +1591,151 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk)
[ ... ]
> +/* Called with the msk socket lock held, instead of closing the subflow */
> +bool mptcp_pm_kernel_retry_reconnect(struct mptcp_sock *msk, struct sock *ssk,
> +				     const struct mptcp_pm_retry *retry)
> +{
> +	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> +	struct sock *sk = (struct sock *)msk;
> +
> +	/* like mptcp_close_ssk() does for the failed attempt */
> +	mptcp_event(MPTCP_EVENT_SUB_CLOSED, msk, ssk, GFP_KERNEL);
> +
> +	MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_SUBFLOWRETRY);
> +	if (mptcp_subflow_reconnect(sk, ssk, &retry->local, &retry->remote))
> +		return false;

[Severity: Medium]
Will this sequence emit a duplicate MPTCP_EVENT_SUB_CLOSED event if the
reconnect attempt fails?

When mptcp_subflow_reconnect() is called, it clears the subflow context
using mptcp_subflow_ctx_reset(), which zeroes the close_event_done flag:

net/mptcp/subflow.c:mptcp_subflow_reconnect() {
...
	mptcp_subflow_ctx_reset(subflow);
...

If the underlying tcp connect call fails, mptcp_subflow_reconnect() returns
an error, causing mptcp_pm_kernel_retry_reconnect() to return false.

[ ... ]
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 60cbd91be6320..f29cfd2ad69b9 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
[ ... ]
> @@ -2827,7 +2829,14 @@ static void __mptcp_close_subflow(struct sock *sk)
>  		if (!skb_queue_empty_lockless(&ssk->sk_receive_queue))
>  			continue;
>  
> +		do_retry = mptcp_pm_kernel_retry_check(msk, subflow, &retry);
> +		if (do_retry && retry.reuse &&
> +		    mptcp_pm_kernel_retry_reconnect(msk, ssk, &retry))
> +			continue;
> +
>  		mptcp_close_ssk(sk, ssk, subflow);

[Severity: Medium]
When mptcp_pm_kernel_retry_reconnect() returns false, the fallback path
here calls mptcp_close_ssk().

Since the close_event_done flag was cleared to false by the failed
reconnect attempt, and the parent MPTCP socket is still in TCP_ESTABLISHED,
mptcp_close_ssk() will unconditionally emit a second MPTCP_EVENT_SUB_CLOSED
event for the exact same subflow socket.

Could this duplicate state notification confuse or desynchronize userspace
path managers?

> +		if (do_retry && !retry.reuse)
> +			mptcp_pm_kernel_retry(msk, &retry);
>  	}
>  
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014143349.768138-1-anilkaushikwireless@gmail.com?part=2

Re: [RFC PATCH mptcp-next v3 0/3] mptcp: pm: kernel: retry subflows closed after an error

[MPTCP CI]

Hi Anil,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Perf: Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/38583978482

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/06eef3d66a29
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1185663


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[Matthieu Baerts]

Hi Anil,

Thank you for the new version. Here is a very quick other review, mostly
questions.

Also, please wait for the netdev Sashiko review (24 to 48h) before
sending a new version. You can remove the RFC tag. Ideally, include a
changelog per patch.

On 14/10/2026 16:33, Anil Kaushik wrote:
> With the in-kernel PM, a subflow closed after an error, e.g. a timeout
> when a path silently drops packets, is never re-created, even once the
> path works again. For example, with a main link and a backup link,
> connections move to the backup after an outage but never come back.
> 
> Add an opt-in sysctl, net.mptcp.subflow_retries (0 by default): the

I confirm that it would be better to avoid the sysctl for this in-kernel
PM specific setting. Instead, please add another limit that can be
configured via netlink. Add a sensible default value (what do you
recommend?), but on by default.

By default, I think the kernel tries to establish a connection for 67
seconds.

A (maybe bad?) idea: I wonder if we shouldn't modify icsk_syn_retries
instead: by doing that, we would do only one retry, but with a long
exponential backoff timeout. The config would set this value directly,
with 0 meaning disabled (and not "no retransmission after the retry"). WDYT?

> maximum number of attempts to re-create such a subflow. When TCP closes
> a subflow created by this side after an error, the PM creates a new one
> with the same addresses right away. Its SYN retransmissions space out
> the attempts, so no extra state or timer is needed. If it cannot
> connect, the same socket is used for the next attempt. The number of
> attempts is kept in the subflow context, and counted from scratch once
> a re-created subflow stayed established for a minute. Re-creating the
> initial subflow uses one of the additional subflows allowed by the
> limits.
> 
> Subflows refused by the peer, reset by the peer without the transient
> flag (RFC 8684 section 3.6), aborted locally, or using a removed address
> are not retried.
> 
> A new counter, SubflowRetry, counts the attempts.

I'm not a fan of a MIB counter only for the in-kernel PM. Do we have
other similar ones? We already expose some diag counters per subflow (ss
-Mte), would that work?

> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index d7c5b50b34cc..b3f7f7b2ef84 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -794,6 +794,14 @@ static void mptcp_pm_rm_addr_or_subflow(struct mptcp_sock *msk,
>  			int how = RCV_SHUTDOWN | SEND_SHUTDOWN;
>  			u8 id = subflow_get_local_id(subflow);
>  
> +			/* a dead subflow still waiting to be closed must not
> +			 * be re-created on a removed address
> +			 */
> +			if ((rm_type == MPTCP_MIB_RMADDR &&
> +			     remote_id == rm_id) ||
> +			    (rm_type == MPTCP_MIB_RMSUBFLOW && id == rm_id))
> +				subflow->retries = MPTCP_SUBFLOW_NO_RETRY;

Is it specific for your case?

BTW, it would be interesting to test cases where the right actions are
taken when a subflow is disconnected or it is trying to reconnect, e.g.
closing the socket, etc. Maybe easier with Packetdrill to get the right
timing? (or extend the existing test to end with a drop, then close the
connetion

Also we should (manually?) check the right netlink events are being
sent, e.g. not having a "closed subflow" event if the subflow wasn't
established before (but we are probably already doing the right thing
there, just to be sure, I didn't check). I guess your current test can
also check the netlink events.

>  			if ((1 << inet_sk_state_load(ssk)) &
>  			    (TCPF_FIN_WAIT1 | TCPF_FIN_WAIT2 | TCPF_CLOSING | TCPF_CLOSE))
>  				continue;
> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index d3014bf57bf3..d020e5f69c01 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -1591,6 +1591,151 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk)
>  	return true;
>  }
>  
> +/* a re-created subflow reset sooner counts as a failed attempt */
> +#define MPTCP_PM_RETRY_STABLE	(60 * HZ)

That feels "fragile" (e.g. with net.ipv4.tcp_syn_retries=0): can we not
check why it failed instead?

> +static bool mptcp_pm_retry_path_error(int err)
> +{
> +	switch (err) {
> +	case ETIMEDOUT:
> +	case EHOSTUNREACH:
> +	case ENETUNREACH:
> +	case EHOSTDOWN:
> +	case ENETDOWN:

Out-of-curiosity: what error do you get when the issue is detected via
TCP Keeplive?

> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +/* A subflow initiated by this side and closed by TCP after an error can be
> + * re-created with the same addresses, see the 'subflow_retries' sysctl. The
> + * SYN retransmissions of the new subflow space out the attempts. Called with
> + * the msk socket lock held, before the subflow is closed.
> + */

(probably only the last sentence is useful)

> +bool mptcp_pm_kernel_retry_check(struct mptcp_sock *msk,
> +				 const struct mptcp_subflow_context *subflow,
> +				 struct mptcp_pm_retry *retry)
> +{
> +	struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> +	bool established = READ_ONCE(subflow->fully_established);
> +	struct sock *sk = (struct sock *)msk;
> +	int err = READ_ONCE(ssk->sk_err);
> +
> +	if (!mptcp_pm_is_kernel(msk) || !err ||
> +	    sk->sk_state != TCP_ESTABLISHED || __mptcp_check_fallback(msk))
> +		return false;
> +
> +	/* mptcp_close_ssk() will not handle it again, e.g. the initial
> +	 * subflow, kept in the list once closed
> +	 */
> +	if (subflow->close_event_done || READ_ONCE(subflow->local_id) < 0)
> +		return false;
> +
> +	/* removed address, see mptcp_pm_rm_addr_or_subflow() */
> +	if (subflow->retries == MPTCP_SUBFLOW_NO_RETRY)
> +		return false;
> +
> +	/* The peer asked not to use this path again (MP_RST without the T
> +	 * flag), the subflow has been aborted locally, or it hit an MPTCP
> +	 * protocol error.
> +	 */
> +	if ((subflow->reset_seen && !subflow->reset_transient) ||
> +	    err == ECONNABORTED || err == EBADMSG)
> +		return false;
> +
> +	/* A subflow that never got established is retried only after network
> +	 * errors: if the peer rejected it, trying again will not help.
> +	 */
> +	if (!established && !mptcp_pm_retry_path_error(err))
> +		return false;
> +
> +	/* Only re-create subflows initiated by this side: MP_JOIN ones, or the
> +	 * initial one on the client side.
> +	 */
> +	if (!subflow->request_join &&
> +	    (subflow->mp_join || READ_ONCE(msk->pm.server_side)))
> +		return false;
> +
> +	/* Count the attempts from scratch only if the path worked for a
> +	 * while: a re-created subflow reset soon after its creation counts
> +	 * as a failed attempt.
> +	 */
> +	retry->retries = subflow->retries;
> +	if (established &&
> +	    (!subflow->retries ||
> +	     (u32)(tcp_jiffies32 - subflow->retry_stamp) >=
> +	     MPTCP_PM_RETRY_STABLE))
> +		retry->retries = 0;
> +	/* a failed attempt is retried with the same socket */
> +	retry->reuse = !established && subflow->retries;

Why can we not always re-used?

If we cannot always do that, maybe we should always re-create one? It
might be better to avoid having multiple paths for that in the code.

> +	if (retry->retries >= mptcp_subflow_retries(sock_net(sk)))
> +		return false;
> +
> +	mptcp_remote_address((struct sock_common *)ssk, &retry->remote);
> +	retry->remote.id = READ_ONCE(subflow->remote_id);
> +	if (!retry->remote.id && READ_ONCE(msk->pm.remote_deny_join_id0))
> +		return false;
> +
> +	mptcp_local_address((struct sock_common *)ssk, &retry->local.addr);
> +	retry->local.addr.port = 0;
> +	retry->local.addr.id = subflow_get_local_id(subflow);
> +	retry->local.flags = subflow->request_bkup ?
> +			     MPTCP_PM_ADDR_FLAG_BACKUP : 0;
> +	retry->local.ifindex = ssk->sk_bound_dev_if;
> +
> +	return true;
> +}
> +
> +/* Called with the msk socket lock held, instead of closing the subflow */
> +bool mptcp_pm_kernel_retry_reconnect(struct mptcp_sock *msk, struct sock *ssk,
> +				     const struct mptcp_pm_retry *retry)
> +{
> +	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> +	struct sock *sk = (struct sock *)msk;
> +
> +	/* like mptcp_close_ssk() does for the failed attempt */
> +	mptcp_event(MPTCP_EVENT_SUB_CLOSED, msk, ssk, GFP_KERNEL);
> +
> +	MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_SUBFLOWRETRY);
> +	if (mptcp_subflow_reconnect(sk, ssk, &retry->local, &retry->remote))
> +		return false;
> +
> +	/* the reset cleared them */
> +	subflow->retries = retry->retries + 1;
> +	subflow->retry_stamp = tcp_jiffies32;
> +	return true;
> +}
> +
> +/* Called with the msk socket lock held, after the subflow has been closed */
> +void mptcp_pm_kernel_retry(struct mptcp_sock *msk,
> +			   const struct mptcp_pm_retry *retry)

Yes, probably better to avoid having both retry_reconnect and retry.

> +{
> +	struct mptcp_subflow_context *subflow;
> +	struct sock *sk = (struct sock *)msk;
> +	bool allowed;
> +
> +	spin_lock_bh(&msk->pm.lock);
> +	allowed = msk->pm.extra_subflows <
> +		  mptcp_pm_get_limit_extra_subflows(msk);
> +	if (allowed)
> +		msk->pm.extra_subflows++;
> +	spin_unlock_bh(&msk->pm.lock);
> +
> +	if (!allowed)
> +		return;
> +
> +	MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_SUBFLOWRETRY);
> +	if (__mptcp_subflow_connect(sk, &retry->local, &retry->remote))
> +		return;
> +
> +	/* the new subflow is the last one: it carries the number of attempts */
> +	subflow = list_last_entry(&msk->conn_list, struct mptcp_subflow_context,
> +				  node);
> +	subflow->retries = retry->retries + 1;
> +	subflow->retry_stamp = tcp_jiffies32;
> +}
> +
>  /* Called under PM lock */
>  void __mptcp_pm_kernel_worker(struct mptcp_sock *msk)
>  {
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 60cbd91be632..f29cfd2ad69b 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2816,6 +2816,8 @@ static void __mptcp_close_subflow(struct sock *sk)
>  	mptcp_for_each_subflow_safe(msk, subflow, tmp) {
>  		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
>  		int ssk_state = inet_sk_state_load(ssk);
> +		struct mptcp_pm_retry retry;
> +		bool do_retry;
>  
>  		if (ssk_state != TCP_CLOSE &&
>  		    (ssk_state != TCP_CLOSE_WAIT ||
> @@ -2827,7 +2829,14 @@ static void __mptcp_close_subflow(struct sock *sk)
>  		if (!skb_queue_empty_lockless(&ssk->sk_receive_queue))
>  			continue;
>  
> +		do_retry = mptcp_pm_kernel_retry_check(msk, subflow, &retry);
> +		if (do_retry && retry.reuse &&
> +		    mptcp_pm_kernel_retry_reconnect(msk, ssk, &retry))
> +			continue;
> +
>  		mptcp_close_ssk(sk, ssk, subflow);
> +		if (do_retry && !retry.reuse)
> +			mptcp_pm_kernel_retry(msk, &retry);

Avoid calling in-kernel specific functions directly: it should be done
via the PM "interface". IOW, call something in pm.c, which would call
the in-kernel function if this pm is being used.

(later, we should do that via a pm ops, but we are not there yet --
still best to avoid calling pm kernel functions directly here in all
conditions)


>  	}
>  
>  }
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index ed2f363bb915..1aa92c9614cd 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -622,6 +622,8 @@ struct mptcp_subflow_context {
>  	u8	reset_transient:1;
>  	u8	reset_reason:4;
>  	u8	stale_count;	    /* Protected by the msk socket lock */
> +	u8	retries;	    /* re-creation attempts, msk socket lock */
> +	u32	retry_stamp;	    /* creation of a re-created subflow */

Probably best not to add that between stale_count and stale + avoid
creating new "holes" in structures (I didn't check with pahole, but I
guess that's the case)

>  	u8	stale;		    /* Protected by the msk socket lock,
>  				     * if set the subflow is unable to snd/rcv
>  				     * data, the schedule should skip it
> @@ -818,6 +820,7 @@ int mptcp_is_checksum_enabled(const struct net *net);
>  int mptcp_allow_join_id0(const struct net *net);
>  unsigned int mptcp_stale_loss_cnt(const struct net *net);
>  unsigned int mptcp_close_timeout(const struct sock *sk);
> +u8 mptcp_subflow_retries(const struct net *net);
>  int mptcp_get_pm_type(const struct net *net);
>  const char *mptcp_get_path_manager(const struct net *net);
>  const char *mptcp_get_scheduler(const struct net *net);
> @@ -859,6 +862,9 @@ void mptcp_remote_address(const struct sock_common *skc,
>  			  struct mptcp_addr_info *addr);
>  
>  /* called with sk socket lock held */
> +int mptcp_subflow_reconnect(struct sock *sk, struct sock *ssk,
> +			    const struct mptcp_pm_local *local,
> +			    const struct mptcp_addr_info *remote);
>  int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_local *local,
>  			    const struct mptcp_addr_info *remote);
>  int mptcp_subflow_create_socket(struct sock *sk, unsigned short family,
> @@ -1258,6 +1264,24 @@ void __init mptcp_pm_userspace_register(void);
>  void __init mptcp_pm_nl_init(void);
>  void mptcp_pm_worker(struct mptcp_sock *msk);
>  void __mptcp_pm_kernel_worker(struct mptcp_sock *msk);
> +
> +/* subflow->retries value preventing any re-creation */
> +#define MPTCP_SUBFLOW_NO_RETRY	U8_MAX
> +
> +struct mptcp_pm_retry {
> +	struct mptcp_pm_local	local;
> +	struct mptcp_addr_info	remote;
> +	u8			retries;
> +	bool			reuse;	/* failed attempt: same socket */
> +};
> +
> +bool mptcp_pm_kernel_retry_check(struct mptcp_sock *msk,
> +				 const struct mptcp_subflow_context *subflow,
> +				 struct mptcp_pm_retry *retry);
> +void mptcp_pm_kernel_retry(struct mptcp_sock *msk,
> +			   const struct mptcp_pm_retry *retry);
> +bool mptcp_pm_kernel_retry_reconnect(struct mptcp_sock *msk, struct sock *ssk,
> +				     const struct mptcp_pm_retry *retry);
>  u8 mptcp_pm_get_endp_signal_max(const struct mptcp_sock *msk);
>  u8 mptcp_pm_get_endp_subflow_max(const struct mptcp_sock *msk);
>  u8 mptcp_pm_get_endp_laminar_max(const struct mptcp_sock *msk);
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 78e886b20258..0cdeae4b45fa 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -1766,6 +1766,69 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_local *local,
>  	return err;
>  }
>  
> +/* Connect again an MPJ subflow that failed to connect, with the same socket.
> + * The subflow never carried data: like for the initial subflow in
> + * __mptcp_subflow_disconnect(), resetting the TCP and the subflow states is
> + * enough. Called with the msk socket lock held.
> + */
> +int mptcp_subflow_reconnect(struct sock *sk, struct sock *ssk,
> +			    const struct mptcp_pm_local *local,
> +			    const struct mptcp_addr_info *remote)
> +{
> +	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> +	struct sockaddr_unsized *uaddr;
> +	struct mptcp_sock *msk = mptcp_sk(sk);
> +	struct sockaddr_storage addr;
> +	u32 remote_token;
> +	int addrlen;
> +	int err;
> +
> +	lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
> +	err = tcp_disconnect(ssk, 0);
> +	if (err)
> +		goto out;
> +
> +	mptcp_subflow_ctx_reset(subflow);
> +	subflow->request_mptcp = 0;
> +	do {
> +		subflow->local_nonce = get_random_u32();
> +	} while (!subflow->local_nonce);
> +	subflow_set_local_id(subflow, local->addr.id);
> +	subflow->remote_key_valid = 1;
> +	subflow->remote_key = READ_ONCE(msk->remote_key);
> +	subflow->local_key = READ_ONCE(msk->local_key);
> +	subflow->token = msk->token;
> +	mptcp_crypto_key_sha(subflow->remote_key, &remote_token, NULL);
> +	subflow->remote_token = remote_token;
> +	WRITE_ONCE(subflow->remote_id, remote->id);
> +	subflow->request_join = 1;
> +	subflow->request_bkup = !!(local->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
> +	subflow->subflow_id = msk->subflow_id++;
> +
> +	mptcp_info2sockaddr(remote, &addr, ssk->sk_family);
> +	addrlen = sizeof(struct sockaddr_in);
> +#if IS_ENABLED(CONFIG_MPTCP_IPV6)
> +	if (addr.ss_family == AF_INET6)
> +		addrlen = sizeof(struct sockaddr_in6);
> +#endif
> +	uaddr = (struct sockaddr_unsized *)&addr;
> +
> +	/* like __inet_stream_connect() for __mptcp_subflow_connect() */
> +	if (BPF_CGROUP_PRE_CONNECT_ENABLED(ssk)) {
> +		err = ssk->sk_prot->pre_connect(ssk, uaddr, addrlen);
> +		if (err)
> +			goto out;
> +	}
> +
> +	err = ssk->sk_prot->connect(ssk, uaddr, addrlen);
> +
> +out:
> +	release_sock(ssk);
> +	MPTCP_INC_STATS(sock_net(sk), err ? MPTCP_MIB_JOINSYNTXCONNECTERR :
> +					    MPTCP_MIB_JOINSYNTX);
> +	return err;
> +}

This might not be used if we always call close_subflow then connect, but
in general: avoid this bunch of duplicated code. Here you could extract
code from the normal connect case and call that from here instead of
re-written with a different way (i.e. not calling kernel_connect()).

> +
>  void __mptcp_inherit_memcg(struct sock *sk, struct sock *ssk, gfp_t gfp)
>  {
>  	/* Only if the msk has been accepted already (and not orphaned).*/

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

Re: [RFC PATCH mptcp-next v3 1/3] selftests: mptcp: connect: retry write() on EAGAIN

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] In copyfd_io_poll()
  (tools/testing/selftests/net/mptcp/mptcp_connect.c), the new EAGAIN…
- [Low] In copyfd_io_poll()
  (tools/testing/selftests/net/mptcp/mptcp_connect.c), `eagain_ms`…

--

selftests: mptcp: connect: retry write() on EAGAIN

In copyfd_io_poll(), this patch stops treating an EAGAIN from write()
after POLLOUT as fatal. The loop sleeps 1ms and goes back to poll().
It still gives up once the EAGAIN count exceeds poll_timeout.

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index 178d98d91fea..03fff859e4d1 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> @@ -638,6 +638,7 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd,
>  		.events = POLLIN | POLLOUT,
>  	};
>  	unsigned int total_wlen = 0, total_rlen = 0;
> +	int eagain_ms = 0;
>  
>  	set_nonblock(peerfd, true);
>  
> @@ -711,6 +712,20 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd,
>  
>  				bw = do_rnd_write(peerfd, winfo->buf + winfo->off, winfo->len);
>  				if (bw < 0) {
> +					/* POLLOUT does not guarantee that
> +					 * the next write will not block:
> +					 * the MPTCP send buffer can shrink
> +					 * in between, e.g. when a subflow
> +					 * is closed. Wait for more space,
> +					 * but not forever: poll() does not
> +					 * check every cause of EAGAIN.
> +					 */
> +					if (errno == EAGAIN &&
> +					    ++eagain_ms <= poll_timeout) {

[Severity: Medium]
What happens here when poll_timeout is -1? The -t option parser in
main() turns any non-positive value into the "no timeout" value:

	case 't':
		poll_timeout = atoi(optarg) * 1000;
		if (poll_timeout <= 0)
			poll_timeout = -1;

With -t 0 or a negative value, the poll() call at the top of the loop
waits forever. But the first EAGAIN from do_rnd_write() sets eagain_ms
to 1, and 1 <= -1 is false.

Control then falls through to perror("write") and return 111. That is
the same fatal exit this patch is meant to remove.

Would a condition like (poll_timeout < 0 || ++eagain_ms <= poll_timeout)
fit the intent better? A separate finite cap would also work.

The in-tree scripts all pass -t ${timeout_poll}, so only an explicit
non-positive -t hits this.

> +						usleep(1000);
> +						continue;
> +					}

[Severity: Low]
Does eagain_ms actually measure milliseconds? It goes up by one per
EAGAIN and is compared with poll_timeout, which is in milliseconds. But
each retry goes back to the top of the loop, and this call gets the full
timeout again:

	switch (poll(&fds, 1, poll_timeout)) {

That iteration can also handle POLLIN with a blocking do_write() to
outfd before the next write attempt.

So each counted retry costs at least 1ms plus timer slack and syscall
overhead. If poll() blocks before reporting POLLOUT, a retry can cost up
to a full poll_timeout.

The counter is reset only after a successful write. Iterations that only
see POLLIN neither increment it nor reset it.

The in-tree scripts pass -t 30, which allows 30000 retries. That is
about 31-35 seconds when POLLOUT is reported right away.

If poll() blocks between EAGAINs, the total can go past the harness kill
timeout in mptcp_connect.sh:

	timeout_test=$((timeout_poll * 2 + 1))

The tool is then killed from outside and never prints its own write()
error.

The commit message says "If EAGAIN lasts longer than the poll timeout,
still fail". Could the bound track elapsed time, for example with
clock_gettime()? Alternatively, the variable could be renamed to show
that it is a retry count.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261014143349.768138-1-anilkaushikwireless%40gmail.com

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 2 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The commit message and the sysctl documentation both say the
  retried subflow's SYN retransmissions space out the attempts, so no…
- [Medium] The window that resets the retry count is measured from when
  the re-created subflow's connect was issued, not from when it became…
- [Medium] When the same-socket retry fails synchronously,
  MPTCP_EVENT_SUB_CLOSED is sent twice for one subflow.
- [Low] MPTCP_SUBFLOW_NO_RETRY is U8_MAX (protocol.h:1269), and the same
  value is a valid attempt count.
- [Low] The sysctl documentation (mptcp-sysctl.rst:163) and the commit
  message say subflows 'reset by the peer without the transient flag ...…

--

mptcp: pm: kernel: retry subflows closed after an error

This patch adds an opt-in sysctl, net.mptcp.subflow_retries. With it, the
in-kernel PM re-creates a locally initiated subflow that TCP closed after
an error, using the same addresses. A failed attempt reuses the same
socket through the new mptcp_subflow_reconnect(). The attempt count
restarts once a re-created subflow has been stable for a minute, and a
new SubflowRetry MIB counter is added.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/440

> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index d3014bf57bf3..d020e5f69c01 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -1591,6 +1591,151 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk)
>  	return true;
>  }
>  
> +/* a re-created subflow reset sooner counts as a failed attempt */
> +#define MPTCP_PM_RETRY_STABLE	(60 * HZ)
> +
> +static bool mptcp_pm_retry_path_error(int err)
> +{
> +	switch (err) {
> +	case ETIMEDOUT:
> +	case EHOSTUNREACH:
> +	case ENETUNREACH:
> +	case EHOSTDOWN:
> +	case ENETDOWN:
> +		return true;
> +	}
> +
> +	return false;
> +}

[Severity: High]
The commit message says "Its SYN retransmissions space out the attempts,
so no extra state or timer is needed". The sysctl documentation also lists
"an unreachable path" as a case that gets retried. Does that spacing still
hold for the EHOSTUNREACH, ENETUNREACH, EHOSTDOWN and ENETDOWN cases that
mptcp_pm_retry_path_error() accepts?

These errors usually come from an ICMP unreachable or a local ARP failure.
For a socket in TCP_SYN_SENT, tcp_v4_err() aborts right away, with no SYN
retransmission:

net/ipv4/tcp_ipv4.c:tcp_v4_err() {
    ...
	case TCP_SYN_SENT:
	case TCP_SYN_RECV:
		...
		if (!sock_owned_by_user(sk))
			tcp_done_with_error(sk, err);
    ...
}

On the next __mptcp_close_subflow() pass, mptcp_pm_kernel_retry_check()
sets retry->reuse = !established && subflow->retries. Then
mptcp_pm_kernel_retry_reconnect() sends a new SYN immediately.

Can the whole subflow_retries budget be used up within a few RTTs, or
about 3 seconds per attempt with an ARP failure? The commit message
implies it would be spread over the ~127 second SYN timeout.

Synchronous failures seem to end recovery even when budget is left. This
happens if ip_route_connect() in tcp_v4_connect() fails with -ENETUNREACH,
or if pre_connect or kernel_bind() fails.

On the reuse path, mptcp_pm_kernel_retry_reconnect() returns false and
__mptcp_close_subflow() calls mptcp_close_ssk(). The replacement is then
skipped because retry.reuse is set:

		mptcp_close_ssk(sk, ssk, subflow);
		if (do_retry && !retry.reuse)
			mptcp_pm_kernel_retry(msk, &retry);

On the new-socket path, __mptcp_subflow_connect() fails and
mptcp_pm_kernel_retry() just returns.

The regular in-kernel PM does not recover either, because
select_local_address() skips IDs that are not set in
msk->pm.id_avail_bitmap:

	if (!test_bit(entry->addr.id, msk->pm.id_avail_bitmap))
		continue;

Take outages such as a gateway going down, or an upstream router sending
host-unreachable. Would connections still stay on the backup path for
good, which is the case the commit message says this fixes? Is a deferred
retry with some backoff needed here?

> +
> +/* A subflow initiated by this side and closed by TCP after an error can be
> + * re-created with the same addresses, see the 'subflow_retries' sysctl. The
> + * SYN retransmissions of the new subflow space out the attempts. Called with
> + * the msk socket lock held, before the subflow is closed.
> + */
> +bool mptcp_pm_kernel_retry_check(struct mptcp_sock *msk,
> +				 const struct mptcp_subflow_context *subflow,
> +				 struct mptcp_pm_retry *retry)
> +{

[ ... ]

> +	/* removed address, see mptcp_pm_rm_addr_or_subflow() */
> +	if (subflow->retries == MPTCP_SUBFLOW_NO_RETRY)
> +		return false;

[Severity: Low]
Can a real attempt count collide with this sentinel?
MPTCP_SUBFLOW_NO_RETRY is U8_MAX. The subflow_retries entry in
mptcp_sysctl_table in ctrl.c sets no extra1/extra2, so
proc_dou8vec_minmax() accepts 255.

With subflow_retries=255, a check with retries=254 passes. Then
mptcp_pm_kernel_retry_reconnect() or mptcp_pm_kernel_retry() stores:

	subflow->retries = retry->retries + 1;

which is 255.

Suppose that re-created subflow then stays established for more than a
minute and later fails. mptcp_pm_kernel_retry_check() returns false here,
before the stability-based restart logic runs, so the subflow is never
retried again.

Would bounding the sysctl with an extra2 below U8_MAX, or using a separate
flag for the removed address case, avoid this?

> +
> +	/* The peer asked not to use this path again (MP_RST without the T
> +	 * flag), the subflow has been aborted locally, or it hit an MPTCP
> +	 * protocol error.
> +	 */
> +	if ((subflow->reset_seen && !subflow->reset_transient) ||
> +	    err == ECONNABORTED || err == EBADMSG)
> +		return false;

[Severity: Low]
The sysctl documentation and the commit message say that subflows "reset
by the peer without the transient flag" are not retried. Does this check
match that?

subflow->reset_seen is only set when mptcp_incoming_options() parses an
MP_TCPRST option:

		if (mp_opt.suboptions & OPTION_MPTCP_RST) {
			subflow->reset_seen = 1;

A plain TCP RST without MP_TCPRST leaves reset_seen at 0 and sets err to
ECONNRESET. For an established subflow, mptcp_pm_retry_path_error() is not
consulted, so the subflow gets retried.

Should the documentation say "MP_RST without the T flag", like the code
comment does? Or should a plain RST be treated as non-transient?

[ ... ]

> +	/* Count the attempts from scratch only if the path worked for a
> +	 * while: a re-created subflow reset soon after its creation counts
> +	 * as a failed attempt.
> +	 */
> +	retry->retries = subflow->retries;
> +	if (established &&
> +	    (!subflow->retries ||
> +	     (u32)(tcp_jiffies32 - subflow->retry_stamp) >=
> +	     MPTCP_PM_RETRY_STABLE))
> +		retry->retries = 0;

[Severity: Medium]
Does this measure how long the subflow stayed established?
mptcp_pm_kernel_retry() and mptcp_pm_kernel_retry_reconnect() set
retry_stamp when the SYN is sent. The field comment in protocol.h calls it
the "creation of a re-created subflow".

The documentation says "The count starts again once a re-created subflow
stayed established for a minute". Here, though, the handshake time counts
toward that minute.

For example, with the default tcp_syn_retries=6, the last SYN
retransmission goes out about 63 seconds after the stamp. Say that SYN is
answered and the subflow is then closed quickly by a retryable error, such
as a plain RST. This check sees more than 60 seconds and resets the count
to 0.

On a path that flaps like this, could net.mptcp.subflow_retries stop
limiting the number of re-creations? Would recording a timestamp when the
subflow becomes fully established match the documented behaviour better?

[ ... ]

> +/* Called with the msk socket lock held, instead of closing the subflow */
> +bool mptcp_pm_kernel_retry_reconnect(struct mptcp_sock *msk, struct sock *ssk,
> +				     const struct mptcp_pm_retry *retry)
> +{
> +	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> +	struct sock *sk = (struct sock *)msk;
> +
> +	/* like mptcp_close_ssk() does for the failed attempt */
> +	mptcp_event(MPTCP_EVENT_SUB_CLOSED, msk, ssk, GFP_KERNEL);
> +
> +	MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_SUBFLOWRETRY);
> +	if (mptcp_subflow_reconnect(sk, ssk, &retry->local, &retry->remote))
> +		return false;

[Severity: Medium]
Can MPTCP_EVENT_SUB_CLOSED be sent twice for the same subflow when the
reconnect fails synchronously?

mptcp_subflow_reconnect() first runs tcp_disconnect(), which always
returns 0 and sets inet_dport to 0. It then calls
mptcp_subflow_ctx_reset(), which clears close_event_done and backup.
After that, subflow_set_local_id() gives local_id a valid value.

pre_connect() or sk_prot->connect() can fail after that, for example with
-ENETUNREACH from ip_route_connect() or a BPF cgroup rejection. In that
case this function returns false and __mptcp_close_subflow() falls through
to mptcp_close_ssk(). The guard there no longer stops it:

	if (subflow->close_event_done || READ_ONCE(subflow->local_id) < 0)
		return;

	subflow->close_event_done = true;

	if (sk->sk_state == TCP_ESTABLISHED)
		mptcp_event(MPTCP_EVENT_SUB_CLOSED, mptcp_sk(sk), ssk, GFP_KERNEL);

The second event is built from a socket with dport 0 and the backup bit
cleared. Would userspace PM listeners then see one subflow closed twice,
the second time with wrong attributes?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261014143349.768138-1-anilkaushikwireless%40gmail.com

Re: [RFC PATCH mptcp-next v3 3/3] selftests: mptcp: join: validate subflow retry after link failure

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new background transfer uses `test_linkfail=131072`
  (mptcp_join.sh:2679).
- [Medium] The two phases wait on absolute, cumulative
  MPTcpExtSubflowRetry values (`sf_retry_after_block 10.0.3.2 2 2` then…
- [Low] The per-step checks in sf_retry_after_block()
  (tools/testing/selftests/net/mptcp/mptcp_join.sh, lines 2208-2222) can…
- [Low] The comment on the new subtest in link_failure_tests() (lines
  2659-2660) says the initial subflow is re-created 'with an MPJ using…

--

selftests: mptcp: join: validate subflow retry after link failure

This adds a subtest to link_failure_tests() that runs a long transfer in
the background. It uses nftables to drop the client packets from the
additional subflow address until MPTcpExtSubflowRetry increases, then lets
them through and checks that a subflow comes back from that address. It
then repeats this for the initial subflow address.

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index f4d8afaeda4f..1048c9cfceb7 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -2151,6 +2151,76 @@ wait_attempt_fail()

[ ... ]

> +# $1: client address to block ; $2: retries ; $3: MPJ, expected after it
> +#
> +# The link stays up, but the client packets from this address are dropped:
> +# the subflow using it is closed after a few retransmissions, then re-created
> +# once the packets go through again.
> +sf_retry_after_block()
> +{
> +	ip netns exec $ns2 nft add rule ip filter OUTPUT ip saddr "${1}" drop
> +	wait_sf_retry $ns2 "${2}"
> +	ip netns exec $ns2 nft flush chain ip filter OUTPUT
> +	wait_mpj_ack "${3}"
> +
> +	print_check "subflow from ${1}"
> +	if ip netns exec $ns2 ss -tnH state established src "${1}" |
> +	   grep -q .; then
> +		print_ok
> +	else
> +		fail_test "no established subflow from ${1}"
> +	fi
> +}

[Severity: Low]
Can this check print OK even if no subflow was re-created?

wait_sf_retry() and wait_mpj_ack() both return 1 on timeout. Here both
results are ignored, so the rule is flushed and the check runs either way.

The ss filter only matches on source address and state. Nothing records
the original source port or 4-tuple before the block. So if the blocked
subflow never reaches the tcp_retries2 timeout, or one of the waits times
out, the original socket is still established and this prints
"[OK] subflow from <addr>".

The final chk_join_nr 3 3 3 would still fail, but it doesn't say which
address failed. Each silent timeout also costs at least timeout_poll
seconds. The same goes for the first wait_mpj_ack 1 call in
link_failure_tests(), which is also unchecked.

Would it make sense to check the wait return values, and to compare the
subflow source port before and after the block?

> @@ -2585,6 +2655,42 @@ link_failure_tests()
>  		chk_stale_nr $ns2 1 -1 2
>  		chk_link_usage $ns2 ns2eth3 $cinsent 50
>  	fi
> +
> +	# subflows closed after a silent link failure, then re-created: the
> +	# additional one, then the initial one, with an MPJ using ID 0

[Severity: Low]
This isn't a bug, but the comment says the initial subflow is re-created
with an MPJ using ID 0, and nothing below checks the address IDs.

mptcp_pm_kernel_retry_check() in net/mptcp/pm_kernel.c has handling that
is specific to ID 0:

	retry->remote.id = READ_ONCE(subflow->remote_id);
	if (!retry->remote.id && READ_ONCE(msk->pm.remote_deny_join_id0))
		return false;
	...
	retry->local.addr.id = subflow_get_local_id(subflow);

The only per-address check is ss -tnH ... src 10.0.1.2, and its output
doesn't include MPTCP IDs. chk_join_nr only counts SYN, SYN-ACK and ACK.
So a re-join with a wrong ID that the peer still accepts would not be
caught.

Could ss -tniH or the subflow events be used to confirm the ID?

[ ... ]

> +		{ timeout_test=120 test_linkfail=131072 \
> +			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null

[Severity: Medium]
Is 131072 the intended value here? run_tests() treats any test_linkfail
value above 2 as a file size in KB:

	if [ "$test_linkfail" -gt 2 ]; then
		size=$test_linkfail
		...
		make_file "$cinfail" "client" $size

So this writes 128 MiB of urandom data into cinfail and another 128 MiB
into sinfail. The transfer also writes cinsent and cout/sout copies. The
other background long-connection tests in this file use test_linkfail=128
with speed throttling. The largest value used elsewhere is 1024.

cinfail and sinfail are global. They are only rewritten by a later
foreground test with test_linkfail above 2, such as fastclose_tests, or
removed by cleanup() at exit. On CI where /tmp is tmpfs, about 256 MiB of
RAM stays in use through several of the test groups that follow.

The files are also generated inside the background job before the
connection starts, while the foreground runs the unchecked wait_mpj_ack 1.
On a slow debug kernel, could that wait time out, so the 10.0.3.2 drop rule
is added before the first MP_JOIN exists? The test would then cover a join
that never gets established, not an established subflow closed by RTO.

> +		local tests_pid=$!
> +
> +		wait_mpj_ack 1
> +		sf_retry_after_block 10.0.3.2 2 2
> +		sf_retry_after_block 10.0.1.2 3 3

[Severity: Medium]
Can the absolute retry thresholds turn the second phase into a no-op?

wait_sf_retry() compares the cumulative MPTcpExtSubflowRetry counter
against an absolute value. Phase 2 therefore assumes phase 1 ended with
exactly 2 retries.

In phase 1, the 10.0.3.2 subflow times out and mptcp_pm_kernel_retry()
bumps the counter to 1. The SYN for that attempt is dropped, and with
tcp_syn_retries=1 it fails after about 3s. mptcp_pm_kernel_retry_reconnect()
then does:

	MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_SUBFLOWRETRY);
	if (mptcp_subflow_reconnect(sk, ssk, &retry->local, &retry->remote))

mptcp_subflow_reconnect() sends the SYN at once through
ssk->sk_prot->connect(), while the drop rule is still installed. Only the
SYN retransmit, 1s later, can get through.

The foreground may need more than about 1s to see the counter and run the
nft flush, for example on loaded CI or on debug or KASAN kernels. In that
case this attempt fails too, and a third attempt bumps the counter to 3
before the join succeeds.

Phase 2 then adds the 10.0.1.2 drop rule, but wait_sf_retry $ns2 3 returns
immediately and the rule is flushed straight away. The initial subflow is
never disrupted. wait_mpj_ack 3 times out silently, the ss check passes on
the original socket, and the run fails at chk_join_nr 3 3 3.

Would it help to read the counters at the start of each
sf_retry_after_block() call and wait for an increase from those values?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261014143349.768138-1-anilkaushikwireless%40gmail.com

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[Anil Kaushik]

Hi Matt,

Thanks for the review.

> Add a sensible default value (what do you recommend?)

15 SYN retries: about 20 minutes with the 120s max RTO.

> modify icsk_syn_retries instead [...] WDYT?

Good idea, I will do that: one new subflow per failure, with its SYN
retries set from the new netlink limit, 0 = disabled. It also removes
the socket reuse, the retries counter and the 60s check.

One point to think though, If ICMP unreachable stops the SYN at once.
I would not retry
again then. Is that OK?

> Do we have other similar ones? [...] ss -Mte, would that work?

I will drop the MIB counter. The test can use the MPJ counters and the
netlink events.

> Is it specific for your case?

No, it came from a Sashiko review: a dead subflow waiting to be closed
could be re-created after its endpoint or the peer's address was
removed. I will try to move it to pm_kernel.c.

> can we not check why it failed instead?

With your idea it is not needed: only one retry, nothing to count.

> what error do you get when the issue is detected via TCP Keepalive?

ETIMEDOUT, so it is retried.

> Why can we not always re-used?

A subflow that carried data needs the full close cleanup. With your
idea we always create a new subflow, so there is only one path.

For the rest: OK. I will go through pm.c, avoid struct holes and
duplicated connect code, drop the RFC tag, add a changelog per patch,
and test closing the connection while retrying plus the netlink events.
I will wait for the netdev Sashiko review before sending.

Thanks,
Anil

On Wed, Oct 14, 2026 at 11:43 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> Hi Anil,
>
> Thank you for the new version. Here is a very quick other review, mostly
> questions.
>
> Also, please wait for the netdev Sashiko review (24 to 48h) before
> sending a new version. You can remove the RFC tag. Ideally, include a
> changelog per patch.
>
> On 14/10/2026 16:33, Anil Kaushik wrote:
> > With the in-kernel PM, a subflow closed after an error, e.g. a timeout
> > when a path silently drops packets, is never re-created, even once the
> > path works again. For example, with a main link and a backup link,
> > connections move to the backup after an outage but never come back.
> >
> > Add an opt-in sysctl, net.mptcp.subflow_retries (0 by default): the
>
> I confirm that it would be better to avoid the sysctl for this in-kernel
> PM specific setting. Instead, please add another limit that can be
> configured via netlink. Add a sensible default value (what do you
> recommend?), but on by default.
>
> By default, I think the kernel tries to establish a connection for 67
> seconds.
>
> A (maybe bad?) idea: I wonder if we shouldn't modify icsk_syn_retries
> instead: by doing that, we would do only one retry, but with a long
> exponential backoff timeout. The config would set this value directly,
> with 0 meaning disabled (and not "no retransmission after the retry"). WDYT?
>
> > maximum number of attempts to re-create such a subflow. When TCP closes
> > a subflow created by this side after an error, the PM creates a new one
> > with the same addresses right away. Its SYN retransmissions space out
> > the attempts, so no extra state or timer is needed. If it cannot
> > connect, the same socket is used for the next attempt. The number of
> > attempts is kept in the subflow context, and counted from scratch once
> > a re-created subflow stayed established for a minute. Re-creating the
> > initial subflow uses one of the additional subflows allowed by the
> > limits.
> >
> > Subflows refused by the peer, reset by the peer without the transient
> > flag (RFC 8684 section 3.6), aborted locally, or using a removed address
> > are not retried.
> >
> > A new counter, SubflowRetry, counts the attempts.
>
> I'm not a fan of a MIB counter only for the in-kernel PM. Do we have
> other similar ones? We already expose some diag counters per subflow (ss
> -Mte), would that work?
>
> > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> > index d7c5b50b34cc..b3f7f7b2ef84 100644
> > --- a/net/mptcp/pm.c
> > +++ b/net/mptcp/pm.c
> > @@ -794,6 +794,14 @@ static void mptcp_pm_rm_addr_or_subflow(struct mptcp_sock *msk,
> >                       int how = RCV_SHUTDOWN | SEND_SHUTDOWN;
> >                       u8 id = subflow_get_local_id(subflow);
> >
> > +                     /* a dead subflow still waiting to be closed must not
> > +                      * be re-created on a removed address
> > +                      */
> > +                     if ((rm_type == MPTCP_MIB_RMADDR &&
> > +                          remote_id == rm_id) ||
> > +                         (rm_type == MPTCP_MIB_RMSUBFLOW && id == rm_id))
> > +                             subflow->retries = MPTCP_SUBFLOW_NO_RETRY;
>
> Is it specific for your case?
>
> BTW, it would be interesting to test cases where the right actions are
> taken when a subflow is disconnected or it is trying to reconnect, e.g.
> closing the socket, etc. Maybe easier with Packetdrill to get the right
> timing? (or extend the existing test to end with a drop, then close the
> connetion
>
> Also we should (manually?) check the right netlink events are being
> sent, e.g. not having a "closed subflow" event if the subflow wasn't
> established before (but we are probably already doing the right thing
> there, just to be sure, I didn't check). I guess your current test can
> also check the netlink events.
>
> >                       if ((1 << inet_sk_state_load(ssk)) &
> >                           (TCPF_FIN_WAIT1 | TCPF_FIN_WAIT2 | TCPF_CLOSING | TCPF_CLOSE))
> >                               continue;
> > diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> > index d3014bf57bf3..d020e5f69c01 100644
> > --- a/net/mptcp/pm_kernel.c
> > +++ b/net/mptcp/pm_kernel.c
> > @@ -1591,6 +1591,151 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk)
> >       return true;
> >  }
> >
> > +/* a re-created subflow reset sooner counts as a failed attempt */
> > +#define MPTCP_PM_RETRY_STABLE        (60 * HZ)
>
> That feels "fragile" (e.g. with net.ipv4.tcp_syn_retries=0): can we not
> check why it failed instead?
>
> > +static bool mptcp_pm_retry_path_error(int err)
> > +{
> > +     switch (err) {
> > +     case ETIMEDOUT:
> > +     case EHOSTUNREACH:
> > +     case ENETUNREACH:
> > +     case EHOSTDOWN:
> > +     case ENETDOWN:
>
> Out-of-curiosity: what error do you get when the issue is detected via
> TCP Keeplive?
>
> > +             return true;
> > +     }
> > +
> > +     return false;
> > +}
> > +
> > +/* A subflow initiated by this side and closed by TCP after an error can be
> > + * re-created with the same addresses, see the 'subflow_retries' sysctl. The
> > + * SYN retransmissions of the new subflow space out the attempts. Called with
> > + * the msk socket lock held, before the subflow is closed.
> > + */
>
> (probably only the last sentence is useful)
>
> > +bool mptcp_pm_kernel_retry_check(struct mptcp_sock *msk,
> > +                              const struct mptcp_subflow_context *subflow,
> > +                              struct mptcp_pm_retry *retry)
> > +{
> > +     struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> > +     bool established = READ_ONCE(subflow->fully_established);
> > +     struct sock *sk = (struct sock *)msk;
> > +     int err = READ_ONCE(ssk->sk_err);
> > +
> > +     if (!mptcp_pm_is_kernel(msk) || !err ||
> > +         sk->sk_state != TCP_ESTABLISHED || __mptcp_check_fallback(msk))
> > +             return false;
> > +
> > +     /* mptcp_close_ssk() will not handle it again, e.g. the initial
> > +      * subflow, kept in the list once closed
> > +      */
> > +     if (subflow->close_event_done || READ_ONCE(subflow->local_id) < 0)
> > +             return false;
> > +
> > +     /* removed address, see mptcp_pm_rm_addr_or_subflow() */
> > +     if (subflow->retries == MPTCP_SUBFLOW_NO_RETRY)
> > +             return false;
> > +
> > +     /* The peer asked not to use this path again (MP_RST without the T
> > +      * flag), the subflow has been aborted locally, or it hit an MPTCP
> > +      * protocol error.
> > +      */
> > +     if ((subflow->reset_seen && !subflow->reset_transient) ||
> > +         err == ECONNABORTED || err == EBADMSG)
> > +             return false;
> > +
> > +     /* A subflow that never got established is retried only after network
> > +      * errors: if the peer rejected it, trying again will not help.
> > +      */
> > +     if (!established && !mptcp_pm_retry_path_error(err))
> > +             return false;
> > +
> > +     /* Only re-create subflows initiated by this side: MP_JOIN ones, or the
> > +      * initial one on the client side.
> > +      */
> > +     if (!subflow->request_join &&
> > +         (subflow->mp_join || READ_ONCE(msk->pm.server_side)))
> > +             return false;
> > +
> > +     /* Count the attempts from scratch only if the path worked for a
> > +      * while: a re-created subflow reset soon after its creation counts
> > +      * as a failed attempt.
> > +      */
> > +     retry->retries = subflow->retries;
> > +     if (established &&
> > +         (!subflow->retries ||
> > +          (u32)(tcp_jiffies32 - subflow->retry_stamp) >=
> > +          MPTCP_PM_RETRY_STABLE))
> > +             retry->retries = 0;
> > +     /* a failed attempt is retried with the same socket */
> > +     retry->reuse = !established && subflow->retries;
>
> Why can we not always re-used?
>
> If we cannot always do that, maybe we should always re-create one? It
> might be better to avoid having multiple paths for that in the code.
>
> > +     if (retry->retries >= mptcp_subflow_retries(sock_net(sk)))
> > +             return false;
> > +
> > +     mptcp_remote_address((struct sock_common *)ssk, &retry->remote);
> > +     retry->remote.id = READ_ONCE(subflow->remote_id);
> > +     if (!retry->remote.id && READ_ONCE(msk->pm.remote_deny_join_id0))
> > +             return false;
> > +
> > +     mptcp_local_address((struct sock_common *)ssk, &retry->local.addr);
> > +     retry->local.addr.port = 0;
> > +     retry->local.addr.id = subflow_get_local_id(subflow);
> > +     retry->local.flags = subflow->request_bkup ?
> > +                          MPTCP_PM_ADDR_FLAG_BACKUP : 0;
> > +     retry->local.ifindex = ssk->sk_bound_dev_if;
> > +
> > +     return true;
> > +}
> > +
> > +/* Called with the msk socket lock held, instead of closing the subflow */
> > +bool mptcp_pm_kernel_retry_reconnect(struct mptcp_sock *msk, struct sock *ssk,
> > +                                  const struct mptcp_pm_retry *retry)
> > +{
> > +     struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> > +     struct sock *sk = (struct sock *)msk;
> > +
> > +     /* like mptcp_close_ssk() does for the failed attempt */
> > +     mptcp_event(MPTCP_EVENT_SUB_CLOSED, msk, ssk, GFP_KERNEL);
> > +
> > +     MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_SUBFLOWRETRY);
> > +     if (mptcp_subflow_reconnect(sk, ssk, &retry->local, &retry->remote))
> > +             return false;
> > +
> > +     /* the reset cleared them */
> > +     subflow->retries = retry->retries + 1;
> > +     subflow->retry_stamp = tcp_jiffies32;
> > +     return true;
> > +}
> > +
> > +/* Called with the msk socket lock held, after the subflow has been closed */
> > +void mptcp_pm_kernel_retry(struct mptcp_sock *msk,
> > +                        const struct mptcp_pm_retry *retry)
>
> Yes, probably better to avoid having both retry_reconnect and retry.
>
> > +{
> > +     struct mptcp_subflow_context *subflow;
> > +     struct sock *sk = (struct sock *)msk;
> > +     bool allowed;
> > +
> > +     spin_lock_bh(&msk->pm.lock);
> > +     allowed = msk->pm.extra_subflows <
> > +               mptcp_pm_get_limit_extra_subflows(msk);
> > +     if (allowed)
> > +             msk->pm.extra_subflows++;
> > +     spin_unlock_bh(&msk->pm.lock);
> > +
> > +     if (!allowed)
> > +             return;
> > +
> > +     MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_SUBFLOWRETRY);
> > +     if (__mptcp_subflow_connect(sk, &retry->local, &retry->remote))
> > +             return;
> > +
> > +     /* the new subflow is the last one: it carries the number of attempts */
> > +     subflow = list_last_entry(&msk->conn_list, struct mptcp_subflow_context,
> > +                               node);
> > +     subflow->retries = retry->retries + 1;
> > +     subflow->retry_stamp = tcp_jiffies32;
> > +}
> > +
> >  /* Called under PM lock */
> >  void __mptcp_pm_kernel_worker(struct mptcp_sock *msk)
> >  {
> > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> > index 60cbd91be632..f29cfd2ad69b 100644
> > --- a/net/mptcp/protocol.c
> > +++ b/net/mptcp/protocol.c
> > @@ -2816,6 +2816,8 @@ static void __mptcp_close_subflow(struct sock *sk)
> >       mptcp_for_each_subflow_safe(msk, subflow, tmp) {
> >               struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> >               int ssk_state = inet_sk_state_load(ssk);
> > +             struct mptcp_pm_retry retry;
> > +             bool do_retry;
> >
> >               if (ssk_state != TCP_CLOSE &&
> >                   (ssk_state != TCP_CLOSE_WAIT ||
> > @@ -2827,7 +2829,14 @@ static void __mptcp_close_subflow(struct sock *sk)
> >               if (!skb_queue_empty_lockless(&ssk->sk_receive_queue))
> >                       continue;
> >
> > +             do_retry = mptcp_pm_kernel_retry_check(msk, subflow, &retry);
> > +             if (do_retry && retry.reuse &&
> > +                 mptcp_pm_kernel_retry_reconnect(msk, ssk, &retry))
> > +                     continue;
> > +
> >               mptcp_close_ssk(sk, ssk, subflow);
> > +             if (do_retry && !retry.reuse)
> > +                     mptcp_pm_kernel_retry(msk, &retry);
>
> Avoid calling in-kernel specific functions directly: it should be done
> via the PM "interface". IOW, call something in pm.c, which would call
> the in-kernel function if this pm is being used.
>
> (later, we should do that via a pm ops, but we are not there yet --
> still best to avoid calling pm kernel functions directly here in all
> conditions)
>
>
> >       }
> >
> >  }
> > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> > index ed2f363bb915..1aa92c9614cd 100644
> > --- a/net/mptcp/protocol.h
> > +++ b/net/mptcp/protocol.h
> > @@ -622,6 +622,8 @@ struct mptcp_subflow_context {
> >       u8      reset_transient:1;
> >       u8      reset_reason:4;
> >       u8      stale_count;        /* Protected by the msk socket lock */
> > +     u8      retries;            /* re-creation attempts, msk socket lock */
> > +     u32     retry_stamp;        /* creation of a re-created subflow */
>
> Probably best not to add that between stale_count and stale + avoid
> creating new "holes" in structures (I didn't check with pahole, but I
> guess that's the case)
>
> >       u8      stale;              /* Protected by the msk socket lock,
> >                                    * if set the subflow is unable to snd/rcv
> >                                    * data, the schedule should skip it
> > @@ -818,6 +820,7 @@ int mptcp_is_checksum_enabled(const struct net *net);
> >  int mptcp_allow_join_id0(const struct net *net);
> >  unsigned int mptcp_stale_loss_cnt(const struct net *net);
> >  unsigned int mptcp_close_timeout(const struct sock *sk);
> > +u8 mptcp_subflow_retries(const struct net *net);
> >  int mptcp_get_pm_type(const struct net *net);
> >  const char *mptcp_get_path_manager(const struct net *net);
> >  const char *mptcp_get_scheduler(const struct net *net);
> > @@ -859,6 +862,9 @@ void mptcp_remote_address(const struct sock_common *skc,
> >                         struct mptcp_addr_info *addr);
> >
> >  /* called with sk socket lock held */
> > +int mptcp_subflow_reconnect(struct sock *sk, struct sock *ssk,
> > +                         const struct mptcp_pm_local *local,
> > +                         const struct mptcp_addr_info *remote);
> >  int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_local *local,
> >                           const struct mptcp_addr_info *remote);
> >  int mptcp_subflow_create_socket(struct sock *sk, unsigned short family,
> > @@ -1258,6 +1264,24 @@ void __init mptcp_pm_userspace_register(void);
> >  void __init mptcp_pm_nl_init(void);
> >  void mptcp_pm_worker(struct mptcp_sock *msk);
> >  void __mptcp_pm_kernel_worker(struct mptcp_sock *msk);
> > +
> > +/* subflow->retries value preventing any re-creation */
> > +#define MPTCP_SUBFLOW_NO_RETRY       U8_MAX
> > +
> > +struct mptcp_pm_retry {
> > +     struct mptcp_pm_local   local;
> > +     struct mptcp_addr_info  remote;
> > +     u8                      retries;
> > +     bool                    reuse;  /* failed attempt: same socket */
> > +};
> > +
> > +bool mptcp_pm_kernel_retry_check(struct mptcp_sock *msk,
> > +                              const struct mptcp_subflow_context *subflow,
> > +                              struct mptcp_pm_retry *retry);
> > +void mptcp_pm_kernel_retry(struct mptcp_sock *msk,
> > +                        const struct mptcp_pm_retry *retry);
> > +bool mptcp_pm_kernel_retry_reconnect(struct mptcp_sock *msk, struct sock *ssk,
> > +                                  const struct mptcp_pm_retry *retry);
> >  u8 mptcp_pm_get_endp_signal_max(const struct mptcp_sock *msk);
> >  u8 mptcp_pm_get_endp_subflow_max(const struct mptcp_sock *msk);
> >  u8 mptcp_pm_get_endp_laminar_max(const struct mptcp_sock *msk);
> > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> > index 78e886b20258..0cdeae4b45fa 100644
> > --- a/net/mptcp/subflow.c
> > +++ b/net/mptcp/subflow.c
> > @@ -1766,6 +1766,69 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_local *local,
> >       return err;
> >  }
> >
> > +/* Connect again an MPJ subflow that failed to connect, with the same socket.
> > + * The subflow never carried data: like for the initial subflow in
> > + * __mptcp_subflow_disconnect(), resetting the TCP and the subflow states is
> > + * enough. Called with the msk socket lock held.
> > + */
> > +int mptcp_subflow_reconnect(struct sock *sk, struct sock *ssk,
> > +                         const struct mptcp_pm_local *local,
> > +                         const struct mptcp_addr_info *remote)
> > +{
> > +     struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> > +     struct sockaddr_unsized *uaddr;
> > +     struct mptcp_sock *msk = mptcp_sk(sk);
> > +     struct sockaddr_storage addr;
> > +     u32 remote_token;
> > +     int addrlen;
> > +     int err;
> > +
> > +     lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
> > +     err = tcp_disconnect(ssk, 0);
> > +     if (err)
> > +             goto out;
> > +
> > +     mptcp_subflow_ctx_reset(subflow);
> > +     subflow->request_mptcp = 0;
> > +     do {
> > +             subflow->local_nonce = get_random_u32();
> > +     } while (!subflow->local_nonce);
> > +     subflow_set_local_id(subflow, local->addr.id);
> > +     subflow->remote_key_valid = 1;
> > +     subflow->remote_key = READ_ONCE(msk->remote_key);
> > +     subflow->local_key = READ_ONCE(msk->local_key);
> > +     subflow->token = msk->token;
> > +     mptcp_crypto_key_sha(subflow->remote_key, &remote_token, NULL);
> > +     subflow->remote_token = remote_token;
> > +     WRITE_ONCE(subflow->remote_id, remote->id);
> > +     subflow->request_join = 1;
> > +     subflow->request_bkup = !!(local->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
> > +     subflow->subflow_id = msk->subflow_id++;
> > +
> > +     mptcp_info2sockaddr(remote, &addr, ssk->sk_family);
> > +     addrlen = sizeof(struct sockaddr_in);
> > +#if IS_ENABLED(CONFIG_MPTCP_IPV6)
> > +     if (addr.ss_family == AF_INET6)
> > +             addrlen = sizeof(struct sockaddr_in6);
> > +#endif
> > +     uaddr = (struct sockaddr_unsized *)&addr;
> > +
> > +     /* like __inet_stream_connect() for __mptcp_subflow_connect() */
> > +     if (BPF_CGROUP_PRE_CONNECT_ENABLED(ssk)) {
> > +             err = ssk->sk_prot->pre_connect(ssk, uaddr, addrlen);
> > +             if (err)
> > +                     goto out;
> > +     }
> > +
> > +     err = ssk->sk_prot->connect(ssk, uaddr, addrlen);
> > +
> > +out:
> > +     release_sock(ssk);
> > +     MPTCP_INC_STATS(sock_net(sk), err ? MPTCP_MIB_JOINSYNTXCONNECTERR :
> > +                                         MPTCP_MIB_JOINSYNTX);
> > +     return err;
> > +}
>
> This might not be used if we always call close_subflow then connect, but
> in general: avoid this bunch of duplicated code. Here you could extract
> code from the normal connect case and call that from here instead of
> re-written with a different way (i.e. not calling kernel_connect()).
>
> > +
> >  void __mptcp_inherit_memcg(struct sock *sk, struct sock *ssk, gfp_t gfp)
> >  {
> >       /* Only if the msk has been accepted already (and not orphaned).*/
>
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
>

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[Matthieu Baerts]

Hi Anil,

Thank you for your reply.

On 16/10/2026 07:49, Anil Kaushik wrote:
>> Add a sensible default value (what do you recommend?)
> 
> 15 SYN retries: about 20 minutes with the 120s max RTO.

I guess it could even be longer, no? With an exponential backoff, it's
not spamming the network too much.

I wonder if we shouldn't get something similar to tcp_retries2. But
here, with the syn, we also need to tcp_syn_linear_timeouts into
account. Either we keep it to 15 by default and, (if not 0), we add
tcp_syn_linear_timeouts, or we initialise it to 19.

>> modify icsk_syn_retries instead [...] WDYT?
> 
> Good idea, I will do that: one new subflow per failure, with its SYN
> retries set from the new netlink limit, 0 = disabled. It also removes
> the socket reuse, the retries counter and the 60s check.
> 
> One point to think though, If ICMP unreachable stops the SYN at once.
> I would not retry
> again then. Is that OK?

Good point. In your specific case, what do you observe?

To me, the best is to have the network manager (something from
userspace) removing the MPTCP endpoints when they are not "usable", but
here it can be specific to the destination. I would then say to look at
this case later. In this case, we could eventually disconnect the
subflow, and add a timer to retry later. But that's also more prone to
bugs, so if we can avoid that, probably better to!

>> Do we have other similar ones? [...] ss -Mte, would that work?
> 
> I will drop the MIB counter. The test can use the MPJ counters and the
> netlink events.

Thanks! You can expose the new limit in "struct mptcp_info", instead of
"reserved".

>> Is it specific for your case?
> 
> No, it came from a Sashiko review: a dead subflow waiting to be closed
> could be re-created after its endpoint or the peer's address was
> removed. I will try to move it to pm_kernel.c.

What did you end-up doing for this?

>> can we not check why it failed instead?
> 
> With your idea it is not needed: only one retry, nothing to count.

Good

>> what error do you get when the issue is detected via TCP Keepalive?
> 
> ETIMEDOUT, so it is retried.

Thanks!

>> Why can we not always re-used?
> 
> A subflow that carried data needs the full close cleanup. With your
> idea we always create a new subflow, so there is only one path.
> 
> For the rest: OK. I will go through pm.c, avoid struct holes and
> duplicated connect code, drop the RFC tag, add a changelog per patch,
> and test closing the connection while retrying plus the netlink events.
> I will wait for the netdev Sashiko review before sending.

Thanks! BTW, it is easier if you reply inline here below, to keep the
context:

https://subspace.kernel.org/etiquette.html#do-not-top-post-when-replying

> On Wed, Oct 14, 2026 at 11:43 PM Matthieu Baerts <matttbe@kernel.org> wrote:
(...)

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

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[Matthieu Baerts]

Hi Anil,

I noticed new versions, but I don't know if you saw my reply here. Do
you mind replying to my previous questions / comments here below, please?

On 16/10/2026 10:43, Matthieu Baerts wrote:
> Hi Anil,
> 
> Thank you for your reply.
> 
> On 16/10/2026 07:49, Anil Kaushik wrote:
>>> Add a sensible default value (what do you recommend?)
>>
>> 15 SYN retries: about 20 minutes with the 120s max RTO.
> 
> I guess it could even be longer, no? With an exponential backoff, it's
> not spamming the network too much.
> 
> I wonder if we shouldn't get something similar to tcp_retries2. But
> here, with the syn, we also need to tcp_syn_linear_timeouts into
> account. Either we keep it to 15 by default and, (if not 0), we add
> tcp_syn_linear_timeouts, or we initialise it to 19.
> 
>>> modify icsk_syn_retries instead [...] WDYT?
>>
>> Good idea, I will do that: one new subflow per failure, with its SYN
>> retries set from the new netlink limit, 0 = disabled. It also removes
>> the socket reuse, the retries counter and the 60s check.
>>
>> One point to think though, If ICMP unreachable stops the SYN at once.
>> I would not retry
>> again then. Is that OK?
> 
> Good point. In your specific case, what do you observe?
> 
> To me, the best is to have the network manager (something from
> userspace) removing the MPTCP endpoints when they are not "usable", but
> here it can be specific to the destination. I would then say to look at
> this case later. In this case, we could eventually disconnect the
> subflow, and add a timer to retry later. But that's also more prone to
> bugs, so if we can avoid that, probably better to!
> 
>>> Do we have other similar ones? [...] ss -Mte, would that work?
>>
>> I will drop the MIB counter. The test can use the MPJ counters and the
>> netlink events.
> 
> Thanks! You can expose the new limit in "struct mptcp_info", instead of
> "reserved".
> 
>>> Is it specific for your case?
>>
>> No, it came from a Sashiko review: a dead subflow waiting to be closed
>> could be re-created after its endpoint or the peer's address was
>> removed. I will try to move it to pm_kernel.c.
> 
> What did you end-up doing for this?
> 
>>> can we not check why it failed instead?
>>
>> With your idea it is not needed: only one retry, nothing to count.
> 
> Good
> 
>>> what error do you get when the issue is detected via TCP Keepalive?
>>
>> ETIMEDOUT, so it is retried.
> 
> Thanks!
> 
>>> Why can we not always re-used?
>>
>> A subflow that carried data needs the full close cleanup. With your
>> idea we always create a new subflow, so there is only one path.
>>
>> For the rest: OK. I will go through pm.c, avoid struct holes and
>> duplicated connect code, drop the RFC tag, add a changelog per patch,
>> and test closing the connection while retrying plus the netlink events.
>> I will wait for the netdev Sashiko review before sending.
> 
> Thanks! BTW, it is easier if you reply inline here below, to keep the
> context:
> 
> https://subspace.kernel.org/etiquette.html#do-not-top-post-when-replying
> 
>> On Wed, Oct 14, 2026 at 11:43 PM Matthieu Baerts <matttbe@kernel.org> wrote:
> (...)
> 
> Cheers,
> Matt

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

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[Anil Kaushik]

Hi Matt,

 Sorry I missed this one. Response was in draft and I missed sending it.

On Wed, Oct 21, 2026 at 11:18 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> Hi Anil,
>
> I noticed new versions, but I don't know if you saw my reply here. Do
> you mind replying to my previous questions / comments here below, please?
>
> On 16/10/2026 10:43, Matthieu Baerts wrote:
> > Hi Anil,
> >
> > Thank you for your reply.
> >
> > On 16/10/2026 07:49, Anil Kaushik wrote:
> >>> Add a sensible default value (what do you recommend?)
> >>
> >> 15 SYN retries: about 20 minutes with the 120s max RTO.
> >
> > I guess it could even be longer, no? With an exponential backoff, it's
> > not spamming the network too much.
> >
> > I wonder if we shouldn't get something similar to tcp_retries2. But
> > here, with the syn, we also need to tcp_syn_linear_timeouts into
> > account. Either we keep it to 15 by default and, (if not 0), we add
> > tcp_syn_linear_timeouts, or we initialise it to 19.
> >

tcp_write_timeout() already adds tcp_syn_linear_timeouts on top of
icsk_syn_retries for SYN_SENT sockets. With the defaults, 15 gives 4
linear + 15 exponential retries, about 20 minutes, close to
tcp_retries2. I can make it longer if you prefer. Let me know please?

> >>> modify icsk_syn_retries instead [...] WDYT?
> >>
> >> Good idea, I will do that: one new subflow per failure, with its SYN
> >> retries set from the new netlink limit, 0 = disabled. It also removes
> >> the socket reuse, the retries counter and the 60s check.
> >>
> >> One point to think though, If ICMP unreachable stops the SYN at once.
> >> I would not retry
> >> again then. Is that OK?
> >
> > Good point. In your specific case, what do you observe?

In my tests, the fiber outage is a silent drop: the SYNs just time
out, and the subflow comes back once the path is up again.

> >
> > To me, the best is to have the network manager (something from
> > userspace) removing the MPTCP endpoints when they are not "usable", but
> > here it can be specific to the destination. I would then say to look at
> > this case later. In this case, we could eventually disconnect the
> > subflow, and add a timer to retry later. But that's also more prone to
> > bugs, so if we can avoid that, probably better to!
> >

Sure, lets take up this case later

> >>> Do we have other similar ones? [...] ss -Mte, would that work?
> >>
> >> I will drop the MIB counter. The test can use the MPJ counters and the
> >> netlink events.
> >
> > Thanks! You can expose the new limit in "struct mptcp_info", instead of
> > "reserved".
> >

Done since v5.

> >>> Is it specific for your case?
> >>
> >> No, it came from a Sashiko review: a dead subflow waiting to be closed
> >> could be re-created after its endpoint or the peer's address was
> >> removed. I will try to move it to pm_kernel.c.
> >
> > What did you end-up doing for this?
> >

When an address is removed, mptcp_pm_rm_addr_or_subflow() now marks the
subflows using it, so they are not re-created.

> >>> can we not check why it failed instead?
> >>
> >> With your idea it is not needed: only one retry, nothing to count.
> >
> > Good
> >
> >>> what error do you get when the issue is detected via TCP Keepalive?
> >>
> >> ETIMEDOUT, so it is retried.
> >
> > Thanks!
> >
> >>> Why can we not always re-used?
> >>
> >> A subflow that carried data needs the full close cleanup. With your
> >> idea we always create a new subflow, so there is only one path.
> >>
> >> For the rest: OK. I will go through pm.c, avoid struct holes and
> >> duplicated connect code, drop the RFC tag, add a changelog per patch,
> >> and test closing the connection while retrying plus the netlink events.
> >> I will wait for the netdev Sashiko review before sending.
> >
> > Thanks! BTW, it is easier if you reply inline here below, to keep the
> > context:
> >

Sorry, will do.


> > https://subspace.kernel.org/etiquette.html#do-not-top-post-when-replying
> >
> >> On Wed, Oct 14, 2026 at 11:43 PM Matthieu Baerts <matttbe@kernel.org> wrote:
> > (...)
> >
> > Cheers,
> > Matt
>
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
>
Regards
Anil

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[Matthieu Baerts]

Hi Anil,

Thank you for your reply.

On 21/10/2026 20:20, Anil Kaushik wrote:
> On Wed, Oct 21, 2026 at 11:18 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>> On 16/10/2026 10:43, Matthieu Baerts wrote:
>>> On 16/10/2026 07:49, Anil Kaushik wrote:
>>>>> Add a sensible default value (what do you recommend?)
>>>>
>>>> 15 SYN retries: about 20 minutes with the 120s max RTO.
>>>
>>> I guess it could even be longer, no? With an exponential backoff, it's
>>> not spamming the network too much.
>>>
>>> I wonder if we shouldn't get something similar to tcp_retries2. But
>>> here, with the syn, we also need to tcp_syn_linear_timeouts into
>>> account. Either we keep it to 15 by default and, (if not 0), we add
>>> tcp_syn_linear_timeouts, or we initialise it to 19.
>>>
> 
> tcp_write_timeout() already adds tcp_syn_linear_timeouts on top of
> icsk_syn_retries for SYN_SENT sockets. With the defaults, 15 gives 4
> linear + 15 exponential retries, about 20 minutes, close to
> tcp_retries2. I can make it longer if you prefer. Let me know please?

It is always tricky to find a good default value. I guess 15-20 minutes
is fine for most people. Connections are usually not that long, but some
can be, e.g. ssh. But then, what do we loose to put it higher by default
to support more cases? Maybe environments with low memory could not like
that, but either they don't manage that many connections, or they don't
use MPTCP.

When I look at nf_conntrack_tcp_timeout_established, it is set to 5 days
by default, so we shouldn't go higher than that. But maybe we should
target a few hours by default? e.g. using 20 (~10.5h?) or maybe better
to use 21 (~21h) or 22 to cover a WE (~42.5h?)?

>>>>> modify icsk_syn_retries instead [...] WDYT?
>>>>
>>>> Good idea, I will do that: one new subflow per failure, with its SYN
>>>> retries set from the new netlink limit, 0 = disabled. It also removes
>>>> the socket reuse, the retries counter and the 60s check.
>>>>
>>>> One point to think though, If ICMP unreachable stops the SYN at once.
>>>> I would not retry
>>>> again then. Is that OK?
>>>
>>> Good point. In your specific case, what do you observe?
> 
> In my tests, the fiber outage is a silent drop: the SYNs just time
> out, and the subflow comes back once the path is up again.

Interesting. Out of curiosity, how long are these outages typically?
Less than ~20 minutes I suppose?

>>> To me, the best is to have the network manager (something from
>>> userspace) removing the MPTCP endpoints when they are not "usable", but
>>> here it can be specific to the destination. I would then say to look at
>>> this case later. In this case, we could eventually disconnect the
>>> subflow, and add a timer to retry later. But that's also more prone to
>>> bugs, so if we can avoid that, probably better to!
>>>
> 
> Sure, lets take up this case later
> 
>>>>> Do we have other similar ones? [...] ss -Mte, would that work?
>>>>
>>>> I will drop the MIB counter. The test can use the MPJ counters and the
>>>> netlink events.
>>>
>>> Thanks! You can expose the new limit in "struct mptcp_info", instead of
>>> "reserved".
>>>
> 
> Done since v5.

Thanks.

>>>>> Is it specific for your case?
>>>>
>>>> No, it came from a Sashiko review: a dead subflow waiting to be closed
>>>> could be re-created after its endpoint or the peer's address was
>>>> removed. I will try to move it to pm_kernel.c.
>>>
>>> What did you end-up doing for this?
>>>
> 
> When an address is removed, mptcp_pm_rm_addr_or_subflow() now marks the
> subflows using it, so they are not re-created.

Good, simpler.

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

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[Anil Kaushik]

Hi Matt,

  Thanks for the review.

My response inline.

On Thu, Oct 22, 2026 at 1:02 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> Hi Anil,
>
> Thank you for your reply.
>
> On 21/10/2026 20:20, Anil Kaushik wrote:
> > On Wed, Oct 21, 2026 at 11:18 PM Matthieu Baerts <matttbe@kernel.org> wrote:
> >> On 16/10/2026 10:43, Matthieu Baerts wrote:
> >>> On 16/10/2026 07:49, Anil Kaushik wrote:
> >>>>> Add a sensible default value (what do you recommend?)
> >>>>
> >>>> 15 SYN retries: about 20 minutes with the 120s max RTO.
> >>>
> >>> I guess it could even be longer, no? With an exponential backoff, it's
> >>> not spamming the network too much.
> >>>
> >>> I wonder if we shouldn't get something similar to tcp_retries2. But
> >>> here, with the syn, we also need to tcp_syn_linear_timeouts into
> >>> account. Either we keep it to 15 by default and, (if not 0), we add
> >>> tcp_syn_linear_timeouts, or we initialise it to 19.
> >>>
> >
> > tcp_write_timeout() already adds tcp_syn_linear_timeouts on top of
> > icsk_syn_retries for SYN_SENT sockets. With the defaults, 15 gives 4
> > linear + 15 exponential retries, about 20 minutes, close to
> > tcp_retries2. I can make it longer if you prefer. Let me know please?
>
> It is always tricky to find a good default value. I guess 15-20 minutes
> is fine for most people. Connections are usually not that long, but some
> can be, e.g. ssh. But then, what do we loose to put it higher by default
> to support more cases? Maybe environments with low memory could not like
> that, but either they don't manage that many connections, or they don't
> use MPTCP.
>
> When I look at nf_conntrack_tcp_timeout_established, it is set to 5 days
> by default, so we shouldn't go higher than that. But maybe we should
> target a few hours by default? e.g. using 20 (~10.5h?) or maybe better
> to use 21 (~21h) or 22 to cover a WE (~42.5h?)?
>
The SYN RTO is capped at TCP_RTO_MAX (120s), so after the first
doubling each retry only adds 2 minutes: 20 is about 30 minutes,and
the maximum, 127 (MAX_TCP_SYNCNT), about 4 hours.
Should I use 127 by default, to get a few hours? Covering a weekend
would need something else, e.g. a new attempt once the SYN retries are
used up, which would bring a timer back. Let me know what you think?

> >>>>> modify icsk_syn_retries instead [...] WDYT?
> >>>>
> >>>> Good idea, I will do that: one new subflow per failure, with its SYN
> >>>> retries set from the new netlink limit, 0 = disabled. It also removes
> >>>> the socket reuse, the retries counter and the 60s check.
> >>>>
> >>>> One point to think though, If ICMP unreachable stops the SYN at once.
> >>>> I would not retry
> >>>> again then. Is that OK?
> >>>
> >>> Good point. In your specific case, what do you observe?
> >
> > In my tests, the fiber outage is a silent drop: the SYNs just time
> > out, and the subflow comes back once the path is up again.
>
> Interesting. Out of curiosity, how long are these outages typically?
> Less than ~20 minutes I suppose?
>
It varies. Short ones are a few minutes, but when the fiber is cut, the
repair can take several hours. So a longer default would help.

> >>> To me, the best is to have the network manager (something from
> >>> userspace) removing the MPTCP endpoints when they are not "usable", but
> >>> here it can be specific to the destination. I would then say to look at
> >>> this case later. In this case, we could eventually disconnect the
> >>> subflow, and add a timer to retry later. But that's also more prone to
> >>> bugs, so if we can avoid that, probably better to!
> >>>
> >
> > Sure, lets take up this case later
> >
> >>>>> Do we have other similar ones? [...] ss -Mte, would that work?
> >>>>
> >>>> I will drop the MIB counter. The test can use the MPJ counters and the
> >>>> netlink events.
> >>>
> >>> Thanks! You can expose the new limit in "struct mptcp_info", instead of
> >>> "reserved".
> >>>
> >
> > Done since v5.
>
> Thanks.
>
> >>>>> Is it specific for your case?
> >>>>
> >>>> No, it came from a Sashiko review: a dead subflow waiting to be closed
> >>>> could be re-created after its endpoint or the peer's address was
> >>>> removed. I will try to move it to pm_kernel.c.
> >>>
> >>> What did you end-up doing for this?
> >>>
> >
> > When an address is removed, mptcp_pm_rm_addr_or_subflow() now marks the
> > subflows using it, so they are not re-created.
>
> Good, simpler.
>
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
>
Regards
Anil

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[Matthieu Baerts]

On 22/10/2026 11:24, Anil Kaushik wrote:
> Hi Matt,
> 
>   Thanks for the review.
> 
> My response inline.
> 
> On Thu, Oct 22, 2026 at 1:02 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>>
>> Hi Anil,
>>
>> Thank you for your reply.
>>
>> On 21/10/2026 20:20, Anil Kaushik wrote:
>>> On Wed, Oct 21, 2026 at 11:18 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>>>> On 16/10/2026 10:43, Matthieu Baerts wrote:
>>>>> On 16/10/2026 07:49, Anil Kaushik wrote:
>>>>>>> Add a sensible default value (what do you recommend?)
>>>>>>
>>>>>> 15 SYN retries: about 20 minutes with the 120s max RTO.
>>>>>
>>>>> I guess it could even be longer, no? With an exponential backoff, it's
>>>>> not spamming the network too much.
>>>>>
>>>>> I wonder if we shouldn't get something similar to tcp_retries2. But
>>>>> here, with the syn, we also need to tcp_syn_linear_timeouts into
>>>>> account. Either we keep it to 15 by default and, (if not 0), we add
>>>>> tcp_syn_linear_timeouts, or we initialise it to 19.
>>>>>
>>>
>>> tcp_write_timeout() already adds tcp_syn_linear_timeouts on top of
>>> icsk_syn_retries for SYN_SENT sockets. With the defaults, 15 gives 4
>>> linear + 15 exponential retries, about 20 minutes, close to
>>> tcp_retries2. I can make it longer if you prefer. Let me know please?
>>
>> It is always tricky to find a good default value. I guess 15-20 minutes
>> is fine for most people. Connections are usually not that long, but some
>> can be, e.g. ssh. But then, what do we loose to put it higher by default
>> to support more cases? Maybe environments with low memory could not like
>> that, but either they don't manage that many connections, or they don't
>> use MPTCP.
>>
>> When I look at nf_conntrack_tcp_timeout_established, it is set to 5 days
>> by default, so we shouldn't go higher than that. But maybe we should
>> target a few hours by default? e.g. using 20 (~10.5h?) or maybe better
>> to use 21 (~21h) or 22 to cover a WE (~42.5h?)?
>>
> The SYN RTO is capped at TCP_RTO_MAX (120s), so after the first
> doubling each retry only adds 2 minutes: 20 is about 30 minutes,and
> the maximum, 127 (MAX_TCP_SYNCNT), about 4 hours.
> Should I use 127 by default, to get a few hours? Covering a weekend
> would need something else, e.g. a new attempt once the SYN retries are
> used up, which would bring a timer back. Let me know what you think?

Ah yes, thank you, I remember something with the TCP_RTO_MAX. OK then
please use the maximum, and add "(127)" after mentionning MAX_TCP_SYNCNT
in the doc.

>>>>>>> modify icsk_syn_retries instead [...] WDYT?
>>>>>>
>>>>>> Good idea, I will do that: one new subflow per failure, with its SYN
>>>>>> retries set from the new netlink limit, 0 = disabled. It also removes
>>>>>> the socket reuse, the retries counter and the 60s check.
>>>>>>
>>>>>> One point to think though, If ICMP unreachable stops the SYN at once.
>>>>>> I would not retry
>>>>>> again then. Is that OK?
>>>>>
>>>>> Good point. In your specific case, what do you observe?
>>>
>>> In my tests, the fiber outage is a silent drop: the SYNs just time
>>> out, and the subflow comes back once the path is up again.
>>
>> Interesting. Out of curiosity, how long are these outages typically?
>> Less than ~20 minutes I suppose?
>>
> It varies. Short ones are a few minutes, but when the fiber is cut, the
> repair can take several hours. So a longer default would help.

When the "fiber is cut", do you still have an IP address or is the
connection still up?

To me, if there is an issue in the network, we generally have two cases:
either we are closed to an end (client / server) and the host should be
disconnected from that network, or it is happening in the "middle" of
the network where BGP will reroute the traffic.
In case of disconnection, I would guess the IP is lost and the
corresponding MPTCP endpoint is removed. The userspace will try to
reconnect, then re-add the MPTCP endpoint, and the PM can restart the
subflows.

But that doesn't seem to be the case on your side :)
Do you know why?

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

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[Anil Kaushik]

Hi Matt,

 Thanks for the response.

On Thu, Oct 22, 2026 at 3:58 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> On 22/10/2026 11:24, Anil Kaushik wrote:
> > Hi Matt,
> >
> >   Thanks for the review.
> >
> > My response inline.
> >
> > On Thu, Oct 22, 2026 at 1:02 PM Matthieu Baerts <matttbe@kernel.org> wrote:
> >>
> >> Hi Anil,
> >>
> >> Thank you for your reply.
> >>
> >> On 21/10/2026 20:20, Anil Kaushik wrote:
> >>> On Wed, Oct 21, 2026 at 11:18 PM Matthieu Baerts <matttbe@kernel.org> wrote:
> >>>> On 16/10/2026 10:43, Matthieu Baerts wrote:
> >>>>> On 16/10/2026 07:49, Anil Kaushik wrote:
> >>>>>>> Add a sensible default value (what do you recommend?)
> >>>>>>
> >>>>>> 15 SYN retries: about 20 minutes with the 120s max RTO.
> >>>>>
> >>>>> I guess it could even be longer, no? With an exponential backoff, it's
> >>>>> not spamming the network too much.
> >>>>>
> >>>>> I wonder if we shouldn't get something similar to tcp_retries2. But
> >>>>> here, with the syn, we also need to tcp_syn_linear_timeouts into
> >>>>> account. Either we keep it to 15 by default and, (if not 0), we add
> >>>>> tcp_syn_linear_timeouts, or we initialise it to 19.
> >>>>>
> >>>
> >>> tcp_write_timeout() already adds tcp_syn_linear_timeouts on top of
> >>> icsk_syn_retries for SYN_SENT sockets. With the defaults, 15 gives 4
> >>> linear + 15 exponential retries, about 20 minutes, close to
> >>> tcp_retries2. I can make it longer if you prefer. Let me know please?
> >>
> >> It is always tricky to find a good default value. I guess 15-20 minutes
> >> is fine for most people. Connections are usually not that long, but some
> >> can be, e.g. ssh. But then, what do we loose to put it higher by default
> >> to support more cases? Maybe environments with low memory could not like
> >> that, but either they don't manage that many connections, or they don't
> >> use MPTCP.
> >>
> >> When I look at nf_conntrack_tcp_timeout_established, it is set to 5 days
> >> by default, so we shouldn't go higher than that. But maybe we should
> >> target a few hours by default? e.g. using 20 (~10.5h?) or maybe better
> >> to use 21 (~21h) or 22 to cover a WE (~42.5h?)?
> >>
> > The SYN RTO is capped at TCP_RTO_MAX (120s), so after the first
> > doubling each retry only adds 2 minutes: 20 is about 30 minutes,and
> > the maximum, 127 (MAX_TCP_SYNCNT), about 4 hours.
> > Should I use 127 by default, to get a few hours? Covering a weekend
> > would need something else, e.g. a new attempt once the SYN retries are
> > used up, which would bring a timer back. Let me know what you think?
>
> Ah yes, thank you, I remember something with the TCP_RTO_MAX. OK then
> please use the maximum, and add "(127)" after mentionning MAX_TCP_SYNCNT
> in the doc.
>
Will do in the next version(v8).
> >>>>>>> modify icsk_syn_retries instead [...] WDYT?
> >>>>>>
> >>>>>> Good idea, I will do that: one new subflow per failure, with its SYN
> >>>>>> retries set from the new netlink limit, 0 = disabled. It also removes
> >>>>>> the socket reuse, the retries counter and the 60s check.
> >>>>>>
> >>>>>> One point to think though, If ICMP unreachable stops the SYN at once.
> >>>>>> I would not retry
> >>>>>> again then. Is that OK?
> >>>>>
> >>>>> Good point. In your specific case, what do you observe?
> >>>
> >>> In my tests, the fiber outage is a silent drop: the SYNs just time
> >>> out, and the subflow comes back once the path is up again.
> >>
> >> Interesting. Out of curiosity, how long are these outages typically?
> >> Less than ~20 minutes I suppose?
> >>
> > It varies. Short ones are a few minutes, but when the fiber is cut, the
> > repair can take several hours. So a longer default would help.
>
> When the "fiber is cut", do you still have an IP address or is the
> connection still up?
>
> To me, if there is an issue in the network, we generally have two cases:
> either we are closed to an end (client / server) and the host should be
> disconnected from that network, or it is happening in the "middle" of
> the network where BGP will reroute the traffic.
> In case of disconnection, I would guess the IP is lost and the
> corresponding MPTCP endpoint is removed. The userspace will try to
> reconnect, then re-add the MPTCP endpoint, and the PM can restart the
> subflows.
>
> But that doesn't seem to be the case on your side :)
> Do you know why?
>
Yes. MPTCP runs on a gateway in front of the fiber and Starlink
routers, connected to each of them over Ethernet. The fiber cut is
between the fiber router and the ISP, so the gateway keeps its link and
its IP address on that side, nothing removes the MPTCP endpoint, and
the packets on that path are just dropped. There will be mix of
deployments like this.

Any thoughts?
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
>
Regards
Anil

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[Matthieu Baerts]

On 22/10/2026 13:04, Anil Kaushik wrote:
> On Thu, Oct 22, 2026 at 3:58 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>> On 22/10/2026 11:24, Anil Kaushik wrote:
>>> On Thu, Oct 22, 2026 at 1:02 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>>>> On 21/10/2026 20:20, Anil Kaushik wrote:
>>>>> On Wed, Oct 21, 2026 at 11:18 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>>>>>> On 16/10/2026 10:43, Matthieu Baerts wrote:
>>>>>>> On 16/10/2026 07:49, Anil Kaushik wrote:

(...)

>>>>>>>>> modify icsk_syn_retries instead [...] WDYT?
>>>>>>>>
>>>>>>>> Good idea, I will do that: one new subflow per failure, with its SYN
>>>>>>>> retries set from the new netlink limit, 0 = disabled. It also removes
>>>>>>>> the socket reuse, the retries counter and the 60s check.
>>>>>>>>
>>>>>>>> One point to think though, If ICMP unreachable stops the SYN at once.
>>>>>>>> I would not retry
>>>>>>>> again then. Is that OK?
>>>>>>>
>>>>>>> Good point. In your specific case, what do you observe?
>>>>>
>>>>> In my tests, the fiber outage is a silent drop: the SYNs just time
>>>>> out, and the subflow comes back once the path is up again.
>>>>
>>>> Interesting. Out of curiosity, how long are these outages typically?
>>>> Less than ~20 minutes I suppose?
>>>>
>>> It varies. Short ones are a few minutes, but when the fiber is cut, the
>>> repair can take several hours. So a longer default would help.
>>
>> When the "fiber is cut", do you still have an IP address or is the
>> connection still up?
>>
>> To me, if there is an issue in the network, we generally have two cases:
>> either we are closed to an end (client / server) and the host should be
>> disconnected from that network, or it is happening in the "middle" of
>> the network where BGP will reroute the traffic.
>> In case of disconnection, I would guess the IP is lost and the
>> corresponding MPTCP endpoint is removed. The userspace will try to
>> reconnect, then re-add the MPTCP endpoint, and the PM can restart the
>> subflows.
>>
>> But that doesn't seem to be the case on your side :)
>> Do you know why?
>>
> Yes. MPTCP runs on a gateway in front of the fiber and Starlink
> routers, connected to each of them over Ethernet. The fiber cut is
> between the fiber router and the ISP, so the gateway keeps its link and
> its IP address on that side, nothing removes the MPTCP endpoint, and
> the packets on that path are just dropped. There will be mix of
> deployments like this.
> 
> Any thoughts?

So when you have an issue between the fiber router and the ISP, the
fiber router keeps its connection up to the ISP, and no ICMP network
unreachable are sent?

In such setup, and if the fiber router gets a notification somehow in
case of issue, I would find it better to forward this notification to
the gateway to disable the MPTCP endpoint. An alternative is to have a
simple service monitoring the links, and removing/re-adding endpoints: a
simple "echo" service in TCP (or ICMP echo) with a server somewhere, one
per link. With that, you have more control and can eventually ack quicker.

Yet, the series you are suggesting here still makes sense because that
would help people, or in case of issues not detected by the network manager.

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

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[Anil Kaushik]

Hi Matt,

 Sorry my earlier response had html content.

Resending my response again. Couple of more points too.

On Thu, Oct 22, 2026 at 7:00 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> On 22/10/2026 13:04, Anil Kaushik wrote:
> > On Thu, Oct 22, 2026 at 3:58 PM Matthieu Baerts <matttbe@kernel.org> wrote:
> >> On 22/10/2026 11:24, Anil Kaushik wrote:
> >>> On Thu, Oct 22, 2026 at 1:02 PM Matthieu Baerts <matttbe@kernel.org> wrote:
> >>>> On 21/10/2026 20:20, Anil Kaushik wrote:
> >>>>> On Wed, Oct 21, 2026 at 11:18 PM Matthieu Baerts <matttbe@kernel.org> wrote:
> >>>>>> On 16/10/2026 10:43, Matthieu Baerts wrote:
> >>>>>>> On 16/10/2026 07:49, Anil Kaushik wrote:
>
> (...)
>
> >>>>>>>>> modify icsk_syn_retries instead [...] WDYT?
> >>>>>>>>
> >>>>>>>> Good idea, I will do that: one new subflow per failure, with its SYN
> >>>>>>>> retries set from the new netlink limit, 0 = disabled. It also removes
> >>>>>>>> the socket reuse, the retries counter and the 60s check.
> >>>>>>>>
> >>>>>>>> One point to think though, If ICMP unreachable stops the SYN at once.
> >>>>>>>> I would not retry
> >>>>>>>> again then. Is that OK?
> >>>>>>>
> >>>>>>> Good point. In your specific case, what do you observe?
> >>>>>
> >>>>> In my tests, the fiber outage is a silent drop: the SYNs just time
> >>>>> out, and the subflow comes back once the path is up again.
> >>>>
> >>>> Interesting. Out of curiosity, how long are these outages typically?
> >>>> Less than ~20 minutes I suppose?
> >>>>
> >>> It varies. Short ones are a few minutes, but when the fiber is cut, the
> >>> repair can take several hours. So a longer default would help.
> >>
> >> When the "fiber is cut", do you still have an IP address or is the
> >> connection still up?
> >>
> >> To me, if there is an issue in the network, we generally have two cases:
> >> either we are closed to an end (client / server) and the host should be
> >> disconnected from that network, or it is happening in the "middle" of
> >> the network where BGP will reroute the traffic.
> >> In case of disconnection, I would guess the IP is lost and the
> >> corresponding MPTCP endpoint is removed. The userspace will try to
> >> reconnect, then re-add the MPTCP endpoint, and the PM can restart the
> >> subflows.
> >>
> >> But that doesn't seem to be the case on your side :)
> >> Do you know why?
> >>
> > Yes. MPTCP runs on a gateway in front of the fiber and Starlink
> > routers, connected to each of them over Ethernet. The fiber cut is
> > between the fiber router and the ISP, so the gateway keeps its link and
> > its IP address on that side, nothing removes the MPTCP endpoint, and
> > the packets on that path are just dropped. There will be mix of
> > deployments like this.
> >
> > Any thoughts?
>
> So when you have an issue between the fiber router and the ISP, the
> fiber router keeps its connection up to the ISP, and no ICMP network
> unreachable are sent?
>
> In such setup, and if the fiber router gets a notification somehow in
> case of issue, I would find it better to forward this notification to
> the gateway to disable the MPTCP endpoint. An alternative is to have a
> simple service monitoring the links, and removing/re-adding endpoints: a
> simple "echo" service in TCP (or ICMP echo) with a server somewhere, one
> per link. With that, you have more control and can eventually ack quicker.
>

Yes. The router still has its default route, so it does not send ICMP
unreachable, it just drops the packets. And when its WAN link really goes
down, it has no way to tell the gateway.

Agreed on the link monitoring. A small probe per link, removing and
re-adding the endpoint, is a good idea and I will send a separate RFC.
Let me know what you think?

> Yet, the series you are suggesting here still makes sense because that
> would help people, or in case of issues not detected by the network manager.
>
I will send v8 with the default set to the maximum, 127.

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

Regards
Anil

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[Matthieu Baerts]

On 22/10/2026 16:31, Anil Kaushik wrote:
> Hi Matt,
> 
>  Sorry my earlier response had html content.
> 
> Resending my response again. Couple of more points too.
> 
> On Thu, Oct 22, 2026 at 7:00 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>>
>> On 22/10/2026 13:04, Anil Kaushik wrote:
>>> On Thu, Oct 22, 2026 at 3:58 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>>>> On 22/10/2026 11:24, Anil Kaushik wrote:
>>>>> On Thu, Oct 22, 2026 at 1:02 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>>>>>> On 21/10/2026 20:20, Anil Kaushik wrote:
>>>>>>> On Wed, Oct 21, 2026 at 11:18 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>>>>>>>> On 16/10/2026 10:43, Matthieu Baerts wrote:
>>>>>>>>> On 16/10/2026 07:49, Anil Kaushik wrote:
>>
>> (...)
>>
>>>>>>>>>>> modify icsk_syn_retries instead [...] WDYT?
>>>>>>>>>>
>>>>>>>>>> Good idea, I will do that: one new subflow per failure, with its SYN
>>>>>>>>>> retries set from the new netlink limit, 0 = disabled. It also removes
>>>>>>>>>> the socket reuse, the retries counter and the 60s check.
>>>>>>>>>>
>>>>>>>>>> One point to think though, If ICMP unreachable stops the SYN at once.
>>>>>>>>>> I would not retry
>>>>>>>>>> again then. Is that OK?
>>>>>>>>>
>>>>>>>>> Good point. In your specific case, what do you observe?
>>>>>>>
>>>>>>> In my tests, the fiber outage is a silent drop: the SYNs just time
>>>>>>> out, and the subflow comes back once the path is up again.
>>>>>>
>>>>>> Interesting. Out of curiosity, how long are these outages typically?
>>>>>> Less than ~20 minutes I suppose?
>>>>>>
>>>>> It varies. Short ones are a few minutes, but when the fiber is cut, the
>>>>> repair can take several hours. So a longer default would help.
>>>>
>>>> When the "fiber is cut", do you still have an IP address or is the
>>>> connection still up?
>>>>
>>>> To me, if there is an issue in the network, we generally have two cases:
>>>> either we are closed to an end (client / server) and the host should be
>>>> disconnected from that network, or it is happening in the "middle" of
>>>> the network where BGP will reroute the traffic.
>>>> In case of disconnection, I would guess the IP is lost and the
>>>> corresponding MPTCP endpoint is removed. The userspace will try to
>>>> reconnect, then re-add the MPTCP endpoint, and the PM can restart the
>>>> subflows.
>>>>
>>>> But that doesn't seem to be the case on your side :)
>>>> Do you know why?
>>>>
>>> Yes. MPTCP runs on a gateway in front of the fiber and Starlink
>>> routers, connected to each of them over Ethernet. The fiber cut is
>>> between the fiber router and the ISP, so the gateway keeps its link and
>>> its IP address on that side, nothing removes the MPTCP endpoint, and
>>> the packets on that path are just dropped. There will be mix of
>>> deployments like this.
>>>
>>> Any thoughts?
>>
>> So when you have an issue between the fiber router and the ISP, the
>> fiber router keeps its connection up to the ISP, and no ICMP network
>> unreachable are sent?
>>
>> In such setup, and if the fiber router gets a notification somehow in
>> case of issue, I would find it better to forward this notification to
>> the gateway to disable the MPTCP endpoint. An alternative is to have a
>> simple service monitoring the links, and removing/re-adding endpoints: a
>> simple "echo" service in TCP (or ICMP echo) with a server somewhere, one
>> per link. With that, you have more control and can eventually ack quicker.
>>
> 
> Yes. The router still has its default route, so it does not send ICMP
> unreachable, it just drops the packets. And when its WAN link really goes
> down, it has no way to tell the gateway.

It could eventually turn off the link to the gateway.

> Agreed on the link monitoring. A small probe per link, removing and
> re-adding the endpoint, is a good idea and I will send a separate RFC.
> Let me know what you think?

That's something that should be done in userspace, no?

>> Yet, the series you are suggesting here still makes sense because that
>> would help people, or in case of issues not detected by the network manager.
>>
> I will send v8 with the default set to the maximum, 127.

Thank you, I will try to have a look.

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

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[Anil Kaushik]

Thanks Matt

On Thu, Oct 22, 2026 at 9:20 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> On 22/10/2026 16:31, Anil Kaushik wrote:
> > Hi Matt,
> >
> >  Sorry my earlier response had html content.
> >
> > Resending my response again. Couple of more points too.
> >
> > On Thu, Oct 22, 2026 at 7:00 PM Matthieu Baerts <matttbe@kernel.org> wrote:
> >>
> >> On 22/10/2026 13:04, Anil Kaushik wrote:
> >>> On Thu, Oct 22, 2026 at 3:58 PM Matthieu Baerts <matttbe@kernel.org> wrote:
> >>>> On 22/10/2026 11:24, Anil Kaushik wrote:
> >>>>> On Thu, Oct 22, 2026 at 1:02 PM Matthieu Baerts <matttbe@kernel.org> wrote:
> >>>>>> On 21/10/2026 20:20, Anil Kaushik wrote:
> >>>>>>> On Wed, Oct 21, 2026 at 11:18 PM Matthieu Baerts <matttbe@kernel.org> wrote:
> >>>>>>>> On 16/10/2026 10:43, Matthieu Baerts wrote:
> >>>>>>>>> On 16/10/2026 07:49, Anil Kaushik wrote:
> >>
> >> (...)
> >>
> >>>>>>>>>>> modify icsk_syn_retries instead [...] WDYT?
> >>>>>>>>>>
> >>>>>>>>>> Good idea, I will do that: one new subflow per failure, with its SYN
> >>>>>>>>>> retries set from the new netlink limit, 0 = disabled. It also removes
> >>>>>>>>>> the socket reuse, the retries counter and the 60s check.
> >>>>>>>>>>
> >>>>>>>>>> One point to think though, If ICMP unreachable stops the SYN at once.
> >>>>>>>>>> I would not retry
> >>>>>>>>>> again then. Is that OK?
> >>>>>>>>>
> >>>>>>>>> Good point. In your specific case, what do you observe?
> >>>>>>>
> >>>>>>> In my tests, the fiber outage is a silent drop: the SYNs just time
> >>>>>>> out, and the subflow comes back once the path is up again.
> >>>>>>
> >>>>>> Interesting. Out of curiosity, how long are these outages typically?
> >>>>>> Less than ~20 minutes I suppose?
> >>>>>>
> >>>>> It varies. Short ones are a few minutes, but when the fiber is cut, the
> >>>>> repair can take several hours. So a longer default would help.
> >>>>
> >>>> When the "fiber is cut", do you still have an IP address or is the
> >>>> connection still up?
> >>>>
> >>>> To me, if there is an issue in the network, we generally have two cases:
> >>>> either we are closed to an end (client / server) and the host should be
> >>>> disconnected from that network, or it is happening in the "middle" of
> >>>> the network where BGP will reroute the traffic.
> >>>> In case of disconnection, I would guess the IP is lost and the
> >>>> corresponding MPTCP endpoint is removed. The userspace will try to
> >>>> reconnect, then re-add the MPTCP endpoint, and the PM can restart the
> >>>> subflows.
> >>>>
> >>>> But that doesn't seem to be the case on your side :)
> >>>> Do you know why?
> >>>>
> >>> Yes. MPTCP runs on a gateway in front of the fiber and Starlink
> >>> routers, connected to each of them over Ethernet. The fiber cut is
> >>> between the fiber router and the ISP, so the gateway keeps its link and
> >>> its IP address on that side, nothing removes the MPTCP endpoint, and
> >>> the packets on that path are just dropped. There will be mix of
> >>> deployments like this.
> >>>
> >>> Any thoughts?
> >>
> >> So when you have an issue between the fiber router and the ISP, the
> >> fiber router keeps its connection up to the ISP, and no ICMP network
> >> unreachable are sent?
> >>
> >> In such setup, and if the fiber router gets a notification somehow in
> >> case of issue, I would find it better to forward this notification to
> >> the gateway to disable the MPTCP endpoint. An alternative is to have a
> >> simple service monitoring the links, and removing/re-adding endpoints: a
> >> simple "echo" service in TCP (or ICMP echo) with a server somewhere, one
> >> per link. With that, you have more control and can eventually ack quicker.
> >>
> >
> > Yes. The router still has its default route, so it does not send ICMP
> > unreachable, it just drops the packets. And when its WAN link really goes
> > down, it has no way to tell the gateway.
>
> It could eventually turn off the link to the gateway.
>
> > Agreed on the link monitoring. A small probe per link, removing and
> > re-adding the endpoint, is a good idea and I will send a separate RFC.
> > Let me know what you think?
>
> That's something that should be done in userspace, no?
>

Yes, in userspace. I was thinking of a plugin for mptcpd, that probes
each link and removes and re-adds the
endpoint. I will look at mptcpd first.
If you have any other thoughts, let me know please.

> >> Yet, the series you are suggesting here still makes sense because that
> >> would help people, or in case of issues not detected by the network manager.
> >>
> > I will send v8 with the default set to the maximum, 127.
>
> Thank you, I will try to have a look.
>
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
>

Regards
Anil

Re: [RFC PATCH mptcp-next v3 2/3] mptcp: pm: kernel: retry subflows closed after an error

[Matthieu Baerts]

On 22/10/2026 18:12, Anil Kaushik wrote:
> Thanks Matt
> 
> On Thu, Oct 22, 2026 at 9:20 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>>
>> On 22/10/2026 16:31, Anil Kaushik wrote:
>>> Hi Matt,
>>>
>>>  Sorry my earlier response had html content.
>>>
>>> Resending my response again. Couple of more points too.
>>>
>>> On Thu, Oct 22, 2026 at 7:00 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>>>>
>>>> On 22/10/2026 13:04, Anil Kaushik wrote:
>>>>> On Thu, Oct 22, 2026 at 3:58 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>>>>>> On 22/10/2026 11:24, Anil Kaushik wrote:
>>>>>>> On Thu, Oct 22, 2026 at 1:02 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>>>>>>>> On 21/10/2026 20:20, Anil Kaushik wrote:
>>>>>>>>> On Wed, Oct 21, 2026 at 11:18 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>>>>>>>>>> On 16/10/2026 10:43, Matthieu Baerts wrote:
>>>>>>>>>>> On 16/10/2026 07:49, Anil Kaushik wrote:
>>>>
>>>> (...)
>>>>
>>>>>>>>>>>>> modify icsk_syn_retries instead [...] WDYT?
>>>>>>>>>>>>
>>>>>>>>>>>> Good idea, I will do that: one new subflow per failure, with its SYN
>>>>>>>>>>>> retries set from the new netlink limit, 0 = disabled. It also removes
>>>>>>>>>>>> the socket reuse, the retries counter and the 60s check.
>>>>>>>>>>>>
>>>>>>>>>>>> One point to think though, If ICMP unreachable stops the SYN at once.
>>>>>>>>>>>> I would not retry
>>>>>>>>>>>> again then. Is that OK?
>>>>>>>>>>>
>>>>>>>>>>> Good point. In your specific case, what do you observe?
>>>>>>>>>
>>>>>>>>> In my tests, the fiber outage is a silent drop: the SYNs just time
>>>>>>>>> out, and the subflow comes back once the path is up again.
>>>>>>>>
>>>>>>>> Interesting. Out of curiosity, how long are these outages typically?
>>>>>>>> Less than ~20 minutes I suppose?
>>>>>>>>
>>>>>>> It varies. Short ones are a few minutes, but when the fiber is cut, the
>>>>>>> repair can take several hours. So a longer default would help.
>>>>>>
>>>>>> When the "fiber is cut", do you still have an IP address or is the
>>>>>> connection still up?
>>>>>>
>>>>>> To me, if there is an issue in the network, we generally have two cases:
>>>>>> either we are closed to an end (client / server) and the host should be
>>>>>> disconnected from that network, or it is happening in the "middle" of
>>>>>> the network where BGP will reroute the traffic.
>>>>>> In case of disconnection, I would guess the IP is lost and the
>>>>>> corresponding MPTCP endpoint is removed. The userspace will try to
>>>>>> reconnect, then re-add the MPTCP endpoint, and the PM can restart the
>>>>>> subflows.
>>>>>>
>>>>>> But that doesn't seem to be the case on your side :)
>>>>>> Do you know why?
>>>>>>
>>>>> Yes. MPTCP runs on a gateway in front of the fiber and Starlink
>>>>> routers, connected to each of them over Ethernet. The fiber cut is
>>>>> between the fiber router and the ISP, so the gateway keeps its link and
>>>>> its IP address on that side, nothing removes the MPTCP endpoint, and
>>>>> the packets on that path are just dropped. There will be mix of
>>>>> deployments like this.
>>>>>
>>>>> Any thoughts?
>>>>
>>>> So when you have an issue between the fiber router and the ISP, the
>>>> fiber router keeps its connection up to the ISP, and no ICMP network
>>>> unreachable are sent?
>>>>
>>>> In such setup, and if the fiber router gets a notification somehow in
>>>> case of issue, I would find it better to forward this notification to
>>>> the gateway to disable the MPTCP endpoint. An alternative is to have a
>>>> simple service monitoring the links, and removing/re-adding endpoints: a
>>>> simple "echo" service in TCP (or ICMP echo) with a server somewhere, one
>>>> per link. With that, you have more control and can eventually ack quicker.
>>>>
>>>
>>> Yes. The router still has its default route, so it does not send ICMP
>>> unreachable, it just drops the packets. And when its WAN link really goes
>>> down, it has no way to tell the gateway.
>>
>> It could eventually turn off the link to the gateway.
>>
>>> Agreed on the link monitoring. A small probe per link, removing and
>>> re-adding the endpoint, is a good idea and I will send a separate RFC.
>>> Let me know what you think?
>>
>> That's something that should be done in userspace, no?
>>
> 
> Yes, in userspace. I was thinking of a plugin for mptcpd, that probes
> each link and removes and re-adds the
> endpoint. I will look at mptcpd first.
> If you have any other thoughts, let me know please.

Good idea to use mptcpd. Or an extension to an existing one.

(Or using tools that already have the capability to probe links like
mwan3 ; or a script using ping/hping, then using "ip mptcp")

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