All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kumar Kartikeya Dwivedi <memxor@gmail.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <martin.lau@kernel.org>,
	Dave Marchevsky <davemarchevsky@meta.com>
Subject: Re: [PATCH bpf-next v10 11/24] bpf: Rewrite kfunc argument handling
Date: Fri, 18 Nov 2022 16:07:30 +0530	[thread overview]
Message-ID: <20221118103730.nbai3gzifkjk45eo@apollo> (raw)
In-Reply-To: <20221118033415.vpy2v2ypb4c2n6cn@MacBook-Pro-5.local>

On Fri, Nov 18, 2022 at 09:04:15AM IST, Alexei Starovoitov wrote:
> On Fri, Nov 18, 2022 at 07:26:01AM +0530, Kumar Kartikeya Dwivedi wrote:
> >  static int btf_check_func_arg_match(struct bpf_verifier_env *env,
> >  				    const struct btf *btf, u32 func_id,
> >  				    struct bpf_reg_state *regs,
> >  				    bool ptr_to_mem_ok,
> > -				    struct bpf_kfunc_arg_meta *kfunc_meta,
> >  				    bool processing_call)
>
> Something odd here.
> Benjamin added the processing_call flag in
> commit 95f2f26f3cac ("bpf: split btf_check_subprog_arg_match in two")
> and we discussed to remove it.
>
> >  		} else if (ptr_to_mem_ok && processing_call) {
>
> since kfunc bit is gone from here the processing_call can be removed.
> ptr_to_mem_ok and processing_call are two bool flags for the same thing, right?
>

I think so, I'll check it out and send a follow up patch.

> > +static int process_kf_arg_ptr_to_kptr_strong(struct bpf_verifier_env *env,
>
> I fixed this bit while applying.
>

Thanks.

> > +static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_arg_meta *meta)
>
> This function looks much better now.
> The split of kfunc vs helper was long overdue.
> Thank you for doing this.
>
> I'm not convinced that KF_ARG_* is necessary, but it's much better than before.
> So it's a step forward.
>

Yes. Eventually we should be merging checks for both helpers and kfuncs, but
that needs more work and would have been out of scope for this set. We can
probably synthesize a bpf_func_proto for the kfunc from BTF and then offload to
check_helper_call.

> Pls watch for CI errors and follow up when necessary.

Will do.

Thanks!

  reply	other threads:[~2022-11-18 10:37 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-18  1:55 [PATCH bpf-next v10 00/24] Allocated objects, BPF linked lists Kumar Kartikeya Dwivedi
2022-11-18  1:55 ` [PATCH bpf-next v10 01/24] bpf: Fix early return in map_check_btf Kumar Kartikeya Dwivedi
2022-11-18  1:55 ` [PATCH bpf-next v10 02/24] bpf: Do btf_record_free outside map_free callback Kumar Kartikeya Dwivedi
2022-11-18  1:55 ` [PATCH bpf-next v10 03/24] bpf: Free inner_map_meta when btf_record_dup fails Kumar Kartikeya Dwivedi
2022-11-18  1:55 ` [PATCH bpf-next v10 04/24] bpf: Populate field_offs for inner_map_meta Kumar Kartikeya Dwivedi
2022-11-18  1:55 ` [PATCH bpf-next v10 05/24] bpf: Introduce allocated objects support Kumar Kartikeya Dwivedi
2022-11-18  1:55 ` [PATCH bpf-next v10 06/24] bpf: Recognize lock and list fields in allocated objects Kumar Kartikeya Dwivedi
2022-11-18  1:55 ` [PATCH bpf-next v10 07/24] bpf: Verify ownership relationships for user BTF types Kumar Kartikeya Dwivedi
2022-11-18  1:55 ` [PATCH bpf-next v10 08/24] bpf: Allow locking bpf_spin_lock in allocated objects Kumar Kartikeya Dwivedi
2022-11-18  1:55 ` [PATCH bpf-next v10 09/24] bpf: Allow locking bpf_spin_lock global variables Kumar Kartikeya Dwivedi
2022-11-18  1:56 ` [PATCH bpf-next v10 10/24] bpf: Allow locking bpf_spin_lock in inner map values Kumar Kartikeya Dwivedi
2022-11-18  1:56 ` [PATCH bpf-next v10 11/24] bpf: Rewrite kfunc argument handling Kumar Kartikeya Dwivedi
2022-11-18  3:34   ` Alexei Starovoitov
2022-11-18 10:37     ` Kumar Kartikeya Dwivedi [this message]
2022-11-18 18:02       ` Alexei Starovoitov
2022-11-18 19:00         ` Kumar Kartikeya Dwivedi
2022-11-18 18:08       ` Alexei Starovoitov
2022-11-18 19:40   ` David Vernet
2022-11-20 19:25     ` Kumar Kartikeya Dwivedi
2022-11-18  1:56 ` [PATCH bpf-next v10 12/24] bpf: Support constant scalar arguments for kfuncs Kumar Kartikeya Dwivedi
2022-11-18  1:56 ` [PATCH bpf-next v10 13/24] bpf: Introduce bpf_obj_new Kumar Kartikeya Dwivedi
2022-11-18  1:56 ` [PATCH bpf-next v10 14/24] bpf: Introduce bpf_obj_drop Kumar Kartikeya Dwivedi
2022-11-18  1:56 ` [PATCH bpf-next v10 15/24] bpf: Permit NULL checking pointer with non-zero fixed offset Kumar Kartikeya Dwivedi
2022-11-18  1:56 ` [PATCH bpf-next v10 16/24] bpf: Introduce single ownership BPF linked list API Kumar Kartikeya Dwivedi
2022-11-21 18:34   ` Nathan Chancellor
2022-11-18  1:56 ` [PATCH bpf-next v10 17/24] bpf: Add 'release on unlock' logic for bpf_list_push_{front,back} Kumar Kartikeya Dwivedi
2022-11-18  1:56 ` [PATCH bpf-next v10 18/24] bpf: Add comments for map BTF matching requirement for bpf_list_head Kumar Kartikeya Dwivedi
2022-11-18  1:56 ` [PATCH bpf-next v10 19/24] selftests/bpf: Add __contains macro to bpf_experimental.h Kumar Kartikeya Dwivedi
2022-11-18  1:56 ` [PATCH bpf-next v10 20/24] selftests/bpf: Update spinlock selftest Kumar Kartikeya Dwivedi
2022-11-18  1:56 ` [PATCH bpf-next v10 21/24] selftests/bpf: Add failure test cases for spin lock pairing Kumar Kartikeya Dwivedi
2022-11-18  1:56 ` [PATCH bpf-next v10 22/24] selftests/bpf: Add BPF linked list API tests Kumar Kartikeya Dwivedi
2023-10-11 22:44   ` Andrii Nakryiko
2023-10-11 23:02     ` Kumar Kartikeya Dwivedi
2023-10-20  0:15       ` Andrii Nakryiko
2023-10-20 14:51         ` Kumar Kartikeya Dwivedi
2022-11-18  1:56 ` [PATCH bpf-next v10 23/24] selftests/bpf: Add BTF sanity tests Kumar Kartikeya Dwivedi
2022-11-18  1:56 ` [PATCH bpf-next v10 24/24] selftests/bpf: Temporarily disable linked list tests Kumar Kartikeya Dwivedi
2022-11-22 17:24   ` Alexei Starovoitov
2022-11-18  3:40 ` [PATCH bpf-next v10 00/24] Allocated objects, BPF linked lists patchwork-bot+netdevbpf

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=20221118103730.nbai3gzifkjk45eo@apollo \
    --to=memxor@gmail.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davemarchevsky@meta.com \
    --cc=martin.lau@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.