[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ec815b89-09aa-9e33-29b4-19e369ccfa21@iogearbox.net>
Date: Fri, 25 Sep 2020 17:42:45 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Alexei Starovoitov <ast@...nel.org>,
john fastabend <john.fastabend@...il.com>,
Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next 4/6] bpf, libbpf: add bpf_tail_call_static helper
for bpf programs
On 9/25/20 12:17 AM, Daniel Borkmann wrote:
> On 9/24/20 10:53 PM, Andrii Nakryiko wrote:
>> On Thu, Sep 24, 2020 at 11:22 AM Daniel Borkmann <daniel@...earbox.net> wrote:
>>>
>>> Port of tail_call_static() helper function from Cilium's BPF code base [0]
>>> to libbpf, so others can easily consume it as well. We've been using this
>>> in production code for some time now. The main idea is that we guarantee
>>> that the kernel's BPF infrastructure and JIT (here: x86_64) can patch the
>>> JITed BPF insns with direct jumps instead of having to fall back to using
>>> expensive retpolines. By using inline asm, we guarantee that the compiler
>>> won't merge the call from different paths with potentially different
>>> content of r2/r3.
>>>
>>> We're also using __throw_build_bug() macro in different places as a neat
>>> trick to trigger compilation errors when compiler does not remove code at
>>> compilation time. This works for the BPF backend as it does not implement
>>> the __builtin_trap().
>>>
>>> [0] https://github.com/cilium/cilium/commit/f5537c26020d5297b70936c6b7d03a1e412a1035
>>>
>>> Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
>>> ---
>>> tools/lib/bpf/bpf_helpers.h | 32 ++++++++++++++++++++++++++++++++
>>> 1 file changed, 32 insertions(+)
>>>
>>> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
>>> index 1106777df00b..18b75a4c82e6 100644
>>> --- a/tools/lib/bpf/bpf_helpers.h
>>> +++ b/tools/lib/bpf/bpf_helpers.h
>>> @@ -53,6 +53,38 @@
>>> })
>>> #endif
>>>
>>> +/*
>>> + * Misc useful helper macros
>>> + */
>>> +#ifndef __throw_build_bug
>>> +# define __throw_build_bug() __builtin_trap()
>>> +#endif
>>
>> this will become part of libbpf stable API, do we want/need to expose
>> it? If we want to expose it, then we should probably provide a better
>> description.
>>
>> But also curious, how is it better than _Static_assert() (see
>> test_cls_redirect.c), which also allows to provide a better error
>> message?
>
> Need to get back to you whether that has same semantics. We use the __throw_build_bug()
> also in __bpf_memzero() and friends [0] as a way to trigger a hard build bug if we hit
> a default switch-case [0], so we detect unsupported sizes which are not covered by the
> implementation yet. If _Static_assert (0, "foo") does the trick, we could also use that;
> will check with our code base.
So _Static_assert() won't work here, for example consider:
# cat f1.c
int main(void)
{
if (0)
_Static_assert(0, "foo");
return 0;
}
# clang -target bpf -Wall -O2 -c f1.c -o f1.o
f1.c:4:3: error: expected expression
_Static_assert(0, "foo");
^
1 error generated.
In order for it to work as required form the use-case, the _Static_assert() must not trigger
here given the path is unreachable and will be optimized away. I'll add a comment to the
__throw_build_bug() helper. Given libbpf we should probably also prefix with bpf_. If you see
a better name that would fit, pls let me know.
> [0] https://github.com/cilium/cilium/blob/master/bpf/include/bpf/builtins.h
Thanks,
Daniel
Powered by blists - more mailing lists