[bpf-next,v7,0/3] Add skb + xdp dynptrs

Message

Joanne Koong Oct. 21, 2022, 1:15 a.m. UTC

This patchset is the 2nd in the dynptr series. The 1st can be found here [0].

This patchset adds skb and xdp type dynptrs, which have two main benefits for
packet parsing:
    * allowing operations on sizes that are not statically known at
      compile-time (eg variable-sized accesses).
    * more ergonomic and less brittle iteration through data (eg does not need
      manual if checking for being within bounds of data_end)

When comparing the differences in runtime for packet parsing without dynptrs
vs. with dynptrs for the more simple cases, there is no noticeable difference.
For the more complex cases where lengths are non-statically known at compile
time, there can be a significant speed-up when using dynptrs (eg a 2x speed up
for cls redirection). Patch 3 contains more details as well as examples of how
to use skb and xdp dynptrs.

[0] https://lore.kernel.org/bpf/[e-mail]/

--
Changelog:

v6 = https://lore.kernel.org/bpf/[e-mail]/
v6 -> v7
    * Change bpf_dynptr_data() to return read-only data slices if the skb prog
      is read-only (Martin)
    * Add test "skb_invalid_write" to test that writes to rd-only data slices
      are rejected

v5 = https://lore.kernel.org/bpf/[e-mail]/
v5 -> v6
    * Address kernel test robot errors by static inlining

v4 = https://lore.kernel.org/bpf/[e-mail]/
v4 -> v5
    * Address kernel test robot errors for configs w/out CONFIG_NET set
    * For data slices, return PTR_TO_MEM instead of PTR_TO_PACKET (Kumar)
    * Split selftests into subtests (Andrii)
    * Remove insn patching. Use rdonly and rdwr protos for dynptr skb
      construction (Andrii)
    * bpf_dynptr_data() returns NULL for rd-only dynptrs. There will be a
      separate bpf_dynptr_data_rdonly() added later (Andrii and Kumar)

v3 = https://lore.kernel.org/bpf/[e-mail]/
v3 -> v4
    * Forgot to commit --amend the kernel test robot error fixups

v2 = https://lore.kernel.org/bpf/[e-mail]/
v2 -> v3
    * Fix kernel test robot build test errors

v1 = https://lore.kernel.org/bpf/[e-mail]/
v1 -> v2
  * Return data slices to rd-only skb dynptrs (Martin)
  * bpf_dynptr_write allows writes to frags for skb dynptrs, but always
    invalidates associated data slices (Martin)
  * Use switch casing instead of ifs (Andrii)
  * Use 0xFD for experimental kind number in the selftest (Zvi)
  * Put selftest conversions w/ dynptrs into new files (Alexei)
  * Add new selftest "test_cls_redirect_dynptr.c" 


Joanne Koong (3):
  bpf: Add skb dynptrs
  bpf: Add xdp dynptrs
  selftests/bpf: tests for using dynptrs to parse skb and xdp buffers

 include/linux/bpf.h                           |  88 +-
 include/linux/filter.h                        |  38 +
 include/uapi/linux/bpf.h                      |  67 +-
 kernel/bpf/helpers.c                          |  91 +-
 kernel/bpf/verifier.c                         | 116 ++-
 net/core/filter.c                             | 119 ++-
 tools/include/uapi/linux/bpf.h                |  67 +-
 .../selftests/bpf/prog_tests/cls_redirect.c   |  25 +
 .../testing/selftests/bpf/prog_tests/dynptr.c |  74 +-
 .../selftests/bpf/prog_tests/l4lb_all.c       |   2 +
 .../bpf/prog_tests/parse_tcp_hdr_opt.c        |  93 ++
 .../selftests/bpf/prog_tests/xdp_attach.c     |  11 +-
 .../testing/selftests/bpf/progs/dynptr_fail.c | 111 ++
 .../selftests/bpf/progs/dynptr_success.c      |  23 +
 .../bpf/progs/test_cls_redirect_dynptr.c      | 968 ++++++++++++++++++
 .../bpf/progs/test_l4lb_noinline_dynptr.c     | 469 +++++++++
 .../bpf/progs/test_parse_tcp_hdr_opt.c        | 119 +++
 .../bpf/progs/test_parse_tcp_hdr_opt_dynptr.c | 110 ++
 .../selftests/bpf/progs/test_xdp_dynptr.c     | 235 +++++
 .../selftests/bpf/test_tcp_hdr_options.h      |   1 +
 20 files changed, 2731 insertions(+), 96 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/parse_tcp_hdr_opt.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_l4lb_noinline_dynptr.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_parse_tcp_hdr_opt.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_parse_tcp_hdr_opt_dynptr.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_dynptr.c

