[PATCH mptcp-next v6 0/6] mptcp: pm: in-kernel: retry subflows closed after an error

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH mptcp-next v6 0/6] mptcp: pm: in-kernel: retry subflows closed after an error
@ 2026-10-18  4:17 Anil Kaushik
  2026-10-18  4:17 ` [PATCH mptcp-next v6 1/6] selftests: mptcp: connect: retry write() on EAGAIN Anil Kaushik
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Anil Kaushik @ 2026-10-18  4:17 UTC (permalink / raw)
  To: mptcp; +Cc: Anil Kaushik

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

When TCP closes an established subflow we created because of an error,
the in-kernel PM now creates a new one to the same addresses. Its SYN
retries space out the attempts. Their number is set by a new limit,
subflow-retries, like TCP_SYNCNT: 15 by default, about 20 minutes, 0
disables it.

Patch 1 fixes a selftest tool issue seen with the new test. Patches 2
and 3 add the new limit, patch 4 the retry, patches 5 and 6 the
selftests.

Changes in v6, after the Sashiko reviews (netdev Sashiko on v4, which
also applied to v5, and sashiko.dev on v5): only subflows that were
established are re-created, so the PM still moves on to the next
endpoints as before; removed remote addresses and the local ID 0 are
handled too; the policy checks the maximum value, so the parse_limit()
patch is gone; and the test uses the net/lib.sh helpers. Details in
each patch.

v5: https://lore.kernel.org/mptcp/20261016160926.1155262-1-anilkaushikwireless@gmail.com/
v4: https://lore.kernel.org/mptcp/20261016055010.980819-1-anilkaushikwireless@gmail.com/
v3: https://lore.kernel.org/mptcp/20261014143349.768138-1-anilkaushikwireless@gmail.com/
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 (6):
  selftests: mptcp: connect: retry write() on EAGAIN
  netlink: specs: mptcp: add the subflow-retries attribute
  mptcp: pm: in-kernel: add the subflow-retries limit
  mptcp: pm: in-kernel: retry subflows closed after an error
  selftests: mptcp: pm_nl_ctl: support the subflow-retries limit
  selftests: mptcp: join: validate subflow retry after link failure

 Documentation/netlink/specs/mptcp_pm.yaml     |  12 ++
 include/uapi/linux/mptcp.h                    |   2 +-
 include/uapi/linux/mptcp_pm.h                 |   1 +
 net/mptcp/mptcp_pm_gen.c                      |  10 +-
 net/mptcp/mptcp_pm_gen.h                      |   4 +-
 net/mptcp/pm.c                                |  39 +++++
 net/mptcp/pm_kernel.c                         | 123 ++++++++++++-
 net/mptcp/protocol.c                          |  23 +++
 net/mptcp/protocol.h                          |  20 +++
 net/mptcp/sockopt.c                           |   2 +
 .../selftests/net/mptcp/mptcp_connect.c       |  18 ++
 .../testing/selftests/net/mptcp/mptcp_join.sh | 164 ++++++++++++++++++
 .../testing/selftests/net/mptcp/pm_netlink.sh |  42 ++++-
 tools/testing/selftests/net/mptcp/pm_nl_ctl.c |  34 +++-
 14 files changed, 477 insertions(+), 17 deletions(-)

-- 
2.25.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH mptcp-next v6 1/6] selftests: mptcp: connect: retry write() on EAGAIN
  2026-10-18  4:17 [PATCH mptcp-next v6 0/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
@ 2026-10-18  4:17 ` Anil Kaushik
  2026-10-18  4:17 ` [PATCH mptcp-next v6 2/6] netlink: specs: mptcp: add the subflow-retries attribute Anil Kaushik
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Anil Kaushik @ 2026-10-18  4:17 UTC (permalink / raw)
  To: mptcp; +Cc: 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 a poll timeout is set, still
fail after as many retries, 1ms apart: 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>
---

Notes:
    v6:
     - the comment says the limit only applies with a poll timeout (netdev
       Sashiko)
    v5:
     - no change
    v4:
     - count the EAGAIN retries, not ms (Sashiko)
     - no limit when there is no poll timeout, -t 0 (Sashiko)
    v3:
     - do not wait forever on EAGAIN (Sashiko)
     - wrap lines over 80 columns
    v2:
     - Fixes tag

 .../selftests/net/mptcp/mptcp_connect.c        | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index 178d98d91fea..49017d00ab69 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_cnt = 0;
 
 	set_nonblock(peerfd, true);
 
@@ -711,6 +712,22 @@ 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 if a poll timeout
+					 * is set: poll() does not check
+					 * every cause of EAGAIN.
+					 */
+					if (errno == EAGAIN &&
+					    (poll_timeout < 0 ||
+					     ++eagain_cnt <= poll_timeout)) {
+						usleep(1000);
+						continue;
+					}
+
 					/* expected reset, continue to read */
 					if (cfg_rcv_trunc &&
 					    (errno == ECONNRESET ||
@@ -723,6 +740,7 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd,
 					return 111;
 				}
 
+				eagain_cnt = 0;
 				winfo->off += bw;
 				winfo->len -= bw;
 				total_wlen += bw;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH mptcp-next v6 2/6] netlink: specs: mptcp: add the subflow-retries attribute
  2026-10-18  4:17 [PATCH mptcp-next v6 0/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
  2026-10-18  4:17 ` [PATCH mptcp-next v6 1/6] selftests: mptcp: connect: retry write() on EAGAIN Anil Kaushik
