* [PATCH mptcp-next v10 1/6] selftests: mptcp: connect: retry write() on EAGAIN
2026-10-26 6:36 [PATCH mptcp-next v10 0/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
@ 2026-10-26 6:36 ` Anil Kaushik
2026-10-26 6:36 ` [PATCH mptcp-next v10 2/6] netlink: specs: mptcp: add the subflow-retries attribute Anil Kaushik
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Anil Kaushik @ 2026-10-26 6:36 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")
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] 12+ messages in thread* [PATCH mptcp-next v10 2/6] netlink: specs: mptcp: add the subflow-retries attribute
2026-10-26 6:36 [PATCH mptcp-next v10 0/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
2026-10-26 6:36 ` [PATCH mptcp-next v10 1/6] selftests: mptcp: connect: retry write() on EAGAIN Anil Kaushik
@ 2026-10-26 6:36 ` Anil Kaushik
2026-10-26 6:36 ` [PATCH mptcp-next v10 3/6] mptcp: pm: in-kernel: add the subflow-retries limit Anil Kaushik
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Anil Kaushik @ 2026-10-26 6:36 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 a timeout, 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.
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---
Notes:
v9:
- after a timeout, not any error, and about 4 hours with the default
RTO settings
v8:
- the default is the maximum, MAX_TCP_SYNCNT (127) (Matthieu)
v7:
- the doc gives the default value (netdev Sashiko)
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 | 13 +++++++++++++
include/uapi/linux/mptcp_pm.h | 1 +
net/mptcp/mptcp_pm_gen.c | 10 ++++++----
net/mptcp/mptcp_pm_gen.h | 4 ++--
4 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/Documentation/netlink/specs/mptcp_pm.yaml b/Documentation/netlink/specs/mptcp_pm.yaml
index 39f3facc38e5..690d3c5dfdd2 100644
--- a/Documentation/netlink/specs/mptcp_pm.yaml
+++ b/Documentation/netlink/specs/mptcp_pm.yaml
@@ -196,6 +196,16 @@ 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 a timeout, like the
+ TCP_SYNCNT socket option, so with tcp_syn_linear_timeouts on top.
+ The default is the maximum, MAX_TCP_SYNCNT (127), about 4 hours
+ with the default RTO settings. 0 to not re-create it.
-
name: event-attr
enum-name: mptcp-event-attr
@@ -327,6 +337,7 @@ operations:
attributes:
- rcv-add-addrs
- subflows
+ - subflow-retries
-
name: get-limits
doc: Get protocol limits
@@ -337,10 +348,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] 12+ messages in thread* [PATCH mptcp-next v10 3/6] mptcp: pm: in-kernel: add the subflow-retries limit
2026-10-26 6:36 [PATCH mptcp-next v10 0/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
2026-10-26 6:36 ` [PATCH mptcp-next v10 1/6] selftests: mptcp: connect: retry write() on EAGAIN Anil Kaushik
2026-10-26 6:36 ` [PATCH mptcp-next v10 2/6] netlink: specs: mptcp: add the subflow-retries attribute Anil Kaushik
@ 2026-10-26 6:36 ` Anil Kaushik
2026-10-26 6:36 ` [PATCH mptcp-next v10 4/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Anil Kaushik @ 2026-10-26 6:36 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 a timeout, like
the TCP_SYNCNT socket option. The default is the maximum, MAX_TCP_SYNCNT
(127), about 4 hours, and 0 to not re-create it. The netlink policy
checks this maximum value.
It is also exposed in struct mptcp_info, in place of the reserved field.
The next patch uses it.
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---
Notes:
v9:
- MAX_TCP_SYNCNT instead of 127 for the default
- after a timeout, not any error
v8:
- the default is the maximum, 127, about 4 hours (Matthieu)
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..7fb4c7874340 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;
+ /* the maximum: about 4 hours with the default RTOs */
+ pernet->limit_subflow_retries = MAX_TCP_SYNCNT;
pernet->next_id = 1;
spin_lock_init(&pernet->lock);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 3be60da12924..4aa4aedb2ba7 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1266,6 +1266,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] 12+ messages in thread* [PATCH mptcp-next v10 4/6] mptcp: pm: in-kernel: retry subflows closed after an error
2026-10-26 6:36 [PATCH mptcp-next v10 0/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
` (2 preceding siblings ...)
2026-10-26 6:36 ` [PATCH mptcp-next v10 3/6] mptcp: pm: in-kernel: add the subflow-retries limit Anil Kaushik
@ 2026-10-26 6:36 ` Anil Kaushik
2026-10-26 9:55 ` sashiko-bot
2026-10-27 21:23 ` netdev-bot+sashiko
2026-10-26 6:36 ` [PATCH mptcp-next v10 5/6] selftests: mptcp: pm_nl_ctl: support the subflow-retries limit Anil Kaushik
` (2 subsequent siblings)
6 siblings, 2 replies; 12+ messages in thread
From: Anil Kaushik @ 2026-10-26 6:36 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 gives up on an established subflow created by this side, after
a retransmission or keepalive timeout, whatever ICMP error was received
from the path before, 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. This is a single connection attempt: an
ICMP error, or a local error when connecting, ends it, and it is not
retried again. It is also closed if the MPTCP socket is closed in the
meantime. If no other subflow can send data, the connection is still
closed after the close timeout, as when all the subflows are closed,
unless the new subflow connects before. 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.
__mptcp_close_ssk() now always cleans the references to the closed
subflow in the MPTCP backlog, not only with MPTCP_CF_PUSH: a re-created
subflow closed with the MPTCP socket can get established in between.
The other paths without MPTCP_CF_PUSH have already purged the backlog.
A subflow reset by the peer, or by anything on the path, is not
re-created: the path works, and re-creating it right away could loop.
A subflow that never got established is not re-created either: the PM
keeps trying the next endpoints, as before. Subflows aborted locally,
also by MPTCP, or using an address that has been removed are not
retried.
TCP only closes a subflow after tcp_retries2 retransmissions, about 15
minutes by default. A subflow that only receives data never gets an
error: the application has to enable TCP keepalive on the MPTCP socket,
which MPTCP applies to all the subflows. Both are documented in
mptcp.rst.
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/440
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---
Notes:
v10:
- keep the connection when the close timeout expires if a
re-created subflow connected in the meantime: an active join
does not stop the timer, mptcp_finish_join() returns early for
it (sashiko.dev)
- close the re-created subflows still connecting once the msk is
no longer established, after a DATA_FIN or a shutdown(), when
no other subflow is left: they cannot join anymore, and would
keep the msk around (sashiko.dev)
v9:
- every timeout is retried, whatever ICMP error TCP reports for it,
the last one received, see tcp_write_err(): resets, local aborts
and MPTCP protocol errors are excluded instead of listing the
timeout errors, so IPv4 and IPv6 behave the same (netdev Sashiko)
- mptcp.rst: 0 does not re-create the subflows (netdev Sashiko)
- the re-created subflows closed with the msk get no SUB_CLOSED
event, like the other subflows closed with it
- if no other subflow can send data, the close timeout still applies:
creating the new subflow no longer keeps the connection open for
the whole SYN retries
- doc and commit message: a single connection attempt, ended by an
ICMP error or a local connect error
- __mptcp_close_ssk() always cleans the msk backlog of the closed
subflow: a re-created subflow closed with the msk can have been
established in between, with data in the backlog (netdev Sashiko).
It is still closed without MPTCP_CF_PUSH, which would re-send all
the pending data
- tcp_sock_set_syncnt(), without the ssk lock
v8:
- mptcp.rst gives the default, 127, about 4 hours
- close the re-created subflows still connecting before
__mptcp_wr_shutdown() disconnects them, and also the already
disconnected ones, e.g. after shutdown(SHUT_WR) (sashiko.dev)
- mptcp.rst: the initial subflow on the client side, the subflows
limit, and only one attempt (netdev Sashiko)
v7:
- only after a path error, a retransmission or keepalive timeout: a
reset from the peer or the path could make it loop (netdev
Sashiko)
- retry and no_retry are bools under the msk socket lock, in the
padding after close_event_done, not bits next to the reset ones
(netdev Sashiko)
- mark all the subflows using a removed address, whatever their
state (netdev Sashiko)
- doc: tcp_retries2, and keepalive for a subflow that only receives
data (Abdul Wasey's tests)
- not retried after a reset by MPTCP itself, even with an old sk_err
left by IP_RECVERR (netdev Sashiko)
- close the re-created subflows still connecting without
MPTCP_CF_PUSH: they never carried data (netdev Sashiko)
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
Documentation/networking/mptcp.rst | 18 +++++
net/mptcp/pm.c | 39 ++++++++++
net/mptcp/pm_kernel.c | 118 +++++++++++++++++++++++++++++
net/mptcp/protocol.c | 108 ++++++++++++++++++++++++--
net/mptcp/protocol.h | 19 +++++
5 files changed, 296 insertions(+), 6 deletions(-)
diff --git a/Documentation/networking/mptcp.rst b/Documentation/networking/mptcp.rst
index b6753ffb9c9a..8a2c9b9d2a4f 100644
--- a/Documentation/networking/mptcp.rst
+++ b/Documentation/networking/mptcp.rst
@@ -73,6 +73,24 @@ To be able to use multiple IP addresses on a host to create multiple *subflows*
addresses can be used. This can be configured with ``ip mptcp endpoint`` for
example.
+When TCP gives up on an established subflow created by this host, after a
+retransmission or keepalive timeout, the in-kernel path manager creates it
+again, with the ``subflow-retries`` limit as its number of SYN retries, like
+``TCP_SYNCNT``: 127 by default, about 4 hours, and 0 to not re-create it. This
+is done for the additional subflows, and for the initial one on the client side:
+the new subflow then takes one of the additional subflows allowed by the
+``subflows`` limit. This is a single connection attempt: an ICMP error, or a
+local error when connecting, ends it, and it is not created again. If no other
+subflow can send data in the meantime, the connection is closed after
+``close_timeout``, as when all the subflows are closed, unless the new subflow
+connects before. A subflow reset by the peer, or by anything on the path, is not
+created again either. TCP only gives up on a subflow after ``tcp_retries2``
+retransmissions, about 15 minutes by default: shorter outages are handled by TCP
+itself. A subflow that only receives data does not notice that its path stopped
+working: the application should then enable TCP keepalive on the MPTCP socket
+(``SO_KEEPALIVE``, ``TCP_KEEPIDLE``, ``TCP_KEEPINTVL`` and ``TCP_KEEPCNT``),
+which MPTCP applies to all its subflows.
+
Packet Scheduler
----------------
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index d7c5b50b34cc..2bb08be95cd1 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 using a removed address must not be re-created, whatever its
+ * state, e.g. closed by TCP but not collected yet, 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)
+{
+ 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)
+ subflow->no_retry = true;
+}
+
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 7fb4c7874340..de84aecd6a62 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -1610,6 +1610,124 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk)
return true;
}
+/* TCP gave up on the path: after a retransmission, keepalive or user
+ * timeout, sk_err is ETIMEDOUT, or the last ICMP error received from the
+ * path, see tcp_write_err(). The other errors on an established subflow
+ * are not path failures: a reset, from the peer or from anything on the
+ * path (ECONNRESET, or EPIPE in CLOSE_WAIT, see tcp_reset()), a local
+ * abort (ECONNABORTED), or an MPTCP protocol error (EBADMSG).
+ */
+static bool mptcp_pm_retry_path_error(int err)
+{
+ switch (err) {
+ case ECONNRESET:
+ case EPIPE:
+ case ECONNABORTED:
+ case EBADMSG:
+ return false;
+ }
+
+ 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 sk_err and the resetting flag 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;
+
+ /* Only when the path stopped working. A reset, from the peer or from
+ * anything on the path, means the path works: re-creating the subflow
+ * right away could loop. Not after a reset by MPTCP itself either, with
+ * mptcp_subflow_reset(), whatever sk_err says.
+ */
+ if (!mptcp_pm_retry_path_error(err) || subflow->resetting)
+ 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;
+ 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);
+ /* from 1 to MAX_TCP_SYNCNT, see mptcp_pm_kernel_retry_check() */
+ tcp_sock_set_syncnt(mptcp_subflow_tcp_sock(subflow),
+ retry->syn_retries);
+ subflow->retry = true;
+}
+
/* 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 02a4f479d9d8..6d3232dc2b19 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2697,8 +2697,11 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
subflow->closing = 1;
- if (flags & MPTCP_CF_PUSH)
- mptcp_cleanup_ssk_backlog(sk, ssk);
+ /* also without MPTCP_CF_PUSH: a re-created subflow closed with the msk
+ * can have been established in between, see
+ * mptcp_close_retry_subflows()
+ */
+ mptcp_cleanup_ssk_backlog(sk, ssk);
/* Borrow the fwd allocated page left-over; fwd memory for the subflow
* could be negative at this point, but will be reach zero soon - when
@@ -2788,6 +2791,78 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
}
}
+/* Re-created subflows still connecting are useless once the msk is closed,
+ * or no longer established: they cannot join anymore, see
+ * mptcp_finish_join(), and would keep it around.
+ */
+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);
+ int state;
+
+ if (!subflow->retry || READ_ONCE(subflow->fully_established))
+ continue;
+
+ /* still connecting, or already disconnected by
+ * mptcp_subflow_shutdown(), e.g. after shutdown(SHUT_WR)
+ */
+ state = inet_sk_state_load(ssk);
+ if (state != TCP_SYN_SENT && state != TCP_CLOSE)
+ continue;
+
+ /* Checked without the ssk lock: it can have been established
+ * in between. __mptcp_close_ssk() cleans its data in the msk
+ * backlog, and the MPTCP retransmissions cover what it sent.
+ * No need to push the pending data again.
+ */
+ mptcp_pm_subflow_check_next(msk, subflow);
+ __mptcp_close_ssk(sk, ssk, subflow, 0);
+ }
+}
+
+static bool mptcp_has_active_subflow(struct mptcp_sock *msk)
+{
+ struct mptcp_subflow_context *subflow;
+
+ mptcp_for_each_subflow(msk, subflow) {
+ if (__mptcp_subflow_active(subflow))
+ return true;
+ }
+
+ return false;
+}
+
+/* With no subflow left to send data, a re-created subflow still connecting
+ * must not keep the connection open longer than the close timeout, as when
+ * all the subflows are closed, see __mptcp_close_ssk(). mptcp_worker() keeps
+ * the connection if the subflow connected in the meantime.
+ */
+static void mptcp_retry_check_tout(struct sock *sk)
+{
+ struct mptcp_subflow_context *subflow;
+ struct mptcp_sock *msk = mptcp_sk(sk);
+ bool connecting = false;
+
+ if (inet_csk(sk)->icsk_mtup.probe_timestamp ||
+ mptcp_has_active_subflow(msk))
+ return;
+
+ mptcp_for_each_subflow(msk, subflow)
+ connecting |= subflow->retry &&
+ !READ_ONCE(subflow->fully_established);
+ if (!connecting)
+ return;
+
+ if (sk->sk_state == TCP_ESTABLISHED)
+ mptcp_start_tout_timer(sk);
+ else
+ mptcp_close_retry_subflows(sk);
+}
+
void mptcp_close_ssk(struct sock *sk, struct sock *ssk,
struct mptcp_subflow_context *subflow)
{
@@ -2806,6 +2881,7 @@ void mptcp_close_ssk(struct sock *sk, struct sock *ssk,
mptcp_pm_subflow_check_next(mptcp_sk(sk), subflow);
__mptcp_close_ssk(sk, ssk, subflow, MPTCP_CF_PUSH);
+ mptcp_retry_check_tout(sk);
}
static unsigned int mptcp_sync_mss(struct sock *sk, u32 pmtu)
@@ -2823,6 +2899,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 ||
@@ -2834,7 +2912,13 @@ 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);
+ /* creating a subflow stopped the close timeout */
+ mptcp_retry_check_tout(sk);
+ }
}
}
@@ -3188,10 +3272,19 @@ static void mptcp_worker(struct work_struct *work)
if (mptcp_close_tout_expired(sk)) {
struct mptcp_subflow_context *subflow, *tmp;
- mptcp_do_fastclose(sk);
- mptcp_for_each_subflow_safe(msk, subflow, tmp)
- __mptcp_close_ssk(sk, subflow->tcp_sock, subflow, 0);
- mptcp_close_wake_up(sk);
+ /* a re-created subflow connected in the meantime, see
+ * mptcp_retry_check_tout()
+ */
+ if (sk->sk_state == TCP_ESTABLISHED &&
+ mptcp_has_active_subflow(msk)) {
+ mptcp_stop_tout_timer(sk);
+ } else {
+ mptcp_do_fastclose(sk);
+ mptcp_for_each_subflow_safe(msk, subflow, tmp)
+ __mptcp_close_ssk(sk, subflow->tcp_sock,
+ subflow, 0);
+ mptcp_close_wake_up(sk);
+ }
}
if (sock_flag(sk, SOCK_DEAD) && sk->sk_state == TCP_CLOSE) {
@@ -3516,6 +3609,9 @@ bool __mptcp_close(struct sock *sk, long timeout)
goto cleanup;
}
+ /* before __mptcp_wr_shutdown() disconnects them */
+ mptcp_close_retry_subflows(sk);
+
if (mptcp_data_avail(msk) || timeout < 0 ||
(sock_flag(sk, SOCK_LINGER) && !sk->sk_lingertime)) {
/* If the msk has read data, or the caller explicitly ask it,
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 4aa4aedb2ba7..d47430fc7dc8 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -610,6 +610,8 @@ struct mptcp_subflow_context {
bool close_event_done; /* has done the post-closed part,
* protected by msk socket lock
*/
+ bool retry; /* re-created after an error, msk lock */
+ bool no_retry; /* uses a removed address, msk lock */
u32 lent_mem_frag;
u32 remote_nonce;
u32 local_nonce;
@@ -1139,6 +1141,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,
@@ -1260,6 +1274,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] 12+ messages in thread* Re: [PATCH mptcp-next v10 4/6] mptcp: pm: in-kernel: retry subflows closed after an error
2026-10-26 6:36 ` [PATCH mptcp-next v10 4/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
@ 2026-10-26 9:55 ` sashiko-bot
2026-10-27 21:23 ` netdev-bot+sashiko
1 sibling, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-10-26 9:55 UTC (permalink / raw)
To: Anil Kaushik, mptcp
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] mptcp: use-after-free in mptcp_close_ssk() via mptcp_close_retry_subflows()
- [High] mptcp: unstopped close timer after retry connect aborts active connections
- [Medium] mptcp: connecting retry subflows not closed when leaving TCP_ESTABLISHED
--
commit f1e9a463142ae1f128b140c9fd8fd0bb5928bda7
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
client-initiated subflows that close after a path timeout error, using the
subflow-retries limit for SYN retries. It also adds helpers to manage the
close timeout and clean up connecting retry subflows when the MPTCP socket
closes or leaves the established state.
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 02a4f479d9d8c..6d3232dc2b19d 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2697,8 +2697,11 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
[ ... ]
> @@ -2788,6 +2791,78 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
> }
> }
>
> +/* Re-created subflows still connecting are useless once the msk is closed,
> + * or no longer established: they cannot join anymore, see
> + * mptcp_finish_join(), and would keep it around.
> + */
> +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);
> + int state;
> +
> + if (!subflow->retry || READ_ONCE(subflow->fully_established))
> + continue;
> +
> + /* still connecting, or already disconnected by
> + * mptcp_subflow_shutdown(), e.g. after shutdown(SHUT_WR)
> + */
> + state = inet_sk_state_load(ssk);
> + if (state != TCP_SYN_SENT && state != TCP_CLOSE)
> + continue;
[Severity: Medium]
Can connecting retry subflows be left behind on msk->conn_list when the
MPTCP socket leaves TCP_ESTABLISHED?
First, mptcp_close_retry_subflows() is only called from __mptcp_close() and
from mptcp_retry_check_tout() when another subflow closes (and only if
icsk_mtup.probe_timestamp is zero and no active subflows remain). When sk
transitions out of TCP_ESTABLISHED via mptcp_shutdown() or
mptcp_check_data_fin(), mptcp_close_retry_subflows() is not called.
For example, if all active subflows have already closed and a retry subflow
is in TCP_SYN_SENT with probe_timestamp set, calling shutdown(fd, SHUT_WR)
moves sk to TCP_FIN_WAIT1:
- If mptcp_send_head(sk) is non-NULL, mptcp_check_send_data_fin() returns
early and leaves the retry subflow in TCP_SYN_SENT even though
mptcp_finish_join() will reject it once sk is no longer TCP_ESTABLISHED.
- If mptcp_send_head(sk) is NULL, mptcp_subflow_shutdown() calls
tcp_disconnect(), which sets ssk->sk_state to TCP_CLOSE and invokes
sk_error_report() rather than sk_state_change(), so
MPTCP_WORK_CLOSE_SUBFLOW is not scheduled and the disconnected retry
subflow stays on msk->conn_list.
Second, mptcp_retry_check_tout() treats any retry subflow with
!READ_ONCE(subflow->fully_established) as connecting, whereas
mptcp_close_retry_subflows() skips states other than TCP_SYN_SENT and
TCP_CLOSE. If a retry subflow received its SYN-ACK before sk left
TCP_ESTABLISHED and is in TCP_ESTABLISHED waiting for the 4th ACK
(!subflow->fully_established) when the last active subflow closes,
__mptcp_close_ssk() will not start the close timer (since msk->conn_list is
not singular), and mptcp_retry_check_tout() -> mptcp_close_retry_subflows()
will skip the subflow because its state is TCP_ESTABLISHED, leaving msk
without an active subflow or a close timeout timer.
> +
> + /* Checked without the ssk lock: it can have been established
> + * in between. __mptcp_close_ssk() cleans its data in the msk
> + * backlog, and the MPTCP retransmissions cover what it sent.
> + * No need to push the pending data again.
> + */
> + mptcp_pm_subflow_check_next(msk, subflow);
> + __mptcp_close_ssk(sk, ssk, subflow, 0);
> + }
> +}
> +
> +static bool mptcp_has_active_subflow(struct mptcp_sock *msk)
> +{
> + struct mptcp_subflow_context *subflow;
> +
> + mptcp_for_each_subflow(msk, subflow) {
> + if (__mptcp_subflow_active(subflow))
> + return true;
> + }
> +
> + return false;
> +}
> +
> +/* With no subflow left to send data, a re-created subflow still connecting
> + * must not keep the connection open longer than the close timeout, as when
> + * all the subflows are closed, see __mptcp_close_ssk(). mptcp_worker() keeps
> + * the connection if the subflow connected in the meantime.
> + */
> +static void mptcp_retry_check_tout(struct sock *sk)
> +{
> + struct mptcp_subflow_context *subflow;
> + struct mptcp_sock *msk = mptcp_sk(sk);
> + bool connecting = false;
> +
> + if (inet_csk(sk)->icsk_mtup.probe_timestamp ||
> + mptcp_has_active_subflow(msk))
> + return;
> +
> + mptcp_for_each_subflow(msk, subflow)
> + connecting |= subflow->retry &&
> + !READ_ONCE(subflow->fully_established);
> + if (!connecting)
> + return;
> +
> + if (sk->sk_state == TCP_ESTABLISHED)
> + mptcp_start_tout_timer(sk);
> + else
> + mptcp_close_retry_subflows(sk);
> +}
> +
> void mptcp_close_ssk(struct sock *sk, struct sock *ssk,
> struct mptcp_subflow_context *subflow)
> {
> @@ -2806,6 +2881,7 @@ void mptcp_close_ssk(struct sock *sk, struct sock *ssk,
> mptcp_pm_subflow_check_next(mptcp_sk(sk), subflow);
>
> __mptcp_close_ssk(sk, ssk, subflow, MPTCP_CF_PUSH);
> + mptcp_retry_check_tout(sk);
> }
[Severity: Critical]
Does calling mptcp_retry_check_tout() inside mptcp_close_ssk() cause a
use-after-free when callers iterate over msk->conn_list with
mptcp_for_each_subflow_safe()?
Both __mptcp_close_subflow() and mptcp_pm_rm_addr_or_subflow() iterate over
msk->conn_list using mptcp_for_each_subflow_safe(msk, subflow, tmp) and call
mptcp_close_ssk(sk, ssk, subflow):
__mptcp_close_subflow() / mptcp_pm_rm_addr_or_subflow()
mptcp_for_each_subflow_safe(msk, subflow, tmp)
mptcp_close_ssk(sk, ssk, subflow)
__mptcp_close_ssk(sk, ssk, subflow, MPTCP_CF_PUSH)
mptcp_retry_check_tout(sk)
mptcp_close_retry_subflows(sk)
__mptcp_close_ssk(sk, ssk_retry, subflow_retry, 0)
list_del(&subflow_retry->node)
Consider what happens when an earlier subflow timed out and was re-created
(appended to msk->conn_list in TCP_SYN_SENT) while another established
subflow S1 was still active (so probe_timestamp is 0):
1. sk leaves TCP_ESTABLISHED (for example, moving to TCP_CLOSE_WAIT on
incoming DATA_FIN or to TCP_FIN_WAIT1 on shutdown(SHUT_WR)).
2. S1 then closes via __mptcp_close_subflow() or
mptcp_pm_rm_addr_or_subflow(). In the outer
mptcp_for_each_subflow_safe(msk, subflow, tmp) loop, subflow is S1 and
the cached next pointer tmp is the connecting retry subflow S_retry.
3. mptcp_close_ssk() closes S1 and calls mptcp_retry_check_tout(sk). Since
no active subflow remains, S_retry is connecting, and
sk->sk_state != TCP_ESTABLISHED, it calls mptcp_close_retry_subflows(sk).
4. mptcp_close_retry_subflows(sk) calls __mptcp_close_ssk() on S_retry
without setting S_retry->close_event_done = true. Because
ssk_retry != msk->first, __mptcp_close_ssk() unlinks S_retry from
msk->conn_list via list_del(&S_retry->node) (poisoning S_retry->node.next
with LIST_POISON1), frees S_retry via kfree_rcu(), and drops msk's
reference to ssk_retry.
5. Control returns to the outer mptcp_for_each_subflow_safe() loop, which
advances subflow to tmp (the unlinked and freed S_retry) and reads
tmp->node.next (LIST_POISON1). In __mptcp_close_subflow(), because
S_retry->close_event_done is still false and ssk_retry is now in
TCP_CLOSE, it calls mptcp_close_ssk() a second time on S_retry before
dereferencing LIST_POISON1 on the next iteration.
[ ... ]
> @@ -3188,10 +3272,19 @@ static void mptcp_worker(struct work_struct *work)
> if (mptcp_close_tout_expired(sk)) {
> struct mptcp_subflow_context *subflow, *tmp;
>
> - mptcp_do_fastclose(sk);
> - mptcp_for_each_subflow_safe(msk, subflow, tmp)
> - __mptcp_close_ssk(sk, subflow->tcp_sock, subflow, 0);
> - mptcp_close_wake_up(sk);
> + /* a re-created subflow connected in the meantime, see
> + * mptcp_retry_check_tout()
> + */
> + if (sk->sk_state == TCP_ESTABLISHED &&
> + mptcp_has_active_subflow(msk)) {
> + mptcp_stop_tout_timer(sk);
> + } else {
> + mptcp_do_fastclose(sk);
> + mptcp_for_each_subflow_safe(msk, subflow, tmp)
> + __mptcp_close_ssk(sk, subflow->tcp_sock,
> + subflow, 0);
> + mptcp_close_wake_up(sk);
> + }
> }
[Severity: High]
Can leaving the close timeout timer running after a retry subflow connects
abort a live connection or leave a stale probe_timestamp?
When the last active subflow times out and is retried,
mptcp_retry_check_tout() starts the close timeout timer via
mptcp_start_tout_timer(sk), setting icsk_mtup.probe_timestamp. If the
re-created subflow connects before the close timeout expires,
mptcp_finish_join() returns early for active joins
(!list_empty(&subflow->node)) without calling mptcp_stop_tout_timer(parent),
and check_fully_established() does not stop the timer either.
Because the timer and probe_timestamp remain active until the original
timeout expires:
1. If the reconnected MPTCP socket (with !sock_flag(sk, SOCK_DEAD)) moves to
TCP_CLOSE_WAIT (on incoming DATA_FIN while still sending data) or
TCP_FIN_WAIT1 / TCP_FIN_WAIT2 (after shutdown(SHUT_WR) while still
receiving data) before the stale timer fires, mptcp_worker() sees
sk->sk_state != TCP_ESTABLISHED and enters the else branch, calling
mptcp_do_fastclose(sk) and resetting the active connection.
2. Because icsk_mtup.probe_timestamp is still non-zero after the retry
subflow becomes active, if another retry subflow is still connecting and
the newly active subflow closes before the original timer expires,
mptcp_retry_check_tout() sees inet_csk(sk)->icsk_mtup.probe_timestamp != 0
and returns early instead of restarting the close timeout from the new
close time (or closing connecting retry subflows if sk is no longer
TCP_ESTABLISHED).
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261026063657.2955783-1-anilkaushikwireless@gmail.com?part=4
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH mptcp-next v10 4/6] mptcp: pm: in-kernel: retry subflows closed after an error
2026-10-26 6:36 ` [PATCH mptcp-next v10 4/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
2026-10-26 9:55 ` sashiko-bot
@ 2026-10-27 21:23 ` netdev-bot+sashiko
1 sibling, 0 replies; 12+ messages in thread
From: netdev-bot+sashiko @ 2026-10-27 21:23 UTC (permalink / raw)
To: anilkaushikwireless; +Cc: mptcp, kuba
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 1 · High: 0 · Medium: 1 · 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).
- [Critical] Use-after-free and list corruption in net/mptcp/protocol.c.
- [Medium] mptcp_retry_check_tout() (net/mptcp/protocol.c) arms the close
timeout while a re-created subflow is connecting.
--
mptcp: pm: in-kernel: retry subflows closed after an error
With this patch, the in-kernel PM re-creates an established subflow it
initiated once TCP gives up on it after a retransmission or keepalive
timeout. The SYN retries of the new subflow come from the subflow-retries
limit. Re-created subflows that are still connecting are closed when the
msk is closed or is no longer established, and the close timeout is kept
when no other subflow can send.
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/440
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 02a4f479d9d8c..6d3232dc2b19d 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
[ ... ]
> @@ -2788,6 +2791,78 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
[ ... ]
> +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) {
[ ... ]
> + mptcp_pm_subflow_check_next(msk, subflow);
> + __mptcp_close_ssk(sk, ssk, subflow, 0);
> + }
> +}
[ ... ]
> +static void mptcp_retry_check_tout(struct sock *sk)
> +{
[ ... ]
> + if (sk->sk_state == TCP_ESTABLISHED)
> + mptcp_start_tout_timer(sk);
> + else
> + mptcp_close_retry_subflows(sk);
> +}
> +
> void mptcp_close_ssk(struct sock *sk, struct sock *ssk,
> struct mptcp_subflow_context *subflow)
> {
> @@ -2806,6 +2881,7 @@ void mptcp_close_ssk(struct sock *sk, struct sock *ssk,
> mptcp_pm_subflow_check_next(mptcp_sk(sk), subflow);
>
> __mptcp_close_ssk(sk, ssk, subflow, MPTCP_CF_PUSH);
> + mptcp_retry_check_tout(sk);
> }
[Severity: Critical]
Can this free a subflow that the caller's list walk has already cached as
the next entry?
mptcp_close_ssk() is called from inside mptcp_for_each_subflow_safe()
loops in __mptcp_close_subflow() and in mptcp_pm_rm_addr_or_subflow() in
net/mptcp/pm.c. Suppose the msk is not TCP_ESTABLISHED, no subflow is
active, and a re-created subflow is still connecting. The new call then
goes to mptcp_close_retry_subflows(), which runs __mptcp_close_ssk() on
other entries of conn_list.
For the re-created subflow, that does list_del() on its node and calls
__tcp_close(). It also drops the msk's ssk reference. Because the context
is disposable, subflow_ulp_release() then frees it with kfree_rcu().
__mptcp_subflow_connect() adds the re-created subflow R with
list_add_tail(). So R is usually the tmp the outer loop has cached while
it processes the initial subflow F. This sequence looks like it reaches
that case:
client msk, in-kernel PM, conn_list [F, A]
A times out, __mptcp_close_subflow() re-creates R: conn_list [F, R]
R stays in SYN_SENT because its SYNs are dropped
peer sends DATA_FIN, mptcp_check_data_fin() sets TCP_CLOSE_WAIT
peer resets F (or sends RM_ADDR for id 0)
__mptcp_close_subflow() /* subflow = F, tmp = R */
mptcp_close_ssk(F)
__mptcp_close_ssk(F)
mptcp_retry_check_tout()
mptcp_close_retry_subflows()
__mptcp_close_ssk(R, 0) /* list_del() and free of R */
loop continues with subflow = R
The outer loop then reads R->tcp_sock and calls mptcp_close_ssk(R) a
second time. close_event_done stays unset because
mptcp_close_retry_subflows() never sets it.
That second call:
- decrements extra_subflows again in mptcp_pm_subflow_check_next()
- calls lock_sock_nested() on a socket that may already be freed
- calls list_del() on a poisoned node
After that, the loop takes its next entry from R->node.next, which is
LIST_POISON1.
The peer, or an on-path attacker, can drive every step of this sequence.
The feature is enabled by default for the in-kernel PM.
Could mptcp_close_retry_subflows() be deferred from here, for example to
the top level of mptcp_worker(), so that it never runs inside a caller's
safe iteration?
> @@ -2823,6 +2899,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;
[ ... ]
> + 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);
> + /* creating a subflow stopped the close timeout */
> + mptcp_retry_check_tout(sk);
> + }
> }
>
> }
> @@ -3188,10 +3272,19 @@ static void mptcp_worker(struct work_struct *work)
> if (mptcp_close_tout_expired(sk)) {
> struct mptcp_subflow_context *subflow, *tmp;
>
> - mptcp_do_fastclose(sk);
> - mptcp_for_each_subflow_safe(msk, subflow, tmp)
> - __mptcp_close_ssk(sk, subflow->tcp_sock, subflow, 0);
> - mptcp_close_wake_up(sk);
> + /* a re-created subflow connected in the meantime, see
> + * mptcp_retry_check_tout()
> + */
> + if (sk->sk_state == TCP_ESTABLISHED &&
> + mptcp_has_active_subflow(msk)) {
> + mptcp_stop_tout_timer(sk);
> + } else {
> + mptcp_do_fastclose(sk);
[Severity: Medium]
Can this fastclose a half-closed connection that already has an
established re-created subflow?
When the msk is TCP_ESTABLISHED and the only subflow left is a re-created
one that is still connecting, mptcp_retry_check_tout() arms the close
timer with mptcp_start_tout_timer(). When that subflow later completes its
MP_JOIN, mptcp_finish_join() returns early because the subflow is already
in conn_list:
if (!list_empty(&subflow->node)) {
...
mptcp_subflow_joined(msk, ssk);
spin_unlock_bh(&msk->fallback_lock);
mptcp_propagate_sndbuf(parent, ssk);
return true;
}
So __mptcp_finish_join(), which calls mptcp_stop_tout_timer(), is never
reached, and the timer stays armed.
The msk can then move to TCP_CLOSE_WAIT before close_timeout expires,
either through a peer DATA_FIN in mptcp_check_data_fin() or through a
local shutdown(SHUT_WR). The check above then fails on sk_state, and
mptcp_do_fastclose() resets the active subflow.
Could the timer be stopped when the re-created subflow joins? Another
option would be for this check to also accept the states where the
connection is still usable.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261026063657.2955783-1-anilkaushikwireless%40gmail.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH mptcp-next v10 5/6] selftests: mptcp: pm_nl_ctl: support the subflow-retries limit
2026-10-26 6:36 [PATCH mptcp-next v10 0/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
` (3 preceding siblings ...)
2026-10-26 6:36 ` [PATCH mptcp-next v10 4/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
@ 2026-10-26 6:36 ` Anil Kaushik
2026-10-27 21:23 ` netdev-bot+sashiko
2026-10-26 6:36 ` [PATCH mptcp-next v10 6/6] selftests: mptcp: join: validate subflow retry after link failure Anil Kaushik
2026-10-26 7:55 ` [PATCH mptcp-next v10 0/6] mptcp: pm: in-kernel: retry subflows closed after an error MPTCP CI
6 siblings, 1 reply; 12+ messages in thread
From: Anil Kaushik @ 2026-10-26 6:36 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.
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---
Notes:
v9:
- try the value above the maximum after setting 0, not the default
value, which is also the maximum: a refused value is then told
apart from a clamped one (netdev Sashiko)
v8:
- the default value is now 127
v7:
- also check that the maximum value, 127, is accepted (netdev
Sashiko)
v5:
- new (Matthieu)
.../testing/selftests/net/mptcp/pm_netlink.sh | 48 ++++++++++++++++++-
tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 34 ++++++++++---
2 files changed, 73 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..9a6209181962 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 127
+ elif mptcp_lib_expect_all_features; then
+ # missing feature: the defaults limits check will fail
+ echo 127
+ 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,28 @@ 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} 0
+ check "get_limits" "$(format_limits ${max_limits} ${max_limits} 0)" \
+ "set subflow retries"
+
+ # not from the default: a refused value differs from a clamped one
+ ip netns exec "${ns1}" ./pm_nl_ctl limits ${max_limits} ${max_limits} \
+ 128 2>/dev/null
+ check "get_limits" "$(format_limits ${max_limits} ${max_limits} 0)" \
+ "subflow retries above hard limit"
+
+ ip netns exec "${ns1}" ./pm_nl_ctl limits ${max_limits} ${max_limits} \
+ 127
+ check "get_limits" "$(format_limits ${max_limits} ${max_limits} 127)" \
+ "set max 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] 12+ messages in thread* Re: [PATCH mptcp-next v10 5/6] selftests: mptcp: pm_nl_ctl: support the subflow-retries limit
2026-10-26 6:36 ` [PATCH mptcp-next v10 5/6] selftests: mptcp: pm_nl_ctl: support the subflow-retries limit Anil Kaushik
@ 2026-10-27 21:23 ` netdev-bot+sashiko
0 siblings, 0 replies; 12+ messages in thread
From: netdev-bot+sashiko @ 2026-10-27 21:23 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] Coverage gap in pm_netlink.sh, from commit "selftests: mptcp:
pm_nl_ctl: support the subflow-retries limit".
- [Low] The 'subflow retries above hard limit' subtest in pm_netlink.sh
(lines 259-263) runs `pm_nl_ctl limits ${max_limits} ${max_limits} 128…
--
selftests: mptcp: pm_nl_ctl: support the subflow-retries limit
This commit lets 'pm_nl_ctl limits' take an optional third value that sets
the subflow-retries limit, and makes it print that limit when the kernel
reports it. pm_netlink.sh is extended to check the default value, that a
value above the maximum is refused, and that the limit can be set.
> diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
> index 21bfe1311f118..9a62091819626 100755
> --- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
> +++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
[ ... ]
> @@ -224,7 +247,28 @@ 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} 0
> + check "get_limits" "$(format_limits ${max_limits} ${max_limits} 0)" \
> + "set subflow retries"
[Severity: Low]
Would it be worth checking here that a 2-value set keeps the retries
limit?
If SET_LIMITS has no MPTCP_PM_ATTR_SUBFLOW_RETRIES attribute,
mptcp_pm_nl_set_limits_doit() keeps the current value:
net/mptcp/pm_kernel.c:mptcp_pm_nl_set_limits_doit() {
...
attr = info->attrs[MPTCP_PM_ATTR_SUBFLOW_RETRIES];
retries = nla_get_u32_default(attr, pernet->limit_subflow_retries);
...
}
iproute2 and mptcp_lib_pm_nl_set_limits() only send RCV_ADD_ADDRS and
SUBFLOWS. Every normal limits update therefore goes through this path.
In pm_netlink.sh, every 2-value set_limits call runs while
subflow_retries is still 127. That value is the default, the maximum and
MAX_TCP_SYNCNT. In the new block, every SET_LIMITS call passes the third
value.
Suppose SET_LIMITS without the attribute reset limit_subflow_retries to
MAX_TCP_SYNCNT. Would any of the new checks catch that?
One way to cover it: after setting 0 here, run
set_limits ${max_limits} ${max_limits} and check that subflow_retries 0
is still reported.
> +
> + # not from the default: a refused value differs from a clamped one
> + ip netns exec "${ns1}" ./pm_nl_ctl limits ${max_limits} ${max_limits} \
> + 128 2>/dev/null
> + check "get_limits" "$(format_limits ${max_limits} ${max_limits} 0)" \
> + "subflow retries above hard limit"
[Severity: Low]
Could this subtest also check that the pm_nl_ctl command fails?
Here stderr is thrown away and the exit status is not checked. The only
check is the get_limits call that follows, which confirms the state did
not change.
pm_nl_ctl already exits non-zero when it gets a netlink error ACK.
nl_error() reports the error, and do_nl_req() then does:
if (err)
error(1, 0, "bailing out due to netlink error[s]");
As written, a kernel that ignored an out-of-range SUBFLOW_RETRIES and
returned success would still pass. NLA_POLICY_MAX(NLA_U32, 127) in
mptcp_pm_set_limits_nl_policy is meant to reject that request.
The comment above the command is accurate for what it tests: starting
from 0 does tell a refused value apart from a clamped one. The existing
'rcv addrs above hard limit' and 'subflows above hard limit' subtests
also check only the state.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261026063657.2955783-1-anilkaushikwireless%40gmail.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH mptcp-next v10 6/6] selftests: mptcp: join: validate subflow retry after link failure
2026-10-26 6:36 [PATCH mptcp-next v10 0/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
` (4 preceding siblings ...)
2026-10-26 6:36 ` [PATCH mptcp-next v10 5/6] selftests: mptcp: pm_nl_ctl: support the subflow-retries limit Anil Kaushik
@ 2026-10-26 6:36 ` Anil Kaushik
2026-10-27 21:23 ` netdev-bot+sashiko
2026-10-26 7:55 ` [PATCH mptcp-next v10 0/6] mptcp: pm: in-kernel: retry subflows closed after an error MPTCP CI
6 siblings, 1 reply; 12+ messages in thread
From: Anil Kaushik @ 2026-10-26 6:36 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.
A second test closes the connection while a subflow is being re-created,
and checks the netlink events. Only the client sends data: the server
reads it all first, in sendfile mode, and keeps its side open. The
client gets a big MPTCP send buffer compared to its subflows, and both
paths are blocked: the data left in it is never sent, nor the DATA_FIN,
so only the close of the MPTCP socket can stop the subflow being
re-created. Only the client is closed: it has nothing left to read, so
the connection is not closed with a fastclose, which would reset all the
subflows anyway. The check is skipped if a fastclose happens. The big
files used for the long connection are emptied at the end.
A third test re-creates the only subflow, and checks the connection is
still closed after the close timeout, the new subflow being unable to
connect. A fourth one lets the path come back before the close timeout,
and checks the connection stays open.
The tests are skipped if pm_nl_ctl does not show the new limit, or if the
filter rules cannot be set.
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
---
Notes:
v10:
- new test: the path of the only subflow comes back before the
close timeout, and the connection stays open (sashiko.dev)
The path comes back as soon as the subflow is re-created, with a
30s close timeout, longer than the SYN backoff, and its transfer
has a longer poll timeout, with no data at all while the only path
is blocked: both are needed on a slow debug kernel
v9:
- wait for absolute counter values, read before blocking the path,
or 0 and 1 in the new netns: the subflow can be re-created before
a later read (netdev Sashiko)
- check that the connection is established before blocking it, in
the close test and the new one (netdev Sashiko)
- the close test blocks the other path too, with a big msk send
buffer: the pending data and the DATA_FIN no longer close the
subflow being re-created, only the close of the MPTCP socket can.
Checked: both the close test and the new test fail without their
fix
- new test: the only subflow is re-created, and the connection is
still closed after the close timeout
- $sinfail as the server input, kill errors ignored, clearer failure
message, $cout and $sout emptied too
- the server of the second test reads all the data from the client
first, in sendfile mode, instead of blocking on a FIFO in poll
mode: the client always has data in flight (netdev Sashiko)
v8:
- comment in run_tests() matching the new condition (netdev Sashiko)
- a separate test closes the connection while a subflow is
re-created: the server sends nothing but keeps its side open, and
only the client is closed, so there is no fastclose, which would
reset all the subflows anyway; the check is skipped if one happens
(netdev Sashiko)
v7 (netdev Sashiko):
- the test_linkfail=1/2 modes regenerate the shared files when they
are empty, e.g. when running link_failure_tests() twice
- read the MP_JOIN ACK counter before letting the packets through,
and do not compare the ports, they can be reused
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 | 399 +++++++++++++++++-
1 file changed, 391 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index c3ccce96b45b..3fa561b1a8fd 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -52,6 +52,7 @@ nr_blank=6
# These var are used only in some tests, make sure they are not already set
unset FAILING_LINKS
unset test_linkfail
+unset server_in
unset addr_nr_ns1
unset addr_nr_ns2
unset cestab_ns1
@@ -1038,7 +1039,12 @@ do_transfer()
fi
extra_srv_args="$extra_args $extra_srv_args"
- if [ "$test_linkfail" -gt 1 ];then
+ if [ -n "${server_in}" ]; then
+ # the server reads all the data from the client before sending
+ # this file: it does not block on its input
+ extra_srv_args="$extra_srv_args -m sendfile"
+ listener_in="${server_in}"
+ elif [ "$test_linkfail" -gt 1 ]; then
listener_in="${sinfail}"
fi
ip netns exec ${listener_ns} \
@@ -1135,9 +1141,9 @@ run_tests()
cinfail=$(mktemp)
fi
make_file "$cinfail" "client" $size
- # create the input file for the failure test when
- # the first failure test run
- elif [ "$test_linkfail" -ne 0 ] && [ -z "$cinfail" ]; then
+ # create the input file for the failure test, when
+ # it is missing or has been emptied
+ elif [ "$test_linkfail" -ne 0 ] && [ ! -s "$cinfail" ]; then
# the client file must be considerably larger
# of the maximum expected cwin value, or the
# link utilization will be not predicable
@@ -1146,23 +1152,27 @@ run_tests()
size=$((size*8192))
size=$((size + ( RANDOM % 8192) ))
- cinfail=$(mktemp)
+ if [ -z "$cinfail" ]; then
+ cinfail=$(mktemp)
+ fi
make_file "$cinfail" "client" $size
fi
- if [ "$test_linkfail" -gt 2 ]; then
+ if [ "$test_linkfail" -gt 2 ] && [ -z "${server_in}" ]; then
size=$test_linkfail
if [ -z "$sinfail" ]; then
sinfail=$(mktemp)
fi
make_file "$sinfail" "server" $size
- elif [ "$test_linkfail" -eq 2 ] && [ -z "$sinfail" ]; then
+ elif [ "$test_linkfail" -eq 2 ] && [ ! -s "$sinfail" ]; then
size=$((RANDOM%16))
size=$((size+1))
size=$((size*2048))
- sinfail=$(mktemp)
+ if [ -z "$sinfail" ]; then
+ sinfail=$(mktemp)
+ fi
make_file "$sinfail" "server" $size
fi
@@ -2151,6 +2161,237 @@ 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: timeout ; $2: ns ; $3: counter ; $4: value to reach
+wait_counter_ge()
+{
+ slowwait "${1}" until_counter_is ">= ${4}" \
+ 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 ack_rx syn_tx
+
+ 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
+
+ # read before the block: the subflow can be re-created right after it
+ syn_tx=$(mptcp_lib_get_counter $ns2 "MPTcpExtMPJoinSynTx")
+ if ! nft_drop_from "${1}"; then
+ fail_test "unable to drop the packets from ${1}"
+ return
+ fi
+ if ! wait_counter_ge "${timeout_poll}" $ns2 "MPTcpExtMPJoinSynTx" \
+ $((syn_tx + 1)); 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
+ # read before the flush: the next SYN can be sent right after it
+ ack_rx=$(mptcp_lib_get_counter $ns1 "MPTcpExtMPJoinAckRx")
+ ip netns exec $ns2 nft flush chain ip filter OUTPUT
+
+ if ! wait_counter_ge "${timeout_poll}" $ns1 "MPTcpExtMPJoinAckRx" \
+ $((ack_rx + 1)); then
+ fail_test "re-created subflow from ${1} not established"
+ return
+ fi
+
+ if [ -z "$(sf_from "${1}")" ]; then
+ fail_test "no established subflow from ${1}"
+ else
+ print_ok
+ fi
+}
+
+# $1: ns ; prints the number of MP_FASTCLOSE sent and received
+fastclose_nr()
+{
+ local tx rx
+
+ tx=$(mptcp_lib_get_counter "${1}" "MPTcpExtMPFastcloseTx")
+ rx=$(mptcp_lib_get_counter "${1}" "MPTcpExtMPFastcloseRx")
+ echo $((${tx:-0} + ${rx:-0}))
+}
+
+no_sf_connecting()
+{
+ ! ip netns exec $ns2 ss -tnH state syn-sent | grep -q .
+}
+
+# $1: client address of the only subflow ; $2: close timeout
+#
+# The subflow is blocked and re-created, and its path comes back right away:
+# the new subflow connects before the close timeout, and keeps the connection
+# open after it.
+chk_sf_retry_path_back()
+{
+ print_check "open after the close timeout"
+ if ! wait_counter_ge $((timeout_poll * 4)) \
+ $ns1 "MPTcpExtMPCapableACKRX" 1; then
+ fail_test "connection not established"
+ return
+ fi
+ if ! nft_drop_from "${1}"; then
+ fail_test "unable to drop the packets from ${1}"
+ return
+ fi
+ # no other subflow: the first MP_JOIN SYN is the re-created one
+ if ! wait_counter_ge "${timeout_poll}" \
+ $ns2 "MPTcpExtMPJoinSynTx" 1; then
+ ip netns exec $ns2 nft flush chain ip filter OUTPUT
+ fail_test "subflow from ${1} not re-created"
+ return
+ fi
+ ip netns exec $ns2 nft flush chain ip filter OUTPUT
+ if ! wait_counter_ge "${timeout_poll}" \
+ $ns1 "MPTcpExtMPJoinAckRx" 1; then
+ fail_test "re-created subflow from ${1} not established"
+ return
+ fi
+
+ # the close timeout started when the subflow was re-created
+ sleep $((${2} + 2))
+ if [ "$(fastclose_nr $ns2)" -gt 0 ]; then
+ fail_test "closed with a fastclose"
+ elif [ -z "$(sf_from "${1}")" ]; then
+ fail_test "no established subflow from ${1}"
+ else
+ print_ok
+ fi
+}
+
+# The client has nothing left to read: it closes the connection without a
+# fastclose, which would reset all the subflows anyway.
+kill_client()
+{
+ local pid
+
+ for pid in $(ip netns pids $ns2); do
+ if [ "$(cat "/proc/${pid}/comm" 2>/dev/null)" = mptcp_connect ]
+ then
+ kill "${pid}" 2>/dev/null
+ fi
+ done
+}
+
+# $1: why the subflow is not being re-created before the close, if it is not
+# $2: number of MP_FASTCLOSE before the close
+chk_no_sf_connecting()
+{
+ local closed
+
+ print_check "no subflow connecting after close"
+ if [ -n "${1}" ]; then
+ fail_test "${1}"
+ return
+ fi
+
+ # closed with the MPTCP socket
+ slowwait "${timeout_poll}" no_sf_connecting
+ closed=$?
+ if [ "$(fastclose_nr $ns2)" -gt "${2}" ]; then
+ print_skip "closed with a fastclose, resetting all the subflows"
+ elif [ "${closed}" -eq 0 ]; then
+ print_ok
+ else
+ fail_test "subflow still connecting after close"
+ fi
+}
+
+# Slow links with short queues: the transfer lasts the whole test on any
+# machine, and the blocked subflow always has data in flight.
+sf_retry_init()
+{
+ 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
+}
+
+sf_retry_cleanup()
+{
+ ip netns exec $ns2 nft flush chain ip filter OUTPUT
+ # big files: empty them, the next tests write their own content
+ : > "$cinfail"
+ : > "$sinfail"
+ : > "$cinsent"
+ : > "$cout"
+ : > "$sout"
+}
+
set_userspace_pm()
{
local ns=$1
@@ -2585,6 +2826,148 @@ 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
+ if reset_with_events "subflow retry after link failure" &&
+ continue_if has_subflow_retries && init_nft_filter; then
+ sf_retry_init
+ # 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_ge $((timeout_poll * 4)) \
+ $ns1 "MPTcpExtMPJoinAckRx" 1
+ sf_retry_after_block 10.0.3.2
+ sf_retry_after_block 10.0.1.2
+ mptcp_lib_kill_group_wait $tests_pid
+ kill_events_pids
+
+ join_syn_tx=3 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 2 error 2
+ sf_retry_cleanup
+ fi
+
+ # close the connection while a subflow is being re-created
+ if reset_with_events "subflow retry, close while connecting" &&
+ continue_if has_subflow_retries && init_nft_filter; then
+ sf_retry_init
+ # A big msk send buffer, compared to the subflow ones: when the
+ # client is closed, some data is not given to any subflow yet,
+ # so the DATA_FIN, which would also close the subflow being
+ # re-created, is not sent.
+ ip netns exec $ns2 sysctl -q \
+ net.ipv4.tcp_wmem="1048576 65536 262144"
+ # The server reads until the client closes, and sends nothing:
+ # the client has nothing to read, and the connection stays
+ # established. The transfer is stopped on purpose, its result
+ # is not checked.
+ : > "$sinfail"
+ { timeout_test=600 test_linkfail=131072 server_in="$sinfail" \
+ run_tests $ns1 $ns2 10.0.1.1 >/dev/null & } 2>/dev/null
+ local tests_pid=$!
+
+ local not_ready="" syn_tx
+ if ! wait_counter_ge $((timeout_poll * 4)) \
+ $ns1 "MPTcpExtMPJoinAckRx" 1; then
+ not_ready="no additional subflow before the block"
+ else
+ # read before the block: the subflow can be re-created
+ # right after it
+ syn_tx=$(mptcp_lib_get_counter $ns2 \
+ "MPTcpExtMPJoinSynTx")
+ if ! nft_drop_from 10.0.3.2; then
+ not_ready="unable to drop the packets"
+ elif ! wait_counter_ge "${timeout_poll}" \
+ $ns2 "MPTcpExtMPJoinSynTx" $((syn_tx + 1)); then
+ not_ready="no subflow re-created before close"
+ fi
+ fi
+ # Block the other path too, so this data is never sent: only
+ # the close of the MPTCP socket can then stop the subflow being
+ # re-created.
+ if [ -z "${not_ready}" ] && ! nft_drop_from 10.0.1.2; then
+ not_ready="unable to drop the packets"
+ fi
+ local fc_before
+ fc_before=$(fastclose_nr $ns2)
+ kill_client
+ chk_no_sf_connecting "${not_ready}" "${fc_before}"
+ mptcp_lib_kill_group_wait $tests_pid
+ kill_events_pids
+
+ join_syn_tx=2 chk_join_nr 1 1 1
+ chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 1
+ chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_CLOSED 1 error 1
+ sf_retry_cleanup
+ fi
+
+ # the only subflow is re-created: the connection is still closed after
+ # the close timeout, the new subflow cannot connect
+ if reset_with_events "subflow retry, no other subflow" &&
+ continue_if has_subflow_retries && init_nft_filter; then
+ sf_retry_init
+ pm_nl_flush_endpoint $ns2
+ ip netns exec $ns2 sysctl -q net.mptcp.close_timeout=5
+ { timeout_test=600 test_linkfail=131072 \
+ run_tests $ns1 $ns2 10.0.1.1 >/dev/null & } 2>/dev/null
+ local tests_pid=$!
+
+ print_check "closed after the close timeout"
+ if ! wait_counter_ge $((timeout_poll * 4)) \
+ $ns1 "MPTcpExtMPCapableACKRX" 1; then
+ fail_test "connection not established"
+ elif ! nft_drop_from 10.0.1.2; then
+ fail_test "unable to drop the packets"
+ # no other subflow: the first MP_JOIN SYN is the re-created one
+ elif ! wait_counter_ge "${timeout_poll}" \
+ $ns2 "MPTcpExtMPJoinSynTx" 1; then
+ fail_test "no subflow re-created"
+ # closed with the connection, after 5 seconds
+ elif slowwait "${timeout_poll}" no_sf_connecting; then
+ print_ok
+ else
+ fail_test "subflow still connecting"
+ fi
+ mptcp_lib_kill_group_wait $tests_pid
+ kill_events_pids
+
+ join_syn_tx=1 chk_join_nr 0 0 0
+ chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_CLOSED 1 error 1
+ sf_retry_cleanup
+ fi
+
+ # the path of the only subflow comes back before the close timeout:
+ # the re-created subflow keeps the connection open
+ if reset_with_events "subflow retry, no other subflow, path back" &&
+ continue_if has_subflow_retries && init_nft_filter; then
+ # The SYN retransmissions back off: long enough for the next one
+ # to be sent once the path is back, also on a slow machine.
+ local close_timeout=30
+
+ sf_retry_init
+ pm_nl_flush_endpoint $ns2
+ ip netns exec $ns2 sysctl -q \
+ net.mptcp.close_timeout=${close_timeout}
+ # no data at all while the only path is blocked: a longer poll
+ # timeout, so the transfer outlives the outage
+ { timeout_poll=$((timeout_poll * 4)) timeout_test=600 \
+ test_linkfail=131072 \
+ run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
+ local tests_pid=$!
+
+ chk_sf_retry_path_back 10.0.1.2 ${close_timeout}
+ mptcp_lib_kill_group_wait $tests_pid
+ kill_events_pids
+
+ join_syn_tx=1 chk_join_nr 1 1 1
+ chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 1
+ chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_CLOSED 1 error 1
+ sf_retry_cleanup
+ fi
}
add_addr_timeout_tests()
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH mptcp-next v10 6/6] selftests: mptcp: join: validate subflow retry after link failure
2026-10-26 6:36 ` [PATCH mptcp-next v10 6/6] selftests: mptcp: join: validate subflow retry after link failure Anil Kaushik
@ 2026-10-27 21:23 ` netdev-bot+sashiko
0 siblings, 0 replies; 12+ messages in thread
From: netdev-bot+sashiko @ 2026-10-27 21:23 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: 2 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] The 'subflow retry, no other subflow' test in
link_failure_tests() (tools/testing/selftests/net/mptcp/mptcp_join.sh)…
- [Medium] With -C, mptcp_join.sh sets validate_checksum=true for every
test (reset(): `validate_checksum=$checksum`).
- [Low] In the 'subflow retry, no other subflow, path back' test, the
background `run_tests` is started with only stderr redirected (`{…
--
selftests: mptcp: join: validate subflow retry after link failure
This adds four mptcp_join.sh tests for the subflow-retries limit. Each one
uses netfilter to drop the client packets from one address until the
subflow is closed and re-created. They cover re-creating the additional and
initial subflows, closing the connection while a subflow is being
re-created, and the close timeout when only a re-created subflow is left,
with and without the path coming back.
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index c3ccce96b45b1..3fa561b1a8fd1 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
[ ... ]
> @@ -2585,6 +2826,148 @@ 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
> + if reset_with_events "subflow retry after link failure" &&
> + continue_if has_subflow_retries && init_nft_filter; then
> + sf_retry_init
> + # 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_ge $((timeout_poll * 4)) \
> + $ns1 "MPTcpExtMPJoinAckRx" 1
> + sf_retry_after_block 10.0.3.2
> + sf_retry_after_block 10.0.1.2
> + mptcp_lib_kill_group_wait $tests_pid
> + kill_events_pids
> +
> + join_syn_tx=3 chk_join_nr 3 3 3
[Severity: Medium]
Can this fail on a correct kernel when mptcp_join.sh is run with -C?
With -C, reset() sets validate_checksum=true. chk_join_nr() then also
runs:
if $validate_checksum; then
chk_csum_nr $csum_ns1 $csum_ns2
chk_fail_nr $fail_nr $fail_nr
chk_rst_nr $rst_nr $rst_nr
join_rst_nr defaults to 0 here, so any MP_RST counted in ns1 or ns2 makes
the check fail.
The transfer is killed while it is still running in both directions at
the full 4mbit link rate, and mptcp_connect has no SIGTERM handler. If
either side still has unread data when its socket is closed at exit,
__mptcp_close() takes the mptcp_data_avail() branch:
__mptcp_close()
mptcp_do_fastclose()
tcp_send_active_reset()
mptcp_established_options()
mptcp_established_options_rst() /* MPTCP_MIB_MPRSTTX++ */
On the peer, mptcp_check_fastclose() can do the same for subflows that are
not closed yet.
The "subflow retry, no other subflow, path back" test kills its transfer
the same way before calling join_syn_tx=1 chk_join_nr 1 1 1.
The existing tests that kill live transfers use rate-limited speeds
(speed=5/20), which leaves a much smaller window for unread data. Do these
two tests need to account for the MP_RST counters when -C is used?
> + chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 3
> + chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_CLOSED 2 error 2
> + sf_retry_cleanup
> + fi
[ ... ]
> + # the only subflow is re-created: the connection is still closed after
> + # the close timeout, the new subflow cannot connect
> + if reset_with_events "subflow retry, no other subflow" &&
> + continue_if has_subflow_retries && init_nft_filter; then
> + sf_retry_init
> + pm_nl_flush_endpoint $ns2
> + ip netns exec $ns2 sysctl -q net.mptcp.close_timeout=5
> + { timeout_test=600 test_linkfail=131072 \
> + run_tests $ns1 $ns2 10.0.1.1 >/dev/null & } 2>/dev/null
> + local tests_pid=$!
> +
> + print_check "closed after the close timeout"
> + if ! wait_counter_ge $((timeout_poll * 4)) \
> + $ns1 "MPTcpExtMPCapableACKRX" 1; then
> + fail_test "connection not established"
> + elif ! nft_drop_from 10.0.1.2; then
> + fail_test "unable to drop the packets"
> + # no other subflow: the first MP_JOIN SYN is the re-created one
> + elif ! wait_counter_ge "${timeout_poll}" \
> + $ns2 "MPTcpExtMPJoinSynTx" 1; then
> + fail_test "no subflow re-created"
> + # closed with the connection, after 5 seconds
> + elif slowwait "${timeout_poll}" no_sf_connecting; then
> + print_ok
[Severity: Medium]
Does this check actually depend on the 5 second close timeout?
The commit message says this test "checks the connection is still closed
after the close timeout". However, the only pass condition is that ns2 has
no SYN-SENT socket within timeout_poll, which is 30s by default:
no_sf_connecting()
{
! ip netns exec $ns2 ss -tnH state syn-sent | grep -q .
}
The background transfer here keeps the default timeout_poll, and
do_transfer() passes it to the client as "mptcp_connect -t ${timeout_poll}".
The path back test raises it, but this test does not.
With the only path blocked, the client stops getting poll events. After
about 30s copyfd_io_poll() returns on poll timeout and the client exits.
Its close goes through:
__mptcp_close()
mptcp_close_retry_subflows()
This closes the retry subflow that is still in SYN_SENT, whether or not
the close timeout ever fired.
The slowwait only starts once MPJoinSynTx reaches 1, a few seconds after
the block (tcp_retries2=3). That puts the application's own close at
around 30s inside the 30s window.
Wouldn't this still print ok if mptcp_retry_check_tout() never started the
close timeout? It would also pass if the retry subflow followed
tcp_syn_retries=1 and went away after about 3 seconds.
The later checks give the same result in all these cases:
join_syn_tx=1 chk_join_nr 0 0 0
chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_CLOSED 1 error 1
MPTCP_LIB_EVENT_CLOSED is not checked, and the exit status of the
background job is ignored.
Could the transfer use a longer timeout_poll, and could the wait be bounded
by close_timeout, or check that the MPTCP connection itself was closed?
> + else
> + fail_test "subflow still connecting"
> + fi
[ ... ]
> + # the path of the only subflow comes back before the close timeout:
> + # the re-created subflow keeps the connection open
> + if reset_with_events "subflow retry, no other subflow, path back" &&
> + continue_if has_subflow_retries && init_nft_filter; then
> + # The SYN retransmissions back off: long enough for the next one
> + # to be sent once the path is back, also on a slow machine.
> + local close_timeout=30
> +
> + sf_retry_init
> + pm_nl_flush_endpoint $ns2
> + ip netns exec $ns2 sysctl -q \
> + net.mptcp.close_timeout=${close_timeout}
> + # no data at all while the only path is blocked: a longer poll
> + # timeout, so the transfer outlives the outage
> + { timeout_poll=$((timeout_poll * 4)) timeout_test=600 \
> + test_linkfail=131072 \
> + run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
[Severity: Low]
Will the output of this test always come out garbled? The second and third
tests send the stdout of the background job to /dev/null, but this one
does not.
chk_sf_retry_path_back() starts with:
print_check "open after the close timeout"
if ! wait_counter_ge $((timeout_poll * 4)) \
$ns1 "MPTcpExtMPCapableACKRX" 1; then
print_check() does not end the line. MPCapableACKRX can only reach 1 once
do_transfer() starts, and that happens after run_tests() has called
make_file() for the two 128 MB files.
So make_file()'s "Test file (size 131072 KB) for client/server" info lines
seem to always land in the middle of the check label, with the
[ OK ]/[FAIL] result on a line of its own.
The first test avoids this by waiting before print_check.
> + local tests_pid=$!
> +
> + chk_sf_retry_path_back 10.0.1.2 ${close_timeout}
> + mptcp_lib_kill_group_wait $tests_pid
> + kill_events_pids
> +
> + join_syn_tx=1 chk_join_nr 1 1 1
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261026063657.2955783-1-anilkaushikwireless%40gmail.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH mptcp-next v10 0/6] mptcp: pm: in-kernel: retry subflows closed after an error
2026-10-26 6:36 [PATCH mptcp-next v10 0/6] mptcp: pm: in-kernel: retry subflows closed after an error Anil Kaushik
` (5 preceding siblings ...)
2026-10-26 6:36 ` [PATCH mptcp-next v10 6/6] selftests: mptcp: join: validate subflow retry after link failure Anil Kaushik
@ 2026-10-26 7:55 ` MPTCP CI
6 siblings, 0 replies; 12+ messages in thread
From: MPTCP CI @ 2026-10-26 7:55 UTC (permalink / raw)
To: Anil Kaushik; +Cc: mptcp
Hi Anil,
Thank you for your modifications, that's great!
Our CI did some validations and here is its report:
- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Perf: Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/40283088555
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/2501a105211b
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1193404
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)
^ permalink raw reply [flat|nested] 12+ messages in thread