Re: [PATCH bpf-next v8 1/8] bpf: refactor kfunc checks using table-driven approach in verifier

Emil Tsalapatis
2026-03-19T15:39:36Z

On Mon Mar 16, 2026 at 7:28 AM EDT, Chengkaitao wrote:
> From: Kaitao Cheng <chengkaitao@kylinos.cn>
>
> Replace per-kfunc btf_id chains in list/rbtree/res_lock and graph node
> checks with btf_id_in_kfunc_table() and static kfunc tables for easier
> maintenance.
>
> Prepare for future extensions to the bpf_list API family.
>
> Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>

The change is valid, can you see if you can remove some of the
is_bpf_*api_kfunc calls areound line 13000 and replace them with
btf_id_in_kfunc_table calls?

And can you also do this refactoring for the other kfunc families? That
way we replace all functions that match on the btf_id into a single call
that takes in the array we are scanning for a match.

> ---
> kernel/bpf/verifier.c | 79 +++++++++++++++++++++++++++++++------------
> 1 file changed, 57 insertions(+), 22 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 4fbacd2149cd..64c1f8343dfa 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -12639,6 +12639,53 @@ BTF_ID(func, bpf_session_is_return)
> BTF_ID(func, bpf_stream_vprintk)
> BTF_ID(func, bpf_stream_print_stack)
>
> +static const enum special_kfunc_type bpf_list_api_kfuncs[] = {
> + KF_bpf_list_push_front_impl,
> + KF_bpf_list_push_back_impl,
> + KF_bpf_list_pop_front,
> + KF_bpf_list_pop_back,
> + KF_bpf_list_front,
> + KF_bpf_list_back,
> +};
> +
> +/* Kfuncs that take a list node argument (bpf_list_node *). */
Nit: Why add a description on just this and bpf_rbtree_node_api_kfuncs?
I think a small comment on top of each if fine if a bit wordy because
it's easy to spot when scanning for a specific kfunc family.
> +static const enum special_kfunc_type bpf_list_node_api_kfuncs[] = {
> + KF_bpf_list_push_front_impl,
> + KF_bpf_list_push_back_impl,
> +};
> +
> +/* Kfuncs that take an rbtree node argument (bpf_rb_node *). */
> +static const enum special_kfunc_type bpf_rbtree_node_api_kfuncs[] = {
> + KF_bpf_rbtree_remove,
> + KF_bpf_rbtree_add_impl,
> + KF_bpf_rbtree_left,
> + KF_bpf_rbtree_right,
> +};
> +
> +static const enum special_kfunc_type bpf_rbtree_api_kfuncs[] = {
> + KF_bpf_rbtree_add_impl,
> + KF_bpf_rbtree_remove,
> + KF_bpf_rbtree_first,
> + KF_bpf_rbtree_root,
> + KF_bpf_rbtree_left,
> + KF_bpf_rbtree_right,
> +};
> +
> +static const enum special_kfunc_type bpf_res_spin_lock_kfuncs[] = {
> + KF_bpf_res_spin_lock,
> + KF_bpf_res_spin_unlock,
> + KF_bpf_res_spin_lock_irqsave,
> + KF_bpf_res_spin_unlock_irqrestore,
> +};
> +
> +static bool btf_id_in_kfunc_table(u32 btf_id, const enum special_kfunc_type *kfuncs, int n)
> +{
> + for (int i = 0; i < n; i++)
> + if (btf_id == special_kfunc_list[kfuncs[i]])
> + return true;
> + return false;
> +}
> +
> static bool is_task_work_add_kfunc(u32 func_id)
> {
> return func_id == special_kfunc_list[KF_bpf_task_work_schedule_signal] ||
> @@ -13038,22 +13085,14 @@ static int check_reg_allocation_locked(struct bpf_verifier_env *env, struct bpf_
>
> static bool is_bpf_list_api_kfunc(u32 btf_id)
> {
> - return btf_id == special_kfunc_list[KF_bpf_list_push_front_impl] ||
> - btf_id == special_kfunc_list[KF_bpf_list_push_back_impl] ||
> - btf_id == special_kfunc_list[KF_bpf_list_pop_front] ||
> - btf_id == special_kfunc_list[KF_bpf_list_pop_back] ||
> - btf_id == special_kfunc_list[KF_bpf_list_front] ||
> - btf_id == special_kfunc_list[KF_bpf_list_back];
> + return btf_id_in_kfunc_table(btf_id, bpf_list_api_kfuncs,
> + ARRAY_SIZE(bpf_list_api_kfuncs));
> }
>
> static bool is_bpf_rbtree_api_kfunc(u32 btf_id)
> {
> - return btf_id == special_kfunc_list[KF_bpf_rbtree_add_impl] ||
> - btf_id == special_kfunc_list[KF_bpf_rbtree_remove] ||
> - btf_id == special_kfunc_list[KF_bpf_rbtree_first] ||
> - btf_id == special_kfunc_list[KF_bpf_rbtree_root] ||
> - btf_id == special_kfunc_list[KF_bpf_rbtree_left] ||
> - btf_id == special_kfunc_list[KF_bpf_rbtree_right];
> + return btf_id_in_kfunc_table(btf_id, bpf_rbtree_api_kfuncs,
> + ARRAY_SIZE(bpf_rbtree_api_kfuncs));
> }
>
> static bool is_bpf_iter_num_api_kfunc(u32 btf_id)
> @@ -13071,10 +13110,8 @@ static bool is_bpf_graph_api_kfunc(u32 btf_id)
>
> static bool is_bpf_res_spin_lock_kfunc(u32 btf_id)
> {
> - return btf_id == special_kfunc_list[KF_bpf_res_spin_lock] ||
> - btf_id == special_kfunc_list[KF_bpf_res_spin_unlock] ||
> - btf_id == special_kfunc_list[KF_bpf_res_spin_lock_irqsave] ||
> - btf_id == special_kfunc_list[KF_bpf_res_spin_unlock_irqrestore];
> + return btf_id_in_kfunc_table(btf_id, bpf_res_spin_lock_kfuncs,
> + ARRAY_SIZE(bpf_res_spin_lock_kfuncs));
> }
>
> static bool is_bpf_arena_kfunc(u32 btf_id)
> @@ -13163,14 +13200,12 @@ static bool check_kfunc_is_graph_node_api(struct bpf_verifier_env *env,
>
> switch (node_field_type) {
> case BPF_LIST_NODE:
> - ret = (kfunc_btf_id == special_kfunc_list[KF_bpf_list_push_front_impl] ||
> - kfunc_btf_id == special_kfunc_list[KF_bpf_list_push_back_impl]);
> + ret = btf_id_in_kfunc_table(kfunc_btf_id, bpf_list_node_api_kfuncs,
> + ARRAY_SIZE(bpf_list_node_api_kfuncs));
> break;
> case BPF_RB_NODE:
> - ret = (kfunc_btf_id == special_kfunc_list[KF_bpf_rbtree_remove] ||
> - kfunc_btf_id == special_kfunc_list[KF_bpf_rbtree_add_impl] ||
> - kfunc_btf_id == special_kfunc_list[KF_bpf_rbtree_left] ||
> - kfunc_btf_id == special_kfunc_list[KF_bpf_rbtree_right]);
> + ret = btf_id_in_kfunc_table(kfunc_btf_id, bpf_rbtree_node_api_kfuncs,
> + ARRAY_SIZE(bpf_rbtree_node_api_kfuncs));
> break;
> default:
> verbose(env, "verifier internal error: unexpected graph node argument type %s\n",