On Wed, 29 Sep 2021 11:36:33 +0100 Lorenz Bauer wrote: > On Mon, 20 Sept 2021 at 23:46, Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: > > > The draft API was: > > > > > > void *xdp_mb_pointer_flush(struct xdp_buff *xdp_md, u32 flags, > > > u32 offset, u32 len, void *stack_buf) > > > > > > Which does not take the ptr returned by header_pointer(), but that's > > > easy to add (well, easy other than the fact it'd be the 6th arg). > > > > I guess we could play some trickery with stuffing offset/len/flags into > > one or two u64s to save an argument or two? > > Adding another pointer arg seems really hard to explain as an API. > What happens if I pass the "wrong" ptr? What happens if I pass NULL? Sure. We can leave the checking to the program then, but that ties our hands for the implementation changes later on. Not sure which pointer type will be chosen for the ret value but it may allow error checking at verification. > How about this: instead of taking stack_ptr, take the return value > from header_pointer as an argument. Then use the already existing > (right ;P) inlining to do the following: > > if (md->ptr + args->off != ret_ptr) > __pointer_flush(...) That only checks for the case where pointer is in the "head" frag, and is not generally correct. You need to check the length of the first frag is smaller than off. Otherwise BPF stack may "happen" to follow the head page and math will work out. It would also be slower than Lorenzo's current code, which allows access to tail pages without copying.
