bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: Kumar Kartikeya Dwivedi <memxor@gmail.com>, bpf@vger.kernel.org
Cc: Tejun Heo <tj@kernel.org>, Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau	 <martin.lau@kernel.org>,
	kkd@meta.com, kernel-team@meta.com
Subject: Re: [PATCH bpf-next v2] bpf: Add __prog tag to pass in prog->aux
Date: Tue, 13 May 2025 01:49:11 -0700	[thread overview]
Message-ID: <32b9c10381d7f0fe358f955437febf96e5d2f58a.camel@gmail.com> (raw)
In-Reply-To: <20250513025747.1519365-1-memxor@gmail.com>

On Mon, 2025-05-12 at 19:57 -0700, Kumar Kartikeya Dwivedi wrote:

[...]

In Documentation/bpf/kfuncs.rst there are descriptions for several
suffixes that are supported, "__prog" should be added there.

Do we want to add a separate test case for this feature?
It looks like there are no tests for other suffixes,
so relying on wq tests passing is probably fine.

Implementation lgtm, a few nits below.

>  include/linux/bpf_verifier.h |  1 +
>  kernel/bpf/helpers.c         |  4 ++--
>  kernel/bpf/verifier.c        | 33 +++++++++++++++++++++++++++------
>  3 files changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 9734544b6957..7dd85ed6059e 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -606,6 +606,7 @@ struct bpf_insn_aux_data {
>  	bool calls_callback;
>  	/* registers alive before this instruction. */
>  	u16 live_regs_before;
> +	u16 arg_prog;

Nit: there is a 4-bit hole after `fastcall_spills_num`,
     `arg_prog` field could be put there and 2 bytes at the tail
     would be remain for future extension.

>  };
> 
>  #define MAX_USED_MAPS 64 /* max number of maps accessed by one eBPF program */
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index fed53da75025..43cbf439b9fb 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -3012,9 +3012,9 @@ __bpf_kfunc int bpf_wq_start(struct bpf_wq *wq, unsigned int flags)
>  __bpf_kfunc int bpf_wq_set_callback_impl(struct bpf_wq *wq,
>  					 int (callback_fn)(void *map, int *key, void *value),
>  					 unsigned int flags,
> -					 void *aux__ign)
> +					 void *aux__prog)
>  {
> -	struct bpf_prog_aux *aux = (struct bpf_prog_aux *)aux__ign;
> +	struct bpf_prog_aux *aux = (struct bpf_prog_aux *)aux__prog;
>  	struct bpf_async_kern *async = (struct bpf_async_kern *)wq;
> 
>  	if (flags)
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 28f5a7899bd6..f409a06099f6 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -343,6 +343,7 @@ struct bpf_kfunc_call_arg_meta {
>  		int uid;
>  	} map;
>  	u64 mem_size;
> +	u32 arg_prog;

Nit: this is a boolean flag, I'd put it after `bool arg_owning_ref`,
     as there is a 3 bytes hole there.

>  };
> 
>  struct btf *btf_vmlinux;

[...]

@@ -12906,6 +12912,17 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
 		if (is_kfunc_arg_ignore(btf, &args[i]))
 			continue;
 
+		if (is_kfunc_arg_prog(btf, &args[i])) {
+			/* Used to reject repeated use of __aux. */
                                                          ^^^^^^
                                               Nit: should be __prog.

+			if (meta->arg_prog) {
+				verbose(env, "Only 1 prog->aux argument supported per-kfunc\n");
+				return -EFAULT;
+			}
+			meta->arg_prog = regno;
+			cur_aux(env)->arg_prog = regno;
+			continue;
+		}
+
 		if (btf_type_is_scalar(t)) {
 			if (reg->type != SCALAR_VALUE) {
 				verbose(env, "R%d is not a scalar\n", regno);


      reply	other threads:[~2025-05-13  8:49 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-13  2:57 [PATCH bpf-next v2] bpf: Add __prog tag to pass in prog->aux Kumar Kartikeya Dwivedi
2025-05-13  8:49 ` Eduard Zingerman [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=32b9c10381d7f0fe358f955437febf96e5d2f58a.camel@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@meta.com \
    --cc=kkd@meta.com \
    --cc=martin.lau@kernel.org \
    --cc=memxor@gmail.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).