On Thu, Mar 10, 2022 at 11:25 PM Kumar Kartikeya Dwivedi
[off-list ref] wrote:

quoted

Also I don't believe that this patch set and exposing
bpf_xdp_pointer as a helper actually gives measurable performance wins.
It looks quirky to me and hard to use.

This is actually inspired from your idea to avoid memcpy when reading and
writing to multi-buff XDP [0]. But instead of passing in the stack or mem
pointer (as discussed in that thread), I let the user set it and detect it
themselves, which makes the implementation simpler.

I am sure accessing a few bytes directly is going to be faster than first
memcpy'ing it to a local buffer, reading, and then possibly writing things
back again using a memcpy, but I will be happy to come with some numbers when
I respin this later, when Joanne posts the dynptr series.

Ofcourse, we could just make return value PTR_TO_MEM even for the 'pass buf
pointer' idea, but then we have to conservatively invalidate the pointer even if
it points to stack buffer on clear_all_pkt_pointers. The current approach looked
better to me.

  [0]: https://lore.kernel.org/bpf/CAADnVQKbrkOxfNoixUx-RLJEWULJLyhqjZ=M_X2cFG_APwNyCg@mail.gmail.com (local)

There is a big difference in what was proposed earlier
vs what you've implemented.
In that proposal bpf_packet_pointer would behave similar to
skb_header_pointer. The kernel developers are familiar with it
and the concept is tested by time.
While your bpf_packet_pointer is different.
It fails on frag boundaries, so the user has to open code
the checks and add explicit bpf_xdp_load_bytes.
I guess you're arguing that it's the same.
My point that it's a big conceptual difference in api.
This kind of difference should be discussed instead
of dumping a patch set. Especially without RFC tag.

Please focus on kptr patches. They were close enough and have
a chance of landing in this release.
This bpf_packet_pointer stuff is certainly not going to make it.