netfilter-devel.vger.kernel.org archive mirror
 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 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).