From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Borkmann Subject: Re: [PATCH v3 bpf-next 03/10] bpf: introduce per-cpu cgroup local storage Date: Fri, 28 Sep 2018 14:03:29 +0200 Message-ID: <21a7ecfb-c7cd-d7aa-01d1-4e055e410817@iogearbox.net> References: <20180926113326.29069-1-guro@fb.com> <20180926113326.29069-4-guro@fb.com> <20180928084528.i5txkac34pmqvs3p@ast-mbp.dhcp.thefacebook.com> <20180928100302.GB9018@castle.DHCP.thefacebook.com> <20180928102458.dbia6xnxkijvkld6@ast-mbp.dhcp.thefacebook.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, Song Liu , linux-kernel@vger.kernel.org, kernel-team@fb.com, Alexei Starovoitov To: Alexei Starovoitov , Roman Gushchin Return-path: In-Reply-To: <20180928102458.dbia6xnxkijvkld6@ast-mbp.dhcp.thefacebook.com> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On 09/28/2018 12:25 PM, Alexei Starovoitov wrote: > On Fri, Sep 28, 2018 at 11:03:03AM +0100, Roman Gushchin wrote: >>>> + >>>> + if (unlikely(map_flags & BPF_EXIST)) >>>> + return -EINVAL; >>> >>> that should have been BPF_NOEXIST ? >> >> Yeah, or maybe even better s/&/!= ? >> It's probably better to require BPF_EXIST flag to update a cgroup storage? >> Agree? If so, let me fix this for both shared and per-cpu versions in >> a follow-up patch. > > I think BPF_ANY is technically valid too. > If we were to require strict BPF_EXIST only, we'd need to fix stable too. > I'm fine with both (BPF_EXIST only and BPF_ANY|BPF_EXIST). > Daniel, what do you think? I'm okay with either option, both seem plausible.