@ 2026-10-18  4:17 ` Anil Kaushik
  2026-10-19 17:46   ` netdev-bot+sashiko
  2026-10-18  4:17 ` [PATCH mptcp-next v6 3/6] mptcp: pm: in-kernel: add the subflow-retries limit Anil Kaushik
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Anil Kaushik @ 2026-10-18  4:17 UTC (permalink / raw)
  To: mptcp; +Cc: Anil Kaushik

It will be used by the in-kernel PM to set the number of SYN
retransmissions of a subflow re-created after an error, with the other
limits: add it to the set-limits and get-limits commands.

The uapi header and the generated files are updated with ynl-regen.

Assisted-by: LLM
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---

Notes:
    v6 (netdev Sashiko):
     - the policy checks the maximum value, 127 (MAX_TCP_SYNCNT)
     - the doc says the value works like TCP_SYNCNT
    v5:
     - new, split from the retry patch (Matthieu)

 Documentation/netlink/specs/mptcp_pm.yaml | 12 ++++++++++++
 include/uapi/linux/mptcp_pm.h             |  1 +
 net/mptcp/mptcp_pm_gen.c                  | 10 ++++++----
 net/mptcp/mptcp_pm_gen.h                  |  4 ++--
 4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/Documentation/netlink/specs/mptcp_pm.yaml b/Documentation/netlink/specs/mptcp_pm.yaml
index 39f3facc38e5..93abd7db0fd5 100644
--- a/Documentation/netlink/specs/mptcp_pm.yaml
+++ b/Documentation/netlink/specs/mptcp_pm.yaml
@@ -196,6 +196,15 @@ attribute-sets:
         name: addr-remote
         type: nest
         nested-attributes: address
+      -
+        name: subflow-retries
+        type: u32
+        checks:
+          max: 127
+        doc: |
+          SYN retries of a subflow re-created after an error, like the
+          TCP_SYNCNT socket option, so with tcp_syn_linear_timeouts on top.
+          0 to not re-create it. The maximum is MAX_TCP_SYNCNT.
   -
     name: event-attr
     enum-name: mptcp-event-attr
@@ -327,6 +336,7 @@ operations:
           attributes:
             - rcv-add-addrs
             - subflows
+            - subflow-retries
     -
       name: get-limits
       doc: Get protocol limits
@@ -337,10 +347,12 @@ operations:
           attributes:
             - rcv-add-addrs
             - subflows
+            - subflow-retries
         reply:
           attributes:
             - rcv-add-addrs
             - subflows
+            - subflow-retries
     -
       name: set-flags
       doc: Change endpoint flags
diff --git a/include/uapi/linux/mptcp_pm.h b/include/uapi/linux/mptcp_pm.h
index fe9863d75350..43e9643deda9 100644
--- a/include/uapi/linux/mptcp_pm.h
+++ b/include/uapi/linux/mptcp_pm.h
@@ -102,6 +102,7 @@ enum {
 	MPTCP_PM_ATTR_TOKEN,
 	MPTCP_PM_ATTR_LOC_ID,
 	MPTCP_PM_ATTR_ADDR_REMOTE,
+	MPTCP_PM_ATTR_SUBFLOW_RETRIES,
 
 	__MPTCP_ATTR_AFTER_LAST
 };
diff --git a/net/mptcp/mptcp_pm_gen.c b/net/mptcp/mptcp_pm_gen.c
index c180930a8e0e..e2d4af1bc1d5 100644
--- a/net/mptcp/mptcp_pm_gen.c
+++ b/net/mptcp/mptcp_pm_gen.c
@@ -44,15 +44,17 @@ const struct nla_policy mptcp_pm_flush_addrs_nl_policy[MPTCP_PM_ENDPOINT_ADDR +
 };
 
 /* MPTCP_PM_CMD_SET_LIMITS - do */
-const struct nla_policy mptcp_pm_set_limits_nl_policy[MPTCP_PM_ATTR_SUBFLOWS + 1] = {
+const struct nla_policy mptcp_pm_set_limits_nl_policy[MPTCP_PM_ATTR_SUBFLOW_RETRIES + 1] = {
 	[MPTCP_PM_ATTR_RCV_ADD_ADDRS] = { .type = NLA_U32, },
 	[MPTCP_PM_ATTR_SUBFLOWS] = { .type = NLA_U32, },
+	[MPTCP_PM_ATTR_SUBFLOW_RETRIES] = NLA_POLICY_MAX(NLA_U32, 127),
 };
 
 /* MPTCP_PM_CMD_GET_LIMITS - do */
-const struct nla_policy mptcp_pm_get_limits_nl_policy[MPTCP_PM_ATTR_SUBFLOWS + 1] = {
+const struct nla_policy mptcp_pm_get_limits_nl_policy[MPTCP_PM_ATTR_SUBFLOW_RETRIES + 1] = {
 	[MPTCP_PM_ATTR_RCV_ADD_ADDRS] = { .type = NLA_U32, },
 	[MPTCP_PM_ATTR_SUBFLOWS] = { .type = NLA_U32, },
+	[MPTCP_PM_ATTR_SUBFLOW_RETRIES] = NLA_POLICY_MAX(NLA_U32, 127),
 };
 
 /* MPTCP_PM_CMD_SET_FLAGS - do */
@@ -127,7 +129,7 @@ const struct genl_ops mptcp_pm_nl_ops[11] = {
 		.validate	= GENL_DONT_VALIDATE_STRICT,
 		.doit		= mptcp_pm_nl_set_limits_doit,
 		.policy		= mptcp_pm_set_limits_nl_policy,
-		.maxattr	= MPTCP_PM_ATTR_SUBFLOWS,
+		.maxattr	= MPTCP_PM_ATTR_SUBFLOW_RETRIES,
 		.flags		= GENL_UNS_ADMIN_PERM,
 	},
 	{
@@ -135,7 +137,7 @@ const struct genl_ops mptcp_pm_nl_ops[11] = {
 		.validate	= GENL_DONT_VALIDATE_STRICT,
 		.doit		= mptcp_pm_nl_get_limits_doit,
 		.policy		= mptcp_pm_get_limits_nl_policy,
-		.maxattr	= MPTCP_PM_ATTR_SUBFLOWS,
+		.maxattr	= MPTCP_PM_ATTR_SUBFLOW_RETRIES,
 	},
 	{
 		.cmd		= MPTCP_PM_CMD_SET_FLAGS,
diff --git a/net/mptcp/mptcp_pm_gen.h b/net/mptcp/mptcp_pm_gen.h
index b9280bcb43f5..5b4dee8a0a34 100644
--- a/net/mptcp/mptcp_pm_gen.h
+++ b/net/mptcp/mptcp_pm_gen.h
@@ -23,9 +23,9 @@ extern const struct nla_policy mptcp_pm_get_addr_nl_policy[MPTCP_PM_ATTR_TOKEN +
 
 extern const struct nla_policy mptcp_pm_flush_addrs_nl_policy[MPTCP_PM_ENDPOINT_ADDR + 1];
 
-extern const struct nla_policy mptcp_pm_set_limits_nl_policy[MPTCP_PM_ATTR_SUBFLOWS + 1];
+extern const struct nla_policy mptcp_pm_set_limits_nl_policy[MPTCP_PM_ATTR_SUBFLOW_RETRIES + 1];
 
-extern const struct nla_policy mptcp_pm_get_limits_nl_policy[MPTCP_PM_ATTR_SUBFLOWS + 1];
+extern const struct nla_policy mptcp_pm_get_limits_nl_policy[MPTCP_PM_ATTR_SUBFLOW_RETRIES + 1];
 
 extern const struct nla_policy mptcp_pm_set_flags_nl_policy[MPTCP_PM_ATTR_ADDR_REMOTE + 1];
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH mptcp-next v6 3/6] mptcp: pm: in-kernel: add the subflow-retries limit
  2026-10-18  4:17 [PATCH mptcp-next v6 0/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
  2026-10-18  4:17 ` [PATCH mptcp-next v6 1/6] selftests: mptcp: connect: retry write() on EAGAIN Anil Kaushik
  2026-10-18  4:17 ` [PATCH mptcp-next v6 2/6] netlink: specs: mptcp: add the subflow-retries attribute Anil Kaushik
@ 2026-10-18  4:17 ` Anil Kaushik
  2026-10-18  4:17 ` [PATCH mptcp-next v6 4/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Anil Kaushik @ 2026-10-18  4:17 UTC (permalink / raw)
  To: mptcp; +Cc: Anil Kaushik

A new limit for the in-kernel PM, set and read with the other limits:
the number of SYN retries of a subflow re-created after an error, like
the TCP_SYNCNT socket option. 15 by default, about 20 minutes, and 0
to not re-create it. The netlink policy checks the maximum value,
MAX_TCP_SYNCNT.

It is also exposed in struct mptcp_info, in place of the reserved field.

The next patch uses it.

Assisted-by: LLM
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---

Notes:
    v6:
     - the netlink policy checks the maximum, no more parse_limit() change
       (netdev Sashiko)
     - the limit works like TCP_SYNCNT, say so (netdev Sashiko)
    v5:
     - new, split from the retry patch (Matthieu)
     - the limit is also in struct mptcp_info (Matthieu)

 include/uapi/linux/mptcp.h |  2 +-
 net/mptcp/pm_kernel.c      | 23 ++++++++++++++++++++++-
 net/mptcp/protocol.h       |  1 +
 net/mptcp/sockopt.c        |  2 ++
 4 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index 72a5d030154e..d7e3f359773d 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -72,7 +72,7 @@ struct mptcp_info {
 	__u8	mptcpi_subflows_total;
 	__u8	mptcpi_endp_laminar_max;
 	__u8	mptcpi_endp_fullmesh_max;
-	__u8	reserved;
+	__u8	mptcpi_limit_subflow_retries;
 	__u32	mptcpi_last_data_sent;
 	__u32	mptcpi_last_data_recv;
 	__u32	mptcpi_last_ack_recv;
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index d3014bf57bf3..0379e1495cf4 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -25,6 +25,7 @@ struct pm_nl_pernet {
 	u8			endp_fullmesh_max;
 	u8			limit_add_addr_accepted;
 	u8			limit_extra_subflows;
+	u8			limit_subflow_retries;
 	u8			next_id;
 	DECLARE_BITMAP(id_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
 };
@@ -96,6 +97,14 @@ u8 mptcp_pm_get_limit_extra_subflows(const struct mptcp_sock *msk)
 }
 EXPORT_SYMBOL_GPL(mptcp_pm_get_limit_extra_subflows);
 
+u8 mptcp_pm_get_limit_subflow_retries(const struct mptcp_sock *msk)
+{
+	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
+
+	return READ_ONCE(pernet->limit_subflow_retries);
+}
+EXPORT_SYMBOL_GPL(mptcp_pm_get_limit_subflow_retries);
+
 static bool has_subflow_daddr(const struct mptcp_sock *msk,
 			      const struct mptcp_addr_info *daddr)
 {
@@ -1419,7 +1428,8 @@ static int parse_limit(struct genl_info *info, int id, unsigned int *limit)
 int mptcp_pm_nl_set_limits_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
-	unsigned int rcv_addrs, subflows;
+	unsigned int rcv_addrs, subflows, retries;
+	struct nlattr *attr;
 	int ret;
 
 	spin_lock_bh(&pernet->lock);
@@ -1433,8 +1443,13 @@ int mptcp_pm_nl_set_limits_doit(struct sk_buff *skb, struct genl_info *info)
 	if (ret)
 		goto unlock;
 
+	/* the maximum, MAX_TCP_SYNCNT, is checked by the policy */
+	attr = info->attrs[MPTCP_PM_ATTR_SUBFLOW_RETRIES];
+	retries = nla_get_u32_default(attr, pernet->limit_subflow_retries);
+
 	WRITE_ONCE(pernet->limit_add_addr_accepted, rcv_addrs);
 	WRITE_ONCE(pernet->limit_extra_subflows, subflows);
+	WRITE_ONCE(pernet->limit_subflow_retries, retries);
 
 unlock:
 	spin_unlock_bh(&pernet->lock);
@@ -1464,6 +1479,10 @@ int mptcp_pm_nl_get_limits_doit(struct sk_buff *skb, struct genl_info *info)
 			READ_ONCE(pernet->limit_extra_subflows)))
 		goto fail;
 
+	if (nla_put_u32(msg, MPTCP_PM_ATTR_SUBFLOW_RETRIES,
+			READ_ONCE(pernet->limit_subflow_retries)))
+		goto fail;
+
 	genlmsg_end(msg, reply);
 	return genlmsg_reply(msg, info);
 
@@ -1618,6 +1637,8 @@ static int __net_init pm_nl_init_net(struct net *net)
 
 	/* Cit. 2 subflows ought to be enough for anybody. */
 	pernet->limit_extra_subflows = 2;
+	/* about 20 minutes with the default RTO values */
+	pernet->limit_subflow_retries = 15;
 	pernet->next_id = 1;
 	spin_lock_init(&pernet->lock);
 
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index ed2f363bb915..6d571328aca9 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1264,6 +1264,7 @@ u8 mptcp_pm_get_endp_laminar_max(const struct mptcp_sock *msk);
 u8 mptcp_pm_get_endp_fullmesh_max(const struct mptcp_sock *msk);
 u8 mptcp_pm_get_limit_add_addr_accepted(const struct mptcp_sock *msk);
 u8 mptcp_pm_get_limit_extra_subflows(const struct mptcp_sock *msk);
+u8 mptcp_pm_get_limit_subflow_retries(const struct mptcp_sock *msk);
 
 /* called under PM lock */
 static inline void __mptcp_pm_close_subflow(struct mptcp_sock *msk)
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 922f6ae5c80c..d3c4abf59ec5 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -1075,6 +1075,8 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 			mptcp_pm_get_endp_laminar_max(msk);
 		info->mptcpi_endp_fullmesh_max =
 			mptcp_pm_get_endp_fullmesh_max(msk);
+		info->mptcpi_limit_subflow_retries =
+			mptcp_pm_get_limit_subflow_retries(msk);
 	}
 
 	if (__mptcp_check_fallback(msk))
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH mptcp-next v6 4/6] mptcp: pm: in-kernel: retry subflows closed after an error
  2026-10-18  4:17 [PATCH mptcp-next v6 0/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
                   ` (2 preceding siblings ...)
  2026-10-18  4:17 ` [PATCH mptcp-next v6 3/6] mptcp: pm: in-kernel: add the subflow-retries limit Anil Kaushik
@ 2026-10-18  4:17 ` Anil Kaushik
  2026-10-18  4:45   ` sashiko-bot
                     ` (2 more replies)
  2026-10-18  4:17 ` [PATCH mptcp-next v6 5/6] selftests: mptcp: pm_nl_ctl: support the subflow-retries limit Anil Kaushik
  2026-10-18  4:17 ` [PATCH mptcp-next v6 6/6] selftests: mptcp: join: validate subflow retry after link failure Anil Kaushik
  5 siblings, 3 replies; 15+ messages in thread
From: Anil Kaushik @ 2026-10-18  4:17 UTC (permalink / raw)
  To: mptcp; +Cc: 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.

When TCP closes an established subflow created by this side after an
error, the PM now creates a new one with the same addresses. Its SYN
retries, set like TCP_SYNCNT by the subflow-retries limit, space out the
attempts. If this new subflow cannot connect, it is not retried again,
and it is closed if the MPTCP socket is closed in the meantime. Once it
is established, it is handled like any other subflow. Re-creating the
initial subflow uses one of the additional subflows allowed by the
limits.

A subflow that never got established is not re-created: the PM keeps
trying the next endpoints, as before. Subflows reset by the peer with an
MP_RST without the transient flag (RFC 8684 section 3.6), aborted
locally, or using an address that has been removed are not retried
either.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/440
Assisted-by: LLM
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---

