On Sat, Apr 4, 2026 at 12:49 PM Ihor Solodrai <[email protected]> wrote: > > > > On 4/3/26 10:41 AM, Chengkaitao wrote: > > On Tue, Mar 31, 2026 at 1:05 AM Alexei Starovoitov > > <[email protected]> wrote: > >> > >> On Sun, Mar 29, 2026 at 7:05 AM Chengkaitao <[email protected]> wrote: > >>> > >>> From: Kaitao Cheng <[email protected]> > >>> > >>> [...] > >>> + > >>> +/* Kfunc family related to spin_lock. */ > >>> +static const enum special_kfunc_type bpf_res_spin_lock_api_kfuncs[] = { > >>> + KF_bpf_res_spin_lock, > >>> + KF_bpf_res_spin_unlock, > >>> + KF_bpf_res_spin_lock_irqsave, > >>> + KF_bpf_res_spin_unlock_irqrestore, > >>> +}; > >> > >> I think it's a step in the wrong direction. > >> I'd wait for Ihor's BTF_ID_NAMED cleanup. > > > > After reading Ihor's messages on the list, if I understand correctly, > > our two approaches seem to target different problems. What Ihor's > > work appears to achieve is the ability to remove the entire enum > > special_kfunc_type. My goal, on the other hand, is to replace many > > scattered func_id == special_kfunc_list[...] comparisons with a > > table-driven approach. > > Hi Kaitao, > > I appreciate your efforts, however after a quick pass over the changes > you propose (both here and in the new series) with respect to BTF_ID > macros and special_kfuncs_list, I don't understand what problem you're > trying to solve. > > The inherent complexity is in the fact that the verifier must know > when a particular BTF id identifies a specific kfunc, or whether it > belongs to some pre-defined set of ids. This is why > special_kfuncs_list and other BTF_ID_SET/LIST-s exist. > > And so there is no way around defining those ids and sets *somewhere*, > and so far BTF_ID_* macros did a fine job of that, all things > considered. > > AFAICT your changes simply move around the same definitions from > functions with if statements to constant arrays with a runtime search > on them (which is slower by the way). What is the benefit of that vs > the current implementation? We still have to maintain those arrays in > the same way we have to maintain the is_foo_kfunc helpers. > > Your newer proposal [1] takes the same idea to the next level, by > introducing an entire new BTF kind, new ELF sections and a bunch of > macros that are no less complicated than existing. And all of that > just moves the same arrays "upstream" to the .BTF_ids section. Again, > I fail to see any benefits to that complexity. Having differentiation > between LIST and SET, and having to mark START and END is not a > problem that needs solving IMO.
Your analysis of the code implementation for the new proposal is correct. Let me elaborate on the purpose behind my approach. ****** Purpose 1 ****** As described in this patch: https://lore.kernel.org/bpf/[email protected]/ If we want to add a new kfunc, bpf_list_add_impl, we would today have to add "btf_id == special_kfunc_list[KF_bpf_list_back]" (or similar) five times in verifier.c. Under the newer proposal, that is no longer necessary: defining the kfunc and its verifier metadata in one place is enough, for example: __bpf_kfunc int bpf_list_add_impl(struct bpf_list_head *head, struct bpf_list_node *new, struct bpf_list_node *prev, void *meta__ign, u64 off) { /* kfunc implementation */ ....... } BPF_VERIF_KFUNC_DEF(bpf_list_add_impl, list_api, graph_node_api, ... ) If BPF_VERIF_KFUNC_DEF is extended further, BTF_ID(func, bpf_list_add_impl) and BTF_ID_FLAGS(func, bpf_list_add_impl) might also become unnecessary, so the snippet above could eventually be close to all the code required to add a new kfunc. ****** Purpose 2 ****** The kernel no longer needs enum special_kfunc_type to list every KF_bpf_* entry. That information is folded into the .BTF_ids.##sfx section instead, so kfunc authors do not have to touch or think about special_kfunc_type. ****** Purpose 3 ****** As described in this patch: https://lore.kernel.org/bpf/[email protected]/ In is_bpf_list_api_kfunc(u32 btf_id) there are on the order of eleven "btf_id == special_kfunc_list[*]" comparisons. As more kfuncs are added, every is_bpf_* helper will only grow longer and the verifier will get more repetitive. With the new design, those is_bpf_* helpers can be removed entirely, including the awkward scattered "btf_id == *" checks. ****** Purpose 4 ****** It pushes us to untangle messy verifier safety cases and make them modular, so they can be expressed as parameters to BPF_VERIF_KFUNC_DEF > The work I was discussing with Alexei [2] is targeted: get rid of > constant enums that mirror existing kfunc symbols by making BTF_ID > internals a bit smarter. And the benefit is clear and simple. > > If you could explain what exact issue you're trying to address with > your BTF_ID refactoring patches, maybe we can try converging on a > reasonable approach to it. > > Thanks! > > [1] https://lore.kernel.org/bpf/[email protected]/ > [2] > https://lore.kernel.org/bpf/[email protected]/ > > > > > > That said, once Ihor's solution lands, my approach would no longer > > be applicable as-is. So I have been thinking about an alternative, > > please see the patch linked below for details: > > https://lore.kernel.org/bpf/[email protected]/ > > > >> Kaitao Cheng, > >> > >> also please start your part of code reviews. > >> Your patches are not going to be landing if you don't code review. > >> > >> https://lore.kernel.org/bpf/CAADnVQ+TKKptnNB25V3=bcdybh5g6c2dyw2sytxvyravnpn...@mail.gmail.com/ > > > > I will do my best to carry this forward. -- Yours, Chengkaitao
