[v13,bpf-next,00/10] Add skb + xdp dynptrs

Message

Joanne Koong March 1, 2023, 3:49 p.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, there is no noticeable difference. Patch 9 contains more
details as well as examples of how to use skb and xdp dynptrs.

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

--
Changelog:

v12 = https://lore.kernel.org/bpf/[e-mail]/
v12 -> v13:
    * Fix missing { } for case statement

v11 = https://lore.kernel.org/bpf/[e-mail]/
v11 -> v12:
    * Change constant mem size checking to use "__szk" kfunc annotation
      for slices
    * Use autoloading for success selftests

v10 = https://lore.kernel.org/bpf/[e-mail]/
v10 -> v11:
    * Reject bpf_dynptr_slice_rdwr() for non-writable progs at load time
      instead of runtime
    * Add additional patch (__uninit kfunc annotation)
    * Expand on documentation
    * Add bpf_dynptr_write() calls for persisting writes in tests

v9 = https://lore.kernel.org/bpf/[e-mail]/
v9 -> v10:
    * Add bpf_dynptr_slice and bpf_dynptr_slice_rdwr interface
    * Add some more tests
    * Split up patchset into more parts to make it easier to review

v8 = https://lore.kernel.org/bpf/[e-mail]/
v8 -> v9:
    * Fix dynptr_get_type() to check non-stack dynptrs 

v7 = https://lore.kernel.org/bpf/[e-mail]/
v7 -> v8:
    * Change helpers to kfuncs
    * Add 2 new patches (1/5 and 2/5)

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 (10):
  bpf: Support "sk_buff" and "xdp_buff" as valid kfunc arg types
  bpf: Refactor process_dynptr_func
  bpf: Allow initializing dynptrs in kfuncs
  bpf: Define no-ops for externally called bpf dynptr functions
  bpf: Refactor verifier dynptr into get_dynptr_arg_reg
  bpf: Add __uninit kfunc annotation
  bpf: Add skb dynptrs
  bpf: Add xdp dynptrs
  bpf: Add bpf_dynptr_slice and bpf_dynptr_slice_rdwr
  selftests/bpf: tests for using dynptrs to parse skb and xdp buffers

 Documentation/bpf/kfuncs.rst                  |  17 +
 include/linux/bpf.h                           |  95 +-
 include/linux/bpf_verifier.h                  |   3 -
 include/linux/filter.h                        |  46 +
 include/uapi/linux/bpf.h                      |  18 +-
 kernel/bpf/btf.c                              |  22 +
 kernel/bpf/helpers.c                          | 221 +++-
 kernel/bpf/verifier.c                         | 415 ++++++--
 net/core/filter.c                             | 108 +-
 tools/include/uapi/linux/bpf.h                |  18 +-
 tools/testing/selftests/bpf/bpf_kfuncs.h      |  38 +
 .../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 | 287 ++++-
 .../selftests/bpf/progs/dynptr_success.c      |  55 +-
 .../bpf/progs/test_cls_redirect_dynptr.c      | 980 ++++++++++++++++++
 .../bpf/progs/test_l4lb_noinline_dynptr.c     | 487 +++++++++
 .../bpf/progs/test_parse_tcp_hdr_opt.c        | 119 +++
 .../bpf/progs/test_parse_tcp_hdr_opt_dynptr.c | 114 ++
 .../selftests/bpf/progs/test_xdp_dynptr.c     | 257 +++++
 .../selftests/bpf/test_tcp_hdr_options.h      |   1 +
 24 files changed, 3320 insertions(+), 186 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/bpf_kfuncs.h
 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

[e-mail] March 1, 2023, 6:10 p.m. UTC | #1

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <[e-mail]>:

On Wed,  1 Mar 2023 07:49:43 -0800 you 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)
> 
> [...]

Here is the summary with links:
  - [v13,bpf-next,01/10] bpf: Support "sk_buff" and "xdp_buff" as valid kfunc arg types
    https://git.kernel.org/bpf/bpf-next/c/2f4643934670
  - [v13,bpf-next,02/10] bpf: Refactor process_dynptr_func
    https://git.kernel.org/bpf/bpf-next/c/7e0dac2807e6
  - [v13,bpf-next,03/10] bpf: Allow initializing dynptrs in kfuncs
    https://git.kernel.org/bpf/bpf-next/c/1d18feb2c915
  - [v13,bpf-next,04/10] bpf: Define no-ops for externally called bpf dynptr functions
    https://git.kernel.org/bpf/bpf-next/c/8357b366cbb0
  - [v13,bpf-next,05/10] bpf: Refactor verifier dynptr into get_dynptr_arg_reg
    https://git.kernel.org/bpf/bpf-next/c/485ec51ef976
  - [v13,bpf-next,06/10] bpf: Add __uninit kfunc annotation
    https://git.kernel.org/bpf/bpf-next/c/d96d937d7c5c
  - [v13,bpf-next,07/10] bpf: Add skb dynptrs
    https://git.kernel.org/bpf/bpf-next/c/b5964b968ac6
  - [v13,bpf-next,08/10] bpf: Add xdp dynptrs
    https://git.kernel.org/bpf/bpf-next/c/05421aecd4ed
  - [v13,bpf-next,09/10] bpf: Add bpf_dynptr_slice and bpf_dynptr_slice_rdwr
    https://git.kernel.org/bpf/bpf-next/c/66e3a13e7c2c
  - [v13,bpf-next,10/10] selftests/bpf: tests for using dynptrs to parse skb and xdp buffers
    https://git.kernel.org/bpf/bpf-next/c/cfa7b011894d

