Re: [PATCH mptcp_net-next 1/1] mptcp: fastopen: only mark MPTFO subflows with SYN data

Hi Ren, Wyatt, Yuan,

> Passive TCP Fast Open accepts a valid-cookie SYN even when it carries
> no data. In that case the child socket's receive queue is intentionally
> left empty.
>
> mptcp_fastopen_subflow_synack_set_params() set is_mptfo before checking
> for queued SYN data. That made data-less TFO SYNs hit a WARN and, if
> the warning was non-fatal, left stale MPTFO state behind. The stale
> flag could later trigger a state-confusion bug in
> check_fully_established().
>
> Only mark the subflow as MPTFO after confirming that a SYN-data skb is
> present. Return quietly when the receive queue is empty.

Thank you for the fix, it looks good to me:

Reviewed-by: Matthieu Baerts (NGI0) <[email protected]>

@Netdev maintainers: this can be applied directly in net.


Sorry for the delay, I initially wanted to understand the reproducer,
then wrote a small packetdrill script, then I lost track of this patch.

As I mentioned to Yuan last week at NetdevConf, it would be great if the
PoC could first try to use Packetdrill when it is doable. For example,
in this case, this packetdrill script appears to be far more readable
than the .c files generated by AI:

  https://github.com/multipath-tcp/packetdrill/pull/202/changes

=> It is easy to read the syscalls, the injected packets (<) and the
expected ones (>) with an output similar to the one from tcpdump.

Note that for MPTCP, this fork should be used:

  https://github.com/multipath-tcp/packetdrill

Otherwise, the official repo can be used:

  https://github.com/google/packetdrill

-- 
Matthieu Baerts (NGI0) <[email protected]>