On 9/25/20 6:50 PM, Andrii Nakryiko wrote:
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
Right, but that won't work for example for the use case to detect switch cases which fall
into default case as mentioned with the mem* optimizations earlier in this thread.
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()?
I think the __bpf_unreachable() sounds best to me, will use that.
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
