On 8/21/24 6:32 PM, Alexei Starovoitov wrote:
On Wed, Aug 21, 2024 at 4:35 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
From: Martin KaFai Lau <martin.lau@xxxxxxxxxx>
The existing prologue has been able to call bpf helper but not a kfunc.
This patch allows the prologue/epilogue to call the kfunc.
The subsystem that implements the .gen_prologue and .gen_epilogue
can add the BPF_PSEUDO_KFUNC_CALL instruction with insn->imm
set to the btf func_id of the kfunc call. This part is the same
as the bpf prog loaded from the sys_bpf.
I don't understand the value of this feature, since it seems
pretty hard to use.
The module (qdisc-bpf or else) would need to do something
like patch 8/8:
+BTF_ID_LIST(st_ops_epilogue_kfunc_list)
+BTF_ID(func, bpf_kfunc_st_ops_inc10)
+BTF_ID(func, bpf_kfunc_st_ops_inc100)
just to be able to:
BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0,
st_ops_epilogue_kfunc_list[0]);
So a bunch of extra work on the module side and
a bunch of work in this patch to enable such a pattern,
but what is the value?
gen_epilogue() can call arbitrary kernel function.
It doesn't have to be a helper.
kfunc-s provide calling convention conversion from bpf to native,
but the same thing is achieved by BPF_CALL_N macro.
The module can use that macro without adding an actual bpf helper
to uapi bpf.h.
Then in gen_epilogue() the extra bpf insn can use:
BPF_EMIT_CALL(module_provided_helper_that_is_not_helper)
which will use
BPF_CALL_IMM(x) ((void *)(x) - (void *)__bpf_call_base)
BPF_EMIT_CALL() was my earlier thought. I switched to the kfunc in this patch
because of the bpf_jit_supports_far_kfunc_call() support for the kernel module.
Using kfunc call will make supporting it the same.
I think the future bpf-qdisc can enforce built-in. bpf-tcp-cc has already been
built-in only also. I think the hid_bpf is built-in only also.
Another consideration is also holding the module refcnt when having an
attachable bpf prog calling a kernel func implemented in a kernel module. iiuc,
this is the reason why aux->kfunc_btf_tab holds a reference to the kernel
module. This should not be a problem to struct_ops though because the struct_ops
map is the one that is attachable instead of the struct_ops prog. The struct_ops
map has already held a refcnt of the module.
to populate imm.
And JITs will emit jump to that wrapper code provided by
BPF_CALL_N.
And no need for this extra complexity in the verifier and
its consumers that have to figure out (module_fd, btf_id) for
kfunc just to fit into kfunc pattern with btf_distill_func_proto().
I guess one can argue that if such kfunc is already available
to bpf prog then extra BPF_CALL_N wrapper for the same thing
is a waste of kernel text, but this patch also adds quite a bit of
kernel text. So the cost of BPF_CALL_N (which is a zero on x86)
is acceptable.
