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

[Matthieu Baerts <matttbe@kernel.org>]

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.