Re: [PATCH bpf-next v1 0/5] Introduce bpf_packet_pointer helper

On Tue, Mar 08, 2022 at 11:18:52AM IST, Andrii Nakryiko wrote:

On Sun, Mar 6, 2022 at 3:43 PM Kumar Kartikeya Dwivedi [off-list ref] wrote:

quoted

Expose existing 'bpf_xdp_pointer' as a BPF helper named 'bpf_packet_pointer'
returning a packet pointer with a fixed immutable range. This can be useful to
enable DPA without having to use memcpy (currently the case in
bpf_xdp_load_bytes and bpf_xdp_store_bytes).

The intended usage to read and write data for multi-buff XDP is:

        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;
        }

Note that bpf_packet_pointer returns a PTR_TO_PACKET, not PTR_TO_MEM, because
these pointers need to be invalidated on clear_all_pkt_pointers invocation, and
it is also more meaningful to the user to see return value as R0=pkt.

This series is meant to collect feedback on the approach, next version can
include a bpf_skb_pointer and exposing it as bpf_packet_pointer helper for TC
hooks, and explore not resetting range to zero on r0 += rX, instead check access
like check_mem_region_access (var_off + off < range), since there would be no
data_end to compare against and obtain a new range.

The common name and func_id is supposed to allow writing generic code using
bpf_packet_pointer that works for both XDP and TC programs.

Please see the individual patches for implementation details.

Joanne is working on a "bpf_dynptr" framework that will support
exactly this feature, in addition to working with dynamically
allocated memory, working with memory of statically unknown size (but
safe and checked at runtime), etc. And all that within a generic
common feature implemented uniformly within the verifier. E.g., it
won't need any of the custom bits of logic added in patch #2 and #3.
So I'm thinking that instead of custom-implementing a partial case of
bpf_dynptr just for skb and xdp packets, let's maybe wait for dynptr
and do it only once there?

Interesting stuff, looking forward to it.

See also my ARG_CONSTANT comment. It seems like a pretty common thing
where input constant is used to characterize some pointer returned
from the helper (e.g., bpf_ringbuf_reserve() case), and we'll need
that for bpf_dynptr for exactly this "give me direct access of N
bytes, if possible" case. So improving/generalizing it now before
dynptr lands makes a lot of sense, outside of bpf_packet_pointer()
feature itself.

No worries, we can continue the discussion in patch 1, I'll split out the arg
changes into a separate patch, and wait for dynptr to be posted before reworking
this.

quoted

Kumar Kartikeya Dwivedi (5):
  bpf: Add ARG_SCALAR and ARG_CONSTANT
  bpf: Introduce pkt_uid concept for PTR_TO_PACKET
  bpf: Introduce bpf_packet_pointer helper to do DPA
  selftests/bpf: Add verifier tests for pkt pointer with pkt_uid
  selftests/bpf: Update xdp_adjust_frags to use bpf_packet_pointer

 include/linux/bpf.h                           |   4 +
 include/linux/bpf_verifier.h                  |   9 +-
 include/uapi/linux/bpf.h                      |  12 ++
 kernel/bpf/verifier.c                         |  97 ++++++++++--
 net/core/filter.c                             |  48 +++---
 tools/include/uapi/linux/bpf.h                |  12 ++
 .../bpf/prog_tests/xdp_adjust_frags.c         |  46 ++++--
 .../bpf/progs/test_xdp_update_frags.c         |  46 ++++--
 tools/testing/selftests/bpf/verifier/xdp.c    | 146 ++++++++++++++++++
 9 files changed, 358 insertions(+), 62 deletions(-)

--
2.35.1

--
Kartikeya