On Fri, Sep 25, 2020 at 8:52 AM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote: > > On 9/25/20 5:42 PM, Daniel Borkmann wrote: > > 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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> > >>>> --- > >>>> 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. > > .. aaand it looks like I need some more coffee. ;-) But result is the same after all: > > # clang -target bpf -Wall -O2 -c f1.c -o f1.o > f1.c:4:3: error: static_assert failed "foo" > _Static_assert(0, "foo"); > ^ ~ > 1 error generated. > > # cat f1.c > int main(void) > { > if (0) { > _Static_assert(0, "foo"); > } > return 0; > } You need still more :-P. For you use case it will look like this: $ cat test-bla.c int bar(int x) { _Static_assert(!__builtin_constant_p(x), "not a constant!"); return x; } int foo() { bar(123); return 0; } $ clang -target bpf -O2 -c test-bla.c -o test-bla.o $ echo $? 0 But in general to ensure unreachable code it's probably useful anyway to have this. How about calling it __bpf_build_error() or maybe even __bpf_unreachable()? > > > 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 >