On Tue, Feb 27, 2024 at 8:51 AM Benjamin Tissoires benjamin.tissoires@redhat.com wrote:
On Tue, Feb 27, 2024 at 5:36 PM Eduard Zingerman eddyz87@gmail.com wrote:
On Tue, 2024-02-27 at 17:18 +0100, Benjamin Tissoires wrote: [...]
Hmm, I must still be missing a piece of the puzzle: if I declare bpf_timer_set_sleepable_cb() to take a third "aux" argument, given that it is declared as kfunc, I also must declare it in my bpf program, or I get the following:
# libbpf: extern (func ksym) 'bpf_timer_set_sleepable_cb': func_proto [264] incompatible with vmlinux [18151]
And if I declare it, then I don't know what to pass, given that this is purely added by the verifier:
43: (85) call bpf_timer_set_sleepable_cb#18152 arg#2 pointer type STRUCT bpf_prog_aux must point to scalar, or struct with scalar
Right, something has to be done about number of arguments and we don't have a convenient mechanism for this afaik.
The simplest way would be to have two kfuncs:
- one with 2 arguments, used form bpf program;
- another with 3 arguments, used at runtime;
- replace former by latter during rewrite.
It's hacky but seems interesting enough to be tested :)
Too hacky imo :)
Let's follow the existing pattern. See: __bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void *meta__ign)
__ign suffix tells the verifier to ignore it.
Then we do: #define bpf_obj_new(type) \ ((type *)bpf_obj_new_impl(bpf_core_type_id_local(type), NULL))
and later the verifier replaces arg2 with the correct pointer.
We also could use the suffix (like __uninit, __k, etc...), but it might introduce more headaches than the 2 kfuncs you are proposing.
Only one kfunc pls. Let's not make it more complex than necessary.
We cannot easily add a suffix to tell libbpf to ignore that arg, since bpf_core_types_are_compat() compares types and there are no argument names in the types. So it will be a significant surgery for libbpf to find the arg name in vmlinux BTF and strcmp the suffix.
Could you please provide more details on what exactly it complains about?
Well, there is a simple reason: that code is never reached because, in that function, there is a `if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL)` above that unconditionally terminates with a `continue`. So basically this part of the code is never hit.
I'll include that new third argument and the dual kfunc call in fixup_kfunc_call() and report if it works from here.
Something is wrong. fixup_kfunc_call() can rewrite args with whatever it wants. Are you sure you've added bpf_timer_set_sleepable_cb to special_kfunc_list ?