Re: [PATCH net v4 0/3] mptcp: Fix conflicts between MPTCP and sockmap

[Matthieu Baerts <matttbe@kernel.org>]

Hi Jiayuan,

On 05/11/2025 12:36, Jiayuan Chen wrote:
> Overall, we encountered a warning [1] that can be triggered by running the
> selftest I provided.

Thank you for the v4!

> sockmap works by replacing sk_data_ready, recvmsg, sendmsg operations and
> implementing fast socket-level forwarding logic:
> 1. Users can obtain file descriptors through userspace socket()/accept()
>    interfaces, then call BPF syscall to perform these replacements.
> 2. Users can also use the bpf_sock_hash_update helper (in sockops programs)
>    to replace handlers when TCP connections enter ESTABLISHED state
>   (BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB/BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB)
> 
> However, when combined with MPTCP, an issue arises: MPTCP creates subflow
> sk's and performs TCP handshakes, so the BPF program obtains subflow sk's
> and may incorrectly replace their sk_prot. We need to reject such
> operations. In patch 1, we set psock_update_sk_prot to NULL in the
> subflow's custom sk_prot.

This new version looks good to me. I have some small comments on patches
1 and 2 that can only be addressed if a v5 is needed I think.

I have some questions for the 3rd patch. It would be good if someone
else with more experience with the BPF selftests can also look at it.

> Additionally, if the server's listening socket has MPTCP enabled and the
> client's TCP also uses MPTCP, we should allow the combination of subflow
> and sockmap. This is because the latest Golang programs have enabled MPTCP
> for listening sockets by default [2]. For programs already using sockmap,
> upgrading Golang should not cause sockmap functionality to fail.

Note: even if these patches here are needed to avoid stream corruption
and other issues, in your specific case with sockmap, I think it would
be better to set this env var until MPTCP support is added to sockmap:

  GODEBUG=multipathtcp=0

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