You are awesome, thank you!

Jakub Kicinski March 8, 2023, 8:16 a.m. UTC | #2

On Wed,  1 Mar 2023 07:49:43 -0800 Joanne Koong 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, there is no noticeable difference. Patch 9 contains more
> details as well as examples of how to use skb and xdp dynptrs.

Oddly I see an error trying to build net-next with clang 15.0.7,
but I'm 90% sure that it built yesterday, has anyone seen:

../kernel/bpf/verifier.c:10298:24: error: array index 16 is past the end of the array (which contains 16 elements) [-Werror,-Warray-bounds]
                                   meta.func_id == special_kfunc_list[KF_bpf_dynptr_slice_rdwr]) {
                                                   ^                  ~~~~~~~~~~~~~~~~~~~~~~~~
../kernel/bpf/verifier.c:9150:1: note: array 'special_kfunc_list' declared here
BTF_ID_LIST(special_kfunc_list)
^
../include/linux/btf_ids.h:207:27: note: expanded from macro 'BTF_ID_LIST'
#define BTF_ID_LIST(name) static u32 __maybe_unused name[16];
                          ^
1 error generated.

Andrii Nakryiko March 8, 2023, 5:08 p.m. UTC | #3

On Wed, Mar 8, 2023 at 12:16 AM Jakub Kicinski <[e-mail]> wrote:
>
> On Wed,  1 Mar 2023 07:49:43 -0800 Joanne Koong 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, there is no noticeable difference. Patch 9 contains more
> > details as well as examples of how to use skb and xdp dynptrs.
>
> Oddly I see an error trying to build net-next with clang 15.0.7,
> but I'm 90% sure that it built yesterday, has anyone seen:

yep, it was fixed in bpf-next:

2d5bcdcda879 ("bpf: Increase size of BTF_ID_LIST without
CONFIG_DEBUG_INFO_BTF again")

>
> ../kernel/bpf/verifier.c:10298:24: error: array index 16 is past the end of the array (which contains 16 elements) [-Werror,-Warray-bounds]
>                                    meta.func_id == special_kfunc_list[KF_bpf_dynptr_slice_rdwr]) {
>                                                    ^                  ~~~~~~~~~~~~~~~~~~~~~~~~
> ../kernel/bpf/verifier.c:9150:1: note: array 'special_kfunc_list' declared here
> BTF_ID_LIST(special_kfunc_list)
> ^
> ../include/linux/btf_ids.h:207:27: note: expanded from macro 'BTF_ID_LIST'
> #define BTF_ID_LIST(name) static u32 __maybe_unused name[16];
>                           ^
> 1 error generated.

Jakub Kicinski March 8, 2023, 5:28 p.m. UTC | #4

On Wed, 8 Mar 2023 09:08:09 -0800 Andrii Nakryiko wrote:
> On Wed, Mar 8, 2023 at 12:16 AM Jakub Kicinski <[e-mail]> wrote:
> > On Wed,  1 Mar 2023 07:49:43 -0800 Joanne Koong 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, there is no noticeable difference. Patch 9 contains more
> > > details as well as examples of how to use skb and xdp dynptrs.  
> >
> > Oddly I see an error trying to build net-next with clang 15.0.7,
> > but I'm 90% sure that it built yesterday, has anyone seen:  
> 
> yep, it was fixed in bpf-next:
> 
> 2d5bcdcda879 ("bpf: Increase size of BTF_ID_LIST without
> CONFIG_DEBUG_INFO_BTF again")

Perfect, thanks! Could you get that to us ASAP, please?

Andrii Nakryiko March 8, 2023, 7:02 p.m. UTC | #5

On Wed, Mar 8, 2023 at 9:28 AM Jakub Kicinski <[e-mail]> wrote:
>
> On Wed, 8 Mar 2023 09:08:09 -0800 Andrii Nakryiko wrote:
> > On Wed, Mar 8, 2023 at 12:16 AM Jakub Kicinski <[e-mail]> wrote:
> > > On Wed,  1 Mar 2023 07:49:43 -0800 Joanne Koong 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, there is no noticeable difference. Patch 9 contains more
> > > > details as well as examples of how to use skb and xdp dynptrs.
> > >
> > > Oddly I see an error trying to build net-next with clang 15.0.7,
> > > but I'm 90% sure that it built yesterday, has anyone seen:
> >
> > yep, it was fixed in bpf-next:
> >
> > 2d5bcdcda879 ("bpf: Increase size of BTF_ID_LIST without
> > CONFIG_DEBUG_INFO_BTF again")
>
> Perfect, thanks! Could you get that to us ASAP, please?

yep, will send PR soon