Notes:
    v6 (netdev Sashiko):
     - only re-create subflows that were established: the PM still tries the
       next endpoints when one never gets established
     - only once TCP closed the subflow, to read its reset flags safely
     - removed addresses, also the remote ones and the local ID 0: the dead
       subflows are marked in mptcp_pm_rm_addr_or_subflow(), instead of
       looking up the endpoint
    v5 (Matthieu):
     - split in 4 patches
     - reverse xmas tree in mptcp_pm_kernel_retry_check()
     - "in-kernel" in the subject, like the other in-kernel PM patches
     - close the re-created subflows still connecting when the socket is
       closed (Matthieu: closing the socket during a retry)
    v4 (Matthieu):
     - netlink limit, on by default, instead of the sysctl
     - one new subflow per failure, its SYN retries set from the limit: no
       socket reuse, no attempts counter, no 60s check
     - no MIB counter
     - not retried if the endpoint was removed, instead of the pm.c change
     - called through pm.c
     - new bit in the subflow context, no new hole
     - commit message: MP_RST, not any RST (Sashiko)
    v3 (Sashiko):
     - no retry after MP_RST without the T flag, local abort or EBADMSG
     - no retry on a removed endpoint or address
     - use only request_bkup for the backup flag
    v2:
     - use the subflow sockets, no list or timer
     - code moved to pm_kernel.c

 net/mptcp/pm.c        |  39 ++++++++++++++++
 net/mptcp/pm_kernel.c | 100 ++++++++++++++++++++++++++++++++++++++++++
 net/mptcp/protocol.c  |  23 ++++++++++
 net/mptcp/protocol.h  |  19 ++++++++
 4 files changed, 181 insertions(+)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index d7c5b50b34cc..54d94bc72963 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -694,6 +694,26 @@ void mptcp_pm_subflow_check_next(struct mptcp_sock *msk,
 	spin_unlock_bh(&pm->lock);
 }
 
+/* Called with the msk socket lock held, before closing a subflow: true if
+ * it has to be re-created once closed, see mptcp_pm_subflow_retry().
+ */
+bool mptcp_pm_subflow_retry_check(struct mptcp_sock *msk,
+				  const struct mptcp_subflow_context *subflow,
+				  struct mptcp_pm_retry *retry)
+{
+	if (!mptcp_pm_is_kernel(msk))
+		return false;
+
+	return mptcp_pm_kernel_retry_check(msk, subflow, retry);
+}
+
+void mptcp_pm_subflow_retry(struct mptcp_sock *msk,
+			    const struct mptcp_pm_retry *retry)
+{
+	if (mptcp_pm_is_kernel(msk))
+		mptcp_pm_kernel_retry(msk, retry);
+}
+
 void mptcp_pm_add_addr_received(const struct sock *ssk,
 				const struct mptcp_addr_info *addr)
 {
@@ -762,6 +782,24 @@ static void mptcp_pm_add_addr_send_ack(struct mptcp_sock *msk)
 	mptcp_pm_schedule_work(msk, MPTCP_PM_ADD_ADDR_SEND_ACK);
 }
 
+/* A subflow closed by TCP but not collected yet must not be re-created with
+ * a removed address, see mptcp_pm_kernel_retry_check().
+ */
+static void mptcp_pm_rm_no_retry(struct mptcp_subflow_context *subflow,
+				 enum linux_mptcp_mib_field rm_type, u8 rm_id)
+{
+	struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+	u8 id;
+
+	if (rm_type == MPTCP_MIB_RMADDR)
+		id = READ_ONCE(subflow->remote_id);
+	else
+		id = subflow_get_local_id(subflow);
+
+	if (id == rm_id && inet_sk_state_load(ssk) == TCP_CLOSE)
+		subflow->no_retry = 1;
+}
+
 static void mptcp_pm_rm_addr_or_subflow(struct mptcp_sock *msk,
 					const struct mptcp_rm_list *rm_list,
 					enum linux_mptcp_mib_field rm_type)
@@ -794,6 +832,7 @@ 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);
 
+			mptcp_pm_rm_no_retry(subflow, rm_type, rm_id);
 			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 0379e1495cf4..e88c236b62e9 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -1610,6 +1610,106 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk)
 	return true;
 }
 
