On Wed, Jan 3, 2024 at 12:57 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > On Tue, 2024-01-02 at 11:00 -0800, Andrii Nakryiko wrote: > > Add a few extra cases of global funcs with context arguments. This time > > rely on "arg:ctx" decl_tag (__arg_ctx macro), but put it next to > > "classic" cases where context argument has to be of an exact type that > > BPF verifier expects (e.g., bpf_user_pt_regs_t for kprobe/uprobe). > > > > Colocating all these cases separately from other global func args that > > rely on arg:xxx decl tags (in verifier_global_subprogs.c) allows for > > simpler backwards compatibility testing on old kernels. All the cases in > > test_global_func_ctx_args.c are supposed to work on older kernels, which > > was manually validated during development. > > > > Acked-by: Jiri Olsa <jolsa@xxxxxxxxxx> > > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > > --- > > At first I thought that we would need to add a new CI job that would > run these tests for some older kernel version. libbpf CI will test this on 5.15 kernel, so we will have regression tests > > However, the transformation of the sub-program parameters happens > unconditionally. So it should be possible to read BTF for some of the > programs after they are loaded and check if transformation is applied > as expected. Thus allowing to check __arg_ctx handling on libbpf side > w/o the need to run on old kernel. Yes, but it's bpf_prog_info to get func_info (actually two calls due to how API is), parse func_info (pain without libbpf internal helpers from libbpf_internal.h, and with it's more coupling) to find subprog's func BTF ID and then check BTF. It's so painful that I don't think it's worth it given we'll test this in libbpf CI (and I did manual check locally as well). Also, nothing actually prevents us from not doing this if the kernel supports __arg_ctx natively, which is just a painful feature detector to write, using low-level APIs, which is why I decided that it's simpler to just do this unconditionally. > I think it's worth it to add such test, wdyt? > I feel like slacking and not adding this :) This will definitely fail in libbpf CI, if it's wrong.
