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:
- Users can obtain file descriptors through userspace socket()/accept() interfaces, then call BPF syscall to perform these replacements.
- 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