Re: [PATCH net v3 0/2] mptcp: fix request migration ownership

Hi Ren, Ruide,

Thank you for the v3! Here is a quick reaction, only about the
reproducers, not about the kernel code:

On 06/08/2026 13:14, Ren Wei wrote:

(...)

> Changes in v3:

(...)

> - Added a packetdrill MP_CAPABLE reproducer and decoded warning.

Nice, very appreciated, thank you!

I confirm they work well!
> // poc for MP_JOIN:
> 
> // Minimal reproducer for a stale subflow_req->msk after reqsk migration.
> --tolerance_usecs=200000
> --non_fatal=packet
> 
> `sysctl -q net.mptcp.enabled=1

Small details to help you next time to look at packetdrill reproducers
for MPTCP:

 - You can add new tests in gtests/net/mptcp/<dir>/<file>.pkt from the
   MPTCP packetdrill repository.
 - Then here above, you can add: `../common/defaults.sh`.
 - And execute your tests with run_all.py from the gtests/net dir, e.g.:
   ./packetdrill/run_all.py -lv4 mptcp/<dir>/<file>.pkt

These tests will then be executed from a dedicated netns, using many
sysctl settings: this helps to reproduce issues in a more controlled
environment.

> sysctl -q net.ipv4.tcp_migrate_req=1
> sysctl -q net.ipv4.tcp_synack_retries=1`

(detail) Here, tcp_synack_retries can be set to 0, then the sleep at the
end can be reduced.
> // Listener A and the owning MPTCP connection.
> +0     socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 3
> +0     setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
> +0     setsockopt(3, SOL_SOCKET, SO_REUSEPORT, [1], 4) = 0
> +0     bind(3, ..., ...) = 0
> +0     listen(3, 8) = 0
> 
> +0.0   <  addr[caddr0] > addr[saddr0]  S   0:0(0)         win 65535  <mss 1460, sackOK, TS val 1000 ecr 0,    nop, wscale 8, mpcapable v1 flags[flag_h] nokey>

Also for next time, when validating the listener side, feel free to drop
the TCP Timestamps option if it is not required (like what you did with
the other test): that makes the .pkt file more readable.

(...)

> MP_CAPABLE packetdrill reproducer:
> 
> // Reproducer for MP_CAPABLE request token ownership during TCP req migration.
> //
> // The first listener owns the request created by the MP_CAPABLE SYN.  A second
> // SO_REUSEPORT listener is added only after that SYN, then the first listener is
> // closed.  The SYN+ACK retransmission timer migrates the request to the second
> // listener, and a later request timer destroys the migrated request.
> //
> // On a vulnerable kernel, inet_reqsk_clone() raw-copies token_node.  The clone
> // is not the token table owner, so destroying the migrated request triggers the
> // MPTCP token ownership bug.
> --tolerance_usecs=250000
> 
> +0     `sysctl -q net.mptcp.enabled=1`
> +0     `sysctl -q net.ipv4.tcp_migrate_req=1`
> +0     `sysctl -q net.ipv4.tcp_synack_retries=2`
> +0     `sysctl -q net.ipv4.tcp_timestamps=1`
> +0     `sysctl -q kernel.panic_on_warn=0`
> +0     `sysctl -q kernel.panic_on_oops=0`
> +0     `ip tcp_metrics flush all >/dev/null 2>&1 || true`
> +0     `tc qdisc replace dev tun0 root pfifo >/dev/null 2>&1 || true`

Same here for `../common/defaults.sh`. Then I guess you only need
tcp_migrate_req=1 and tcp_synack_retries=2 (set to 0 to reduce the last
wait), and "wscale" will always be 8 in the S(.) instead of depending on
the environment.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.