Re: [PATCH bpf-next v3 2/9] bpf: Support __arena and __arena_nullable on struct_ops arguments

On Mon, 2026-08-03 at 14:51 +0200, Kumar Kartikeya Dwivedi wrote:

Lgtm except for several nits below.

...

> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c

...

> @@ -226,26 +229,30 @@ static int prepare_arg_info(struct btf *btf,
>  	info = info_buf;
>  	for (arg_no = 0; arg_no < nargs; arg_no++) {
>  		/* Skip arguments that is not suffixed with
> -		 * "__nullable or __ref".
> +		 * "__nullable", "__ref", "__arena" or "__arena_nullable".
>  		 */
>  		is_nullable = btf_param_match_suffix(btf, &stub_args[arg_no],
>  						     MAYBE_NULL_SUFFIX);
>  		is_refcounted = btf_param_match_suffix(btf, &stub_args[arg_no],
>  						       REFCOUNTED_SUFFIX);
> +		is_arena_nullable = btf_param_match_suffix(btf, &stub_args[arg_no],
> +							   ARENA_MAYBE_NULL_SUFFIX);
> +		is_arena = btf_param_match_suffix(btf, &stub_args[arg_no], ARENA_SUFFIX);
>  
>  		if (is_nullable)
>  			suffix = MAYBE_NULL_SUFFIX;
>  		else if (is_refcounted)
>  			suffix = REFCOUNTED_SUFFIX;
> +		else if (is_arena_nullable)
> +			suffix = ARENA_MAYBE_NULL_SUFFIX;
> +		else if (is_arena)
> +			suffix = ARENA_SUFFIX;

As discussed already, that's unfortunate that p__arena and p__nullable
are valid, but p__arena__nullable is not, substituted by
p__arena_nullable. Same goes for global subprogs.

>  		else
>  			continue;
>  
> -		/* Should be a pointer to struct */
> -		pointed_type = btf_type_resolve_ptr(btf,
> -						    args[arg_no].type,
> -						    &arg_btf_id);
> -		if (!pointed_type ||
> -		    !btf_type_is_struct(pointed_type)) {
> +		/* Should be a pointer to struct, or any pointer for __arena/__arena_nullable */
> +		pointed_type = btf_type_resolve_ptr(btf, args[arg_no].type, &arg_btf_id);
> +		if (!pointed_type || (!is_arena && !is_arena_nullable && !btf_type_is_struct(pointed_type))) {
>  			pr_warn("stub function %s has %s tagging to an unsupported type\n",
>  				stub_fname, suffix);
>  			goto err_out;

...

> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index ed7999ad6c66..d2d7a2904345 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c

...

> @@ -695,6 +743,22 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
>  		goto out;
>  	}
>  
> +	/*
> +	 * Arena ctx args are converted only by the struct_ops indirect
> +	 * trampoline, which dispatches to a single known prog. Generic
> +	 * trampolines can mix progs with different arenas, so no conversion
> +	 * is possible here. Not reachable today: only struct_ops progs get
> +	 * arena ctx args and they never ride generic trampolines.
> +	 */
> +	for (kind = 0; kind < BPF_TRAMP_MAX; kind++) {
> +		for (i = 0; i < tnodes[kind].nr_nodes; i++) {
> +			if (bpf_prog_has_arena_ctx_arg(tnodes[kind].nodes[i]->link->prog)) {
> +				err = -ENOTSUPP;
> +				goto out;
> +			}
> +		}
> +	}

Nit: should this be done in __bpf_trampoline_link_prog() or
     bpf_trampoline_add_prog()? To avoid doing the same check on unlink.
     Since this is an invariant violation, do we want to add WARN_ONCE?

> +
>  	/* clear all bits except SHARE_IPMODIFY and TAIL_CALL_CTX */
>  	tr->flags &= (BPF_TRAMP_F_SHARE_IPMODIFY | BPF_TRAMP_F_TAIL_CALL_CTX);
>  

...