* [PATCH mptcp-net v5 0/2] mptcp: pm: in-kernel: fullmesh: re-add of the initial subflow endp
@ 2026-10-21 7:03 Abdul Wasey
2026-10-21 7:03 ` [PATCH mptcp-net v5 1/2] mptcp: pm: in-kernel: fullmesh: skip closed subflows Abdul Wasey
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Abdul Wasey @ 2026-10-21 7:03 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts, Mat Martineau, Geliang Tang, Paolo Abeni
With fullmesh endpoints, removing and re-adding the endpoint used by the
initial subflow never creates a new subflow from it: the closed initial
subflow stays in the conn_list, and makes the PM think the remote ID 0
is already used, or picks its reset remote address. If the initial
subflow was the only one, no address is left for the remote ID 0 at all.
Patch 1 fixes that, patch 2 adds tests for it.
Found with a router flapping one of its links: the link carrying the
initial subflow never rejoined the existing connections. Tested on top
of export-net: the new tests fail without patch 1, except the one adding
another endpoint, which already worked and guards against breaking it.
The whole mptcp_join.sh passes with patch 1. Patch 2 uses iptables like
the other tests there: when merged with net-next, it will need the same
conversion to nftables as the others.
Changes in v5:
- Patch 1: when no subflow is left to take the remote ID 0 address from,
use the remote address of the MPTCP socket, as fill_remote_addr()
does. Without it, v4 also broke adding another endpoint after the
removal of the initial one, when it was the only subflow (Sashiko on
netdev).
- Patch 2: add two tests for the initial subflow alone: re-adding its
endpoint, and adding another endpoint.
- v4: https://lore.kernel.org/mptcp/20261019190558.465270-1-w453y.me@gmail.com/
Changes in v4:
- Patch 2: only check the joins when the iptables rule has been added
(Sashiko).
- v3: https://lore.kernel.org/mptcp/20261019185512.458313-1-w453y.me@gmail.com/
Changes in v3:
- Patch 2: use iptables instead of init_nftables(), which is only in
net-next (Sashiko).
- v2: https://lore.kernel.org/mptcp/20261019180306.382039-1-w453y.me@gmail.com/
Changes in v2:
- Patch 1: also skip the subflows in TCP_CLOSE: the initial subflow can
be disconnected and its context reset, clearing the 'closing' flag and
zeroing its remote address (Sashiko).
- Patch 2: validate this case too, by resetting the initial subflow
before removing its endpoint.
- v1: https://lore.kernel.org/mptcp/20261019170751.200446-1-w453y.me@gmail.com/
Abdul Wasey (2):
mptcp: pm: in-kernel: fullmesh: skip closed subflows
selftests: mptcp: join: re-add the initial subflow endp with fullmesh
net/mptcp/pm_kernel.c | 33 +++++-
.../testing/selftests/net/mptcp/mptcp_join.sh | 108 ++++++++++++++++++
2 files changed, 140 insertions(+), 1 deletion(-)
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH mptcp-net v5 1/2] mptcp: pm: in-kernel: fullmesh: skip closed subflows
2026-10-21 7:03 [PATCH mptcp-net v5 0/2] mptcp: pm: in-kernel: fullmesh: re-add of the initial subflow endp Abdul Wasey
@ 2026-10-21 7:03 ` Abdul Wasey
2026-10-22 20:10 ` netdev-bot+sashiko
2026-10-21 7:03 ` [PATCH mptcp-net v5 2/2] selftests: mptcp: join: re-add the initial subflow endp with fullmesh Abdul Wasey
2026-10-21 13:21 ` [PATCH mptcp-net v5 0/2] mptcp: pm: in-kernel: fullmesh: re-add of the initial subflow endp MPTCP CI
2 siblings, 1 reply; 6+ messages in thread
From: Abdul Wasey @ 2026-10-21 7:03 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts, Mat Martineau, Geliang Tang, Paolo Abeni, stable
With fullmesh endpoints, if the endpoint used by the initial subflow is
removed and then added back, no new subflow is created from it: the
connection never uses this local address again.
The initial subflow is kept in the conn_list once closed. If it was
still established when closed, it keeps the local ID 0, which is also
the ID used by the re-added endpoint, and fill_remote_addresses_fullmesh()
then marks its remote ID as unavailable. If it was already in TCP_CLOSE,
e.g. after a reset, it is disconnected and its context is reset: the
'closing' flag and the remote ID are cleared, and its remote address
and port are zeroed. It is then picked as a remote address, with the
address 0.0.0.0, the port 0 and the ID 0, which also marks the remote
ID 0 as unavailable. In both cases, when the peer has a single address,
all subflows use the remote ID 0, and no valid remote address is left.
Skip the subflows being closed or in TCP_CLOSE state, both when looking
for the remote IDs already in use and when picking the remote addresses.
__mptcp_subflow_active() cannot be used here: subflows still being
established have to keep their remote ID marked as in use, to avoid
creating duplicated subflows.
When no subflow is left to take the remote ID 0 address from, e.g. when
the initial subflow was the only one, use the remote address of the MPTCP
socket, as fill_remote_addr() does for non-fullmesh endpoints. Before,
re-adding the initial endpoint created no subflow in this case either,
and another endpoint only worked by picking the closed initial subflow.
Without the fullmesh flag, the remote address is taken from the MPTCP
socket, so this issue is specific to fullmesh endpoints.
Fixes: 045e9d812868 ("mptcp: fix duplicate subflow creation")
Cc: stable@vger.kernel.org
Signed-off-by: Abdul Wasey <w453y.me@gmail.com>
---
net/mptcp/pm_kernel.c | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 1a7750813..c240c1056 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -199,6 +199,16 @@ fill_remote_addr(struct mptcp_sock *msk, struct mptcp_addr_info *local,
return 1;
}
+/* Closed subflows can stay in the conn_list, e.g. the initial one: their
+ * addresses and IDs are not reliable, possibly even reset.
+ */
+static bool mptcp_subflow_is_closed(struct mptcp_subflow_context *subflow)
+{
+ struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+
+ return subflow->closing || inet_sk_state_load(ssk) == TCP_CLOSE;
+}
+
static unsigned int
fill_remote_addresses_fullmesh(struct mptcp_sock *msk,
struct mptcp_addr_info *local,
@@ -216,11 +226,18 @@ fill_remote_addresses_fullmesh(struct mptcp_sock *msk,
* already created by incoming ADD_ADDR
*/
bitmap_zero(unavail_id, MPTCP_PM_MAX_ADDR_ID + 1);
- mptcp_for_each_subflow(msk, subflow)
+ mptcp_for_each_subflow(msk, subflow) {
+ if (mptcp_subflow_is_closed(subflow))
+ continue;
+
if (READ_ONCE(subflow->local_id) == local->id)
__set_bit(subflow->remote_id, unavail_id);
+ }
mptcp_for_each_subflow(msk, subflow) {
+ if (mptcp_subflow_is_closed(subflow))
+ continue;
+
ssk = mptcp_subflow_tcp_sock(subflow);
mptcp_remote_address((struct sock_common *)ssk, &addrs[i]);
addrs[i].id = READ_ONCE(subflow->remote_id);
@@ -243,6 +260,20 @@ fill_remote_addresses_fullmesh(struct mptcp_sock *msk,
break;
}
+ /* The initial subflow can be closed: like fill_remote_addr(), use the
+ * remote address of the MPTCP socket for the remote ID 0.
+ */
+ if (!deny_id0 && !test_bit(0, unavail_id) && i < addrs_size &&
+ msk->pm.extra_subflows < limit_extra_subflows) {
+ struct mptcp_addr_info remote = { 0 };
+
+ mptcp_remote_address((struct sock_common *)sk, &remote);
+ if (mptcp_pm_addr_families_match(sk, local, &remote)) {
+ addrs[i++] = remote;
+ msk->pm.extra_subflows++;
+ }
+ }
+
return i;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH mptcp-net v5 2/2] selftests: mptcp: join: re-add the initial subflow endp with fullmesh
2026-10-21 7:03 [PATCH mptcp-net v5 0/2] mptcp: pm: in-kernel: fullmesh: re-add of the initial subflow endp Abdul Wasey
2026-10-21 7:03 ` [PATCH mptcp-net v5 1/2] mptcp: pm: in-kernel: fullmesh: skip closed subflows Abdul Wasey
@ 2026-10-21 7:03 ` Abdul Wasey
2026-10-22 20:10 ` netdev-bot+sashiko
2026-10-21 13:21 ` [PATCH mptcp-net v5 0/2] mptcp: pm: in-kernel: fullmesh: re-add of the initial subflow endp MPTCP CI
2 siblings, 1 reply; 6+ messages in thread
From: Abdul Wasey @ 2026-10-21 7:03 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts, Mat Martineau, Geliang Tang, Paolo Abeni, stable
The "delete and re-add" test validates the removal and the re-addition
of the endpoint used by the initial subflow, but without the fullmesh
flag.
Do the same with the fullmesh flag: with a second subflow, after a reset
of the initial subflow, and with the initial subflow only, then re-adding
its endpoint or adding another one. Without the previous patch, no new
subflow is created, except when adding another endpoint.
The 'Fixes' tag here below is the same as the one from the previous
commit: this patch here is not fixing anything wrong in the selftests,
but it validates the previous fix for an issue introduced by this commit
ID.
Fixes: 045e9d812868 ("mptcp: fix duplicate subflow creation")
Cc: stable@vger.kernel.org
Signed-off-by: Abdul Wasey <w453y.me@gmail.com>
---
.../testing/selftests/net/mptcp/mptcp_join.sh | 108 ++++++++++++++++++
1 file changed, 108 insertions(+)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 18ce7136a..6d2cc01ac 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -4445,6 +4445,114 @@ endpoint_tests()
chk_rm_nr 4 4
fi
+ # remove and re-add the endpoint of the initial subflow, with fullmesh
+ if reset_with_events "delete and re-add, fullmesh" &&
+ continue_if mptcp_lib_kallsyms_has "subflow_rebuild_header$"; then
+ pm_nl_set_limits $ns1 0 2
+ pm_nl_set_limits $ns2 0 2
+ pm_nl_add_endpoint $ns2 10.0.1.2 id 1 dev ns2eth1 flags subflow,fullmesh
+ pm_nl_add_endpoint $ns2 10.0.2.2 id 2 dev ns2eth2 flags subflow,fullmesh
+ { timeout_test=120 test_linkfail=128 speed=5 \
+ run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
+ local tests_pid=$!
+
+ wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
+ wait_mpj 1
+ chk_subflow_nr "before delete id 0" 2
+
+ pm_nl_del_endpoint $ns2 1 10.0.1.2
+ sleep 0.5
+ chk_subflow_nr "after delete id 0" 1
+
+ pm_nl_add_endpoint $ns2 10.0.1.2 id 1 dev ns2eth1 flags subflow,fullmesh
+ wait_mpj 2
+ chk_subflow_nr "after re-add id 0" 2
+
+ mptcp_lib_kill_group_wait $tests_pid
+ kill_events_pids
+
+ chk_join_nr 2 2 2
+ chk_rm_nr 1 1
+ fi
+
+ # same, but the initial subflow is reset before removing its endpoint
+ if reset_with_events "delete and re-add, fullmesh, after reset" &&
+ continue_if mptcp_lib_kallsyms_has "subflow_rebuild_header$"; then
+ pm_nl_set_limits $ns1 0 2
+ pm_nl_set_limits $ns2 0 2
+ pm_nl_add_endpoint $ns2 10.0.1.2 id 1 dev ns2eth1 flags subflow,fullmesh
+ pm_nl_add_endpoint $ns2 10.0.2.2 id 2 dev ns2eth2 flags subflow,fullmesh
+ { timeout_test=120 test_linkfail=128 speed=5 \
+ run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
+ local tests_pid=$!
+
+ wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
+ wait_mpj 1
+ chk_subflow_nr "before reset" 2
+
+ if ip netns exec "${ns1}" ${iptables} -A INPUT -s "10.0.1.2" \
+ -p tcp --tcp-option 30 -j REJECT --reject-with tcp-reset; then
+ wait_event ns2 MPTCP_LIB_EVENT_SUB_CLOSED 1
+ ip netns exec "${ns1}" ${iptables} -F INPUT
+
+ pm_nl_del_endpoint $ns2 1 10.0.1.2
+ pm_nl_add_endpoint $ns2 10.0.1.2 id 1 dev ns2eth1 flags subflow,fullmesh
+ wait_mpj 2
+ chk_mptcp_info subflows 2 subflows 2
+ chk_join_nr 2 2 2
+ fi
+
+ mptcp_lib_kill_group_wait $tests_pid
+ kill_events_pids
+ fi
+
+ # only the initial subflow: once its endpoint is removed, no live subflow
+ # is left to take the remote address from
+ if reset_with_events "delete and re-add, fullmesh, single subflow" &&
+ continue_if mptcp_lib_kallsyms_has "subflow_rebuild_header$"; then
+ pm_nl_set_limits $ns1 0 2
+ pm_nl_set_limits $ns2 0 2
+ pm_nl_add_endpoint $ns2 10.0.1.2 id 1 dev ns2eth1 flags subflow,fullmesh
+ { timeout_test=120 test_linkfail=128 speed=5 \
+ run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
+ local tests_pid=$!
+
+ wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
+ pm_nl_del_endpoint $ns2 1 10.0.1.2
+ sleep 0.5
+ pm_nl_add_endpoint $ns2 10.0.1.2 id 1 dev ns2eth1 flags subflow,fullmesh
+ wait_mpj 1
+ chk_mptcp_info subflows 1 subflows 1
+
+ mptcp_lib_kill_group_wait $tests_pid
+ kill_events_pids
+
+ chk_join_nr 1 1 1
+ fi
+
+ # same, but another endpoint is added after the removal
+ if reset_with_events "delete and add another, fullmesh, single subflow" &&
+ continue_if mptcp_lib_kallsyms_has "subflow_rebuild_header$"; then
+ pm_nl_set_limits $ns1 0 2
+ pm_nl_set_limits $ns2 0 2
+ pm_nl_add_endpoint $ns2 10.0.1.2 id 1 dev ns2eth1 flags subflow,fullmesh
+ { timeout_test=120 test_linkfail=128 speed=5 \
+ run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
+ local tests_pid=$!
+
+ wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
+ pm_nl_del_endpoint $ns2 1 10.0.1.2
+ sleep 0.5
+ pm_nl_add_endpoint $ns2 10.0.2.2 id 2 dev ns2eth2 flags subflow,fullmesh
+ wait_mpj 1
+ chk_mptcp_info subflows 1 subflows 1
+
+ mptcp_lib_kill_group_wait $tests_pid
+ kill_events_pids
+
+ chk_join_nr 1 1 1
+ fi
+
# remove and re-add
if reset_with_events "delete re-add signal" &&
continue_if mptcp_lib_kallsyms_has "subflow_rebuild_header$"; then
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH mptcp-net v5 0/2] mptcp: pm: in-kernel: fullmesh: re-add of the initial subflow endp
2026-10-21 7:03 [PATCH mptcp-net v5 0/2] mptcp: pm: in-kernel: fullmesh: re-add of the initial subflow endp Abdul Wasey
2026-10-21 7:03 ` [PATCH mptcp-net v5 1/2] mptcp: pm: in-kernel: fullmesh: skip closed subflows Abdul Wasey
2026-10-21 7:03 ` [PATCH mptcp-net v5 2/2] selftests: mptcp: join: re-add the initial subflow endp with fullmesh Abdul Wasey
@ 2026-10-21 13:21 ` MPTCP CI
2 siblings, 0 replies; 6+ messages in thread
From: MPTCP CI @ 2026-10-21 13:21 UTC (permalink / raw)
To: Abdul Wasey; +Cc: mptcp
Hi Abdul,
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: Critical: No conclusion ❓
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/39580754081
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/2a9b455644b4
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1190258
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] 6+ messages in thread
* Re: [PATCH mptcp-net v5 1/2] mptcp: pm: in-kernel: fullmesh: skip closed subflows
2026-10-21 7:03 ` [PATCH mptcp-net v5 1/2] mptcp: pm: in-kernel: fullmesh: skip closed subflows Abdul Wasey
@ 2026-10-22 20:10 ` netdev-bot+sashiko
0 siblings, 0 replies; 6+ messages in thread
From: netdev-bot+sashiko @ 2026-10-22 20:10 UTC (permalink / raw)
To: w453y.me; +Cc: mptcp, matttbe, martineau, geliang, pabeni, stable, kuba
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The commit message gets the failure mechanism partly wrong for
the case where the initial subflow is already in TCP_CLOSE.
--
mptcp: pm: in-kernel: fullmesh: skip closed subflows
This patch changes fill_remote_addresses_fullmesh() to skip subflows that
are closing or in TCP_CLOSE. They are skipped both when collecting the
remote IDs in use and when picking remote addresses. If no subflow is left
to provide the remote ID 0 address, the remote address of the MPTCP socket
is used instead, as fill_remote_addr() does.
> If it was already in TCP_CLOSE,
> e.g. after a reset, it is disconnected and its context is reset: the
> 'closing' flag and the remote ID are cleared, and its remote address
> and port are zeroed. It is then picked as a remote address, with the
> address 0.0.0.0, the port 0 and the ID 0, which also marks the remote
> ID 0 as unavailable.
[Severity: Low]
Is this description of the TCP_CLOSE case accurate?
>From __mptcp_close_ssk(), the initial subflow stays in the conn_list and
goes through __mptcp_subflow_disconnect():
net/mptcp/protocol.c:__mptcp_subflow_disconnect() {
...
WARN_ON_ONCE(tcp_disconnect(ssk, 0));
mptcp_subflow_ctx_reset(subflow);
...
}
On that path, tcp_disconnect() resets only the destination port and the
source address:
net/ipv4/tcp.c:tcp_disconnect() {
...
inet->inet_dport = 0;
inet_bhash2_reset_saddr(sk);
...
}
inet_bhash2_reset_saddr() ends up in inet_reset_saddr(). That clears
inet_saddr, inet_rcv_saddr and the IPv6 source addresses, but not
sk_daddr or skc_v6_daddr. mptcp_subflow_ctx_reset() clears the reset
group (closing, remote_id, ...) and sets local_id to -1. It does not
touch the socket addresses.
So in the old fill_remote_addresses_fullmesh(), mptcp_remote_address(ssk)
would return the real peer address with port 0 and remote ID 0, not
0.0.0.0:0.
The conclusion still holds, since port 0 is unusable and remote ID 0
still gets marked as unavailable. Could the changelog say that only the
destination port, the source address and the IDs are reset, and that the
peer address is kept?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261021070324.50533-1-w453y.me%40gmail.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH mptcp-net v5 2/2] selftests: mptcp: join: re-add the initial subflow endp with fullmesh
2026-10-21 7:03 ` [PATCH mptcp-net v5 2/2] selftests: mptcp: join: re-add the initial subflow endp with fullmesh Abdul Wasey
@ 2026-10-22 20:10 ` netdev-bot+sashiko
0 siblings, 0 replies; 6+ messages in thread
From: netdev-bot+sashiko @ 2026-10-22 20:10 UTC (permalink / raw)
To: w453y.me; +Cc: mptcp, matttbe, martineau, geliang, pabeni, stable, kuba
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] The two new single-subflow subtests, "delete and re-add,
fullmesh, single subflow" (lines 4520-4523) and "delete and add…
- [Low] In the new subtest "delete and re-add, fullmesh, after reset"
(endpoint_tests() in tools/testing/selftests/net/mptcp/mptcp_join.sh),…
--
selftests: mptcp: join: re-add the initial subflow endp with fullmesh
This commit adds four subtests to endpoint_tests() in mptcp_join.sh. Each
one removes and re-adds the endpoint used by the initial subflow, with the
fullmesh flag set. They cover three cases: a second subflow is present,
the initial subflow is reset first, and only the initial subflow exists
(re-adding the same endpoint or adding another one).
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 18ce7136a2b0e..6d2cc01ac4492 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -4445,6 +4445,114 @@ endpoint_tests()
[ ... ]
> + # same, but the initial subflow is reset before removing its endpoint
> + if reset_with_events "delete and re-add, fullmesh, after reset" &&
> + continue_if mptcp_lib_kallsyms_has "subflow_rebuild_header$"; then
[ ... ]
> + wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
> + wait_mpj 1
> + chk_subflow_nr "before reset" 2
> +
> + if ip netns exec "${ns1}" ${iptables} -A INPUT -s "10.0.1.2" \
> + -p tcp --tcp-option 30 -j REJECT --reject-with tcp-reset; then
> + wait_event ns2 MPTCP_LIB_EVENT_SUB_CLOSED 1
> + ip netns exec "${ns1}" ${iptables} -F INPUT
> +
> + pm_nl_del_endpoint $ns2 1 10.0.1.2
> + pm_nl_add_endpoint $ns2 10.0.1.2 id 1 dev ns2eth1 flags subflow,fullmesh
> + wait_mpj 2
> + chk_mptcp_info subflows 2 subflows 2
> + chk_join_nr 2 2 2
> + fi
[Severity: Low]
What happens if the iptables rule can't be installed here? For example,
the REJECT target or the tcp-option match might be unavailable.
There is no else branch. In that case the SUB_CLOSED wait, the endpoint
delete and re-add, wait_mpj 2, chk_mptcp_info and chk_join_nr are all
skipped. Nothing calls mark_as_skipped() or fail_test(), so
append_prev_results() reports the subtest as a pass even though only
chk_subflow_nr "before reset" 2 ran.
The rule is the reason this subtest exists: it is there to exercise the
TCP_CLOSE branch of mptcp_subflow_is_closed(). Should it mark the test as
skipped, the way reset_with_tcp_filter() does?
reset_with_tcp_filter() {
...
if ! ip netns exec "${ns}" ${iptables} \
-A "${chain}" \
-s "${src}" \
-p tcp \
-j "${target}"; then
mark_as_skipped "unable to set the filter rules"
return 1
fi
...
}
The "userspace pm no dup close events after error" subtest uses a
similar bare if. Commit 560edd99b5f5 accepted that because another subtest
that depends fully on iptables would be marked as skipped. Nothing like
that covers this case.
> +
> + mptcp_lib_kill_group_wait $tests_pid
> + kill_events_pids
> + fi
> +
> + # only the initial subflow: once its endpoint is removed, no live subflow
> + # is left to take the remote address from
> + if reset_with_events "delete and re-add, fullmesh, single subflow" &&
> + continue_if mptcp_lib_kallsyms_has "subflow_rebuild_header$"; then
> + pm_nl_set_limits $ns1 0 2
> + pm_nl_set_limits $ns2 0 2
> + pm_nl_add_endpoint $ns2 10.0.1.2 id 1 dev ns2eth1 flags subflow,fullmesh
> + { timeout_test=120 test_linkfail=128 speed=5 \
> + run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
> + local tests_pid=$!
> +
> + wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
> + pm_nl_del_endpoint $ns2 1 10.0.1.2
> + sleep 0.5
> + pm_nl_add_endpoint $ns2 10.0.1.2 id 1 dev ns2eth1 flags subflow,fullmesh
> + wait_mpj 1
> + chk_mptcp_info subflows 1 subflows 1
[Severity: Medium]
Is the ESTABLISHED event enough synchronization before deleting endpoint
id 1?
mptcp_pm_fully_established() only schedules the PM work, then emits the
event:
net/mptcp/pm.c:mptcp_pm_fully_established() {
...
if (READ_ONCE(pm->work_pending) &&
!(pm->status & BIT(MPTCP_PM_ALREADY_ESTABLISHED)))
mptcp_pm_schedule_work(msk, MPTCP_PM_ESTABLISHED);
...
if (announce)
mptcp_event(MPTCP_EVENT_ESTABLISHED, msk, ssk, GFP_ATOMIC);
}
msk->mpc_endpoint_id is set later, from the worker, in
mptcp_mpc_endpoint_setup():
entry = __lookup_addr(pernet, &mpc_addr);
if (entry) {
__clear_bit(entry->addr.id, msk->pm.id_avail_bitmap);
msk->mpc_endpoint_id = entry->addr.id;
...
}
Suppose pm_nl_del_endpoint runs before the worker. Then this call in
mptcp_nl_remove_subflow_and_signal_addr() returns 1 rather than 0:
list.ids[0] = mptcp_endp_get_local_id(msk, addr);
mptcp_pm_rm_addr_or_subflow() then skips the initial subflow, because its
local id is 0, so the subflow stays established. When the worker runs it
finds no endpoint for 10.0.1.2, and mpc_endpoint_id stays 0.
In this re-add variant, the re-add then maps endpoint 1 to local id 0.
The initial subflow is still alive and keeps remote id 0 unavailable, so
no new subflow is created. Could wait_mpj 1, chk_mptcp_info subflows 1
and chk_join_nr 1 1 1 then fail spuriously?
The "delete and add another, fullmesh, single subflow" subtest below has
the same race. There, endpoint 2 would still join using the remote
address of the live initial subflow. The test would pass without covering
the case where no subflow is left.
The two-subflow subtests avoid this, because their wait_mpj 1 runs after
the worker has called mptcp_mpc_endpoint_setup(). Neither single-subflow
subtest checks the subflow count after the delete, the way the first new
subtest does with chk_subflow_nr.
The window is probably small, but nothing seems to prevent it on loaded
CI machines or debug kernels. Should these subtests wait for the PM
worker, or check that the initial subflow is gone, before re-adding the
endpoint?
> +
> + mptcp_lib_kill_group_wait $tests_pid
> + kill_events_pids
> +
> + chk_join_nr 1 1 1
> + fi
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261021070324.50533-1-w453y.me%40gmail.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-22 20:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-21 7:03 [PATCH mptcp-net v5 0/2] mptcp: pm: in-kernel: fullmesh: re-add of the initial subflow endp Abdul Wasey
2026-10-21 7:03 ` [PATCH mptcp-net v5 1/2] mptcp: pm: in-kernel: fullmesh: skip closed subflows Abdul Wasey
2026-10-22 20:10 ` netdev-bot+sashiko
2026-10-21 7:03 ` [PATCH mptcp-net v5 2/2] selftests: mptcp: join: re-add the initial subflow endp with fullmesh Abdul Wasey
2026-10-22 20:10 ` netdev-bot+sashiko
2026-10-21 13:21 ` [PATCH mptcp-net v5 0/2] mptcp: pm: in-kernel: fullmesh: re-add of the initial subflow endp MPTCP CI
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.