Comments

Tariq Toukan Jan. 15, 2023, 12:41 p.m. UTC | #1

Hi,

I'm reviving this thread, following the discussion here:
https://lore.kernel.org/bpf/[e-mail]/

On 21/10/2022 4:19, Alexei Starovoitov wrote:
> On Thu, Oct 20, 2022 at 6:15 PM Joanne Koong <[e-mail]> wrote:
>>
>> This patchset is the 2nd in the dynptr series. The 1st can be found here [0].
>>
>> This patchset adds skb and xdp type dynptrs, which have two main benefits for
>> packet parsing:
>>      * allowing operations on sizes that are not statically known at
>>        compile-time (eg variable-sized accesses).
>>      * more ergonomic and less brittle iteration through data (eg does not need
>>        manual if checking for being within bounds of data_end)
>>
>> When comparing the differences in runtime for packet parsing without dynptrs
>> vs. with dynptrs for the more simple cases, there is no noticeable difference.
>> For the more complex cases where lengths are non-statically known at compile
>> time, there can be a significant speed-up when using dynptrs (eg a 2x speed up
>> for cls redirection). Patch 3 contains more details as well as examples of how
>> to use skb and xdp dynptrs.
> 
> Before proceeding with this patchset I think we gotta resolve the
> issues with dynptr-s that Kumar found.

Just to make sure I'm following: The issues that are discussed here?
https://lore.kernel.org/all/CAP01T74icBDXOM=[e-mail]/

What is the current status of dynptrs?
Any updates since October?
Do we have any agreement or a plan for this?

Regards,
Tariq

Joanne Koong Jan. 17, 2023, 5:20 p.m. UTC | #2

On Sun, Jan 15, 2023 at 4:41 AM Tariq Toukan <[e-mail]> wrote:
>
> Hi,
>
> I'm reviving this thread, following the discussion here:
> https://lore.kernel.org/bpf/[e-mail]/
>
> On 21/10/2022 4:19, Alexei Starovoitov wrote:
> > On Thu, Oct 20, 2022 at 6:15 PM Joanne Koong <[e-mail]> wrote:
> >>
> >> This patchset is the 2nd in the dynptr series. The 1st can be found here [0].
> >>
> >> This patchset adds skb and xdp type dynptrs, which have two main benefits for
> >> packet parsing:
> >>      * allowing operations on sizes that are not statically known at
> >>        compile-time (eg variable-sized accesses).
> >>      * more ergonomic and less brittle iteration through data (eg does not need
> >>        manual if checking for being within bounds of data_end)
> >>
> >> When comparing the differences in runtime for packet parsing without dynptrs
> >> vs. with dynptrs for the more simple cases, there is no noticeable difference.
> >> For the more complex cases where lengths are non-statically known at compile
> >> time, there can be a significant speed-up when using dynptrs (eg a 2x speed up
> >> for cls redirection). Patch 3 contains more details as well as examples of how
> >> to use skb and xdp dynptrs.
> >
> > Before proceeding with this patchset I think we gotta resolve the
> > issues with dynptr-s that Kumar found.
>
> Just to make sure I'm following: The issues that are discussed here?
> https://lore.kernel.org/all/CAP01T74icBDXOM=[e-mail]/
>
> What is the current status of dynptrs?
> Any updates since October?
> Do we have any agreement or a plan for this?

Hi Tariq,

The current status of dynptrs is blocked on two things: 1) the fixes
by Kumar in [1] landing upstream and 2) a bigger question of whether
bpf development should proceed with kfuncs or helpers (thread in [2]).
We had a bpf office hour session last Thursday about whether helpers
should be frozen (#2), but did not come to a conclusion in the time
allotted. As a follow-up to the office hour, Toke and David Vernet, I
believe, are working on a document outlining how kfunc stability will
work. After this document is sent out, I think we will probably have
another office hour session or discussion upstream about which
direction to take.

[1] https://lore.kernel.org/bpf/[e-mail]/
[2] https://lore.kernel.org/bpf/[e-mail]/t/#u

>
> Regards,
> Tariq