* [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint
@ 2026-05-29 6:43 Kalpan Jani
2026-05-29 6:55 ` Matthieu Baerts
2026-05-29 7:58 ` MPTCP CI
0 siblings, 2 replies; 12+ messages in thread
From: Kalpan Jani @ 2026-05-29 6:43 UTC (permalink / raw)
To: mptcp
Cc: matttbe, martineau, pabeni, shardul.b, janak, kalpanjani009,
shardulsb08, syzbot+55c2a5c871441261ed14, Kalpan Jani
syzkaller hit the WARN_ON_ONCE() in mptcp_pm_alloc_anno_list() with
the in-kernel path manager.
When a signal endpoint is removed, the pending ADD_ADDR has to be
cancelled: its retransmit timer stopped and the anno_list entry
unlinked and freed. For a non-zero id endpoint this is done via
mptcp_nl_remove_subflow_and_signal_addr() ->
mptcp_pm_remove_anno_addr() -> mptcp_remove_anno_list_by_saddr().
The id 0 removal path, mptcp_nl_remove_id_zero_address(), does not do
this: it only queues a RM_ADDR and marks the id available again, but
leaves any pending anno_list entry and its armed retransmit timer
alive.
So when the id 0 endpoint is removed and re-added while its previously
sent ADD_ADDR is still awaiting the echo, the stale entry survives.
The kernel PM reselects id 0, reaches mptcp_pm_alloc_anno_list() a
second time, finds the stale entry and hits the WARN.
Make the id 0 removal path symmetric with the non-zero one: drop the
pending ADD_ADDR before queuing the RM_ADDR, and decrement
add_addr_signaled if the address had been announced. This closes the
race at its source, so the WARN_ON_ONCE() stays a valid assertion.
Fixes: 740d798e8767 ("mptcp: remove id 0 address")
Reported-by: syzbot+55c2a5c871441261ed14@syzkaller.appspotmail.com
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/620
Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com>
---
v1 -> v2:
- Do not drop the WARN_ON_ONCE(); it is a valid assertion.
(Matthieu Baerts)
- Fix mptcp_nl_remove_id_zero_address() to tear down the pending
ADD_ADDR entry: call mptcp_remove_anno_list_by_saddr() and
decrement add_addr_signaled, mirroring the non-zero id path.
(Matthieu Baerts)
net/mptcp/pm_kernel.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index fc818b63752e..ed0ad1c1b140 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -1126,6 +1126,7 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
struct sock *sk = (struct sock *)msk;
struct mptcp_addr_info msk_local;
+ bool announced;
if (list_empty(&msk->conn_list) || mptcp_pm_is_userspace(msk))
goto next;
@@ -1135,7 +1136,16 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
goto next;
lock_sock(sk);
+ /* Drop a possibly pending ADD_ADDR for this address: stop its
+ * retransmit timer and unlink the anno_list entry, so a later
+ * re-add cannot find a stale entry and hit the WARN_ON_ONCE()
+ * in mptcp_pm_alloc_anno_list(). Mirrors the non-zero id path
+ * in mptcp_pm_remove_anno_addr().
+ */
+ announced = mptcp_remove_anno_list_by_saddr(msk, &msk_local);
spin_lock_bh(&msk->pm.lock);
+ if (announced)
+ msk->pm.add_addr_signaled--;
mptcp_pm_remove_addr(msk, &list);
mptcp_pm_rm_subflow(msk, &list);
__mark_subflow_endp_available(msk, 0);
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint 2026-05-29 6:43 [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint Kalpan Jani @ 2026-05-29 6:55 ` Matthieu Baerts 2026-05-29 7:18 ` Kalpan Jani 2026-05-29 7:58 ` MPTCP CI 1 sibling, 1 reply; 12+ messages in thread From: Matthieu Baerts @ 2026-05-29 6:55 UTC (permalink / raw) To: Kalpan Jani Cc: mptcp, martineau, pabeni, shardul.b, janak, kalpanjani009, shardulsb08, syzbot+55c2a5c871441261ed14 Hi Kalpan, Thank you for the v2. 29 May 2026 16:44:12 Kalpan Jani <kalpan.jani@mpiricsoftware.com>: > syzkaller hit the WARN_ON_ONCE() in mptcp_pm_alloc_anno_list() with > the in-kernel path manager. > > When a signal endpoint is removed, the pending ADD_ADDR has to be > cancelled: its retransmit timer stopped and the anno_list entry > unlinked and freed. For a non-zero id endpoint this is done via > mptcp_nl_remove_subflow_and_signal_addr() -> > mptcp_pm_remove_anno_addr() -> mptcp_remove_anno_list_by_saddr(). > > The id 0 removal path, mptcp_nl_remove_id_zero_address(), does not do > this: it only queues a RM_ADDR and marks the id available again, but > leaves any pending anno_list entry and its armed retransmit timer > alive. > > So when the id 0 endpoint is removed and re-added while its previously > sent ADD_ADDR is still awaiting the echo, the stale entry survives. > The kernel PM reselects id 0, reaches mptcp_pm_alloc_anno_list() a > second time, finds the stale entry and hits the WARN. > > Make the id 0 removal path symmetric with the non-zero one: drop the > pending ADD_ADDR before queuing the RM_ADDR, and decrement > add_addr_signaled if the address had been announced. This closes the > race at its source, so the WARN_ON_ONCE() stays a valid assertion. > > Fixes: 740d798e8767 ("mptcp: remove id 0 address") > Reported-by: syzbot+55c2a5c871441261ed14@syzkaller.appspotmail.com > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/620 > Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com> > --- > > v1 -> v2: > - Do not drop the WARN_ON_ONCE(); it is a valid assertion. > (Matthieu Baerts) > - Fix mptcp_nl_remove_id_zero_address() to tear down the pending > ADD_ADDR entry: call mptcp_remove_anno_list_by_saddr() and > decrement add_addr_signaled, mirroring the non-zero id path. > (Matthieu Baerts) > > net/mptcp/pm_kernel.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c > index fc818b63752e..ed0ad1c1b140 100644 > --- a/net/mptcp/pm_kernel.c > +++ b/net/mptcp/pm_kernel.c > @@ -1126,6 +1126,7 @@ static int mptcp_nl_remove_id_zero_address(struct net *net, > while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) { > struct sock *sk = (struct sock *)msk; > struct mptcp_addr_info msk_local; > + bool announced; > > if (list_empty(&msk->conn_list) || mptcp_pm_is_userspace(msk)) > goto next; > @@ -1135,7 +1136,16 @@ static int mptcp_nl_remove_id_zero_address(struct net *net, > goto next; > > lock_sock(sk); > + /* Drop a possibly pending ADD_ADDR for this address: stop its > + * retransmit timer and unlink the anno_list entry, so a later > + * re-add cannot find a stale entry and hit the WARN_ON_ONCE() > + * in mptcp_pm_alloc_anno_list(). Mirrors the non-zero id path > + * in mptcp_pm_remove_anno_addr(). > + */ I'm not sure such a long comment in the code is that useful, and it might: it can quickly become outdated when other parts are modified. Limit it to the first line, the rest can be seen in the commit message. > + announced = mptcp_remove_anno_list_by_saddr(msk, &msk_local); > spin_lock_bh(&msk->pm.lock); > + if (announced) > + msk->pm.add_addr_signaled--; It would be good to check this counter on the test/reproducer you will provide. Did it work with packetdrill? Cheers, Matt > mptcp_pm_remove_addr(msk, &list); > mptcp_pm_rm_subflow(msk, &list); > __mark_subflow_endp_available(msk, 0); > -- > 2.43.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint 2026-05-29 6:55 ` Matthieu Baerts @ 2026-05-29 7:18 ` Kalpan Jani 2026-05-31 6:39 ` Matthieu Baerts 0 siblings, 1 reply; 12+ messages in thread From: Kalpan Jani @ 2026-05-29 7:18 UTC (permalink / raw) To: Matthieu Baerts Cc: mptcp, martineau, pabeni, shardul.b, janak, kalpanjani009, shardulsb08, syzbot+55c2a5c871441261ed14 Hi Matt, > Did it work with packetdrill? I worked on a packetdrill test based on add_addr4_server.pkt with the MP_JOIN extra subflow and the delete/re-add sequence. The environment is working (add_addr4_server.pkt passes cleanly), but I was not able to reliably trigger the WARN through packetdrill within the test timeframe. The difficulty is that the id 0 ADD_ADDR is only sent when the endpoint is re-added after the connection is established AND after mptcp_nl_remove_id_zero_address() has run to restore the id_avail_bitmap bit via __mark_subflow_endp_available(). Getting packetdrill to control this timing precisely — while also keeping conn_list non-empty so mptcp_nl_remove_id_zero_address() actually processes the msk — proved tricky. syzbot is reproducing the bug reliably on current upstream (3 crashes between 2026/05/19 and 2026/05/22 on kernels including cca95436be15). The syzbot reproducer is the most reliable pre-fix evidence available. I will continue working on the packetdrill test and send it as a follow-up once I have a clean before/after result. For v3 I have addressed your comment shortening request. Thanks, Kalpan Jani From: Matthieu Baerts <matttbe@kernel.org> To: "Kalpan Jani"<kalpan.jani@mpiricsoftware.com> Cc: <mptcp@lists.linux.dev>, <martineau@kernel.org>, <pabeni@redhat.com>, <shardul.b@mpiricsoftware.com>, <janak@mpiric.us>, <kalpanjani009@gmail.com>, <shardulsb08@gmail.com>, <syzbot+55c2a5c871441261ed14@syzkaller.appspotmail.com> Date: Fri, 29 May 2026 12:25:09 +0530 Subject: Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint > Hi Kalpan, > > Thank you for the v2. > > 29 May 2026 16:44:12 Kalpan Jani <kalpan.jani@mpiricsoftware.com>: > > > syzkaller hit the WARN_ON_ONCE() in mptcp_pm_alloc_anno_list() with > > the in-kernel path manager. > > > > When a signal endpoint is removed, the pending ADD_ADDR has to be > > cancelled: its retransmit timer stopped and the anno_list entry > > unlinked and freed. For a non-zero id endpoint this is done via > > mptcp_nl_remove_subflow_and_signal_addr() -> > > mptcp_pm_remove_anno_addr() -> mptcp_remove_anno_list_by_saddr(). > > > > The id 0 removal path, mptcp_nl_remove_id_zero_address(), does not do > > this: it only queues a RM_ADDR and marks the id available again, but > > leaves any pending anno_list entry and its armed retransmit timer > > alive. > > > > So when the id 0 endpoint is removed and re-added while its previously > > sent ADD_ADDR is still awaiting the echo, the stale entry survives. > > The kernel PM reselects id 0, reaches mptcp_pm_alloc_anno_list() a > > second time, finds the stale entry and hits the WARN. > > > > Make the id 0 removal path symmetric with the non-zero one: drop the > > pending ADD_ADDR before queuing the RM_ADDR, and decrement > > add_addr_signaled if the address had been announced. This closes the > > race at its source, so the WARN_ON_ONCE() stays a valid assertion. > > > > Fixes: 740d798e8767 ("mptcp: remove id 0 address") > > Reported-by: syzbot+55c2a5c871441261ed14@syzkaller.appspotmail.com > > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/620 > > Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com> > > --- > > > > v1 -> v2: > > - Do not drop the WARN_ON_ONCE(); it is a valid assertion. > > (Matthieu Baerts) > > - Fix mptcp_nl_remove_id_zero_address() to tear down the pending > > ADD_ADDR entry: call mptcp_remove_anno_list_by_saddr() and > > decrement add_addr_signaled, mirroring the non-zero id path. > > (Matthieu Baerts) > > > > net/mptcp/pm_kernel.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c > > index fc818b63752e..ed0ad1c1b140 100644 > > --- a/net/mptcp/pm_kernel.c > > +++ b/net/mptcp/pm_kernel.c > > @@ -1126,6 +1126,7 @@ static int mptcp_nl_remove_id_zero_address(struct net *net, > > while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) { > > struct sock *sk = (struct sock *)msk; > > struct mptcp_addr_info msk_local; > > + bool announced; > > > > if (list_empty(&msk->conn_list) || mptcp_pm_is_userspace(msk)) > > goto next; > > @@ -1135,7 +1136,16 @@ static int mptcp_nl_remove_id_zero_address(struct net *net, > > goto next; > > > > lock_sock(sk); > > + /* Drop a possibly pending ADD_ADDR for this address: stop its > > + * retransmit timer and unlink the anno_list entry, so a later > > + * re-add cannot find a stale entry and hit the WARN_ON_ONCE() > > + * in mptcp_pm_alloc_anno_list(). Mirrors the non-zero id path > > + * in mptcp_pm_remove_anno_addr(). > > + */ > > I'm not sure such a long comment in the code is that useful, and it > might: it can quickly become outdated when other parts are modified. > Limit it to the first line, the rest can be seen in the commit message. > > > + announced = mptcp_remove_anno_list_by_saddr(msk, &msk_local); > > spin_lock_bh(&msk->pm.lock); > > + if (announced) > > + msk->pm.add_addr_signaled--; > > It would be good to check this counter on the test/reproducer you will > provide. Did it work with packetdrill? > > Cheers, > Matt > > > mptcp_pm_remove_addr(msk, &list); > > mptcp_pm_rm_subflow(msk, &list); > > __mark_subflow_endp_available(msk, 0); > > -- > > 2.43.0 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint 2026-05-29 7:18 ` Kalpan Jani @ 2026-05-31 6:39 ` Matthieu Baerts 2026-06-01 11:59 ` Kalpan Jani 0 siblings, 1 reply; 12+ messages in thread From: Matthieu Baerts @ 2026-05-31 6:39 UTC (permalink / raw) To: Kalpan Jani Cc: mptcp, martineau, pabeni, shardul.b, janak, kalpanjani009, shardulsb08, syzbot+55c2a5c871441261ed14 Hi Kalpan, On 29/05/2026 17:18, Kalpan Jani wrote: > Hi Matt, > >> Did it work with packetdrill? > > I worked on a packetdrill test based on add_addr4_server.pkt with the > MP_JOIN extra subflow and the delete/re-add sequence. The environment > is working (add_addr4_server.pkt passes cleanly), but I was not able > to reliably trigger the WARN through packetdrill within the test > timeframe. > > The difficulty is that the id 0 ADD_ADDR is only sent when the > endpoint is re-added after the connection is established AND after > mptcp_nl_remove_id_zero_address() has run to restore the > id_avail_bitmap bit via __mark_subflow_endp_available(). Getting > packetdrill to control this timing precisely — while also keeping > conn_list non-empty so mptcp_nl_remove_id_zero_address() actually > processes the msk — proved tricky. I understand it might be tricky, but do you mind sharing what you have so far. Maybe someone else can help you to reproduce the issue. Also, it might be enough to have a test that can reproduce the WARN not 100% of the time, as long as it is not 0. I will wait for this test before applying the v3. At least having something exercising new paths: here, having the new call to mptcp_remove_anno_list_by_saddr() returning true. > syzbot is reproducing the bug reliably on current upstream (3 crashes > between 2026/05/19 and 2026/05/22 on kernels including cca95436be15). > The syzbot reproducer is the most reliable pre-fix evidence available. Because syzbot didn't manage to generate a reproducer, we cannot really rely on syzbot to say that the bug is fixed. 3 crashes out of how many thousands/millions attempts? :) > I will continue working on the packetdrill test and send it as a > follow-up once I have a clean before/after result. For v3 I have > addressed your comment shortening request. Thanks! Do not forget to wait ~1 day between submissions, otherwise it is even harder for reviewers and various tools to follow the modifications and discussions. Cheers, Matt -- Sponsored by the NGI0 Core fund. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint 2026-05-31 6:39 ` Matthieu Baerts @ 2026-06-01 11:59 ` Kalpan Jani 2026-07-06 16:46 ` Matthieu Baerts 0 siblings, 1 reply; 12+ messages in thread From: Kalpan Jani @ 2026-06-01 11:59 UTC (permalink / raw) To: Matthieu Baerts Cc: mptcp, martineau, pabeni, shardul.b, janak, kalpanjani009, shardulsb08, syzbot+55c2a5c871441261ed14 [-- Attachment #1: Type: text/plain, Size: 5096 bytes --] Hi Matt, > do you mind sharing what you have so far. Maybe someone else can help > you to reproduce the issue. Attached is the WIP packetdrill test (server side, based on the existing add_addr/mp_join server tests, with an extra MP_JOIN subflow). I have not been able to reproduce the WARN with it yet, so please treat it as a work-in-progress rather than a finished test -- the status and the open question are written at the top of the file. The trigger is "ip mptcp endpoint delete id 0 <saddr>", which is the only op that reaches mptcp_nl_remove_id_zero_address(). (flush is not usable: it goes through mptcp_pm_flush_addrs_and_subflows(), which already calls mptcp_pm_announced_remove(), so that path was never buggy.) Where I am stuck is getting ADD_ADDR(id 0) announced in the first place. The signal endpoint on the MPC address has its id bit cleared by mptcp_mpc_endpoint_setup(), so it is not announced on connect. "delete id 0" frees the bit again, but I could not confirm that the PM worker re-runs and announces id 0 on an otherwise idle connection. Without that announcement no pending anno_list entry is created, so the second "delete id 0" has nothing stale to leave behind and the WARN does not trigger. So my question: what is the minimal way to get the id-0 address actually announced via packetdrill? Does this need the MP_JOIN subflow and/or a data exchange to wake the PM worker between the operations, or is there a simpler approach I am missing? > I will wait for this test before applying the v3. Understood, I will keep working on it. One thing I wanted to ask directly: is a deterministic packetdrill test a hard requirement for applying v3, or could the fix go in on the reasoning + the syzbot report alone? The change only makes the id-0 removal path symmetric with the non-zero id path (tear down the pending ADD_ADDR and decrement add_addr_signaled), so it is fairly contained and independent of whether I can script a reliable trigger. (Note: the anno-list helpers were renamed in current export, so v3 no longer applies as-is; I can send a trivially rebased v4 that swaps mptcp_remove_anno_list_by_saddr() for mptcp_pm_announced_remove() if useful.) Cheers, Kalpan Jani From: Matthieu Baerts <matttbe@kernel.org> To: "Kalpan Jani"<kalpan.jani@mpiricsoftware.com> Cc: "mptcp"<mptcp@lists.linux.dev>, "martineau"<martineau@kernel.org>, "pabeni"<pabeni@redhat.com>, "shardul.b"<shardul.b@mpiricsoftware.com>, "janak"<janak@mpiric.us>, "kalpanjani009"<kalpanjani009@gmail.com>, "shardulsb08"<shardulsb08@gmail.com>, "syzbot+55c2a5c871441261ed14"<syzbot+55c2a5c871441261ed14@syzkaller.appspotmail.com> Date: Sun, 31 May 2026 12:09:53 +0530 Subject: Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint > Hi Kalpan, > > On 29/05/2026 17:18, Kalpan Jani wrote: > > Hi Matt, > > > >> Did it work with packetdrill? > > > > I worked on a packetdrill test based on add_addr4_server.pkt with the > > MP_JOIN extra subflow and the delete/re-add sequence. The environment > > is working (add_addr4_server.pkt passes cleanly), but I was not able > > to reliably trigger the WARN through packetdrill within the test > > timeframe. > > > > The difficulty is that the id 0 ADD_ADDR is only sent when the > > endpoint is re-added after the connection is established AND after > > mptcp_nl_remove_id_zero_address() has run to restore the > > id_avail_bitmap bit via __mark_subflow_endp_available(). Getting > > packetdrill to control this timing precisely — while also keeping > > conn_list non-empty so mptcp_nl_remove_id_zero_address() actually > > processes the msk — proved tricky. > > I understand it might be tricky, but do you mind sharing what you have > so far. Maybe someone else can help you to reproduce the issue. Also, it > might be enough to have a test that can reproduce the WARN not 100% of > the time, as long as it is not 0. I will wait for this test before > applying the v3. > > At least having something exercising new paths: here, having the new > call to mptcp_remove_anno_list_by_saddr() returning true. > > > syzbot is reproducing the bug reliably on current upstream (3 crashes > > between 2026/05/19 and 2026/05/22 on kernels including cca95436be15). > > The syzbot reproducer is the most reliable pre-fix evidence available. > > Because syzbot didn't manage to generate a reproducer, we cannot really > rely on syzbot to say that the bug is fixed. 3 crashes out of how many > thousands/millions attempts? :) > > > I will continue working on the packetdrill test and send it as a > > follow-up once I have a clean before/after result. For v3 I have > > addressed your comment shortening request. > > Thanks! > > Do not forget to wait ~1 day between submissions, otherwise it is even > harder for reviewers and various tools to follow the modifications and > discussions. > > Cheers, > Matt > -- > Sponsored by the NGI0 Core fund. > > [-- Attachment #2: add_addr_id0_readd_server_WIP.pkt --] [-- Type: application/octet-stream, Size: 5538 bytes --] // SPDX-License-Identifier: GPL-2.0 // // ============================ STATUS: WIP ============================ // Work-in-progress reproducer for issue #620. It does NOT reproduce the // WARN yet; sharing it per the suggestion to post what I have so far so // others can help close the gap. Please do not treat it as a passing test. // // What it is meant to do: drive the in-kernel PM into reselecting address // ID 0 while a previously announced ADD_ADDR(id 0) is still pending (echo // withheld), which on an unfixed kernel leaves a stale anno_list entry and // trips WARN_ON_ONCE() in mptcp_pm_announced_alloc(). // // Where I am stuck: I have not been able to confirm that ADD_ADDR(id 0) is // actually announced in this setup. The signal endpoint on the MPC address // has its id bit cleared by mptcp_mpc_endpoint_setup(), so it is not // announced on connect; "delete id 0" frees the bit again, but I am not sure // the PM worker re-runs to announce id 0 on an otherwise idle connection. // Without that announcement no pending entry is created and the WARN cannot // trigger. The "> ..." kernel-reply lines below are therefore my best guess // and are NOT calibrated against captured output (in particular the DSS // data-ack values are placeholders). // // Open question: what is the minimal way to get the id-0 address announced // via packetdrill -- is the MP_JOIN subflow and/or an interleaved data // segment required to wake the PM worker between the operations? // // Reminder for anyone trying it: the trigger must be "endpoint delete id 0", // NOT "flush". Only the delete-id-0 op reaches mptcp_nl_remove_id_zero_address(); // flush goes through mptcp_pm_flush_addrs_and_subflows(), which already calls // mptcp_pm_announced_remove() and so was never buggy. // ===================================================================== // // Roles: packetdrill is the peer (caddr*); the kernel under test is local // (saddr0 == $OPT_LOCAL_IP) and runs the in-kernel PM as the server. --tolerance_usecs=750000 `../common/defaults.sh` +0 `../common/multi-ep.sh -e 0` +0 `sysctl -q net.mptcp.add_addr_timeout=0` +0.0 socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 3 +0.0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0 +0.0 fcntl(3, F_GETFL) = 0x2 (flags O_RDWR) +0.0 fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK) = 0 +0 bind(3, ..., ...) = 0 +0 listen(3, 1) = 0 // ---- MPC connection on saddr0 ---- +0.1 < addr[caddr0] > addr[saddr0] S 0:0(0) win 65535 <mss 1460, sackOK, TS val 4074410674 ecr 0, nop, wscale 8, mpcapable v1 flags[flag_h] nokey> +0 > S. 0:0(0) ack 1 <mss 1460, sackOK, TS val 4074410674 ecr 4074410674, nop, wscale 8, mpcapable v1 flags[flag_h] key[skey]> +0.2 < . 1:1(0) ack 1 win 256 <nop, nop, TS val 4074410674 ecr 4074410674, mpcapable v1 flags[flag_h] key[ckey=2, skey]> +0 accept(3, ..., ...) = 4 +0.0 < addr[caddr0] > addr[saddr0] P. 1:3(2) ack 1 win 256 <nop, nop, TS val 4074418292 ecr 4074410674, mpcapable v1 flags[flag_h] key[skey, ckey] mpcdatalen 2, nop, nop> +0.0 > . 1:1(0) ack 3 <nop, nop, TS val 4074418293 ecr 4074418292, dss dack8=3 dll=0 nocs> // ---- Extra subflow via MP_JOIN (likely needed to wake the PM worker) ---- +0.1 < addr[caddr1] > addr[saddr0] S 0:0(0) win 65535 <mss 1460, sackOK, TS val 448955294 ecr 0, nop, wscale 8, mp_join_syn address_id=1 token=sha256_32(skey)> +0 > S. 0:0(0) ack 1 <mss 1460, sackOK, TS val 448955294 ecr 448955294, nop, wscale 8, mp_join_syn_ack address_id=0 sender_hmac=auto> +0.1 < . 1:1(0) ack 1 win 256 <nop, nop, TS val 448955294 ecr 448955294, mp_join_ack sender_hmac=auto> +0 > . 1:1(0) ack 1 <nop, nop, TS val 448955294 ecr 448955294, dss dack8=3 nocs> // ---- Signal endpoint on the local (MPC / ID 0) address ---- // mpc_endpoint_id is set and its id bit cleared, so NO ADD_ADDR is sent yet. +0.1 `ip mptcp endpoint add $OPT_LOCAL_IP id 1 signal` // ---- First "delete id 0": intended to free id 0 so the PM announces it ---- +0.3 `ip mptcp endpoint delete id 0 $OPT_LOCAL_IP` // UNCALIBRATED expected RM_ADDR(id 0) ride-along (DSS dack value is a guess): +0 > . 1:1(0) ack 1 <nop, nop, TS val 448955294 ecr 448955294, dss dack8=3 nocs, remove_address address_id=[0]> // UNCALIBRATED: ADD_ADDR(id 0) -- this is the step I could not confirm fires: +0.1 > . 1:1(0) ack 1 <nop, nop, TS val 448955294 ecr 448955294, add_address address_id=0 addr[saddr0] hmac=auto> // Withhold the echo so the ADD_ADDR(id 0) entry would stay pending. // ---- Second "delete id 0": THIS is the path the patch fixes ---- // * unfixed: leaves the stale entry -> later re-announce hits the WARN; // * fixed: mptcp_pm_announced_remove() returns true, entry freed, // add_addr_signaled decremented. +0.2 `ip mptcp endpoint delete id 0 $OPT_LOCAL_IP` // UNCALIBRATED expected RM_ADDR(id 0): +0 > . 1:1(0) ack 1 <nop, nop, TS val 448955294 ecr 448955294, dss dack8=3 nocs, remove_address address_id=[0]> // A 2nd signal endpoint lifts the signal cap so the PM can reselect id 0; // on an unfixed kernel the reselect should hit the stale entry -> WARN. +0.2 `ip mptcp endpoint add 192.168.0.2 id 2 signal` ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint 2026-06-01 11:59 ` Kalpan Jani @ 2026-07-06 16:46 ` Matthieu Baerts 2026-07-07 6:15 ` Kalpan Jani 0 siblings, 1 reply; 12+ messages in thread From: Matthieu Baerts @ 2026-07-06 16:46 UTC (permalink / raw) To: Kalpan Jani Cc: mptcp, martineau, pabeni, shardul.b, janak, kalpanjani009, shardulsb08 Hi Kalpan, Sorry for the delay. On 01/06/2026 13:59, Kalpan Jani wrote: >> do you mind sharing what you have so far. Maybe someone else can help >> you to reproduce the issue. > > Attached is the WIP packetdrill test (server side, based on the existing > add_addr/mp_join server tests, with an extra MP_JOIN subflow). I have not > been able to reproduce the WARN with it yet, so please treat it as a > work-in-progress rather than a finished test -- the status and the open > question are written at the top of the file. > > The trigger is "ip mptcp endpoint delete id 0 <saddr>", which is the only > op that reaches mptcp_nl_remove_id_zero_address(). (flush is not usable: it > goes through mptcp_pm_flush_addrs_and_subflows(), which already calls > mptcp_pm_announced_remove(), so that path was never buggy.) > > Where I am stuck is getting ADD_ADDR(id 0) announced in the first place. > The signal endpoint on the MPC address has its id bit cleared by > mptcp_mpc_endpoint_setup(), so it is not announced on connect. "delete id 0" > frees the bit again, but I could not confirm that the PM worker re-runs and > announces id 0 on an otherwise idle connection. Without that announcement no > pending anno_list entry is created, so the second "delete id 0" has nothing > stale to leave behind and the WARN does not trigger. > > So my question: what is the minimal way to get the id-0 address actually > announced via packetdrill? Does this need the MP_JOIN subflow and/or a data > exchange to wake the PM worker between the operations, or is there a simpler > approach I am missing? With the in-kernel PM, I think you need to: - create a subflow using a second local IP address (maybe optional): use multi-ep.sh (or add an IP address and inject a SYN+MPJ) - remove the endpoint linked to the initial subflow: ip mptcp endp del - re-create the same endpoint: ip mptcp endp add (or adapt multi-ep.sh to avoid the flush) That's what the mptcp_join.sh's "delete re-add signal" subtest does if I remember well. >> I will wait for this test before applying the v3. > > Understood, I will keep working on it. One thing I wanted to ask directly: > is a deterministic packetdrill test a hard requirement for applying v3, or > could the fix go in on the reasoning + the syzbot report alone? The change > only makes the id-0 removal path symmetric with the non-zero id path (tear > down the pending ADD_ADDR and decrement add_addr_signaled), so it is fairly > contained and independent of whether I can script a reliable trigger. Having a reproducer (packetdrill, selftests, syzbot repro) are not just added to avoid having the same issue later on, but it also helps the reviewers to understand what is being fixed. Here, we don't have a syzbot reproducer, so I think it is even more important to prove that something is being fixed here: maybe syzbot found another path, and having such test here would really help for later to exercise one "known path". > (Note: the anno-list helpers were renamed in current export, so v3 no longer > applies as-is; I can send a trivially rebased v4 that swaps > mptcp_remove_anno_list_by_saddr() for mptcp_pm_announced_remove() if useful.) I see that you sent a v5 for this. I guess you did the rebase, and you added a reproducer in the selftest. It should be enough for now. But here, I would like to get your feedback if you don't mind: now that you have a clearer view on what needed to be tested, do you think there was/is a blocking point to get such validation in packetdrill? I do think Packetdrill tests are often better in terms of maintenance, clarity, and execution time to reproduce exactly what is needed (not just with MPTCP), so I think it is important to make sure this tool is still adapted, and understood by multiple people. Cheers, Matt -- Sponsored by the NGI0 Core fund. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint 2026-07-06 16:46 ` Matthieu Baerts @ 2026-07-07 6:15 ` Kalpan Jani 2026-07-07 11:27 ` Matthieu Baerts 0 siblings, 1 reply; 12+ messages in thread From: Kalpan Jani @ 2026-07-07 6:15 UTC (permalink / raw) To: Matthieu Baerts Cc: mptcp, martineau, pabeni, shardul.b, janak, kalpanjani009, shardulsb08 Hi Matt, Thank you for the clear question — here is my honest assessment. The blocking point for a packetdrill test of this specific bug is that the trigger sequence requires the kernel PM worker to run between a "del id 0" and a second signal endpoint add, and packetdrill's shell-escape timing is coarser than what reliably controls that window. More specifically, the reproducer needs: 1. A fully-established MPTCP connection. 2. A second subflow via MP_JOIN (to keep the connection alive across the id 0 removal) — this part packetdrill handles well, as shown in mp_join_server.pkt. 3. An ADD_ADDR for the id-0 address already in flight with the echo withheld — also doable: just don't send the echo packet. 4. `ip mptcp endpoint delete id 0 <addr>` via a shell escape — packetdrill supports this. 5. `ip mptcp endpoint add <addr> flags signal` to force PM reselection — also a shell escape. The difficulty is between steps 3 and 4: the ADD_ADDR for id 0 only gets sent after mptcp_mpc_endpoint_setup() has accounted the MPC endpoint and the PM worker has run. In the mptcp_join.sh reproducer this is handled by waiting for the second join (wait_mpj) and checking the add_addr_signal counter before proceeding. In packetdrill, the closest control is expecting the `> add_address address_id=0 ...` line before issuing the shell escape — and that does work in principle. Where it gets fragile: after the del+re-add, the PM reselects id 0 and the WARN fires inside the add_addr_doit netlink handler, which is synchronous with the shell-escape command. Packetdrill would then need to expect a second `> add_address address_id=0 ...` line that a fixed kernel would *not* send (since mptcp_pm_announced_alloc returns false silently). That asymmetry — expecting a packet that a fixed kernel suppresses — means the test structure naturally inverts: you'd write it to expect *no* retransmission after the del+re-add, which is harder to express cleanly than "this packet must appear". The mptcp_join.sh counter check (`add_addr_signal` going from 2 to 1 after the del) is a more direct assertion. That said, I think a packetdrill test *is* achievable if it's structured around the ADD_ADDR retransmit that fires on an unfixed kernel after the del+re-add: on an unfixed kernel the stale entry triggers a second `> add_address address_id=0` packet; a fixed kernel does not send it. So the test would: - inject the MP_JOIN to create the second subflow; - see `> add_address address_id=0` (the first, legitimate one); - not send the echo; - issue `ip mptcp endpoint delete id 0 $OPT_LOCAL_IP` as a shell escape; - issue `ip mptcp endpoint add $OPT_LOCAL_IP flags signal`; - add a ~`*` line that expects *no* spurious second add_address — which is the opposite of what unfixed kernels produce. The main uncertainty is whether packetdrill's `*` (any-packet) handling can cleanly express "this packet must NOT appear within N ms", or whether we'd need a short timeout + close to flush the pipe. That's the part I haven't mastered yet in packetdrill. If you can point me at an existing test that uses a negative packet assertion (or a timeout to prove absence), I can turn the mptcp_join.sh reproducer into a packetdrill test for the next version. Otherwise I'll send the mptcp_join.sh subtest as patch 2 of v6 (with the `addr` fix in patch 1), and follow up with a packetdrill test separately once I understand that idiom better. Cheers, Kalpan Jani From: Matthieu Baerts <matttbe@kernel.org> To: "Kalpan Jani"<kalpan.jani@mpiricsoftware.com> Cc: "mptcp"<mptcp@lists.linux.dev>, "martineau"<martineau@kernel.org>, "pabeni"<pabeni@redhat.com>, "shardul.b"<shardul.b@mpiricsoftware.com>, "janak"<janak@mpiric.us>, "kalpanjani009"<kalpanjani009@gmail.com>, "shardulsb08"<shardulsb08@gmail.com> Date: Mon, 06 Jul 2026 22:16:55 +0530 Subject: Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint > Hi Kalpan, > > Sorry for the delay. > > On 01/06/2026 13:59, Kalpan Jani wrote: > >> do you mind sharing what you have so far. Maybe someone else can help > >> you to reproduce the issue. > > > > Attached is the WIP packetdrill test (server side, based on the existing > > add_addr/mp_join server tests, with an extra MP_JOIN subflow). I have not > > been able to reproduce the WARN with it yet, so please treat it as a > > work-in-progress rather than a finished test -- the status and the open > > question are written at the top of the file. > > > > The trigger is "ip mptcp endpoint delete id 0 <saddr>", which is the only > > op that reaches mptcp_nl_remove_id_zero_address(). (flush is not usable: it > > goes through mptcp_pm_flush_addrs_and_subflows(), which already calls > > mptcp_pm_announced_remove(), so that path was never buggy.) > > > > Where I am stuck is getting ADD_ADDR(id 0) announced in the first place. > > The signal endpoint on the MPC address has its id bit cleared by > > mptcp_mpc_endpoint_setup(), so it is not announced on connect. "delete id 0" > > frees the bit again, but I could not confirm that the PM worker re-runs and > > announces id 0 on an otherwise idle connection. Without that announcement no > > pending anno_list entry is created, so the second "delete id 0" has nothing > > stale to leave behind and the WARN does not trigger. > > > > So my question: what is the minimal way to get the id-0 address actually > > announced via packetdrill? Does this need the MP_JOIN subflow and/or a data > > exchange to wake the PM worker between the operations, or is there a simpler > > approach I am missing? > > With the in-kernel PM, I think you need to: > > - create a subflow using a second local IP address (maybe optional): use > multi-ep.sh (or add an IP address and inject a SYN+MPJ) > > - remove the endpoint linked to the initial subflow: ip mptcp endp del > > - re-create the same endpoint: ip mptcp endp add (or adapt multi-ep.sh > to avoid the flush) > > That's what the mptcp_join.sh's "delete re-add signal" subtest does if I > remember well. > > >> I will wait for this test before applying the v3. > > > > Understood, I will keep working on it. One thing I wanted to ask directly: > > is a deterministic packetdrill test a hard requirement for applying v3, or > > could the fix go in on the reasoning + the syzbot report alone? The change > > only makes the id-0 removal path symmetric with the non-zero id path (tear > > down the pending ADD_ADDR and decrement add_addr_signaled), so it is fairly > > contained and independent of whether I can script a reliable trigger. > > Having a reproducer (packetdrill, selftests, syzbot repro) are not just > added to avoid having the same issue later on, but it also helps the > reviewers to understand what is being fixed. Here, we don't have a > syzbot reproducer, so I think it is even more important to prove that > something is being fixed here: maybe syzbot found another path, and > having such test here would really help for later to exercise one "known > path". > > > (Note: the anno-list helpers were renamed in current export, so v3 no longer > > applies as-is; I can send a trivially rebased v4 that swaps > > mptcp_remove_anno_list_by_saddr() for mptcp_pm_announced_remove() if useful.) > I see that you sent a v5 for this. I guess you did the rebase, and you > added a reproducer in the selftest. It should be enough for now. But > here, I would like to get your feedback if you don't mind: now that you > have a clearer view on what needed to be tested, do you think there > was/is a blocking point to get such validation in packetdrill? I do > think Packetdrill tests are often better in terms of maintenance, > clarity, and execution time to reproduce exactly what is needed (not > just with MPTCP), so I think it is important to make sure this tool is > still adapted, and understood by multiple people. > > Cheers, > Matt > -- > Sponsored by the NGI0 Core fund. > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint 2026-07-07 6:15 ` Kalpan Jani @ 2026-07-07 11:27 ` Matthieu Baerts 2026-07-07 13:35 ` Kalpan Jani 0 siblings, 1 reply; 12+ messages in thread From: Matthieu Baerts @ 2026-07-07 11:27 UTC (permalink / raw) To: Kalpan Jani Cc: mptcp, martineau, pabeni, shardul.b, janak, kalpanjani009, shardulsb08 Hi Kalpan, Thank you for your reply. On 07/07/2026 08:15, Kalpan Jani wrote: > Hi Matt, > > Thank you for the clear question — here is my honest assessment. > > The blocking point for a packetdrill test of this specific bug is > that the trigger sequence requires the kernel PM worker to run > between a "del id 0" and a second signal endpoint add, and > packetdrill's shell-escape timing is coarser than what reliably > controls that window. > > More specifically, the reproducer needs: > > 1. A fully-established MPTCP connection. > 2. A second subflow via MP_JOIN (to keep the connection alive > across the id 0 removal) — this part packetdrill handles well, > as shown in mp_join_server.pkt. > 3. An ADD_ADDR for the id-0 address already in flight with the > echo withheld — also doable: just don't send the echo packet. > 4. `ip mptcp endpoint delete id 0 <addr>` via a shell escape — > packetdrill supports this. > 5. `ip mptcp endpoint add <addr> flags signal` to force PM > reselection — also a shell escape. > > The difficulty is between steps 3 and 4: the ADD_ADDR for id 0 > only gets sent after mptcp_mpc_endpoint_setup() has accounted the > MPC endpoint and the PM worker has run. In the mptcp_join.sh > reproducer this is handled by waiting for the second join > (wait_mpj) and checking the add_addr_signal counter before > proceeding. In packetdrill, the closest control is expecting the > `> add_address address_id=0 ...` line before issuing the shell > escape — and that does work in principle. Note that if needed (but I don't think it is if I understood correctly), you can always call commands or scripts in Packetdrill, e.g. to wait for an internal state to change. > Where it gets fragile: after the del+re-add, the PM reselects id 0 > and the WARN fires inside the add_addr_doit netlink handler, which > is synchronous with the shell-escape command. Packetdrill would > then need to expect a second `> add_address address_id=0 ...` line > that a fixed kernel would *not* send (since mptcp_pm_announced_alloc > returns false silently). That asymmetry — expecting a packet that > a fixed kernel suppresses — means the test structure naturally > inverts: you'd write it to expect *no* retransmission after the > del+re-add, which is harder to express cleanly than "this packet > must appear". The mptcp_join.sh counter check (`add_addr_signal` > going from 2 to 1 after the del) is a more direct assertion. You can also check ss' status. But here, you can also simply inject another packet after a small delay, and expect an ACK without ADD_ADDR to be sent instead. > That said, I think a packetdrill test *is* achievable if it's > structured around the ADD_ADDR retransmit that fires on an unfixed > kernel after the del+re-add: on an unfixed kernel the stale entry > triggers a second `> add_address address_id=0` packet; a fixed > kernel does not send it. So the test would: > > - inject the MP_JOIN to create the second subflow; > - see `> add_address address_id=0` (the first, legitimate one); > - not send the echo; > - issue `ip mptcp endpoint delete id 0 $OPT_LOCAL_IP` as a > shell escape; > - issue `ip mptcp endpoint add $OPT_LOCAL_IP flags signal`; > - add a ~`*` line that expects *no* spurious second > add_address — which is the opposite of what unfixed kernels > produce. > > The main uncertainty is whether packetdrill's `*` (any-packet) > handling can cleanly express "this packet must NOT appear within > N ms", or whether we'd need a short timeout + close to flush the > pipe. That's the part I haven't mastered yet in packetdrill. > > If you can point me at an existing test that uses a negative > packet assertion (or a timeout to prove absence), I can turn the > mptcp_join.sh reproducer into a packetdrill test for the next > version. Otherwise I'll send the mptcp_join.sh subtest as patch 2 > of v6 (with the `addr` fix in patch 1), and follow up with a > packetdrill test separately once I understand that idiom better. I think the easiest is to inject a packet after the timeout to force the kernel to react, e.g. sending a DATA_FIN, data, an out of window packet, etc. Then see that the reaction is the expected one, not a wrong retransmission. I just noticed that's what is done in add_addr_retry_errors.pkt, lines 32-38. In general, about Packetdrill, I recommend this presentation: https://netdevconf.info/0x19/sessions/tutorial/tutorial-using-packetdrill-to-write-automated-tests-for-the-linux-networking-stack.html e.g. on page 15, there are some explanations about timing: https://netdevconf.info/0x19/docs/netdev-0x19-paper23-talk-slides/packetdrill%20-%20NetDev%202025-03-10.pdf Back to the series you sent: we could say that the mptcp_join.sh reproducer is enough, but I think it would be useful for you -- and to avoid a new "speed=slow" subtest: mptcp_join.sh's execution is already long enough -- to try a bit more with Packetdrill if that's OK. WDYT? Cheers, Matt -- Sponsored by the NGI0 Core fund. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint 2026-07-07 11:27 ` Matthieu Baerts @ 2026-07-07 13:35 ` Kalpan Jani 2026-07-07 14:14 ` Matthieu Baerts 0 siblings, 1 reply; 12+ messages in thread From: Kalpan Jani @ 2026-07-07 13:35 UTC (permalink / raw) To: Matthieu Baerts Cc: mptcp, martineau, pabeni, shardul.b, janak, kalpanjani009, shardulsb08 Hi Matt, Thank you for pointing me to add_addr_retry_errors.pkt — that makes the idiom clear. Inject a data packet after the del+re-add sequence and expect a plain DSS ACK back with no add_address option piggybacked. On an unfixed kernel the stale entry causes a spurious ADD_ADDR(id=0) retransmit to appear on that ACK, which packetdrill catches as an unexpected option. I will write the packetdrill test using that pattern and send v6 with: Patch 1: kernel fix (also correcting the addr argument — the &anno_addr copy in v5 is fragile for with-port endpoints) Patch 2: packetdrill regression test in gtests/net/mptcp/regressions/ Also noted on the NetDev slides — reading those before drafting the test. Cheers, Kalpan Jani From: Matthieu Baerts <matttbe@kernel.org> To: "Kalpan Jani"<kalpan.jani@mpiricsoftware.com> Cc: "mptcp"<mptcp@lists.linux.dev>, "martineau"<martineau@kernel.org>, "pabeni"<pabeni@redhat.com>, "shardul.b"<shardul.b@mpiricsoftware.com>, "janak"<janak@mpiric.us>, "kalpanjani009"<kalpanjani009@gmail.com>, "shardulsb08"<shardulsb08@gmail.com> Date: Tue, 07 Jul 2026 16:57:06 +0530 Subject: Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint > Hi Kalpan, > > Thank you for your reply. > > On 07/07/2026 08:15, Kalpan Jani wrote: > > Hi Matt, > > > > Thank you for the clear question — here is my honest assessment. > > > > The blocking point for a packetdrill test of this specific bug is > > that the trigger sequence requires the kernel PM worker to run > > between a "del id 0" and a second signal endpoint add, and > > packetdrill's shell-escape timing is coarser than what reliably > > controls that window. > > > > More specifically, the reproducer needs: > > > > 1. A fully-established MPTCP connection. > > 2. A second subflow via MP_JOIN (to keep the connection alive > > across the id 0 removal) — this part packetdrill handles well, > > as shown in mp_join_server.pkt. > > 3. An ADD_ADDR for the id-0 address already in flight with the > > echo withheld — also doable: just don't send the echo packet. > > 4. `ip mptcp endpoint delete id 0 <addr>` via a shell escape — > > packetdrill supports this. > > 5. `ip mptcp endpoint add <addr> flags signal` to force PM > > reselection — also a shell escape. > > > > The difficulty is between steps 3 and 4: the ADD_ADDR for id 0 > > only gets sent after mptcp_mpc_endpoint_setup() has accounted the > > MPC endpoint and the PM worker has run. In the mptcp_join.sh > > reproducer this is handled by waiting for the second join > > (wait_mpj) and checking the add_addr_signal counter before > > proceeding. In packetdrill, the closest control is expecting the > > `> add_address address_id=0 ...` line before issuing the shell > > escape — and that does work in principle. > > Note that if needed (but I don't think it is if I understood correctly), > you can always call commands or scripts in Packetdrill, e.g. to wait for > an internal state to change. > > > Where it gets fragile: after the del+re-add, the PM reselects id 0 > > and the WARN fires inside the add_addr_doit netlink handler, which > > is synchronous with the shell-escape command. Packetdrill would > > then need to expect a second `> add_address address_id=0 ...` line > > that a fixed kernel would *not* send (since mptcp_pm_announced_alloc > > returns false silently). That asymmetry — expecting a packet that > > a fixed kernel suppresses — means the test structure naturally > > inverts: you'd write it to expect *no* retransmission after the > > del+re-add, which is harder to express cleanly than "this packet > > must appear". The mptcp_join.sh counter check (`add_addr_signal` > > going from 2 to 1 after the del) is a more direct assertion. > > You can also check ss' status. But here, you can also simply inject > another packet after a small delay, and expect an ACK without ADD_ADDR > to be sent instead. > > > That said, I think a packetdrill test *is* achievable if it's > > structured around the ADD_ADDR retransmit that fires on an unfixed > > kernel after the del+re-add: on an unfixed kernel the stale entry > > triggers a second `> add_address address_id=0` packet; a fixed > > kernel does not send it. So the test would: > > > > - inject the MP_JOIN to create the second subflow; > > - see `> add_address address_id=0` (the first, legitimate one); > > - not send the echo; > > - issue `ip mptcp endpoint delete id 0 $OPT_LOCAL_IP` as a > > shell escape; > > - issue `ip mptcp endpoint add $OPT_LOCAL_IP flags signal`; > > - add a ~`*` line that expects *no* spurious second > > add_address — which is the opposite of what unfixed kernels > > produce. > > > > The main uncertainty is whether packetdrill's `*` (any-packet) > > handling can cleanly express "this packet must NOT appear within > > N ms", or whether we'd need a short timeout + close to flush the > > pipe. That's the part I haven't mastered yet in packetdrill. > > > > If you can point me at an existing test that uses a negative > > packet assertion (or a timeout to prove absence), I can turn the > > mptcp_join.sh reproducer into a packetdrill test for the next > > version. Otherwise I'll send the mptcp_join.sh subtest as patch 2 > > of v6 (with the `addr` fix in patch 1), and follow up with a > > packetdrill test separately once I understand that idiom better. > > I think the easiest is to inject a packet after the timeout to force the > kernel to react, e.g. sending a DATA_FIN, data, an out of window packet, > etc. Then see that the reaction is the expected one, not a wrong > retransmission. I just noticed that's what is done in > add_addr_retry_errors.pkt, lines 32-38. > > In general, about Packetdrill, I recommend this presentation: > > > https://netdevconf.info/0x19/sessions/tutorial/tutorial-using-packetdrill-to-write-automated-tests-for-the-linux-networking-stack.html > > e.g. on page 15, there are some explanations about timing: > > > https://netdevconf.info/0x19/docs/netdev-0x19-paper23-talk-slides/packetdrill%20-%20NetDev%202025-03-10.pdf > > Back to the series you sent: we could say that the mptcp_join.sh > reproducer is enough, but I think it would be useful for you -- and to > avoid a new "speed=slow" subtest: mptcp_join.sh's execution is already > long enough -- to try a bit more with Packetdrill if that's OK. WDYT? > > Cheers, > Matt > -- > Sponsored by the NGI0 Core fund. > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint 2026-07-07 13:35 ` Kalpan Jani @ 2026-07-07 14:14 ` Matthieu Baerts 2026-07-08 5:31 ` Kalpan Jani 0 siblings, 1 reply; 12+ messages in thread From: Matthieu Baerts @ 2026-07-07 14:14 UTC (permalink / raw) To: Kalpan Jani Cc: mptcp, martineau, pabeni, shardul.b, janak, kalpanjani009, shardulsb08 On 07/07/2026 15:35, Kalpan Jani wrote: > Hi Matt, > Thank you for pointing me to add_addr_retry_errors.pkt — that makes the idiom clear. Inject a data packet after the del+re-add sequence and expect a plain DSS ACK back with no add_address option piggybacked. On an unfixed kernel the stale entry causes a spurious ADD_ADDR(id=0) retransmit to appear on that ACK, which packetdrill catches as an unexpected option. Great! > I will write the packetdrill test using that pattern and send v6 with: > > Patch 1: kernel fix (also correcting the addr argument — the &anno_addr copy in v5 is fragile for with-port endpoints) If that's the same version as the v5, no need to resend it. > Patch 2: packetdrill regression test in gtests/net/mptcp/regressions/ Can you open a PR instead, please? https://github.com/multipath-tcp/packetdrill/ Cheers, Matt -- Sponsored by the NGI0 Core fund. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint 2026-07-07 14:14 ` Matthieu Baerts @ 2026-07-08 5:31 ` Kalpan Jani 0 siblings, 0 replies; 12+ messages in thread From: Kalpan Jani @ 2026-07-08 5:31 UTC (permalink / raw) To: Matthieu Baerts Cc: mptcp, martineau, pabeni, shardul.b, janak, kalpanjani009, shardulsb08 Hi Matt, PR opened: https://github.com/multipath-tcp/packetdrill/pull/201 One note on patch 1: v5 passes &anno_addr (a zeroed copy of msk_local) instead of addr directly. This works for no-port endpoints but is fragile for with-port ones. Should I send a corrected v6 for patch 1, or can you fix it up while applying? Cheers, Kalpan Jani From: Matthieu Baerts <matttbe@kernel.org> To: "Kalpan Jani"<kalpan.jani@mpiricsoftware.com> Cc: "mptcp"<mptcp@lists.linux.dev>, "martineau"<martineau@kernel.org>, "pabeni"<pabeni@redhat.com>, "shardul.b"<shardul.b@mpiricsoftware.com>, "janak"<janak@mpiric.us>, "kalpanjani009"<kalpanjani009@gmail.com>, "shardulsb08"<shardulsb08@gmail.com> Date: Tue, 07 Jul 2026 19:44:57 +0530 Subject: Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint > On 07/07/2026 15:35, Kalpan Jani wrote: > > Hi Matt, > > Thank you for pointing me to add_addr_retry_errors.pkt — that makes the idiom clear. Inject a data packet after the del+re-add sequence and expect a plain DSS ACK back with no add_address option piggybacked. On an unfixed kernel the stale entry causes a spurious ADD_ADDR(id=0) retransmit to appear on that ACK, which packetdrill catches as an unexpected option. > > Great! > > > I will write the packetdrill test using that pattern and send v6 with: > > > > Patch 1: kernel fix (also correcting the addr argument — the &anno_addr copy in v5 is fragile for with-port endpoints) > > If that's the same version as the v5, no need to resend it. > > > Patch 2: packetdrill regression test in gtests/net/mptcp/regressions/ > > Can you open a PR instead, please? > > https://github.com/multipath-tcp/packetdrill/ > > Cheers, > Matt > -- > Sponsored by the NGI0 Core fund. > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint 2026-05-29 6:43 [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint Kalpan Jani 2026-05-29 6:55 ` Matthieu Baerts @ 2026-05-29 7:58 ` MPTCP CI 1 sibling, 0 replies; 12+ messages in thread From: MPTCP CI @ 2026-05-29 7:58 UTC (permalink / raw) To: Kalpan Jani; +Cc: mptcp Hi Kalpan, 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! ✅ - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/26623410540 Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/ed6268d8c29c Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1102655 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
end of thread, other threads:[~2026-07-08 5:31 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-29 6:43 [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint Kalpan Jani 2026-05-29 6:55 ` Matthieu Baerts 2026-05-29 7:18 ` Kalpan Jani 2026-05-31 6:39 ` Matthieu Baerts 2026-06-01 11:59 ` Kalpan Jani 2026-07-06 16:46 ` Matthieu Baerts 2026-07-07 6:15 ` Kalpan Jani 2026-07-07 11:27 ` Matthieu Baerts 2026-07-07 13:35 ` Kalpan Jani 2026-07-07 14:14 ` Matthieu Baerts 2026-07-08 5:31 ` Kalpan Jani 2026-05-29 7:58 ` 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.