On Fri, 5 Aug 2022 00:58:01 +0200 Kumar Kartikeya Dwivedi wrote:
> When I worked on [0], I actually did it a bit like you described in
> the original discussion under the xdp multi-buff thread, but I left
> the other case (where data to be read resides across frag boundaries)
> up to the user to handle, instead of automatically passing in pointer
> to stack and doing the copy for them, so in my case
> xdp_load_bytes/xdp_store_bytes is the fallback if you can't get a
> bpf_packet_pointer for a ctx, offset, len which you can directly
> access. But this was only for XDP, not for skb.
>
> The advantage with a dynptr is that len for the slice from
> bpf_packet_pointer style helper doesn't have to be a constant, it can
> be a runtime value and since it is checked at runtime anyway, the
> helper's code is the same but access can be done for slices whose
> length is unknown to the verifier in a safe manner. The dynptr is very
> useful as the return value of such a helper.
I see.
> The suggested usage was like this:
>
> int err = 0;
> char buf[N];
>
> off &= 0xffff;
> ptr = bpf_packet_pointer(ctx, off, sizeof(buf), &err);
> if (unlikely(!ptr)) {
> if (err < 0)
> return XDP_ABORTED;
> err = bpf_xdp_load_bytes(ctx, off, buf, sizeof(buf));
> if (err < 0)
> return XDP_ABORTED;
> ptr = buf;
> }
> ...
> // Do some stores and loads in [ptr, ptr + N) region
> ...
> if (unlikely(ptr == buf)) {
> err = bpf_xdp_store_bytes(ctx, off, buf, sizeof(buf));
> if (err < 0)
> return XDP_ABORTED;
> }
>
> So the idea was the same, there is still a "flush" (in that unlikely
> branch), but it is done explicitly by the user (which I found less
> confusing than it being done automagically or a by a new flush helper
> which will do the same thing we do here, but YMMV).
Ack, the flush is awkward to create an API for. I presume that's
the main reason the xdp mbuf discussion wasn't fruitful.
