Re: [Patch bpf-next v7 00/13] sockmap: introduce BPF_SK_SKB_VERDICT and support UDP
From: Cong Wang <hidden>
Date: 2021-03-29 16:58:50
Also in:
bpf
On Mon, Mar 29, 2021 at 8:03 AM John Fastabend [off-list ref] wrote:
Alexei Starovoitov wrote:quoted
On Sun, Mar 28, 2021 at 01:20:00PM -0700, Cong Wang wrote:quoted
From: Cong Wang <redacted> We have thousands of services connected to a daemon on every host via AF_UNIX dgram sockets, after they are moved into VM, we have to add a proxy to forward these communications from VM to host, because rewriting thousands of them is not practical. This proxy uses an AF_UNIX socket connected to services and a UDP socket to connect to the host. It is inefficient because data is copied between kernel space and user space twice, and we can not use splice() which only supports TCP. Therefore, we want to use sockmap to do the splicing without going to user-space at all (after the initial setup). Currently sockmap only fully supports TCP, UDP is partially supported as it is only allowed to add into sockmap. This patchset, as the second part of the original large patchset, extends sockmap with: 1) cross-protocol support with BPF_SK_SKB_VERDICT; 2) full UDP support. On the high level, ->read_sock() is required for each protocol to support sockmap redirection, and in order to do sock proto update, a new ops ->psock_update_sk_prot() is introduced, which is also required. And the BPF ->recvmsg() is also needed to replace the original ->recvmsg() to retrieve skmsg. To make life easier, we have to get rid of lock_sock() in sk_psock_handle_skb(), otherwise we would have to implement ->sendmsg_locked() on top of ->sendmsg(), which is ugly. Please see each patch for more details. To see the big picture, the original patchset is available here: https://github.com/congwang/linux/tree/sockmap this patchset is also available: https://github.com/congwang/linux/tree/sockmap2 --- v7: use work_mutex to protect psock->work return err in udp_read_sock() add patch 6/13 clean up test caseThe feature looks great to me. I think the selftest is a bit light in terms of coverage, but it's acceptable.+1
Well, the first half of this patchset still focuses on the existing code, which is already covered by existing test cases. The second half adds BPF_SK_SKB_VERDICT and UDP support, which are already covered by my new test case. And apparently UDP will never support other sockmap programs like TCP, for example, BPF_SK_SKB_STREAM_PARSER, hence it of course has much less test cases than TCP. Cross-protocol test case will be added in the next patchset when AF_UNIX comes in. If I miss anything, please be specific. Just saying the test case is light does not help me to understand what I need to add. Thanks.