netdev - Re: [Patch bpf-next v7 00/13] sockmap: introduce BPF_SK_SKB

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6061ec2bc63e0_3673a20848@john-XPS-13-9370.notmuch>
Date:   Mon, 29 Mar 2021 08:03:07 -0700
From:   John Fastabend <john.fastabend@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Cong Wang <xiyou.wangcong@...il.com>
Cc:     netdev@...r.kernel.org, bpf@...r.kernel.org,
        duanxiongchun@...edance.com, wangdongdong.6@...edance.com,
        jiang.wang@...edance.com, Cong Wang <cong.wang@...edance.com>
Subject: Re: [Patch bpf-next v7 00/13] sockmap: introduce BPF_SK_SKB_VERDICT
 and support UDP

Alexei Starovoitov wrote:
> On Sun, Mar 28, 2021 at 01:20:00PM -0700, Cong Wang wrote:
> > From: Cong Wang <cong.wang@...edance.com>
> > 
> > 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

> I'd like to see the final Acks from John/Daniel and Jakub/Lorenz before merging.
> Folks,
> please prioritize the review of these patches.

This is getting really close I'll take another pass over it today. Thanks