+/* 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);
+	struct sock *sk = (struct sock *)msk;
+	int err;
+
+	/* Only once TCP closed it: the acquire in inet_sk_state_load() makes
+	 * sure the reset flags checked below are the ones of this close.
+	 */
+	if (inet_sk_state_load(ssk) != TCP_CLOSE)
+		return false;
+
+	err = READ_ONCE(ssk->sk_err);
+	retry->syn_retries = mptcp_pm_get_limit_subflow_retries(msk);
+	if (!retry->syn_retries || !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;
+
+	/* Only a path that worked is tried again. If a subflow never got
+	 * established, the PM moves on to the next endpoints instead, and a
+	 * re-created subflow that cannot connect is not retried again: its SYN
+	 * retransmissions already spaced out the tries.
+	 */
+	if (!READ_ONCE(subflow->fully_established))
+		return false;
+
+	/* its address has been removed, see mptcp_pm_rm_addr_or_subflow() */
+	if (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;
+
+	/* 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;
+
+	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, 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;
+	struct sock *ssk;
+	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 ||
+	    __mptcp_subflow_connect(sk, &retry->local, &retry->remote))
+		return;
+
+	/* the new subflow is the last one */
+	subflow = list_last_entry(&msk->conn_list, struct mptcp_subflow_context,
+				  node);
+	ssk = mptcp_subflow_tcp_sock(subflow);
+
+	lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
+	WRITE_ONCE(inet_csk(ssk)->icsk_syn_retries, retry->syn_retries);
+	subflow->retry = 1;
+	release_sock(ssk);
+}
+
 /* 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..354b80156bce 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,10 @@ static void __mptcp_close_subflow(struct sock *sk)
 		if (!skb_queue_empty_lockless(&ssk->sk_receive_queue))
 			continue;
 
+		do_retry = mptcp_pm_subflow_retry_check(msk, subflow, &retry);
 		mptcp_close_ssk(sk, ssk, subflow);
+		if (do_retry)
+			mptcp_pm_subflow_retry(msk, &retry);
 	}
 
 }
@@ -3494,6 +3499,22 @@ static void mptcp_check_listen_stop(struct sock *sk)
 	release_sock(ssk);
 }
 
+/* Re-created subflows still connecting are useless once the msk is closed,
+ * and their SYN retransmissions could keep them around for a long time.
+ */
+static void mptcp_close_retry_subflows(struct sock *sk)
+{
+	struct mptcp_subflow_context *subflow, *tmp;
+	struct mptcp_sock *msk = mptcp_sk(sk);
+
+	mptcp_for_each_subflow_safe(msk, subflow, tmp) {
+		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+
+		if (subflow->retry && inet_sk_state_load(ssk) == TCP_SYN_SENT)
+			mptcp_close_ssk(sk, ssk, subflow);
+	}
+}
+
 bool __mptcp_close(struct sock *sk, long timeout)
 {
 	struct mptcp_subflow_context *subflow;
@@ -3520,6 +3541,8 @@ bool __mptcp_close(struct sock *sk, long timeout)
 		__mptcp_wr_shutdown(sk);
 	}
 
+	mptcp_close_retry_subflows(sk);
+
 	sk_stream_wait_close(sk, timeout);
 
 cleanup:
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 6d571328aca9..0dc591d92fdc 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -621,6 +621,8 @@ struct mptcp_subflow_context {
 	u8	reset_seen:1;
 	u8	reset_transient:1;
 	u8	reset_reason:4;
+	u8	retry:1;	    /* re-created after an error */
+	u8	no_retry:1;	    /* closed, with a removed address */
 	u8	stale_count;	    /* Protected by the msk socket lock */
 	u8	stale;		    /* Protected by the msk socket lock,
 				     * if set the subflow is unable to snd/rcv
@@ -1137,6 +1139,18 @@ void mptcp_pm_subflow_established(struct mptcp_sock *msk);
 bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk);
 void mptcp_pm_subflow_check_next(struct mptcp_sock *msk,
 				 const struct mptcp_subflow_context *subflow);
+
+struct mptcp_pm_retry {
+	struct mptcp_pm_local	local;
+	struct mptcp_addr_info	remote;
+	u8			syn_retries;
+};
+
+bool mptcp_pm_subflow_retry_check(struct mptcp_sock *msk,
+				  const struct mptcp_subflow_context *subflow,
+				  struct mptcp_pm_retry *retry);
+void mptcp_pm_subflow_retry(struct mptcp_sock *msk,
+			    const struct mptcp_pm_retry *retry);
 void mptcp_pm_add_addr_received(const struct sock *ssk,
 				const struct mptcp_addr_info *addr);
 void mptcp_pm_add_addr_echoed(struct mptcp_sock *msk,
@@ -1258,6 +1272,11 @@ 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);
+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);
 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);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH mptcp-next v6 5/6] selftests: mptcp: pm_nl_ctl: support the subflow-retries limit
  2026-10-18  4:17 [PATCH mptcp-next v6 0/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
                   ` (3 preceding siblings ...)
  2026-10-18  4:17 ` [PATCH mptcp-next v6 4/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
@ 2026-10-18  4:17 ` Anil Kaushik
  2026-10-18  4:17 ` [PATCH mptcp-next v6 6/6] selftests: mptcp: join: validate subflow retry after link failure Anil Kaushik
  5 siblings, 0 replies; 15+ messages in thread
From: Anil Kaushik @ 2026-10-18  4:17 UTC (permalink / raw)
  To: mptcp; +Cc: Anil Kaushik

'pm_nl_ctl limits' can now set the subflow-retries limit with a third
value, and shows it when the kernel supports it.

pm_netlink.sh checks its default value, that a value above the maximum
is refused, and that it can be set. This is only done with pm_nl_ctl
for the moment: iproute2 does not support this limit yet.

Assisted-by: LLM
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---

Notes:
    v5:
     - new (Matthieu)

 .../testing/selftests/net/mptcp/pm_netlink.sh | 42 ++++++++++++++++++-
 tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 34 +++++++++++----
 2 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
index 21bfe1311f11..ddf2899e4830 100755
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -46,15 +46,20 @@ trap cleanup EXIT
 
 mptcp_lib_ns_init ns1
 
+# $3: subflow retries, only shown by pm_nl_ctl, if supported
 format_limits() {
 	local accept="${1}"
 	local subflows="${2}"
+	local retries="${3}"
 
 	if mptcp_lib_is_ip_mptcp; then
 		# with a space at the end
 		printf "add_addr_accepted %d subflows %d \n" "${accept}" "${subflows}"
 	else
 		printf "accept %d\nsubflows %d\n" "${accept}" "${subflows}"
+		if [ -n "${retries}" ]; then
+			printf "subflow_retries %d\n" "${retries}"
+		fi
 	fi
 }
 
@@ -75,6 +80,22 @@ get_limits_nb() {
 	fi
 }
 
+# The subflow retries limit is only set and shown with pm_nl_ctl for now.
+# Prints its default value if it is supported.
+get_default_retries() {
+	if mptcp_lib_is_ip_mptcp; then
+		return
+	fi
+
+	if ip netns exec "${ns1}" ./pm_nl_ctl limits |
+	   grep -q "^subflow_retries"; then
+		echo 15
+	elif mptcp_lib_expect_all_features; then
+		# missing feature: the defaults limits check will fail
+		echo 15
+	fi
+}
+
 format_endpoints() {
 	mptcp_lib_pm_nl_format_endpoints "${@}"
 }
@@ -154,8 +175,10 @@ mptcp_lib_subtests_last_ts_reset
 check "show_endpoints" "" "defaults addr list"
 
 default_limits="$(get_limits)"
+default_retries="$(get_default_retries)"
 if mptcp_lib_expect_all_features; then
-	check "get_limits" "$(format_limits 0 2)" "defaults limits"
+	check "get_limits" "$(format_limits 0 2 "${default_retries}")" \
+		"defaults limits"
 fi
 
 add_endpoint 10.0.1.1
@@ -224,7 +247,22 @@ check "get_limits" "${default_limits}" "subflows above hard limit"
 
 set_limits ${max_limits} ${max_limits}
 flush_endpoint  ## to make sure it doesn't affect the limits
-check "get_limits" "$(format_limits ${max_limits} ${max_limits})" "set limits"
+max_limits_out="$(format_limits ${max_limits} ${max_limits} \
+		 "${default_retries}")"
+check "get_limits" "${max_limits_out}" "set limits"
+
+if [ -n "${default_retries}" ]; then
+	ip netns exec "${ns1}" ./pm_nl_ctl limits ${max_limits} ${max_limits} \
+		128 2>/dev/null
+	check "get_limits" "${max_limits_out}" \
+		"subflow retries above hard limit"
+
+	ip netns exec "${ns1}" ./pm_nl_ctl limits ${max_limits} ${max_limits} 0
+	check "get_limits" "$(format_limits ${max_limits} ${max_limits} 0)" \
+		"set subflow retries"
+	ip netns exec "${ns1}" ./pm_nl_ctl limits ${max_limits} ${max_limits} \
+		"${default_retries}"
+fi
 
 flush_endpoint
 add_endpoint 10.0.1.1
diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
index 78180da1efcc..41467b997018 100644
--- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
+++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
@@ -39,7 +39,7 @@ static void syntax(char *argv[])
 	fprintf(stderr, "\tset [<ip>] [id <nr>] flags [no]backup|[no]fullmesh [port <nr>] [token <token>] [rip <ip>] [rport <port>]\n");
 	fprintf(stderr, "\tflush\n");
 	fprintf(stderr, "\tdump\n");
-	fprintf(stderr, "\tlimits [<rcv addr max> <subflow max>]\n");
+	fprintf(stderr, "\tlimits [<rcv addr max> <subflow max> [<retry>]]\n");
 	fprintf(stderr, "\tevents\n");
 	fprintf(stderr, "\tlisten <local-ip> <local-port>\n");
 	exit(0);
@@ -1217,6 +1217,7 @@ int flush_addrs(int fd, int pm_family, int argc, char *argv[])
 static void print_limits(struct nlmsghdr *nh, int pm_family, int total_len)
 {
 	struct rtattr *attrs;
+	const char *name;
 	uint32_t max;
 
 	for (; NLMSG_OK(nh, total_len); nh = NLMSG_NEXT(nh, total_len)) {
@@ -1235,13 +1236,22 @@ static void print_limits(struct nlmsghdr *nh, int pm_family, int total_len)
 		while (RTA_OK(attrs, len)) {
 			int type = attrs->rta_type;
 
-			if (type != MPTCP_PM_ATTR_RCV_ADD_ADDRS &&
-			    type != MPTCP_PM_ATTR_SUBFLOWS)
+			switch (type) {
+			case MPTCP_PM_ATTR_RCV_ADD_ADDRS:
+				name = "accept";
+				break;
+			case MPTCP_PM_ATTR_SUBFLOWS:
+				name = "subflows";
+				break;
+			case MPTCP_PM_ATTR_SUBFLOW_RETRIES:
+				name = "subflow_retries";
+				break;
+			default:
 				goto next;
+			}
 
 			memcpy(&max, RTA_DATA(attrs), 4);
-			printf("%s %u\n", type == MPTCP_PM_ATTR_SUBFLOWS ?
-					  "subflows" : "accept", max);
+			printf("%s %u\n", name, max);
 
 next:
 			attrs = RTA_NEXT(attrs, len);
@@ -1254,15 +1264,17 @@ int get_set_limits(int fd, int pm_family, int argc, char *argv[])
 	char data[NLMSG_ALIGN(sizeof(struct nlmsghdr)) +
 		  NLMSG_ALIGN(sizeof(struct genlmsghdr)) +
 		  1024];
-	uint32_t rcv_addr = 0, subflows = 0;
+	uint32_t rcv_addr = 0, subflows = 0, retries = 0;
 	int cmd, len = sizeof(data);
 	struct nlmsghdr *nh;
 	int off = 0;
 
 	/* limit */
-	if (argc == 4) {
+	if (argc == 4 || argc == 5) {
 		rcv_addr = atoi(argv[2]);
 		subflows = atoi(argv[3]);
+		if (argc == 5)
+			retries = atoi(argv[4]);
 		cmd = MPTCP_PM_CMD_SET_LIMITS;
 	} else {
 		cmd = MPTCP_PM_CMD_GET_LIMITS;
@@ -1287,6 +1299,14 @@ int get_set_limits(int fd, int pm_family, int argc, char *argv[])
 		memcpy(RTA_DATA(rta), &subflows, 4);
 		off += NLMSG_ALIGN(rta->rta_len);
 
+		if (argc == 5) {
+			rta = (void *)(data + off);
+			rta->rta_type = MPTCP_PM_ATTR_SUBFLOW_RETRIES;
+			rta->rta_len = RTA_LENGTH(4);
+			memcpy(RTA_DATA(rta), &retries, 4);
+			off += NLMSG_ALIGN(rta->rta_len);
+		}
+
 		/* do not expect a reply */
 		len = 0;
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH mptcp-next v6 6/6] selftests: mptcp: join: validate subflow retry after link failure
  2026-10-18  4:17 [PATCH mptcp-next v6 0/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
                   ` (4 preceding siblings ...)
  2026-10-18  4:17 ` [PATCH mptcp-next v6 5/6] selftests: mptcp: pm_nl_ctl: support the subflow-retries limit Anil Kaushik
@ 2026-10-18  4:17 ` Anil Kaushik
  2026-10-19 17:46   ` netdev-bot+sashiko
  5 siblings, 1 reply; 15+ messages in thread
From: Anil Kaushik @ 2026-10-18  4:17 UTC (permalink / raw)
  To: mptcp; +Cc: Anil Kaushik

Test the subflow-retries limit, with its default value, and 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 re-created. Let them through again
once the SYN of the new subflow has been retransmitted more times than
tcp_syn_retries allows, and check the subflow comes back from the same
address. Then do the same with the initial subflow.

Finally, close the connection while a subflow is being re-created, and
check the netlink events. The big files used for the long connection
are emptied at the end.

The test is skipped if pm_nl_ctl does not show the new limit, or if the
filter rules cannot be set.

Assisted-by: LLM
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---

Notes:
    v6 (Sashiko):
     - use the slowwait helpers from net/lib.sh
     - longer first wait, while the 128 MB files are created
     - skip if the filter rules cannot be set, check each drop rule
       (netdev Sashiko)
     - empty the big files at the end, cinsent too, keep the variables
       (netdev Sashiko)
    v5 (Matthieu):
     - skipped if pm_nl_ctl does not show the new limit
     - wait for the SYN retransmissions instead of a sleep, with
       tcp_syn_linear_timeouts=0
     - helper for the check after the close
     - slower links with short queues, so the transfer lasts the whole
       test on any machine, longer timeout (killed at the end)
    v4:
     - default limit, no sysctl or MIB counter
     - more SYN retransmissions than tcp_syn_retries
     - close the connection while a subflow is re-created
     - check the netlink events
     - check the waits and the new subflow port, remove the big files at
       the end (Sashiko)
    v3:
     - check the subflow address, create the files before the background job
    v2:
     - one test, with netfilter

 .../testing/selftests/net/mptcp/mptcp_join.sh | 164 ++++++++++++++++++
 1 file changed, 164 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index f4d8afaeda4f..d4b4146cbd64 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -2151,6 +2151,111 @@ wait_attempt_fail()
 	return 1
 }
 
+# $1: timeout in seconds ; $2: ns ; $3: counter ; $4: increment, 1 by default
+wait_counter_inc()
+{
+	slowwait_for_counter "${1}" "${4:-1}" \
+		mptcp_lib_get_counter "${2}" "${3}" >/dev/null
+}
+
+# $1: client address ; prints the local address and port of its subflow
+sf_from()
+{
+	ip netns exec $ns2 ss -tnH state established src "${1}" |
+		awk '{ print $3; exit }'
+}
+
+has_subflow_retries()
+{
+	ip netns exec $ns2 ./pm_nl_ctl limits | grep -q "^subflow_retries"
+}
+
+# the client packets are dropped with rules in this chain
+init_nft_filter()
+{
+	init_nftables
+	if ! ip netns exec $ns2 nft list chain ip filter OUTPUT \
+	     &>/dev/null; then
+		mark_as_skipped "unable to set the filter rules"
+		return 1
+	fi
+}
+
+# $1: client address
+nft_drop_from()
+{
+	ip netns exec $ns2 nft add rule ip filter OUTPUT ip saddr "${1}" drop
+}
+
+# $1: client address to block
+#
+# The link stays up, but the client packets from this address are dropped:
+# the subflow using it is closed after a few retransmissions, and re-created.
+# The packets go through again once the SYN has been retransmitted more times
+# than tcp_syn_retries allows: the new subflow still connects thanks to the
+# subflow-retries limit.
+sf_retry_after_block()
+{
+	local before after
+
+	before=$(sf_from "${1}")
+
+	print_check "subflow from ${1}"
+	if [ -z "${before}" ]; then
+		fail_test "no established subflow from ${1} before the block"
+		return
+	fi
+
+	if ! nft_drop_from "${1}"; then
+		fail_test "unable to drop the packets from ${1}"
+		return
+	fi
+	if ! wait_counter_inc "${timeout_poll}" $ns2 "MPTcpExtMPJoinSynTx"; then
+		ip netns exec $ns2 nft flush chain ip filter OUTPUT
+		fail_test "subflow from ${1} not re-created"
+		return
+	fi
+	# tcp_syn_retries=1: one more retransmission than allowed
+	if ! wait_counter_inc "${timeout_poll}" \
+		$ns2 "TcpExtTCPSynRetrans" 2; then
+		ip netns exec $ns2 nft flush chain ip filter OUTPUT
+		fail_test "SYN from ${1} not retransmitted enough"
+		return
+	fi
+	ip netns exec $ns2 nft flush chain ip filter OUTPUT
+
+	if ! wait_counter_inc "${timeout_poll}" $ns1 "MPTcpExtMPJoinAckRx"; then
+		fail_test "re-created subflow from ${1} not established"
+		return
+	fi
+
+	after=$(sf_from "${1}")
+	if [ -z "${after}" ] || [ "${after}" = "${before}" ]; then
+		fail_test "no new subflow from ${1}: ${before} -> ${after}"
+	else
+		print_ok
+	fi
+}
+
+no_sf_connecting()
+{
+	! ip netns exec $ns2 ss -tnH state syn-sent | grep -q .
+}
+
+# $1: result of the wait for the subflow being re-created before the close
+chk_no_sf_connecting()
+{
+	print_check "no subflow connecting after close"
+	if [ "${1}" -ne 0 ]; then
+		fail_test "no subflow re-created before the close"
+	# closed with the MPTCP socket
+	elif slowwait "${timeout_poll}" no_sf_connecting; then
+		print_ok
+	else
+		fail_test "subflow still connecting after close"
+	fi
+}
+
 set_userspace_pm()
 {
 	local ns=$1
@@ -2585,6 +2690,65 @@ 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. Then close the connection
+	# while a subflow is being re-created.
+	if reset_with_events "subflow retry after link failure" &&
+	   continue_if has_subflow_retries && init_nft_filter; then
+		# slow links, short queues: the transfer lasts the whole test on
+		# any machine, and the blocked subflow always has data in flight
+		local i
+		for i in 1 3; do
+			tc -n $ns1 qdisc add dev ns1eth$i root netem \
+				rate 4mbit limit 30 delay 1ms
+			tc -n $ns2 qdisc add dev ns2eth$i root netem \
+				rate 4mbit limit 30 delay 1ms
+		done
+		ip netns exec $ns2 sysctl -q net.ipv4.tcp_retries2=3
+		ip netns exec $ns2 sysctl -q net.ipv4.tcp_syn_retries=1
+		ip netns exec $ns2 sysctl -q net.ipv4.tcp_syn_linear_timeouts=0
+		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
+		# killed at the end: the timeout only stops a stuck test
+		{ timeout_test=600 test_linkfail=131072 \
+			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
+		local tests_pid=$!
+
+		# the 128 MB files are created first, that can take a while
+		wait_counter_inc $((timeout_poll * 4)) \
+			$ns1 "MPTcpExtMPJoinAckRx"
+		sf_retry_after_block 10.0.3.2
+		sf_retry_after_block 10.0.1.2
+
+		local retrying=1
+		if nft_drop_from 10.0.3.2; then
+			wait_counter_inc "${timeout_poll}" \
+				$ns2 "MPTcpExtMPJoinSynTx"
+			retrying=$?
+		fi
+		mptcp_lib_kill_group_wait $tests_pid
+		chk_no_sf_connecting ${retrying}
+		ip netns exec $ns2 nft flush chain ip filter OUTPUT
+		kill_events_pids
+
+		join_syn_tx=4 chk_join_nr 3 3 3
+		chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 3
+		chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_CLOSED 3 error 3
+
+		# big files: empty them, the next tests write their own content
+		: > "$cinfail"
+		: > "$sinfail"
+		: > "$cinsent"
+	fi
 }
 
 add_addr_timeout_tests()
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH mptcp-next v6 4/6] mptcp: pm: in-kernel: retry subflows closed after an error
  2026-10-18  4:17 ` [PATCH mptcp-next v6 4/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
@ 2026-10-18  4:45   ` sashiko-bot
  2026-10-19 17:26   ` Abdul Wasey
  2026-10-19 17:46   ` netdev-bot+sashiko
  2 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-10-18  4:45 UTC (permalink / raw)
  To: Anil Kaushik, mptcp

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] mptcp: pm: TOCTOU race on sk_state in mptcp_pm_rm_addr_or_subflow()
- [Medium] mptcp: pm: in-kernel: missing subflow->resetting check in mptcp_pm_kernel_retry_check()
--

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

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

This commit teaches the in-kernel path manager to re-create established
subflows initiated by this peer when TCP closes them after an error, while
avoiding retries when the peer sent an non-transient MP_RST, when the
subflow was aborted locally, or when its address was removed.

> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index d7c5b50b34cc9..54d94bc729630 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c

[ ... ]

> @@ -762,6 +782,24 @@ static void mptcp_pm_add_addr_send_ack(struct mptcp_sock *msk)
>  	mptcp_pm_schedule_work(msk, MPTCP_PM_ADD_ADDR_SEND_ACK);
>  }
>  
> +/* A subflow closed by TCP but not collected yet must not be re-created with
> + * a removed address, see mptcp_pm_kernel_retry_check().
> + */
> +static void mptcp_pm_rm_no_retry(struct mptcp_subflow_context *subflow,
> +				 enum linux_mptcp_mib_field rm_type, u8 rm_id)
> +{
> +	struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> +	u8 id;
> +
> +	if (rm_type == MPTCP_MIB_RMADDR)
> +		id = READ_ONCE(subflow->remote_id);
> +	else
> +		id = subflow_get_local_id(subflow);
> +
> +	if (id == rm_id && inet_sk_state_load(ssk) == TCP_CLOSE)
> +		subflow->no_retry = 1;
> +}
> +
>  static void mptcp_pm_rm_addr_or_subflow(struct mptcp_sock *msk,
>  					const struct mptcp_rm_list *rm_list,
>  					enum linux_mptcp_mib_field rm_type)
> @@ -794,6 +832,7 @@ 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);
>  
> +			mptcp_pm_rm_no_retry(subflow, rm_type, rm_id);
>  			if ((1 << inet_sk_state_load(ssk)) &
>  			    (TCPF_FIN_WAIT1 | TCPF_FIN_WAIT2 | TCPF_CLOSING | TCPF_CLOSE))
>  				continue;

