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

[PATCH mptcp-next v4 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 in-kernel
PM now creates a new one to the same addresses. Its SYN retransmissions
space out the attempts. Their number is set by a new limit,
subflow-retries: 15 by default, about 20 minutes, 0 disables it.

Changes in v4, after Matthieu's review: netlink limit instead of the
sysctl, one new subflow with more SYN retransmissions instead of counted
retries, no MIB counter, and a test closing the connection during a
retry. Details in each patch.

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 (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/netlink/specs/mptcp_pm.yaml     |   9 ++
 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                                |  20 +++
 net/mptcp/pm_kernel.c                         | 149 +++++++++++++++++-
 net/mptcp/protocol.c                          |   5 +
 net/mptcp/protocol.h                          |  18 +++
 .../selftests/net/mptcp/mptcp_connect.c       |  17 ++
 .../testing/selftests/net/mptcp/mptcp_join.sh | 124 +++++++++++++++
 10 files changed, 344 insertions(+), 13 deletions(-)

-- 
2.25.1

[PATCH mptcp-next v4 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 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:
    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

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

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index 178d98d91fea..b9fd0bdab748 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,21 @@ 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 &&
+					    (poll_timeout < 0 ||
+					     ++eagain_cnt <= poll_timeout)) {
+						usleep(1000);
+						continue;
+					}
+
 					/* expected reset, continue to read */
 					if (cfg_rcv_trunc &&
 					    (errno == ECONNRESET ||
@@ -723,6 +739,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

Re: [PATCH mptcp-next v4 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: OOB stack read via unsigned len underflow
--

commit 2c05e206fd91fc4ebb90da18cf7072e457266159
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..b9fd0bdab7486 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
[ ... ]
> @@ -711,6 +712,21 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd,
>  
>  				bw = do_rnd_write(peerfd, winfo->buf + winfo->off, winfo->len);
[Severity: Medium]
Does this sequence allow an out-of-bounds stack read if the preceding read
call encounters an error?

In copyfd_io_poll(), winfo->len is assigned the result of read(). If read()
fails and returns -1, the signed value is assigned to the unsigned int
winfo->len, wrapping to its maximum value.

The condition checking if winfo->len > 0 then evaluates to true and bypasses
the error handling. This massive length is passed here to do_rnd_write(),
which computes a random write length up to 65535:

do_rnd_write() {
    ...
    do_w = rand() & 0xffff;
    if (do_w == 0 || do_w > len)
        do_w = len;

    bw = write(fd, buf, do_w);
}

Since the winfo->buf size is only 8192 bytes, could write() read uninitialized
data beyond the stack buffer boundary?

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

[PATCH mptcp-next v4 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.

When TCP closes a subflow created by this side after an error, the PM
now creates a new one with the same addresses. Its SYN retransmissions
space out the attempts. Their number is set by a new limit,
subflow-retries: 15 by default, about 20 minutes, and 0 disables it. If
this new subflow cannot connect, it is not retried again. 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.

Subflows refused by the peer, reset by the peer with an MP_RST without
the transient flag (RFC 8684 section 3.6), aborted locally, or whose
endpoint has been removed are not retried.

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

Notes:
    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

 Documentation/netlink/specs/mptcp_pm.yaml |   9 ++
 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                            |  20 +++
 net/mptcp/pm_kernel.c                     | 149 +++++++++++++++++++++-
 net/mptcp/protocol.c                      |   5 +
 net/mptcp/protocol.h                      |  18 +++
 8 files changed, 203 insertions(+), 13 deletions(-)

diff --git a/Documentation/netlink/specs/mptcp_pm.yaml b/Documentation/netlink/specs/mptcp_pm.yaml
index 39f3facc38e5..1b481908980c 100644
--- a/Documentation/netlink/specs/mptcp_pm.yaml
+++ b/Documentation/netlink/specs/mptcp_pm.yaml
@@ -196,6 +196,12 @@ attribute-sets:
         name: addr-remote
         type: nest
         nested-attributes: address
+      -
+        name: subflow-retries
+        type: u32
+        doc: |
+          SYN retransmissions of a subflow re-created after an error, 0 to
+          not re-create it.
   -
     name: event-attr
     enum-name: mptcp-event-attr
@@ -327,6 +333,7 @@ operations:
           attributes:
             - rcv-add-addrs
             - subflows
+            - subflow-retries
     -
       name: get-limits
       doc: Get protocol limits
@@ -337,10 +344,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..127a1fadaac4 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] = { .type = NLA_U32, },
 };
 
 /* 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] = { .type = NLA_U32, },
 };
 
 /* 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];
 
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index d7c5b50b34cc..1bea4012d7f1 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)
 {
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index d3014bf57bf3..6c081a9c664c 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);
 };
@@ -1399,7 +1400,8 @@ int mptcp_pm_nl_dump_addr(struct sk_buff *msg,
 	return msg->len;
 }
 
-static int parse_limit(struct genl_info *info, int id, unsigned int *limit)
+static int parse_limit(struct genl_info *info, int id, unsigned int *limit,
+		       unsigned int max)
 {
 	struct nlattr *attr = info->attrs[id];
 
@@ -1407,10 +1409,9 @@ static int parse_limit(struct genl_info *info, int id, unsigned int *limit)
 		return 0;
 
 	*limit = nla_get_u32(attr);
-	if (*limit > MPTCP_PM_SUBFLOWS_MAX) {
+	if (*limit > max) {
 		NL_SET_ERR_MSG_ATTR_FMT(info->extack, attr,
-					"limit greater than maximum (%u)",
-					MPTCP_PM_SUBFLOWS_MAX);
+					"limit greater than maximum (%u)", max);
 		return -EINVAL;
 	}
 	return 0;
@@ -1419,22 +1420,31 @@ 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;
 	int ret;
 
 	spin_lock_bh(&pernet->lock);
 	rcv_addrs = pernet->limit_add_addr_accepted;
-	ret = parse_limit(info, MPTCP_PM_ATTR_RCV_ADD_ADDRS, &rcv_addrs);
+	ret = parse_limit(info, MPTCP_PM_ATTR_RCV_ADD_ADDRS, &rcv_addrs,
+			  MPTCP_PM_SUBFLOWS_MAX);
 	if (ret)
 		goto unlock;
 
 	subflows = pernet->limit_extra_subflows;
-	ret = parse_limit(info, MPTCP_PM_ATTR_SUBFLOWS, &subflows);
+	ret = parse_limit(info, MPTCP_PM_ATTR_SUBFLOWS, &subflows,
+			  MPTCP_PM_SUBFLOWS_MAX);
+	if (ret)
+		goto unlock;
+
+	retries = pernet->limit_subflow_retries;
+	ret = parse_limit(info, MPTCP_PM_ATTR_SUBFLOW_RETRIES, &retries,
+			  MAX_TCP_SYNCNT);
 	if (ret)
 		goto unlock;
 
 	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 +1474,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);
 
@@ -1591,6 +1605,125 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk)
 	return true;
 }
 
+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;
+}
+
+/* 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 pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
+	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);
+	bool endp_found;
+	u8 id;
+
+	retry->syn_retries = READ_ONCE(pernet->limit_subflow_retries);
+	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;
+
+	/* A single attempt: its SYN retransmissions space out the tries */
+	if (subflow->retry && !established)
+		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;
+
+	/* not if its endpoint has been removed */
+	id = subflow_get_local_id(subflow);
+	if (id) {
+		rcu_read_lock();
+		endp_found = !!__lookup_addr_by_id(pernet, id);
+		rcu_read_unlock();
+		if (!endp_found)
+			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 = id;
+	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)
 {
@@ -1618,6 +1751,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.c b/net/mptcp/protocol.c
index 60cbd91be632..307a818d4783 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);
 	}
 
 }
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index ed2f363bb915..ab916c75e406 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -621,6 +621,7 @@ 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	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 +1138,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 +1271,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

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

[Matthieu Baerts]

Hi Anil,

On 16/10/2026 07:50, 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.
> 
> When TCP closes a subflow created by this side after an error, the PM
> now creates a new one with the same addresses. Its SYN retransmissions
> space out the attempts. Their number is set by a new limit,
> subflow-retries: 15 by default, about 20 minutes, and 0 disables it. If
> this new subflow cannot connect, it is not retried again. 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.
> 
> Subflows refused by the peer, reset by the peer with an MP_RST without
> the transient flag (RFC 8684 section 3.6), aborted locally, or whose
> endpoint has been removed are not retried.
Thank you for the new version. I had a very quick look:

- The patch is quite big, can you split it please? e.g.

  - the modification of parse_limit() before adding the new limit
  - the modification of the netlink specs and the generated files
  - the new limit: parse, store, expose
  - the retry feature

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

- in mptcp_pm_kernel_retry_check(), move "struct sock *ssk =
  mptcp_subflow_tcp_sock(subflow);" one line down for the reverse xmas
  tree
  https://docs.kernel.org/process/maintainer-netdev.html

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

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

[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
later 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 removed at the end.

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

Notes:
    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 | 124 ++++++++++++++++++
 1 file changed, 124 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index f4d8afaeda4f..bbeff6739c5a 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -2151,6 +2151,69 @@ wait_attempt_fail()
 	return 1
 }
 
+# $1: ns ; $2: counter ; $3: expected minimum value
+wait_counter()
+{
+	local cnt
+
+	for _ in $(seq $((timeout_poll * 10))); do
+		cnt=$(mptcp_lib_get_counter "${1}" "${2}")
+		[ "${cnt:-0}" -ge "${3}" ] && return 0
+		sleep 0.1
+	done
+
+	return 1
+}
+
+# $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 }'
+}
+
+# $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 later than tcp_syn_retries allows: the new
+# subflow still connects thanks to the subflow-retries limit.
+sf_retry_after_block()
+{
+	local syn_tx ack_rx before after
+
+	syn_tx=$(mptcp_lib_get_counter $ns2 "MPTcpExtMPJoinSynTx")
+	ack_rx=$(mptcp_lib_get_counter $ns1 "MPTcpExtMPJoinAckRx")
+	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
+
+	ip netns exec $ns2 nft add rule ip filter OUTPUT ip saddr "${1}" drop
+	if ! wait_counter $ns2 "MPTcpExtMPJoinSynTx" $((${syn_tx:-0} + 1)); then
+		ip netns exec $ns2 nft flush chain ip filter OUTPUT
+		fail_test "subflow from ${1} not re-created"
+		return
+	fi
+	sleep 5
+	ip netns exec $ns2 nft flush chain ip filter OUTPUT
+
+	if ! wait_counter $ns1 "MPTcpExtMPJoinAckRx" $((${ack_rx:-0} + 1)); 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
+}
+
 set_userspace_pm()
 {
 	local ns=$1
@@ -2585,6 +2648,67 @@ 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 mptcp_lib_kallsyms_has "mptcp_pm_kernel_retry$"; then
+		init_nftables
+		init_shapers
+		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.mptcp.close_timeout=5
+		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_counter $ns1 "MPTcpExtMPJoinAckRx" 1
+		sf_retry_after_block 10.0.3.2
+		sf_retry_after_block 10.0.1.2
+
+		ip netns exec $ns2 nft add rule ip filter OUTPUT \
+			ip saddr 10.0.3.2 drop
+		wait_counter $ns2 "MPTcpExtMPJoinSynTx" 4
+		local retrying=$?
+		mptcp_lib_kill_group_wait $tests_pid
+
+		# stopped at the latest by the close timeout
+		print_check "no subflow connecting after close"
+		for _ in $(seq $((timeout_poll * 10))); do
+			ip netns exec $ns2 ss -tnH state syn-sent | grep -q . ||
+				break
+			sleep 0.1
+		done
+		if [ "${retrying}" -ne 0 ]; then
+			fail_test "no subflow re-created before the close"
+		elif ip netns exec $ns2 ss -tnH state syn-sent | grep -q .; then
+			fail_test "subflow still connecting after close"
+		else
+			print_ok
+		fi
+		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: do not keep them, the next tests create their own
+		rm -f "$cinfail" "$sinfail"
+		cinfail=""
+		sinfail=""
+	fi
 }
 
 add_addr_timeout_tests()
-- 
2.25.1

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

[Matthieu Baerts]

On 16/10/2026 07:50, Anil Kaushik wrote:
> 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
> later 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 removed at the end.
> 
> Assisted-by: LLM
> Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
> ---
> 
> Notes:
>     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 | 124 ++++++++++++++++++
>  1 file changed, 124 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index f4d8afaeda4f..bbeff6739c5a 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -2151,6 +2151,69 @@ wait_attempt_fail()
>  	return 1
>  }
>  
> +# $1: ns ; $2: counter ; $3: expected minimum value
> +wait_counter()
> +{
> +	local cnt
> +
> +	for _ in $(seq $((timeout_poll * 10))); do
> +		cnt=$(mptcp_lib_get_counter "${1}" "${2}")
> +		[ "${cnt:-0}" -ge "${3}" ] && return 0
> +		sleep 0.1
> +	done
> +
> +	return 1
> +}
> +
> +# $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 }'
> +}
> +
> +# $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 later than tcp_syn_retries allows: the new
> +# subflow still connects thanks to the subflow-retries limit.
> +sf_retry_after_block()
> +{
> +	local syn_tx ack_rx before after
> +
> +	syn_tx=$(mptcp_lib_get_counter $ns2 "MPTcpExtMPJoinSynTx")
> +	ack_rx=$(mptcp_lib_get_counter $ns1 "MPTcpExtMPJoinAckRx")
> +	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
> +
> +	ip netns exec $ns2 nft add rule ip filter OUTPUT ip saddr "${1}" drop
> +	if ! wait_counter $ns2 "MPTcpExtMPJoinSynTx" $((${syn_tx:-0} + 1)); then
> +		ip netns exec $ns2 nft flush chain ip filter OUTPUT
> +		fail_test "subflow from ${1} not re-created"
> +		return
> +	fi
> +	sleep 5

Do you need a long sleep here?

We usually try to have "quick" tests. Here it looks like we do nothing
for 5 seconds, and possibly the sleep is not even needed.

> +	ip netns exec $ns2 nft flush chain ip filter OUTPUT
> +
> +	if ! wait_counter $ns1 "MPTcpExtMPJoinAckRx" $((${ack_rx:-0} + 1)); 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
> +}
> +
>  set_userspace_pm()
>  {
>  	local ns=$1
> @@ -2585,6 +2648,67 @@ 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 mptcp_lib_kallsyms_has "mptcp_pm_kernel_retry$"; then

Probably best to check if the new limit exists: modifying pm_nl_ctl (in
a dedicated commit) and see if it is there. Maybe pm_netlink.sh will
need to be modified as well.

(And iproute2 too if you don't mind, please :) )

> +		init_nftables
> +		init_shapers
> +		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.mptcp.close_timeout=5
> +		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_counter $ns1 "MPTcpExtMPJoinAckRx" 1
> +		sf_retry_after_block 10.0.3.2
> +		sf_retry_after_block 10.0.1.2
> +
> +		ip netns exec $ns2 nft add rule ip filter OUTPUT \
> +			ip saddr 10.0.3.2 drop
> +		wait_counter $ns2 "MPTcpExtMPJoinSynTx" 4
> +		local retrying=$?
> +		mptcp_lib_kill_group_wait $tests_pid
> +
> +		# stopped at the latest by the close timeout
> +		print_check "no subflow connecting after close"
> +		for _ in $(seq $((timeout_poll * 10))); do
> +			ip netns exec $ns2 ss -tnH state syn-sent | grep -q . ||
> +				break
> +			sleep 0.1
> +		done
> +		if [ "${retrying}" -ne 0 ]; then
> +			fail_test "no subflow re-created before the close"
> +		elif ip netns exec $ns2 ss -tnH state syn-sent | grep -q .; then
> +			fail_test "subflow still connecting after close"
> +		else
> +			print_ok
> +		fi

Best to move this block from print_check in a new helper: we usually
avoid adding "subchecks" in tests directly.

> +		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: do not keep them, the next tests create their own
> +		rm -f "$cinfail" "$sinfail"
> +		cinfail=""
> +		sinfail=""
> +	fi
>  }
>  
>  add_addr_timeout_tests()

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

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

[Anil Kaushik]

Hi Matt,

 Thanks for the review!

On Fri, Oct 16, 2026 at 2:31 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> On 16/10/2026 07:50, Anil Kaushik wrote:
> > 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
> > later 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 removed at the end.
> >
> > Assisted-by: LLM
> > Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
> > ---
> >
> > Notes:
> >     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 | 124 ++++++++++++++++++
> >  1 file changed, 124 insertions(+)
> >
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > index f4d8afaeda4f..bbeff6739c5a 100755
> > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > @@ -2151,6 +2151,69 @@ wait_attempt_fail()
> >       return 1
> >  }
> >
> > +# $1: ns ; $2: counter ; $3: expected minimum value
> > +wait_counter()
> > +{
> > +     local cnt
> > +
> > +     for _ in $(seq $((timeout_poll * 10))); do
> > +             cnt=$(mptcp_lib_get_counter "${1}" "${2}")
> > +             [ "${cnt:-0}" -ge "${3}" ] && return 0
> > +             sleep 0.1
> > +     done
> > +
> > +     return 1
> > +}
> > +
> > +# $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 }'
> > +}
> > +
> > +# $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 later than tcp_syn_retries allows: the new
> > +# subflow still connects thanks to the subflow-retries limit.
> > +sf_retry_after_block()
> > +{
> > +     local syn_tx ack_rx before after
> > +
> > +     syn_tx=$(mptcp_lib_get_counter $ns2 "MPTcpExtMPJoinSynTx")
> > +     ack_rx=$(mptcp_lib_get_counter $ns1 "MPTcpExtMPJoinAckRx")
> > +     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
> > +
> > +     ip netns exec $ns2 nft add rule ip filter OUTPUT ip saddr "${1}" drop
> > +     if ! wait_counter $ns2 "MPTcpExtMPJoinSynTx" $((${syn_tx:-0} + 1)); then
> > +             ip netns exec $ns2 nft flush chain ip filter OUTPUT
> > +             fail_test "subflow from ${1} not re-created"
> > +             return
> > +     fi
> > +     sleep 5
>
> Do you need a long sleep here?
It was there to show the new subflow outlives tcp_syn_retries. In v5,
the test waits for the SYN retransmissions instead, with
tcp_syn_linear_timeouts=0, so no sleep.

>
> We usually try to have "quick" tests. Here it looks like we do nothing
> for 5 seconds, and possibly the sleep is not even needed.
>
> > +     ip netns exec $ns2 nft flush chain ip filter OUTPUT
> > +
> > +     if ! wait_counter $ns1 "MPTcpExtMPJoinAckRx" $((${ack_rx:-0} + 1)); 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
> > +}
> > +
> >  set_userspace_pm()
> >  {
> >       local ns=$1
> > @@ -2585,6 +2648,67 @@ 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 mptcp_lib_kallsyms_has "mptcp_pm_kernel_retry$"; then
>
> Probably best to check if the new limit exists: modifying pm_nl_ctl (in
> a dedicated commit) and see if it is there. Maybe pm_netlink.sh will
> need to be modified as well.
>
> (And iproute2 too if you don't mind, please :) )
Sure, I will send it after this series.
>
> > +             init_nftables
> > +             init_shapers
> > +             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.mptcp.close_timeout=5
> > +             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_counter $ns1 "MPTcpExtMPJoinAckRx" 1
> > +             sf_retry_after_block 10.0.3.2
> > +             sf_retry_after_block 10.0.1.2
> > +
> > +             ip netns exec $ns2 nft add rule ip filter OUTPUT \
> > +                     ip saddr 10.0.3.2 drop
> > +             wait_counter $ns2 "MPTcpExtMPJoinSynTx" 4
> > +             local retrying=$?
> > +             mptcp_lib_kill_group_wait $tests_pid
> > +
> > +             # stopped at the latest by the close timeout
> > +             print_check "no subflow connecting after close"
> > +             for _ in $(seq $((timeout_poll * 10))); do
> > +                     ip netns exec $ns2 ss -tnH state syn-sent | grep -q . ||
> > +                             break
> > +                     sleep 0.1
> > +             done
> > +             if [ "${retrying}" -ne 0 ]; then
> > +                     fail_test "no subflow re-created before the close"
> > +             elif ip netns exec $ns2 ss -tnH state syn-sent | grep -q .; then
> > +                     fail_test "subflow still connecting after close"
> > +             else
> > +                     print_ok
> > +             fi
>
> Best to move this block from print_check in a new helper: we usually
> avoid adding "subchecks" in tests directly.
>
> > +             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: do not keep them, the next tests create their own
> > +             rm -f "$cinfail" "$sinfail"
> > +             cinfail=""
> > +             sinfail=""
> > +     fi
> >  }
> >
> >  add_addr_timeout_tests()
>
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
>

Regards
Anil

Re: [PATCH mptcp-next v4 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): Unstable: 1 failed test(s): selftest_mptcp_join ⚠️ 
- 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/38859846683

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


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)