This patch set defines a new map type, BPF_MAP_TYPE_USER_RINGBUF, which
provides single-user-space-producer / single-kernel-consumer semantics over
a ringbuffer. Along with the new map type, a helper function called
bpf_user_ringbuf_drain() is added which allows a BPF program to specify a
callback with the following signature, to which samples are posted by the
helper:
void (struct bpf_dynptr *dynptr, void *context);
The program can then use the bpf_dynptr_read() or bpf_dynptr_data() helper
functions to safely read the sample from the dynptr. There are currently no
helpers available to determine the size of the sample, but one could easily
be added if required.
On the user-space side, libbpf has been updated to export a new
'struct ring_buffer_user' type, along with the following symbols:
struct ring_buffer_user *
ring_buffer_user__new(int map_fd,
const struct ring_buffer_user_opts *opts);
void ring_buffer_user__free(struct ring_buffer_user *rb);
void *ring_buffer_user__reserve(struct ring_buffer_user *rb, uint32_t size);
void *ring_buffer_user__poll(struct ring_buffer_user *rb, uint32_t size,
int timeout_ms);
void ring_buffer_user__discard(struct ring_buffer_user *rb, void *sample);
void ring_buffer_user__submit(struct ring_buffer_user *rb, void *sample);
These symbols are exported for inclusion in libbpf version 1.0.0.
Note that one thing that is not included in this patch-set is the ability
to kick the kernel from user-space to have it drain messages. The selftests
included in this patch-set currently just use progs with syscall hooks to
"kick" the kernel and have it drain samples from a user-producer
ringbuffer, but being able to kick the kernel using some other mechanism
that doesn't rely on such hooks would be very useful as well. I'm planning
on adding this in a future patch-set.
Signed-off-by: David Vernet <[email protected]>
--
David Vernet (5):
bpf: Clear callee saved regs after updating REG0
bpf: Define new BPF_MAP_TYPE_USER_RINGBUF map type
bpf: Add bpf_user_ringbuf_drain() helper
bpf: Add libbpf logic for user-space ring buffer
selftests/bpf: Add selftests validating the user ringbuf
include/linux/bpf.h | 6 +-
include/linux/bpf_types.h | 1 +
include/uapi/linux/bpf.h | 9 +
kernel/bpf/helpers.c | 2 +
kernel/bpf/ringbuf.c | 232 ++++++-
kernel/bpf/verifier.c | 73 ++-
tools/include/uapi/linux/bpf.h | 9 +
tools/lib/bpf/libbpf.c | 11 +-
tools/lib/bpf/libbpf.h | 19 +
tools/lib/bpf/libbpf.map | 6 +
tools/lib/bpf/libbpf_probes.c | 1 +
tools/lib/bpf/ringbuf.c | 214 +++++++
.../selftests/bpf/prog_tests/user_ringbuf.c | 592 ++++++++++++++++++
.../selftests/bpf/progs/user_ringbuf_fail.c | 174 +++++
.../bpf/progs/user_ringbuf_success.c | 227 +++++++
.../testing/selftests/bpf/test_user_ringbuf.h | 28 +
16 files changed, 1579 insertions(+), 25 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
create mode 100644 tools/testing/selftests/bpf/progs/user_ringbuf_fail.c
create mode 100644 tools/testing/selftests/bpf/progs/user_ringbuf_success.c
create mode 100644 tools/testing/selftests/bpf/test_user_ringbuf.h
--
2.30.2
Hi David,
On Mon, Aug 8, 2022 at 8:52 AM David Vernet <[email protected]> wrote:
>
> This patch set defines a new map type, BPF_MAP_TYPE_USER_RINGBUF, which
> provides single-user-space-producer / single-kernel-consumer semantics over
> a ringbuffer. Along with the new map type, a helper function called
> bpf_user_ringbuf_drain() is added which allows a BPF program to specify a
> callback with the following signature, to which samples are posted by the
> helper:
>
> void (struct bpf_dynptr *dynptr, void *context);
>
> The program can then use the bpf_dynptr_read() or bpf_dynptr_data() helper
> functions to safely read the sample from the dynptr. There are currently no
> helpers available to determine the size of the sample, but one could easily
> be added if required.
>
> On the user-space side, libbpf has been updated to export a new
> 'struct ring_buffer_user' type, along with the following symbols:
>
> struct ring_buffer_user *
> ring_buffer_user__new(int map_fd,
> const struct ring_buffer_user_opts *opts);
> void ring_buffer_user__free(struct ring_buffer_user *rb);
> void *ring_buffer_user__reserve(struct ring_buffer_user *rb, uint32_t size);
> void *ring_buffer_user__poll(struct ring_buffer_user *rb, uint32_t size,
> int timeout_ms);
> void ring_buffer_user__discard(struct ring_buffer_user *rb, void *sample);
> void ring_buffer_user__submit(struct ring_buffer_user *rb, void *sample);
>
> These symbols are exported for inclusion in libbpf version 1.0.0.
>
> Note that one thing that is not included in this patch-set is the ability
> to kick the kernel from user-space to have it drain messages. The selftests
> included in this patch-set currently just use progs with syscall hooks to
> "kick" the kernel and have it drain samples from a user-producer
> ringbuffer, but being able to kick the kernel using some other mechanism
> that doesn't rely on such hooks would be very useful as well. I'm planning
> on adding this in a future patch-set.
>
This could be done using iters. Basically, you can perform draining in
bpf_iter programs and export iter links as bpffs files. Then to kick
the kernel, you simply just read() the file.
> Signed-off-by: David Vernet <[email protected]>
> --
>
> David Vernet (5):
> bpf: Clear callee saved regs after updating REG0
> bpf: Define new BPF_MAP_TYPE_USER_RINGBUF map type
> bpf: Add bpf_user_ringbuf_drain() helper
> bpf: Add libbpf logic for user-space ring buffer
> selftests/bpf: Add selftests validating the user ringbuf
>
> include/linux/bpf.h | 6 +-
> include/linux/bpf_types.h | 1 +
> include/uapi/linux/bpf.h | 9 +
> kernel/bpf/helpers.c | 2 +
> kernel/bpf/ringbuf.c | 232 ++++++-
> kernel/bpf/verifier.c | 73 ++-
> tools/include/uapi/linux/bpf.h | 9 +
> tools/lib/bpf/libbpf.c | 11 +-
> tools/lib/bpf/libbpf.h | 19 +
> tools/lib/bpf/libbpf.map | 6 +
> tools/lib/bpf/libbpf_probes.c | 1 +
> tools/lib/bpf/ringbuf.c | 214 +++++++
> .../selftests/bpf/prog_tests/user_ringbuf.c | 592 ++++++++++++++++++
> .../selftests/bpf/progs/user_ringbuf_fail.c | 174 +++++
> .../bpf/progs/user_ringbuf_success.c | 227 +++++++
> .../testing/selftests/bpf/test_user_ringbuf.h | 28 +
> 16 files changed, 1579 insertions(+), 25 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
> create mode 100644 tools/testing/selftests/bpf/progs/user_ringbuf_fail.c
> create mode 100644 tools/testing/selftests/bpf/progs/user_ringbuf_success.c
> create mode 100644 tools/testing/selftests/bpf/test_user_ringbuf.h
>
> --
> 2.30.2
>
Hi Hao,
On Mon, Aug 08, 2022 at 11:57:53AM -0700, Hao Luo wrote:
> > Note that one thing that is not included in this patch-set is the ability
> > to kick the kernel from user-space to have it drain messages. The selftests
> > included in this patch-set currently just use progs with syscall hooks to
> > "kick" the kernel and have it drain samples from a user-producer
> > ringbuffer, but being able to kick the kernel using some other mechanism
> > that doesn't rely on such hooks would be very useful as well. I'm planning
> > on adding this in a future patch-set.
> >
>
> This could be done using iters. Basically, you can perform draining in
> bpf_iter programs and export iter links as bpffs files. Then to kick
> the kernel, you simply just read() the file.
Thanks for pointing this out. I agree that iters could be used this way to
kick the kernel, and perhaps that would be a sufficient solution. My
thinking, however, was that it would be useful to provide some APIs that
are a bit more ergonomic, and specifically meant to enable kicking
arbitrary "pre-attached" callbacks in a BPF prog, possibly along with some
payload from userspace.
Iters allow userspace to kick the kernel, but IMO they're meant to enable
data extraction from the kernel, and dumping kernel data into user-space.
What I'm proposing is a more generalizable way of driving logic in the
kernel from user-space.
Does that make sense? Looking forward to hearing your thoughts.
Thanks,
David
On Tue, Aug 9, 2022 at 6:15 PM David Vernet <[email protected]> wrote:
>
> Hi Hao,
>
> On Mon, Aug 08, 2022 at 11:57:53AM -0700, Hao Luo wrote:
> > > Note that one thing that is not included in this patch-set is the ability
> > > to kick the kernel from user-space to have it drain messages. The selftests
> > > included in this patch-set currently just use progs with syscall hooks to
> > > "kick" the kernel and have it drain samples from a user-producer
> > > ringbuffer, but being able to kick the kernel using some other mechanism
> > > that doesn't rely on such hooks would be very useful as well. I'm planning
> > > on adding this in a future patch-set.
> > >
> >
> > This could be done using iters. Basically, you can perform draining in
> > bpf_iter programs and export iter links as bpffs files. Then to kick
> > the kernel, you simply just read() the file.
>
> Thanks for pointing this out. I agree that iters could be used this way to
> kick the kernel, and perhaps that would be a sufficient solution. My
> thinking, however, was that it would be useful to provide some APIs that
> are a bit more ergonomic, and specifically meant to enable kicking
> arbitrary "pre-attached" callbacks in a BPF prog, possibly along with some
> payload from userspace.
David, very sorry about the late reply. Thank you for sharing your
thoughts. I am looking at your v2 and understand you need a way to
trigger the kernel to consume samples in the ringbuf, which seems a
reasonable motivation to me.
>
> Iters allow userspace to kick the kernel, but IMO they're meant to enable
> data extraction from the kernel, and dumping kernel data into user-space.
Not necessarily extracting data and dumping data. It could be used to
do operations on a set of objects, the operation could be
notification. Iterating and notifying are orthogonal IMHO.
> What I'm proposing is a more generalizable way of driving logic in the
> kernel from user-space.
> Does that make sense? Looking forward to hearing your thoughts.
Yes, sort of. I see the difference between iter and the proposed
interface. But I am not clear about the motivation of a new APis for
kicking callbacks from userspace. I guess maybe it will become clear,
when you publish a concerte RFC of that interface and integrates with
your userspace publisher.
Hao
On Mon, Aug 15, 2022 at 02:13:13PM -0700, Hao Luo wrote:
> >
> > Iters allow userspace to kick the kernel, but IMO they're meant to enable
> > data extraction from the kernel, and dumping kernel data into user-space.
>
> Not necessarily extracting data and dumping data. It could be used to
> do operations on a set of objects, the operation could be
> notification. Iterating and notifying are orthogonal IMHO.
>
> > What I'm proposing is a more generalizable way of driving logic in the
> > kernel from user-space.
> > Does that make sense? Looking forward to hearing your thoughts.
>
> Yes, sort of. I see the difference between iter and the proposed
> interface. But I am not clear about the motivation of a new APis for
> kicking callbacks from userspace. I guess maybe it will become clear,
> when you publish a concerte RFC of that interface and integrates with
> your userspace publisher.
Fair enough -- let me remove this from the cover letter in future
versions of the patch-set. To your point, there's probably little to be
gained in debating the merits of adding such APIs until there's a
concrete use-case.
Thanks,
David
On Tue, Aug 16, 2022 at 8:10 AM David Vernet <[email protected]> wrote:
>
> On Mon, Aug 15, 2022 at 02:13:13PM -0700, Hao Luo wrote:
> > >
> > > Iters allow userspace to kick the kernel, but IMO they're meant to enable
> > > data extraction from the kernel, and dumping kernel data into user-space.
> >
> > Not necessarily extracting data and dumping data. It could be used to
> > do operations on a set of objects, the operation could be
> > notification. Iterating and notifying are orthogonal IMHO.
> >
> > > What I'm proposing is a more generalizable way of driving logic in the
> > > kernel from user-space.
> > > Does that make sense? Looking forward to hearing your thoughts.
> >
> > Yes, sort of. I see the difference between iter and the proposed
> > interface. But I am not clear about the motivation of a new APis for
> > kicking callbacks from userspace. I guess maybe it will become clear,
> > when you publish a concerte RFC of that interface and integrates with
> > your userspace publisher.
>
> Fair enough -- let me remove this from the cover letter in future
> versions of the patch-set. To your point, there's probably little to be
> gained in debating the merits of adding such APIs until there's a
> concrete use-case.
>
Yep, sounds good. I don't mean to debate :) I would like to help. If
we could build on top of existing infra and make improvements, IMHO it
would be easier to maintain. Anyway, I'm looking forward to your
proposed APIs.
> Thanks,
> David
On Tue, Aug 16, 2022 at 10:01:38AM -0700, Hao Luo wrote:
> On Tue, Aug 16, 2022 at 8:10 AM David Vernet <[email protected]> wrote:
> >
> > On Mon, Aug 15, 2022 at 02:13:13PM -0700, Hao Luo wrote:
> > > >
> > > > Iters allow userspace to kick the kernel, but IMO they're meant to enable
> > > > data extraction from the kernel, and dumping kernel data into user-space.
> > >
> > > Not necessarily extracting data and dumping data. It could be used to
> > > do operations on a set of objects, the operation could be
> > > notification. Iterating and notifying are orthogonal IMHO.
> > >
> > > > What I'm proposing is a more generalizable way of driving logic in the
> > > > kernel from user-space.
> > > > Does that make sense? Looking forward to hearing your thoughts.
> > >
> > > Yes, sort of. I see the difference between iter and the proposed
> > > interface. But I am not clear about the motivation of a new APis for
> > > kicking callbacks from userspace. I guess maybe it will become clear,
> > > when you publish a concerte RFC of that interface and integrates with
> > > your userspace publisher.
> >
> > Fair enough -- let me remove this from the cover letter in future
> > versions of the patch-set. To your point, there's probably little to be
> > gained in debating the merits of adding such APIs until there's a
> > concrete use-case.
> >
>
> Yep, sounds good. I don't mean to debate :) I would like to help. If
> we could build on top of existing infra and make improvements, IMHO it
> would be easier to maintain. Anyway, I'm looking forward to your
> proposed APIs.
Don't worry, I did not take it that you were debating. I very much
appreciate your thoughts and help. If and when I send out that RFC
patchset, I'll be sure to cc you (if not reach out beforehand as well to
discuss).