[Severity: Medium]
Can a subflow whose address is being removed still be retried if its
subflow socket transitions to TCP_CLOSE concurrently?

mptcp_pm_rm_addr_or_subflow() runs under the msk socket lock and
msk->pm.lock without holding the subflow socket lock (ssk), and reads
ssk->sk_state twice without synchronization.

If ssk is not yet in TCP_CLOSE when mptcp_pm_rm_no_retry() checks
inet_sk_state_load(ssk) == TCP_CLOSE, subflow->no_retry stays 0. If a
concurrent TCP timer or softirq (e.g., tcp_write_err() or tcp_reset()
under bh_lock_sock(ssk)) moves ssk to TCP_CLOSE before the second
inet_sk_state_load(ssk) in mptcp_pm_rm_addr_or_subflow(), or if ssk was
in TCP_FIN_WAIT1, TCP_FIN_WAIT2, or TCP_CLOSING and later closes with an
error, the loop takes the continue branch without calling
mptcp_close_ssk().

When __mptcp_close_subflow() later processes ssk, both
subflow->close_event_done and subflow->no_retry are 0, so
mptcp_pm_kernel_retry_check() will re-create the subflow using the
removed address. Would setting subflow->no_retry = 1 whenever id == rm_id
(regardless of whether ssk has already reached TCP_CLOSE) avoid this race?

> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index 0379e1495cf4d..e88c236b62e9f 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -1610,6 +1610,106 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk)
>  	return true;
>  }
>  
> +/* 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)
> +{

[ ... ]

> +	/* 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: Medium]
Should this check also reject subflows where subflow->resetting is set?

When a subflow is locally aborted due to an MPTCP protocol error via
mptcp_subflow_reset() (for example from mptcp_mp_fail_no_response(),
mptcp_update_infinite_map(), or mptcp_dss_corruption()), it sets
subflow->resetting = 1 and calls tcp_done(ssk) without setting
ssk->sk_err:

net/mptcp/subflow.c:mptcp_subflow_reset() {
	...
	subflow->resetting = 1;

	/* No need to delay the actual close for to-be discarded data. */
	__skb_queue_purge(&ssk->sk_receive_queue);
	mptcp_send_active_reset_reason(ssk);
	tcp_done(ssk);
	...
}

If IP_RECVERR or IPV6_RECVERR was enabled on the socket, an earlier
non-fatal ICMP error on the established subflow leaves a non-zero
ssk->sk_err (such as EHOSTUNREACH or EPROTO) untouched because
__mptcp_subflow_error_report() skips sock_error(ssk) on established
non-fallback sockets:

net/mptcp/protocol.c:__mptcp_subflow_error_report() {
	...
	if (sk->sk_state != TCP_SYN_SENT && !__mptcp_check_fallback(mptcp_sk(sk)))
		goto out;

	err = sock_error(ssk);
	...
}

If mptcp_subflow_reset() later closes that subflow,
mptcp_pm_kernel_retry_check() will see the stale ssk->sk_err (which is
neither ECONNABORTED nor EBADMSG) and re-create a subflow that was reset
locally for an MPTCP protocol error.

[ ... ]

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH mptcp-next v6 4/6] mptcp: pm: in-kernel: retry subflows closed after an error
  2026-10-18  4:17 ` [PATCH mptcp-next v6 4/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
  2026-10-18  4:45   ` sashiko-bot
@ 2026-10-19 17:26   ` Abdul Wasey
  2026-10-19 17:56     ` Anil Kaushik
  2026-10-19 17:46   ` netdev-bot+sashiko
  2 siblings, 1 reply; 15+ messages in thread
From: Abdul Wasey @ 2026-10-19 17:26 UTC (permalink / raw)
  To: Anil Kaushik; +Cc: mptcp, Matthieu Baerts

Hi Anil,

I tested v6 on top of export (8bb146c4c) in netns: two 10 Mbit paths
with 15 ms delay, the second one blackholed with netem loss 100% in both
directions during an iperf3 transfer, tcp_retries2=3.

Upload (the client sends) works: without the series the path never
comes back, with it the path carries data again 4.5 s after it returns
for a 20 s outage, and 12.6 s for a 60 s one, the extra time being the
SYN backoff.

Download (the server sends) is not recovered. The server's subflow times
out and is freed without sending anything, while the client's subflow
on that path only receives: it stays established and idle, never gets
an error, so it is never retried. With keepalive enabled on the client
socket (idle 5 s, interval 2 s, count 3), the idle subflow times out and
your patch re-creates it: the path is back 3 s after it returns for a
20 s outage.

So for a client mostly receiving data, which is probably the common
case, the retry only helps if the application sets keepalive. Would it
make sense for the PM to probe idle subflows it initiated, or would you
rather document the keepalive requirement? I'd be happy to work on the
download case if you have a preferred direction.

With the default tcp_retries2=15, the subflows are not closed during 20
to 180 s outages, and the old ones resume by themselves 17 to 24 s after
the path returns, with or without the series.

Thanks,
Abdul Wasey

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH mptcp-next v6 2/6] netlink: specs: mptcp: add the subflow-retries attribute
  2026-10-18  4:17 ` [PATCH mptcp-next v6 2/6] netlink: specs: mptcp: add the subflow-retries attribute Anil Kaushik
