Re: [PATCH net v5 1/2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint

Hi Matt,

Sorry for the long silence on this.

The packetdrill test is now up at
https://github.com/multipath-tcp/packetdrill/pull/201 - I rewrote it and this
time validated it in the virtme-docker environment: 3 failing without the
kernel fix, 3 passing with it, on export/20260622T173908. Your suggestion about
the endpoint linked to the initial subflow is what unblocked it. Details and
the limitations I hit are in the PR.

Two things about v5 itself.

First, patch 1/2 is unchanged from what you have. I said I would rework the
addr argument because the anno_addr copy looked fragile for endpoints with a
port, but after re-checking I no longer think that case is reachable: the MPC
endpoint cannot be a ported endpoint, since both __lookup_addr() and
mptcp_nl_add_subflow_or_signal_addr() compare the port when the endpoint has
one, and that would require it to match the initial subflow's local port -
which mptcp_pm_nl_create_listen_socket() binding the same address:port makes
unreachable in practice. So the entry reachable from
mptcp_nl_remove_id_zero_address() always has port 0 and clearing it is correct.
Tell me if you see a case I have missed.

Second, and I should have raised this earlier: please drop patch 2/2. The
selftest does not exercise the fixed path. I used addr_nr_ns1=-1, which deletes
the endpoint by its real ID via pm_nl_del_endpoint ${listener_ns} $id and
therefore goes through mptcp_nl_remove_subflow_and_signal_addr() - the path
that was already correct. Only -9 reaches mptcp_nl_remove_id_zero_address(),
and the endpoint is 10.0.2.1, so the address check there would not have matched
even with -9. The subtest passes identically with and without patch 1/2, which
is why CI was green everywhere, and it largely duplicates the existing "remove
single address" subtest. Sorry for not catching that before sending v5.

With the packetdrill test covering the same ground, I think patch 1/2 alone is
the right thing to apply. Happy to send a corrected mptcp_join.sh subtest as
well if you would rather have both.

Cheers,
Kalpan Jani


From: Kalpan Jani <[email protected]>
To: <[email protected]>
Cc: <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, "Kalpan Jani"<[email protected]>, <[email protected]>, "Tao Cui"<[email protected]>
Date: Tue, 30 Jun 2026 20:36:31 +0530
Subject: [PATCH net v5 1/2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint

 > The in-kernel MPTCP path manager can leave a stale ADD_ADDR announcement
 > entry alive when removing the id 0 endpoint. This happens because the id 0
 > removal path does not tear down pending announcements, unlike the non-zero
 > id path.
 > 
 > When the PM later reselects id 0 after adding another signal endpoint, it
 > finds the stale anno_list entry and hits WARN_ON_ONCE(mptcp_pm_is_kernel())
 > in mptcp_pm_alloc_anno_list().
 > 
 > Root cause: asymmetry between removal paths.
 > - Non-zero id path: mptcp_nl_remove_subflow_and_signal_addr() calls
 >   mptcp_pm_remove_anno_addr() to clean up.
 > - Id 0 path: mptcp_nl_remove_id_zero_address() skips cleanup entirely.
 > 
 > Fix by making the id 0 path symmetric: call mptcp_pm_announced_remove()
 > and decrement add_addr_signaled before queuing the RM_ADDR.
 > 
 > Subtle detail: signal endpoints are stored in anno_list with port 0, but
 > msk_local carries the connection's local port. mptcp_pm_announced_remove()
 > uses use_port=true for comparison, so clear the port before the lookup.
 > 
 > Fixes: 740d798e8767 ("mptcp: remove id 0 address")
 > Reported-by: [email protected]
 > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/620
 > Suggested-by: Tao Cui <[email protected]>
 > Signed-off-by: Kalpan Jani <[email protected]>
 > ---
 >  net/mptcp/pm_kernel.c | 8 ++++++++
 >  1 file changed, 8 insertions(+)
 > 
 > diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
 > index 331f6fa99014..d3014bf57bf3 100644
 > --- a/net/mptcp/pm_kernel.c
 > +++ b/net/mptcp/pm_kernel.c
 > @@ -1138,6 +1138,8 @@ 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;
 > +        struct mptcp_addr_info anno_addr;
 > +        bool announced;
 >  
 >          if (list_empty(&msk->conn_list) || mptcp_pm_is_userspace(msk))
 >              goto next;
 > @@ -1147,7 +1149,13 @@ 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. */
 > +        anno_addr = msk_local;
 > +        anno_addr.port = 0;
 > +        announced = mptcp_pm_announced_remove(msk, &anno_addr);
 >          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
 > 
 >