netdev - Re: [PATCH bpf-next 1/3] bpf: Single-cpu updates for per-cpu maps

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]

Date:   Wed, 18 Dec 2019 16:49:27 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Yonghong Song <yhs@...com>
Cc:     Paul Chaignon <paul.chaignon@...nge.com>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "paul.chaignon@...il.com" <paul.chaignon@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin Lau <kafai@...com>, Song Liu <songliubraving@...com>,
        Andrii Nakryiko <andriin@...com>
Subject: Re: [PATCH bpf-next 1/3] bpf: Single-cpu updates for per-cpu maps

On Wed, Dec 18, 2019 at 11:11 AM Yonghong Song <yhs@...com> wrote:
>
>
>
> On 12/18/19 6:23 AM, Paul Chaignon wrote:
> > Currently, userspace programs have to update the values of all CPUs at
> > once when updating per-cpu maps.  This limitation prevents the update of
> > a single CPU's value without the risk of missing concurrent updates on
> > other CPU's values.
> >
> > This patch allows userspace to update the value of a specific CPU in
> > per-cpu maps.  The CPU whose value should be updated is encoded in the
> > 32 upper-bits of the flags argument, as follows.  The new BPF_CPU flag
> > can be combined with existing flags.
> >
> >    bpf_map_update_elem(..., cpuid << 32 | BPF_CPU)
>
> Some additional comments beyond Alexei's one.
>
> >
> > Signed-off-by: Paul Chaignon <paul.chaignon@...nge.com>
> > ---
> >   include/uapi/linux/bpf.h       |  4 +++
> >   kernel/bpf/arraymap.c          | 19 ++++++++-----
> >   kernel/bpf/hashtab.c           | 49 ++++++++++++++++++++--------------
> >   kernel/bpf/local_storage.c     | 16 +++++++----
> >   kernel/bpf/syscall.c           | 17 +++++++++---
> >   tools/include/uapi/linux/bpf.h |  4 +++
> >   6 files changed, 74 insertions(+), 35 deletions(-)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index dbbcf0b02970..2efb17d2c77a 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -316,6 +316,10 @@ enum bpf_attach_type {
> >   #define BPF_NOEXIST 1 /* create new element if it didn't exist */
> >   #define BPF_EXIST   2 /* update existing element */
> >   #define BPF_F_LOCK  4 /* spin_lock-ed map_lookup/map_update */
> > +#define BPF_CPU              8 /* single-cpu update for per-cpu maps */
> > +
> > +/* CPU mask for single-cpu updates */
> > +#define BPF_CPU_MASK 0xFFFFFFFF00000000ULL
> BPF_F_CPU_MASK?

Maybe even define this as a function-like macro:

#define BPF_F_CPU_NR(cpu) ((cpu) << 32)

so that it can be easily combined with other flags in code like so:

bpf_map_update_element(...., BPF_F_LOCK | BPF_F_CPU_NR(10))

BPF_F_CPU_NR can automatically set BPF_F_CPU flag as well, btw.

[...]