@ 2026-10-19 17:46   ` netdev-bot+sashiko
  0 siblings, 0 replies; 15+ messages in thread
From: netdev-bot+sashiko @ 2026-10-19 17:46 UTC (permalink / raw)
  To: anilkaushikwireless; +Cc: mptcp, kuba

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The concern: commit "netlink: specs: mptcp: add the subflow-
  retries attribute" adds MPTCP_PM_ATTR_SUBFLOW_RETRIES to the uAPI…

--

netlink: specs: mptcp: add the subflow-retries attribute

This commit adds the subflow-retries attribute to the MPTCP PM netlink
spec for the set-limits and get-limits commands. ynl-regen regenerates the
uapi header and the generated policy files. The new policy limits the value
to 127 (MAX_TCP_SYNCNT).

> diff --git a/net/mptcp/mptcp_pm_gen.c b/net/mptcp/mptcp_pm_gen.c
> index c180930a8e0e4..e2d4af1bc1d5d 100644
> --- a/net/mptcp/mptcp_pm_gen.c
> +++ b/net/mptcp/mptcp_pm_gen.c
> @@ -44,15 +44,17 @@ const struct nla_policy mptcp_pm_flush_addrs_nl_policy[MPTCP_PM_ENDPOINT_ADDR +
>  };
>  
>  /* MPTCP_PM_CMD_SET_LIMITS - do */
> -const struct nla_policy mptcp_pm_set_limits_nl_policy[MPTCP_PM_ATTR_SUBFLOWS + 1] = {
> +const struct nla_policy mptcp_pm_set_limits_nl_policy[MPTCP_PM_ATTR_SUBFLOW_RETRIES + 1] = {
>  	[MPTCP_PM_ATTR_RCV_ADD_ADDRS] = { .type = NLA_U32, },
>  	[MPTCP_PM_ATTR_SUBFLOWS] = { .type = NLA_U32, },
> +	[MPTCP_PM_ATTR_SUBFLOW_RETRIES] = NLA_POLICY_MAX(NLA_U32, 127),
>  };

[Severity: Low]
With only this commit applied, does the kernel accept
MPTCP_PM_ATTR_SUBFLOW_RETRIES and then silently drop it?

The policy checks the value (max 127). But mptcp_pm_nl_set_limits_doit()
in net/mptcp/pm_kernel.c still parses only MPTCP_PM_ATTR_RCV_ADD_ADDRS and
MPTCP_PM_ATTR_SUBFLOWS. A set-limits request that carries the new attribute
returns success and the value is not stored.

The same applies to mptcp_pm_nl_get_limits_doit(): it does not put the
attribute in its reply, although the spec lists it in the get-limits reply.

The next commit in this series, "mptcp: pm: in-kernel: add the
subflow-retries limit", fixes this. In that commit, set-limits does:

	attr = info->attrs[MPTCP_PM_ATTR_SUBFLOW_RETRIES];
	retries = nla_get_u32_default(attr, pernet->limit_subflow_retries);

and get-limits does:

	if (nla_put_u32(msg, MPTCP_PM_ATTR_SUBFLOW_RETRIES,
			READ_ONCE(pernet->limit_subflow_retries)))

So this only affects a kernel built at exactly this commit, for example
while bisecting. Would it be better to fold the spec change into that
commit? Or is the usual netdev split, with the ynl-regen patch landing
before its user, acceptable here?

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH mptcp-next v6 4/6] mptcp: pm: in-kernel: retry subflows closed after an error
  2026-10-18  4:17 ` [PATCH mptcp-next v6 4/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
  2026-10-18  4:45   ` sashiko-bot
  2026-10-19 17:26   ` Abdul Wasey
@ 2026-10-19 17:46   ` netdev-bot+sashiko
  2 siblings, 0 replies; 15+ messages in thread
From: netdev-bot+sashiko @ 2026-10-19 17:46 UTC (permalink / raw)
  To: anilkaushikwireless; +Cc: mptcp, kuba

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

Critical: 0 · High: 0 · Medium: 4 · Low: 0

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] A remote peer can keep the in-kernel PM in an endless subflow
  re-creation loop, with no backoff.
- [Medium] mptcp_pm_rm_addr_or_subflow() (net/mptcp/pm.c) now loads the
  subflow TCP state twice without the ssk lock.
- [Medium] The new `no_retry:1` and `retry:1` bitfields share a byte with
  `reset_seen:1`, `reset_transient:1` and `reset_reason:4` in struct…
- [Medium] mptcp: pm: in-kernel: missing subflow->resetting check in
  mptcp_pm_kernel_retry_check()

--

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

With the in-kernel PM, this patch re-creates a subflow initiated by this
side when TCP closes it after an error, as long as it had been fully
established. The SYN retries of the new subflow come from the
subflow-retries limit, and re-created subflows still connecting are closed
when the MPTCP socket is closed. Subflows reset with a non-transient MP_RST,
aborted locally, or using a removed address are not retried.

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

> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index d7c5b50b34cc9..54d94bc729630 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c

[ ... ]

> @@ -762,6 +782,24 @@ static void mptcp_pm_add_addr_send_ack(struct mptcp_sock *msk)
>  	mptcp_pm_schedule_work(msk, MPTCP_PM_ADD_ADDR_SEND_ACK);
>  }
>  
> +/* A subflow closed by TCP but not collected yet must not be re-created with
> + * a removed address, see mptcp_pm_kernel_retry_check().
> + */
> +static void mptcp_pm_rm_no_retry(struct mptcp_subflow_context *subflow,
> +				 enum linux_mptcp_mib_field rm_type, u8 rm_id)
> +{
> +	struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> +	u8 id;
> +
> +	if (rm_type == MPTCP_MIB_RMADDR)
> +		id = READ_ONCE(subflow->remote_id);
> +	else
> +		id = subflow_get_local_id(subflow);
> +
> +	if (id == rm_id && inet_sk_state_load(ssk) == TCP_CLOSE)
> +		subflow->no_retry = 1;

[Severity: Medium]
Is this plain bitfield write safe without the ssk lock?

In struct mptcp_subflow_context, the new retry and no_retry bits share a
byte with the reset_* bits:

	u8	reset_seen:1;
	u8	reset_transient:1;
	u8	reset_reason:4;
	u8	retry:1;	    /* re-created after an error */
	u8	no_retry:1;	    /* closed, with a removed address */

mptcp_pm_rm_no_retry() does a read-modify-write of that byte while holding
only the msk socket lock and pm.lock. Before this patch, every writer of
this byte ran under the ssk lock.

Seeing TCP_CLOSE doesn't seem to mean that the ssk side has stopped writing.
tcp_done() publishes TCP_CLOSE first and then calls sk->sk_state_change():

tcp_done()
  tcp_set_state(sk, TCP_CLOSE)
  sk->sk_state_change(sk)
    subflow_state_change()
      mptcp_subflow_data_available()
        subflow_check_data_avail()

When data is still queued and the mapping is bad, subflow_check_data_avail()
writes to the same byte in softirq, holding only the ssk lock:

	subflow->reset_transient = 0;
	MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_DSSRESET);
	subflow->reset_reason = status == MAPPING_NODSS ?
	...

mptcp_send_active_reset_reason() later reads these values back to build the
MP_RST.

Suppose this races with an RM_ADDR or RM_SUBFLOW. Can the no_retry update
be lost, so the subflow is retried on the removed address? Or can
reset_transient and reset_reason be overwritten with stale values, so the
MP_RST sent to the peer carries the wrong flag or reason?

Would it help to move retry and no_retry next to close_event_done, which
the msk socket lock protects, or to take the ssk lock here?

> +}
> +
>  static void mptcp_pm_rm_addr_or_subflow(struct mptcp_sock *msk,
>  					const struct mptcp_rm_list *rm_list,
>  					enum linux_mptcp_mib_field rm_type)
> @@ -794,6 +832,7 @@ 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);
>  
> +			mptcp_pm_rm_no_retry(subflow, rm_type, rm_id);
>  			if ((1 << inet_sk_state_load(ssk)) &
>  			    (TCPF_FIN_WAIT1 | TCPF_FIN_WAIT2 | TCPF_CLOSING | TCPF_CLOSE))
>  				continue;

[Severity: Medium]
mptcp_pm_rm_addr_or_subflow() now loads the subflow state twice without
the ssk lock: once inside mptcp_pm_rm_no_retry(), and again in the check
above.

What happens if a matching subflow moves from ESTABLISHED to TCP_CLOSE in
softirq between those two loads? For example, tcp_reset() or
tcp_write_err() can end in tcp_done().

In that case mptcp_pm_rm_no_retry() sees a state that is not closed and
leaves no_retry at 0. The second load then sees TCP_CLOSE and takes the
continue, so mptcp_subflow_shutdown() and mptcp_close_ssk() are skipped and
close_event_done stays 0.

On the next worker run, __mptcp_close_subflow() calls
mptcp_pm_kernel_retry_check(). It sees TCP_CLOSE, sk_err set,
fully_established set, and close_event_done and no_retry both 0. Wouldn't
mptcp_pm_kernel_retry() then re-create the subflow on the address that was
just removed?

The commit message says:

  "... or using an address that has been removed are not retried either."

The window is small, but nothing seems to prevent it. Later patches in the
series only touch selftests, so it doesn't look like this is handled later.
Could the state be loaded only once, or could every matching subflow be
marked whatever its state?

> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index 0379e1495cf4d..e88c236b62e9f 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -1610,6 +1610,106 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk)

[ ... ]

> +	/* Only a path that worked is tried again. If a subflow never got
> +	 * established, the PM moves on to the next endpoints instead, and a
> +	 * re-created subflow that cannot connect is not retried again: its SYN
> +	 * retransmissions already spaced out the tries.
> +	 */
> +	if (!READ_ONCE(subflow->fully_established))
> +		return false;
> +
> +	/* its address has been removed, see mptcp_pm_rm_addr_or_subflow() */
> +	if (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;

[Severity: Medium]
Does this catch the local resets done through mptcp_subflow_reset()?

The comment above says that subflows aborted locally or hitting an
MPTCP protocol error are not retried, but the check only relies on
sk_err. mptcp_subflow_reset() is used by mptcp_mp_fail_no_response(),
mptcp_update_infinite_map() and mptcp_dss_corruption(). It sets
subflow->resetting, sends the MP_RST and calls tcp_done(ssk), but it
never writes ssk->sk_err.

So err is whatever was left on the subflow before. Can it be non-zero
at that point? If IP_RECVERR or IPV6_RECVERR is set on the MPTCP
socket, mptcp_sockopt_sync_locked() propagates it to the subflows.
tcp_v4_err() or tcp_v6_err() can then store an ICMP error such as
EHOSTUNREACH in ssk->sk_err while the subflow is established.
__mptcp_subflow_error_report() returns before sock_error(ssk) when the
subflow is not in SYN_SENT and the msk did not fall back, so that error
is not cleared.

Suppose an MP_FAIL timeout or a DSS corruption then resets that
subflow. mptcp_pm_kernel_retry_check() finds TCP_CLOSE, an err that is
neither ECONNABORTED nor EBADMSG, fully_established set and reset_seen
clear, and asks for a retry.

In the MP_FAIL cases, msk->allow_subflows is already false, so the new
subflow sends a SYN with MP_JOIN, only for mptcp_finish_join() to
reject it if the peer answers. With mptcp_dss_corruption(), the path
that just hit a protocol error is used again, which the comment says
should not happen.

Would checking subflow->resetting here, and maybe
READ_ONCE(msk->allow_subflows), be enough? Or should
mptcp_subflow_reset() set an error such as ECONNABORTED on the ssk, so
that the existing check catches it?

[ ... ]

> +/* 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;
> +	struct sock *ssk;
> +	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 ||
> +	    __mptcp_subflow_connect(sk, &retry->local, &retry->remote))
> +		return;

[Severity: Medium]
Can a remote peer keep the in-kernel PM in an endless subflow re-creation
loop here?

In mptcp_pm_kernel_retry_check(), the only "path worked" check is
subflow->fully_established, and the peer controls it.
check_fully_established() sets it as soon as an in-sequence packet carries
a DSS ack, which is the 4th ACK of the MP_JOIN:

	if (subflow->remote_key_valid &&
	    (((mp_opt->suboptions & OPTION_MPTCP_DSS) && mp_opt->use_ack) ||
	    ...
		goto set_fully_established;

The reset check after it lets two cases through: a plain TCP RST
(sk_err == ECONNRESET, reset_seen == 0) and an MP_RST with the T flag.
Nothing checks subflow->retry, the number of earlier attempts, or the time
since the last one.

mptcp_pm_kernel_retry() then connects immediately, and its only limit is
limit_extra_subflows. Just before, in __mptcp_close_subflow(),
mptcp_close_ssk() released that slot:

mptcp_close_ssk()
  mptcp_pm_subflow_check_next()
    __mptcp_pm_close_subflow()

The new subflow takes the slot straight back. It has request_join set, so
it can be retried again the next time it closes.

Consider a peer, or a middlebox injecting a RST after the MP_JOIN
handshake, that accepts each join, sends the 4th ACK and then resets the
subflow. Wouldn't the client then re-create the subflow every RTT or two,
for as long as the MPTCP socket stays established? Each cycle costs a
socket, a bind, HMAC work, SUB_ESTABLISHED and SUB_CLOSED netlink events,
and an ephemeral port.

The commit message says:

  "Its SYN retries, set like TCP_SYNCNT by the subflow-retries limit, space
  out the attempts."

That only seems to hold when SYNs are silently dropped, not when the peer
answers and then resets. Later patches in the series only touch selftests,
so this is still the case at the end of the series.

Would checking subflow->retry, or adding some backoff or a limit on
attempts per path, be needed here?

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH mptcp-next v6 6/6] selftests: mptcp: join: validate subflow retry after link failure
  2026-10-18  4:17 ` [PATCH mptcp-next v6 6/6] selftests: mptcp: join: validate subflow retry after link failure Anil Kaushik
@ 2026-10-19 17:46   ` netdev-bot+sashiko
  2026-10-19 18:07     ` Anil Kaushik
  0 siblings, 1 reply; 15+ messages in thread
From: netdev-bot+sashiko @ 2026-10-19 17:46 UTC (permalink / raw)
  To: anilkaushikwireless; +Cc: mptcp, kuba

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

Critical: 0 · High: 0 · Medium: 1 · 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] In link_failure_tests(), the new "subflow retry after link
  failure" block empties the shared input files at the end (`: >…
