On Thu, 14 Dec 2023 at 17:08, Kumar Kartikeya Dwivedi memxor@gmail.com wrote:
On Thu, 14 Dec 2023 at 00:49, Eyal Birger eyal.birger@gmail.com wrote:
On Wed, Dec 13, 2023 at 3:15 PM Daniel Xu dxu@dxuuu.xyz wrote:
[...]
diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c index c0dd38616562..f00dba85ac5d 100644 --- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c +++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c @@ -8,8 +8,9 @@ */ #include "vmlinux.h" #include <bpf/bpf_core_read.h> -#include <bpf/bpf_helpers.h> #include <bpf/bpf_endian.h> +#include <bpf/bpf_helpers.h> +#include "bpf_experimental.h" #include "bpf_kfuncs.h" #include "bpf_tracing_net.h"
@@ -988,8 +989,9 @@ int xfrm_get_state_xdp(struct xdp_md *xdp) opts.family = AF_INET;
x = bpf_xdp_get_xfrm_state(xdp, &opts, sizeof(opts));
if (!x || opts.error)
if (!x) goto out;bpf_assert_with(opts.error == 0, XDP_PASS); if (!x->replay_esn) goto out;results in:
57: (b7) r1 = 2 ; R1_w=2 refs=5 58: (85) call bpf_throw#115436 calling kernel function bpf_throw is not allowed
I think this might be because bpf_throw is not registered for use by BPF_PROG_TYPE_XDP. I would simply register the generic_kfunc_set for this program type as well, since it's already done for TC.
Ah yeah, that was it.
It looks like the above error comes from verifier.c:fetch_kfunc_meta, but I can run the exceptions selftests just fine with the same bzImage. So I'm thinking it's not a kfunc registration or BTF issue.
Maybe it's cuz I'm holding onto KFUNC_ACQUIRE'd `x`? Not sure.
Yes, even once you enable this, this will fail for now. I am sending out a series later this week that enables bpf_throw with acquired references, but until then may I suggest the following:
#define bpf_assert_if(cond) for (int ___i = 0, ___j = (cond); !(___j) \ && !___j; bpf_throw(), ___i++)
This will allow you to insert some cleanup code with an assertion. Then in my series, I will convert this temporary bpf_assert_if back to the normal bpf_assert.
It would look like: bpf_assert_if(opts.error == 0) { // Execute if assertion failed bpf_xdp_xfrm_state_release(x); }
Likewise for bpf_assert_with_if, you get the idea.
I gave it a try and I'm getting this compile error:
progs/test_tunnel_kern.c:996:2: error: variable '___j' used in loop condition not modified in loop body [-Werror,-Wfor-loop-analysis] bpf_assert_with_if(opts.error == 0, XDP_PASS) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/dxu/dev/linux/tools/testing/selftests/bpf/bpf_experimental.h:295:38: note: expanded from macro 'bpf_assert_with_if' for (int ___i = 0, ___j = (cond); !(___j) && !___j; bpf_throw(value), ___i++) ^~~~ ~~~~ 1 error generated. make: *** [Makefile:618: /home/dxu/dev/linux/tools/testing/selftests/bpf/test_tunnel_kern.bpf.o] Error 1Seems like the compiler is being clever.
It looks like ___j is used twice - maybe it was meant to be ___i? i.e.:
for (int ___i = 0, ___j = (cond); !(___j) && !___i; bpf_throw(value), ___i++)
Ah, yes, that's a typo. Eyal is right, it should be ___i.
Additionally, I would modify the macro to do ___j = !!(cond).