| Message ID | 20220808155248.2475981-1-void@manifault.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | bpf: Add user-space-publisher ringbuffer map type | expand |
Hi David, On Mon, Aug 8, 2022 at 8:52 AM David Vernet <void@manifault.com> 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 <void@manifault.com> > -- > > 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 <void@manifault.com> 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 <void@manifault.com> 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 <void@manifault.com> 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).
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 <void@manifault.com> -- 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