From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Borkmann Subject: Re: [PATCH net-next 1/2] bpf: enable non-root eBPF programs Date: Tue, 06 Oct 2015 20:03:55 +0200 Message-ID: <56140D0B.2040504@iogearbox.net> References: <1444078101-29060-1-git-send-email-ast@plumgrid.com> <1444078101-29060-2-git-send-email-ast@plumgrid.com> <5612F639.2050305@iogearbox.net> <56131B1F.80002@plumgrid.com> <20151006071347.GB14093@gmail.com> <561380BB.4040506@iogearbox.net> <20151006082048.GA18287@gmail.com> <561388D1.30406@iogearbox.net> <561409EC.5050005@plumgrid.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <561409EC.5050005-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> Sender: linux-api-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Alexei Starovoitov , Ingo Molnar Cc: "David S. Miller" , Andy Lutomirski , Hannes Frederic Sowa , Eric Dumazet , Kees Cook , linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-api@vger.kernel.org On 10/06/2015 07:50 PM, Alexei Starovoitov wrote: > On 10/6/15 1:39 AM, Daniel Borkmann wrote: >>>> [...] Also classic BPF would then need to test for it, since a socket >>>> filter >>>> doesn't really know whether native eBPF is loaded there or a >>>> classic-to-eBPF >>>> transformed one, and classic never makes use of this. Anyway, it >>>> could be done >>>> by adding a bit flag cb_access:1 to the bpf_prog, set it during eBPF >>>> verification phase, and test it inside sk_filter() if I see it >>>> correctly. >>> >>> That could also be done in an unlikely() branch, to keep the cost to >>> the non-eBPF >>> case near zero. >> >> Yes, agreed. For the time being, the majority of users are coming from the >> classic BPF side anyway and the unlikely() could still be changed later on >> if it should not be the case anymore. The flag and bpf_func would share the >> same cacheline as well. > > was also thinking that we can do it only in paths that actually > have multiple protocol layers, since today bpf is mainly used with > tcpdump(raw_socket) and new af_packet fanout both have cb cleared > on RX, because it just came out of alloc_skb and no layers were called, > and on TX we can clear 20 bytes in dev_queue_xmit_nit(). > af_unix/netlink also have clean skb. Need to analyze tun and sctp... > but it feels overly fragile to save a branch in sk_filter, > so planning to go with > if(unlikely(prog->cb_access)) memset in sk_filter(). I was also thinking that for dev_queue_xmit_nit(), since we do the skb_clone() there, to have a clone version (w/o affecting performance of the current one) that instead of copying cb[] over, it would just do a memset(). But that would just be limited to AF_PACKET, and doesn't catch all sk_filter() users. Thanks, Daniel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752810AbbJFSEI (ORCPT ); Tue, 6 Oct 2015 14:04:08 -0400 Received: from www62.your-server.de ([213.133.104.62]:37967 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751531AbbJFSEG (ORCPT ); Tue, 6 Oct 2015 14:04:06 -0400 Message-ID: <56140D0B.2040504@iogearbox.net> Date: Tue, 06 Oct 2015 20:03:55 +0200 From: Daniel Borkmann User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Alexei Starovoitov , Ingo Molnar CC: "David S. Miller" , Andy Lutomirski , Hannes Frederic Sowa , Eric Dumazet , Kees Cook , linux-api@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH net-next 1/2] bpf: enable non-root eBPF programs References: <1444078101-29060-1-git-send-email-ast@plumgrid.com> <1444078101-29060-2-git-send-email-ast@plumgrid.com> <5612F639.2050305@iogearbox.net> <56131B1F.80002@plumgrid.com> <20151006071347.GB14093@gmail.com> <561380BB.4040506@iogearbox.net> <20151006082048.GA18287@gmail.com> <561388D1.30406@iogearbox.net> <561409EC.5050005@plumgrid.com> In-Reply-To: <561409EC.5050005@plumgrid.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Authenticated-Sender: daniel@iogearbox.net Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/06/2015 07:50 PM, Alexei Starovoitov wrote: > On 10/6/15 1:39 AM, Daniel Borkmann wrote: >>>> [...] Also classic BPF would then need to test for it, since a socket >>>> filter >>>> doesn't really know whether native eBPF is loaded there or a >>>> classic-to-eBPF >>>> transformed one, and classic never makes use of this. Anyway, it >>>> could be done >>>> by adding a bit flag cb_access:1 to the bpf_prog, set it during eBPF >>>> verification phase, and test it inside sk_filter() if I see it >>>> correctly. >>> >>> That could also be done in an unlikely() branch, to keep the cost to >>> the non-eBPF >>> case near zero. >> >> Yes, agreed. For the time being, the majority of users are coming from the >> classic BPF side anyway and the unlikely() could still be changed later on >> if it should not be the case anymore. The flag and bpf_func would share the >> same cacheline as well. > > was also thinking that we can do it only in paths that actually > have multiple protocol layers, since today bpf is mainly used with > tcpdump(raw_socket) and new af_packet fanout both have cb cleared > on RX, because it just came out of alloc_skb and no layers were called, > and on TX we can clear 20 bytes in dev_queue_xmit_nit(). > af_unix/netlink also have clean skb. Need to analyze tun and sctp... > but it feels overly fragile to save a branch in sk_filter, > so planning to go with > if(unlikely(prog->cb_access)) memset in sk_filter(). I was also thinking that for dev_queue_xmit_nit(), since we do the skb_clone() there, to have a clone version (w/o affecting performance of the current one) that instead of copying cb[] over, it would just do a memset(). But that would just be limited to AF_PACKET, and doesn't catch all sk_filter() users. Thanks, Daniel