netdev - Re: [PATCH bpf-next 2/2] libbpf, xsk: add libbpf_smp_store_release libbpf_smp_load

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 2 Mar 2021 20:38:49 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Björn Töpel <bjorn.topel@...il.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Björn Töpel <bjorn.topel@...el.com>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        Jonathan Lemon <jonathan.lemon@...il.com>, maximmi@...dia.com,
        Andrii Nakryiko <andrii@...nel.org>
Subject: Re: [PATCH bpf-next 2/2] libbpf, xsk: add libbpf_smp_store_release libbpf_smp_load_acquire

On Mon, Mar 1, 2021 at 2:43 AM Björn Töpel <bjorn.topel@...il.com> wrote:
>
> From: Björn Töpel <bjorn.topel@...el.com>
>
> Now that the AF_XDP rings have load-acquire/store-release semantics,
> move libbpf to that as well.
>
> The library-internal libbpf_smp_{load_acquire,store_release} are only
> valid for 32-bit words on ARM64.
>
> Also, remove the barriers that are no longer in use.
>
> Signed-off-by: Björn Töpel <bjorn.topel@...el.com>
> ---
>  tools/lib/bpf/libbpf_util.h | 72 +++++++++++++++++++++++++------------
>  tools/lib/bpf/xsk.h         | 17 +++------
>  2 files changed, 55 insertions(+), 34 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf_util.h b/tools/lib/bpf/libbpf_util.h
> index 59c779c5790c..94a0d7bb6f3c 100644
> --- a/tools/lib/bpf/libbpf_util.h
> +++ b/tools/lib/bpf/libbpf_util.h
> @@ -5,6 +5,7 @@
>  #define __LIBBPF_LIBBPF_UTIL_H
>
>  #include <stdbool.h>
> +#include <linux/compiler.h>
>
>  #ifdef __cplusplus
>  extern "C" {
> @@ -15,29 +16,56 @@ extern "C" {
>   * application that uses libbpf.
>   */
>  #if defined(__i386__) || defined(__x86_64__)
> -# define libbpf_smp_rmb() asm volatile("" : : : "memory")
> -# define libbpf_smp_wmb() asm volatile("" : : : "memory")
> -# define libbpf_smp_mb() \
> -       asm volatile("lock; addl $0,-4(%%rsp)" : : : "memory", "cc")
> -/* Hinders stores to be observed before older loads. */
> -# define libbpf_smp_rwmb() asm volatile("" : : : "memory")

So, technically, these four are part of libbpf's API, as libbpf_util.h
is actually installed on target hosts. Seems like xsk.h is the only
one that is using them, though.

So the question is whether it's ok to remove them now?

And also, why wasn't this part of xsk.h in the first place?

> +# define libbpf_smp_store_release(p, v)                                        \
> +       do {                                                            \
> +               asm volatile("" : : : "memory");                        \
> +               WRITE_ONCE(*p, v);                                      \
> +       } while (0)
> +# define libbpf_smp_load_acquire(p)                                    \
> +       ({                                                              \
> +               typeof(*p) ___p1 = READ_ONCE(*p);                       \
> +               asm volatile("" : : : "memory");                        \
> +               ___p1;                                                  \
> +       })

[...]