linux-kernel - Re: [PATCH v2 bpf-next 00/13] Atomics for eBPF

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]

Message-ID: <5fc72beede900_15eb720850@john-XPS-13-9370.notmuch>
Date:   Tue, 01 Dec 2020 21:53:50 -0800
From:   John Fastabend <john.fastabend@...il.com>
To:     Yonghong Song <yhs@...com>,
        Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Brendan Jackman <jackmanb@...gle.com>,
        bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        KP Singh <kpsingh@...omium.org>,
        Florent Revest <revest@...omium.org>,
        open list <linux-kernel@...r.kernel.org>,
        Jann Horn <jannh@...gle.com>
Subject: Re: [PATCH v2 bpf-next 00/13] Atomics for eBPF

Yonghong Song wrote:
> 
> 

[...]

> > Great, this means that all existing valid uses of
> > __sync_fetch_and_add() will generate BPF_XADD instructions and will
> > work on old kernels, right?
> 
> That is correct.
> 
> > 
> > If that's the case, do we still need cpu=v4? The new instructions are
> > *only* going to be generated if the user uses previously unsupported
> > __sync_fetch_xxx() intrinsics. So, in effect, the user consciously
> > opts into using new BPF instructions. cpu=v4 seems like an unnecessary
> > tautology then?
> 
> This is a very good question. Essentially this boils to when users can 
> use the new functionality including meaningful return value  of 
> __sync_fetch_and_add().
>    (1). user can write a small bpf program to test the feature. If user
>         gets a failed compilation (fatal error), it won't be supported.
>         Otherwise, it is supported.
>    (2). compiler provides some way to tell user it is safe to use, e.g.,
>         -mcpu=v4, or some clang macro suggested by Brendan earlier.
> 
> I guess since kernel already did a lot of feature discovery. Option (1)
> is probably fine.

For option (2) we can use BTF with kernel version check. If kernel is
greater than kernel this lands in we use the the new instructions if
not we use a slower locked version. That should work for all cases
unless someone backports patches into an older case.

At least thats what I'll probably end up wrapping in a helper function.