- [Low] In sf_retry_after_block(), the drop rule is flushed (`ip netns
  exec $ns2 nft flush chain ip filter OUTPUT`, line 2225) before…
- [Low] sf_retry_after_block() decides whether the subflow was re-created
  by comparing the local address:port strings from `ss` before and after…

--

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

This adds a test to link_failure_tests(). It uses nftables to drop the
client packets from one subflow address until the subflow is closed and
re-created, and lets them through again once the new SYN has been
retransmitted more times than tcp_syn_retries allows. This is done for
the additional subflow and then the initial one. The test then closes
the connection while a subflow is being re-created, checks the netlink
events, and empties the big input files at the end.

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index f4d8afaeda4f3..d4b4146cbd64b 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -2151,6 +2151,111 @@ wait_attempt_fail()
>  	return 1
>  }
>  
> +# $1: timeout in seconds ; $2: ns ; $3: counter ; $4: increment, 1 by default
> +wait_counter_inc()
> +{
> +	slowwait_for_counter "${1}" "${4:-1}" \
> +		mptcp_lib_get_counter "${2}" "${3}" >/dev/null
> +}

[ ... ]

> +sf_retry_after_block()
> +{
> +	local before after
> +
> +	before=$(sf_from "${1}")

[ ... ]

> +	# tcp_syn_retries=1: one more retransmission than allowed
> +	if ! wait_counter_inc "${timeout_poll}" \
> +		$ns2 "TcpExtTCPSynRetrans" 2; then
> +		ip netns exec $ns2 nft flush chain ip filter OUTPUT
> +		fail_test "SYN from ${1} not retransmitted enough"
> +		return
> +	fi
> +	ip netns exec $ns2 nft flush chain ip filter OUTPUT
> +
> +	if ! wait_counter_inc "${timeout_poll}" $ns1 "MPTcpExtMPJoinAckRx"; then

[Severity: Low]
Can this wait miss the MP_JOIN ACK it is waiting for?

The drop rule is flushed before wait_counter_inc() reads its baseline.
slowwait_for_counter() in net/lib.sh only reads the baseline when it is
called:

tools/testing/selftests/net/lib.sh:slowwait_for_counter() {
	...
	local base=$("$@")
	slowwait "$timeout" until_counter_is ">= $((base + delta))" "$@"
}

The re-created subflow keeps retransmitting its SYN on its own timer
(mptcp_pm_kernel_retry() sets icsk_syn_retries). If a retransmission
lands between the nft flush and the nstat read, the handshake can finish
and MPTcpExtMPJoinAckRx can go up before the baseline is taken.

The wait would then need one more MP_JOIN ACK that never arrives. It
would time out after timeout_poll and report "re-created subflow from
... not established", even though the subflow came back.

With exponential backoff, the next retransmission is about 4 seconds
away, so this only happens if the script stalls. Would it still be
more robust to read the counter before the flush and poll against that
saved value?

> +		fail_test "re-created subflow from ${1} not established"
> +		return
> +	fi
> +
> +	after=$(sf_from "${1}")
> +	if [ -z "${after}" ] || [ "${after}" = "${before}" ]; then
> +		fail_test "no new subflow from ${1}: ${before} -> ${after}"

[Severity: Low]
Is the address:port string always different for a re-created subflow?

The retry binds with port 0:

net/mptcp/pm_kernel.c:mptcp_pm_kernel_retry_check() {
	...
	retry->local.addr.port = 0;
	...
}

__mptcp_subflow_connect() then passes this address to kernel_bind(), so
the kernel picks an ephemeral port. The old subflow was also bound with
port 0. It released its port when it reached TCP_CLOSE, and the retry
check runs at that point, so the allocator can hand out the same port
again.

If that happens, this reports "no new subflow from 10.0.3.2: X -> X",
even though the MPJoinSynTx, TCPSynRetrans and MPJoinAckRx waits above
have already shown that a new subflow was established.

The odds are low, roughly 1 in 14000 per run for the 10.0.3.2 case.
Could the check rely on the MPJoinAckRx wait and an established subflow
being present, instead of comparing ports?

[ ... ]

> @@ -2585,6 +2690,65 @@ link_failure_tests()

[ ... ]

> +		join_syn_tx=4 chk_join_nr 3 3 3
> +		chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 3
> +		chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_CLOSED 3 error 3
> +
> +		# big files: empty them, the next tests write their own content
> +		: > "$cinfail"
> +		: > "$sinfail"
> +		: > "$cinsent"

[Severity: Medium]
Does the comment "the next tests write their own content" hold for the
test_linkfail=1 and test_linkfail=2 callers?

The files are emptied, but cinfail and sinfail still hold their paths.
For those modes, run_tests() only fills the files when the variable is
empty:

tools/testing/selftests/net/mptcp/mptcp_join.sh:run_tests() {
	...
	elif [ "$test_linkfail" -ne 0 ] && [ -z "$cinfail" ]; then
	...
	elif [ "$test_linkfail" -eq 2 ] && [ -z "$sinfail" ]; then
	...
}

Only the test_linkfail > 2 branches always rewrite the files.

The default order isn't affected, because this block is the last one in
link_failure_tests(). However, the getopts loop adds one entry per
option and doesn't remove duplicates:

	tests+=("${all_tests[${opt}]}")

So "mptcp_join.sh -l -l" runs link_failure_tests() twice. On the second
run, the test_linkfail=1/2 subtests would send 0-byte files and leave
$cinsent empty. chk_stale_nr() would then fail, and chk_link_usage()
would divide by zero:

	local tx_rate=$((tx_link * 100 / tx_total))

Reordering the groups, or adding a test_linkfail=1/2 case after this
one, would cause the same failures. Would it be better to remove the
files and reset cinfail and sinfail, or to have those branches check
[ ! -s "$cinfail" ] instead?

> +	fi
>  }

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH mptcp-next v6 4/6] mptcp: pm: in-kernel: retry subflows closed after an error
  2026-10-19 17:26   ` Abdul Wasey
@ 2026-10-19 17:56     ` Anil Kaushik
  2026-10-19 18:55       ` Abdul Wasey
  0 siblings, 1 reply; 15+ messages in thread
From: Anil Kaushik @ 2026-10-19 17:56 UTC (permalink / raw)
  To: Abdul Wasey; +Cc: mptcp, Matthieu Baerts

Hi Abdul,

Thanks for testing.

My comments are inline.

Regards
Anil

On Mon, Oct 19, 2026 at 10:56 PM Abdul Wasey <w453y.me@gmail.com> wrote:
>
> Hi Anil,
>
> I tested v6 on top of export (8bb146c4c) in netns: two 10 Mbit paths
> with 15 ms delay, the second one blackholed with netem loss 100% in both
> directions during an iperf3 transfer, tcp_retries2=3.
>
> Upload (the client sends) works: without the series the path never
> comes back, with it the path carries data again 4.5 s after it returns
> for a 20 s outage, and 12.6 s for a 60 s one, the extra time being the
> SYN backoff.
>
> Download (the server sends) is not recovered. The server's subflow times
> out and is freed without sending anything, while the client's subflow
> on that path only receives: it stays established and idle, never gets
> an error, so it is never retried. With keepalive enabled on the client
> socket (idle 5 s, interval 2 s, count 3), the idle subflow times out and
> your patch re-creates it: the path is back 3 s after it returns for a
> 20 s outage.
>
> So for a client mostly receiving data, which is probably the common
> case, the retry only helps if the application sets keepalive. Would it
> make sense for the PM to probe idle subflows it initiated, or would you
> rather document the keepalive requirement? I'd be happy to work on the
> download case if you have a preferred direction.

I will document it in the next version.
 Application that only receives data should enable TCP keepalive on MPTCP
socket. MPTCP already copies these options to every subflow, including
the re-created ones, so a dead path is then detected even when the
subflow is idle.

Probing from the PM can be a separate series. Please go ahead if you
want, Matt may have an opinion on it. I could also work with you. Let me know.


> With the default tcp_retries2=15, the subflows are not closed during 20
> to 180 s outages, and the old ones resume by themselves 17 to 24 s after
> the path returns, with or without the series.
>
Yes, this is for longer outages, or a lower tcp_retries2. I will add it
to the doc.


> Thanks,
> Abdul Wasey

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH mptcp-next v6 6/6] selftests: mptcp: join: validate subflow retry after link failure
  2026-10-19 17:46   ` netdev-bot+sashiko
