On Mon, May 9, 2022 at 12:44 PM Joanne Koong <joannelkoong@xxxxxxxxx> wrote: > > On Fri, May 6, 2022 at 4:41 PM Andrii Nakryiko > <andrii.nakryiko@xxxxxxxxx> wrote: > > > > On Thu, Apr 28, 2022 at 2:12 PM Joanne Koong <joannelkoong@xxxxxxxxx> wrote: > > > > > > Currently, our only way of writing dynamically-sized data into a ring > > > buffer is through bpf_ringbuf_output but this incurs an extra memcpy > > > cost. bpf_ringbuf_reserve + bpf_ringbuf_commit avoids this extra > > > memcpy, but it can only safely support reservation sizes that are > > > statically known since the verifier cannot guarantee that the bpf > > > program won’t access memory outside the reserved space. > > > > > > The bpf_dynptr abstraction allows for dynamically-sized ring buffer > > > reservations without the extra memcpy. > > > > > > There are 3 new APIs: > > > > > > long bpf_ringbuf_reserve_dynptr(void *ringbuf, u32 size, u64 flags, struct bpf_dynptr *ptr); > > > void bpf_ringbuf_submit_dynptr(struct bpf_dynptr *ptr, u64 flags); > > > void bpf_ringbuf_discard_dynptr(struct bpf_dynptr *ptr, u64 flags); > > > > > > These closely follow the functionalities of the original ringbuf APIs. > > > For example, all ringbuffer dynptrs that have been reserved must be > > > either submitted or discarded before the program exits. > > > > > > Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx> > > > --- > > > > Looks great! Modulo those four underscores, they are super confusing... > > > > > include/linux/bpf.h | 10 ++++- > > > include/uapi/linux/bpf.h | 35 +++++++++++++++++ > > > kernel/bpf/helpers.c | 6 +++ > > > kernel/bpf/ringbuf.c | 71 ++++++++++++++++++++++++++++++++++ > > > kernel/bpf/verifier.c | 18 +++++++-- > > > tools/include/uapi/linux/bpf.h | 35 +++++++++++++++++ > > > 6 files changed, 171 insertions(+), 4 deletions(-) > > > > [...] > > > > > > > [...] > > > > > + * > > > + * void bpf_ringbuf_discard_dynptr(struct bpf_dynptr *ptr, u64 flags) > > > + * Description > > > + * Discard reserved ring buffer sample through the dynptr > > > + * interface. This is a no-op if the dynptr is invalid/null. > > > + * > > > + * For more information on *flags*, please see > > > + * 'bpf_ringbuf_discard'. > > > + * Return > > > + * Nothing. Always succeeds. > > > */ > > > > let's also add bpf_dynptr_is_null() (or bpf_dynptr_is_valid(), not > > sure which one is more appropriate, probably just null one), so we can > > check in code whether some reservation was successful without knowing > > bpf_ringbuf_reserve_dynptr()'s return value > I'm planning to add bpf_dynptr_is_null() in the 3rd dynptr patchset > (convenience helpers). Do you prefer that this be part of this > patchset instead? If so, do you think this should be part of the 2nd > patch (aka the one where we set up the infra for dynptrs + implement > malloc-type dynptrs) or this ringbuf patch or its own patch? No problem adding it in a follow up patch. BTW, is it still in the plan to be able to create bpf_dynptr() from map_value, global variables, etc? I.e., it's a LOCAL dynptr except memory is not on STACK. Something like int k = 123; struct my_val *v; struct bpf_dynptr p; v = bpf_map_lookup_elem(&my_map, &k); if (!v) return 0; bpf_dynptr_from_mem(&v->my_data, &p); /* p points inside my_map's value */ ? > > > > > [...] > > [...]
