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? > > [...] > [...]
