Re: [PATCH net v2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint

[Matthieu Baerts <matttbe@kernel.org>]

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.