On Fri, Sep 17, 2021 at 12:00 PM Jakub Kicinski [off-list ref] wrote:

On Fri, 17 Sep 2021 11:43:07 -0700 Alexei Starovoitov wrote:

quoted

quoted

If bpf_xdp_load_bytes() / bpf_xdp_store_bytes() works for most we
can start with that. In all honesty I don't know what the exact
use cases for looking at data are, either. I'm primarily worried
about exposing the kernel internals too early.

I don't mind the xdp equivalent of skb_load_bytes,
but skb_header_pointer() idea is superior.
When we did xdp with data/data_end there was no refine_retval_range
concept in the verifier (iirc or we just missed that opportunity).
We'd need something more advanced: a pointer with valid range
refined by input argument 'len' or NULL.
The verifier doesn't have such thing yet, but it fits as a combination of
value_or_null plus refine_retval_range.
The bpf_xdp_header_pointer() and bpf_skb_header_pointer()
would probably simplify bpf programs as well.
There would be no need to deal with data/data_end.

What are your thoughts on inlining? Can we inline the common case
of the header being in the "head"? Otherwise data/end comparisons
would be faster.

Yeah. It can be inlined by the verifier.
It would still look like a call from bpf prog pov with llvm doing spill/fill
of scratched regs, but it's minor.

Also we can use the same bpf_header_pointer(ctx, ...)
helper for both xdp and skb program types. They will have different
implementation underneath, but this might make possible writing bpf
programs that could work in both xdp and skb context.
I believe cilium has fancy macros to achieve that.