Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752281AbaKDXIG (ORCPT ); Tue, 4 Nov 2014 18:08:06 -0500 Received: from mail-wi0-f169.google.com ([209.85.212.169]:52622 "EHLO mail-wi0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751303AbaKDXIE (ORCPT ); Tue, 4 Nov 2014 18:08:04 -0500 MIME-Version: 1.0 In-Reply-To: <5458A17B.7030904@redhat.com> References: <1415069656-14138-1-git-send-email-ast@plumgrid.com> <1415069656-14138-7-git-send-email-ast@plumgrid.com> <5458A17B.7030904@redhat.com> Date: Tue, 4 Nov 2014 15:08:03 -0800 Message-ID: Subject: Re: [PATCH net-next 6/7] bpf: allow eBPF programs to use maps From: Alexei Starovoitov To: Daniel Borkmann Cc: "David S. Miller" , Ingo Molnar , Andy Lutomirski , Hannes Frederic Sowa , Eric Dumazet , Linux API , Network Development , LKML Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 4, 2014 at 1:50 AM, Daniel Borkmann wrote: > These WARN_ON_ONCE(!rcu_read_lock_held()) seem odd. While I see the point > that > you're holding RCU read lock on the lookup, can you elaborate on your RCU > usage > here and why it's necessary for delete/update? > > I suspect due to the synchronize_rcu() you're using and not using any RCU > accessors but plain memcpy() e.g. in case of the array ...? Correct in case of array. Also hash delete/update() call into lookup() internally that is using _rcu() helpers... Future map types might have much more complex implementations (like LPM), so it helps to state the rules early. Another reason is more complex to explain: A program that intends to access maps has to be one rcu critical section. So all lookup/update/delete calls are under rcu_lock_held. Since programs by themselves cannot have WARN_ON inside them, I've added WARN_ON in these three functions that will be called from the programs to make sure that kernel subsystems don't do (*prog->bpf_func)(...) without taking rcu_lock if they intend to let programs access maps. Having said that in the future we might have a case for programs that don't call into these functions at all and execute instructions only. Those won't need rcu_lock() wrap. I experimented with that for the patch where I replaced pred-tree walker with eBPF program. There is no rcu there. And no calls to map accessors. Has to be noted, that socket filters use rcu to protect sk_filter pointer and program itself. So for that use case we'll keep using rcu for foreseeable future. For tracing filters I had to add rcu_lock() around BPF_PROG_RUN() invocation and these WARN_ON checks saved me a lot of headache, so I prefer to keep them since they cost nothing when lockdep is off. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/