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.
