All of lore.kernel.org
 help / color / mirror / Atom feed
From: Song Liu <song@kernel.org>
To: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: bpf <bpf@vger.kernel.org>, "Alexei Starovoitov" <ast@kernel.org>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>,
	"Jesper Dangaard Brouer" <hawk@kernel.org>,
	netfilter-devel@vger.kernel.org,
	Networking <netdev@vger.kernel.org>
Subject: Re: [PATCH bpf-next v1 00/15] Introduce typed pointer support in BPF maps
Date: Tue, 22 Feb 2022 23:29:53 -0800	[thread overview]
Message-ID: <CAPhsuW70ggLxKFopbKb9cr7WknqZW6utoSOLoLg4Yw971oD22g@mail.gmail.com> (raw)
In-Reply-To: <20220222082129.yivvpm6yo3474dp3@apollo.legion>

On Tue, Feb 22, 2022 at 12:21 AM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
[...]


> >
> > I guess I missed some context here. Could you please provide some reference
> > to the use cases of these features?
> >
>
> The common usecase is caching references to objects inside BPF maps, to avoid
> costly lookups, and being able to raise it once for the duration of program
> invocation when passing it to multiple helpers (to avoid further re-lookups).
> Storing references also allows you to control object lifetime.
>
> One other use case is enabling xdp_frame queueing in XDP using this, but that
> still needs some integration work after this lands, so it's a bit early to
> comment on the specifics.
>
> Other than that, I think Alexei already mentioned this could be easily extended
> to do memory allocation returning a PTR_TO_BTF_ID in a BPF program [0] in the
> future.
>
>   [0]: https://lore.kernel.org/bpf/20220216230615.po6huyrgkswk7u67@ast-mbp.dhcp.thefacebook.com
>
> > For Unreferenced kernel pointer and userspace pointer, it seems that there is
> > no guarantee the pointer will still be valid during access (we only know it is
> > valid when it is stored in the map). Is this correct?
> >
>
> That is correct. In the case of unreferenced and referenced kernel pointers,
> when you do a BPF_LDX, both are marked as PTR_UNTRUSTED, and it is not allowed
> to pass them into helpers or kfuncs, because from that point onwards we cannot
> claim that the object is still alive when pointer is used later. Still,
> dereference is permitted because verifier handles faults for bad accesses using
> PROBE_MEM conversion for PTR_TO_BTF_ID loads in convert_ctx_accesses (which is
> then later detected by JIT to build exception table used by exception handler).
>
> In case of reading unreferenced pointer, in some cases you know that the pointer
> will stay valid, so you can just store it in the map and load and directly
> access it, it imposes very little restrictions.
>
> For the referenced case, and BPF_LDX marking it as PTR_UNTRUSTED, you could say
> that this makes it a lot less useful, because if BPF program already holds
> reference, just to make sure I _read valid data_, I still have to use the
> kptr_get style helper to raise and put reference to ensure the object is alive
> when it is accessed.
>
> So in that case, for RCU protected objects, it should still wait for BPF program
> to hit BPF_EXIT before the actual release, but for other cases like the case of
> sleepable programs, or objects where refcount alone manages lifetime, you can
> also detect writer presence of the other BPF program (to detect if pointer
> during our access was xchg'd out) using a seqlock style scheme:
>
>         v = bpf_map_lookup_elem(&map, ...);
>         if (!v)
>                 return 0;
>         seq_begin = v->seq;
>         atomic_thread_fence(memory_order_acquire); // A
>         <do access>
>         atomic_thread_fence(memory_order_acquire); // B
>         seq_end = v->seq;
>         if (seq_begin & 1 || seq_begin != seq_end)
>                 goto bad_read;
>         <use data>
>
> Ofcourse, barriers are not yet in BPF, but you get the idea (it should work on
> x86). The updater BPF program will increment v->seq before and after xchg,
> ensuring proper ordering. v->seq starts as 0, so odd seq indicates writer update
> is in progress.
>
> This would allow you to not raise refcount, while still ensuring that as long as
> object was accessed, it was still valid between A and B. Even if raising
> uncontended refcount is cheap, this is much cheaper.
>
> The case of userspace pointer is different, it sets the MEM_USER flag, so the
> only useful thing to do is calling bpf_probe_read_user, you can't even
> dereference it. You are right that in most cases that userspace pointer won't be
> useful, but for some cooperative cases between BPF program and userspace thread,
> it can act as a way to share certain thread local areas/userspace memory that
> the BPF program can then store keyed by the task_struct *, where using a BPF map
> to share memory is not always possible.

Thanks for the explanation! I can see the referenced kernel pointer be very
powerful in many use cases. The per cpu pointer is also interesting.

Song

      reply	other threads:[~2022-02-23  7:30 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-20 13:47 [PATCH bpf-next v1 00/15] Introduce typed pointer support in BPF maps Kumar Kartikeya Dwivedi
2022-02-20 13:47 ` [PATCH bpf-next v1 01/15] bpf: Factor out fd returning from bpf_btf_find_by_name_kind Kumar Kartikeya Dwivedi
2022-02-22  5:28   ` Alexei Starovoitov
2022-02-23  3:05     ` Kumar Kartikeya Dwivedi
2022-02-20 13:48 ` [PATCH bpf-next v1 02/15] bpf: Make btf_find_field more generic Kumar Kartikeya Dwivedi
2022-02-20 13:48 ` [PATCH bpf-next v1 03/15] bpf: Allow storing PTR_TO_BTF_ID in map Kumar Kartikeya Dwivedi
2022-02-22  6:46   ` Alexei Starovoitov
2022-02-23  3:09     ` Kumar Kartikeya Dwivedi
2022-02-23 21:46       ` Alexei Starovoitov
2022-02-20 13:48 ` [PATCH bpf-next v1 04/15] bpf: Allow storing referenced " Kumar Kartikeya Dwivedi
2022-02-22  6:53   ` Alexei Starovoitov
2022-02-22  7:10     ` Kumar Kartikeya Dwivedi
2022-02-22 16:20       ` Alexei Starovoitov
2022-02-23  3:04         ` Kumar Kartikeya Dwivedi
2022-02-23 21:52           ` Alexei Starovoitov
2022-02-24  8:43             ` Kumar Kartikeya Dwivedi
2022-02-20 13:48 ` [PATCH bpf-next v1 05/15] bpf: Allow storing PTR_TO_PERCPU_BTF_ID " Kumar Kartikeya Dwivedi
2022-02-20 20:40   ` kernel test robot
2022-02-20 13:48 ` [PATCH bpf-next v1 06/15] bpf: Allow storing __user PTR_TO_BTF_ID " Kumar Kartikeya Dwivedi
2022-02-20 13:48 ` [PATCH bpf-next v1 07/15] bpf: Prevent escaping of pointers loaded from maps Kumar Kartikeya Dwivedi
2022-02-20 13:48 ` [PATCH bpf-next v1 08/15] bpf: Adapt copy_map_value for multiple offset case Kumar Kartikeya Dwivedi
2022-02-22  7:04   ` Alexei Starovoitov
2022-02-23  3:13     ` Kumar Kartikeya Dwivedi
2022-02-23 21:41       ` Alexei Starovoitov
2022-02-20 13:48 ` [PATCH bpf-next v1 09/15] bpf: Populate pairs of btf_id and destructor kfunc in btf Kumar Kartikeya Dwivedi
2022-02-20 13:48 ` [PATCH bpf-next v1 10/15] bpf: Wire up freeing of referenced PTR_TO_BTF_ID in map Kumar Kartikeya Dwivedi
2022-02-20 21:43   ` kernel test robot
2022-02-20 22:55   ` kernel test robot
2022-02-21  0:39   ` kernel test robot
2022-02-20 13:48 ` [PATCH bpf-next v1 11/15] bpf: Teach verifier about kptr_get style kfunc helpers Kumar Kartikeya Dwivedi
2022-02-20 13:48 ` [PATCH bpf-next v1 12/15] net/netfilter: Add bpf_ct_kptr_get helper Kumar Kartikeya Dwivedi
2022-02-21  4:35   ` kernel test robot
2022-02-20 13:48 ` [PATCH bpf-next v1 13/15] libbpf: Add __kptr* macros to bpf_helpers.h Kumar Kartikeya Dwivedi
2022-02-20 13:48 ` [PATCH bpf-next v1 14/15] selftests/bpf: Add C tests for PTR_TO_BTF_ID in map Kumar Kartikeya Dwivedi
2022-02-20 13:48 ` [PATCH bpf-next v1 15/15] selftests/bpf: Add verifier " Kumar Kartikeya Dwivedi
2022-02-22  6:05 ` [PATCH bpf-next v1 00/15] Introduce typed pointer support in BPF maps Song Liu
2022-02-22  8:21   ` Kumar Kartikeya Dwivedi
2022-02-23  7:29     ` Song Liu [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=CAPhsuW70ggLxKFopbKb9cr7WknqZW6utoSOLoLg4Yw971oD22g@mail.gmail.com \
    --to=song@kernel.org \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=hawk@kernel.org \
    --cc=memxor@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=toke@redhat.com \
    /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.