@ 2026-10-19 18:07     ` Anil Kaushik
  0 siblings, 0 replies; 15+ messages in thread
From: Anil Kaushik @ 2026-10-19 18:07 UTC (permalink / raw)
  To: netdev-bot+sashiko; +Cc: mptcp, kuba

Thanks, all four are valid, they will be fixed in v7.

Regards
Anil

On Mon, Oct 19, 2026 at 11:16 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 · 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] In link_failure_tests(), the new "subflow retry after link
>   failure" block empties the shared input files at the end (`: >…
> - [Low] In sf_retry_after_block(), the drop rule is flushed (`ip netns
>   exec $ns2 nft flush chain ip filter OUTPUT`, line 2225) before…
> - [Low] sf_retry_after_block() decides whether the subflow was re-created
>   by comparing the local address:port strings from `ss` before and after…
>
> --
>
> selftests: mptcp: join: validate subflow retry after link failure
>
> This adds a test to link_failure_tests(). It uses nftables to drop the
> client packets from one subflow address until the subflow is closed and
> re-created, and lets them through again once the new SYN has been
> retransmitted more times than tcp_syn_retries allows. This is done for
> the additional subflow and then the initial one. The test then closes
> the connection while a subflow is being re-created, checks the netlink
> events, and empties the big input files at the end.
>
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > index f4d8afaeda4f3..d4b4146cbd64b 100755
> > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > @@ -2151,6 +2151,111 @@ wait_attempt_fail()
> >       return 1
> >  }
> >
> > +# $1: timeout in seconds ; $2: ns ; $3: counter ; $4: increment, 1 by default
> > +wait_counter_inc()
> > +{
> > +     slowwait_for_counter "${1}" "${4:-1}" \
> > +             mptcp_lib_get_counter "${2}" "${3}" >/dev/null
> > +}
>
> [ ... ]
>
> > +sf_retry_after_block()
> > +{
> > +     local before after
> > +
> > +     before=$(sf_from "${1}")
>
> [ ... ]
>
> > +     # tcp_syn_retries=1: one more retransmission than allowed
> > +     if ! wait_counter_inc "${timeout_poll}" \
> > +             $ns2 "TcpExtTCPSynRetrans" 2; then
> > +             ip netns exec $ns2 nft flush chain ip filter OUTPUT
> > +             fail_test "SYN from ${1} not retransmitted enough"
> > +             return
> > +     fi
> > +     ip netns exec $ns2 nft flush chain ip filter OUTPUT
> > +
> > +     if ! wait_counter_inc "${timeout_poll}" $ns1 "MPTcpExtMPJoinAckRx"; then
>
> [Severity: Low]
> Can this wait miss the MP_JOIN ACK it is waiting for?
>
> The drop rule is flushed before wait_counter_inc() reads its baseline.
> slowwait_for_counter() in net/lib.sh only reads the baseline when it is
> called:
>
> tools/testing/selftests/net/lib.sh:slowwait_for_counter() {
>         ...
>         local base=$("$@")
>         slowwait "$timeout" until_counter_is ">= $((base + delta))" "$@"
> }
>
> The re-created subflow keeps retransmitting its SYN on its own timer
> (mptcp_pm_kernel_retry() sets icsk_syn_retries). If a retransmission
> lands between the nft flush and the nstat read, the handshake can finish
> and MPTcpExtMPJoinAckRx can go up before the baseline is taken.
>
> The wait would then need one more MP_JOIN ACK that never arrives. It
> would time out after timeout_poll and report "re-created subflow from
> ... not established", even though the subflow came back.
>
> With exponential backoff, the next retransmission is about 4 seconds
> away, so this only happens if the script stalls. Would it still be
> more robust to read the counter before the flush and poll against that
> saved value?
>
> > +             fail_test "re-created subflow from ${1} not established"
> > +             return
> > +     fi
> > +
> > +     after=$(sf_from "${1}")
> > +     if [ -z "${after}" ] || [ "${after}" = "${before}" ]; then
> > +             fail_test "no new subflow from ${1}: ${before} -> ${after}"
>
> [Severity: Low]
> Is the address:port string always different for a re-created subflow?
>
> The retry binds with port 0:
>
> net/mptcp/pm_kernel.c:mptcp_pm_kernel_retry_check() {
>         ...
>         retry->local.addr.port = 0;
>         ...
> }
>
> __mptcp_subflow_connect() then passes this address to kernel_bind(), so
> the kernel picks an ephemeral port. The old subflow was also bound with
> port 0. It released its port when it reached TCP_CLOSE, and the retry
> check runs at that point, so the allocator can hand out the same port
> again.
>
> If that happens, this reports "no new subflow from 10.0.3.2: X -> X",
> even though the MPJoinSynTx, TCPSynRetrans and MPJoinAckRx waits above
> have already shown that a new subflow was established.
>
> The odds are low, roughly 1 in 14000 per run for the 10.0.3.2 case.
> Could the check rely on the MPJoinAckRx wait and an established subflow
> being present, instead of comparing ports?
>
> [ ... ]
>
> > @@ -2585,6 +2690,65 @@ link_failure_tests()
>
> [ ... ]
>
> > +             join_syn_tx=4 chk_join_nr 3 3 3
> > +             chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 3
> > +             chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_CLOSED 3 error 3
> > +
> > +             # big files: empty them, the next tests write their own content
> > +             : > "$cinfail"
> > +             : > "$sinfail"
> > +             : > "$cinsent"
>
> [Severity: Medium]
> Does the comment "the next tests write their own content" hold for the
> test_linkfail=1 and test_linkfail=2 callers?
>
> The files are emptied, but cinfail and sinfail still hold their paths.
> For those modes, run_tests() only fills the files when the variable is
> empty:
>
> tools/testing/selftests/net/mptcp/mptcp_join.sh:run_tests() {
>         ...
>         elif [ "$test_linkfail" -ne 0 ] && [ -z "$cinfail" ]; then
>         ...
>         elif [ "$test_linkfail" -eq 2 ] && [ -z "$sinfail" ]; then
>         ...
> }
>
> Only the test_linkfail > 2 branches always rewrite the files.
>
> The default order isn't affected, because this block is the last one in
> link_failure_tests(). However, the getopts loop adds one entry per
> option and doesn't remove duplicates:
>
>         tests+=("${all_tests[${opt}]}")
>
> So "mptcp_join.sh -l -l" runs link_failure_tests() twice. On the second
> run, the test_linkfail=1/2 subtests would send 0-byte files and leave
> $cinsent empty. chk_stale_nr() would then fail, and chk_link_usage()
> would divide by zero:
>
>         local tx_rate=$((tx_link * 100 / tx_total))
>
> Reordering the groups, or adding a test_linkfail=1/2 case after this
> one, would cause the same failures. Would it be better to remove the
> files and reset cinfail and sinfail, or to have those branches check
> [ ! -s "$cinfail" ] instead?
>
> > +     fi
> >  }
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018041748.1655711-1-anilkaushikwireless%40gmail.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH mptcp-next v6 4/6] mptcp: pm: in-kernel: retry subflows closed after an error
  2026-10-19 17:56     ` Anil Kaushik
@ 2026-10-19 18:55       ` Abdul Wasey
  0 siblings, 0 replies; 15+ messages in thread
From: Abdul Wasey @ 2026-10-19 18:55 UTC (permalink / raw)
  To: Anil Kaushik; +Cc: mptcp, Matthieu Baerts

Hi Anil,

Documenting it sounds good to me.

I'll do the probing from the PM as a separate series on top of yours.
I'll send an RFC first to see what Matthieu thinks of the approach, and
Cc you on it. Would be glad to work on it together.

Thanks,
Abdul Wasey

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2026-10-19 18:55 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-18  4:17 [PATCH mptcp-next v6 0/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
2026-10-18  4:17 ` [PATCH mptcp-next v6 1/6] selftests: mptcp: connect: retry write() on EAGAIN Anil Kaushik
2026-10-18  4:17 ` [PATCH mptcp-next v6 2/6] netlink: specs: mptcp: add the subflow-retries attribute Anil Kaushik
2026-10-19 17:46   ` netdev-bot+sashiko
2026-10-18  4:17 ` [PATCH mptcp-next v6 3/6] mptcp: pm: in-kernel: add the subflow-retries limit Anil Kaushik
2026-10-18  4:17 ` [PATCH mptcp-next v6 4/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
2026-10-18  4:45   ` sashiko-bot
2026-10-19 17:26   ` Abdul Wasey
2026-10-19 17:56     ` Anil Kaushik
2026-10-19 18:55       ` Abdul Wasey
2026-10-19 17:46   ` netdev-bot+sashiko
2026-10-18  4:17 ` [PATCH mptcp-next v6 5/6] selftests: mptcp: pm_nl_ctl: support the subflow-retries limit Anil Kaushik
2026-10-18  4:17 ` [PATCH mptcp-next v6 6/6] selftests: mptcp: join: validate subflow retry after link failure Anil Kaushik
2026-10-19 17:46   ` netdev-bot+sashiko
2026-10-19 18:07     ` Anil Kaushik

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.