On Mon, Mar 29, 2021 at 8:03 AM John Fastabend <john.fastabend@xxxxxxxxx> wrote: > > Alexei Starovoitov wrote: > > On Sun, Mar 28, 2021 at 01:20:00PM -0700, Cong Wang wrote: > > > From: Cong Wang <cong.wang@xxxxxxxxxxxxx> > > > > > > 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 case > > > > The 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.