| Message ID | 20210216105713.45052-1-lmb@cloudflare.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | PROG_TEST_RUN support for sk_lookup programs | expand |
Lorenz Bauer wrote: > We don't have PROG_TEST_RUN support for sk_lookup programs at the > moment. So far this hasn't been a problem, since we can run our > tests in a separate network namespace. For benchmarking it's nice > to have PROG_TEST_RUN, so I've gone and implemented it. > > Multiple sk_lookup programs can be attached at once to the same > netns. This can't be expressed with the current PROG_TEST_RUN > API, so I'm proposing to extend it with an array of prog_fd. > > Patches 1-2 are clean ups. Patches 3-4 add the new UAPI and > implement PROG_TEST_RUN for sk_lookup. Patch 5 adds a new > function to libbpf to access multi prog tests. Patches 6-8 add > tests. > > Andrii, for patch 4 I decided on the following API: > > int bpf_prog_test_run_array(__u32 *prog_fds, __u32 prog_fds_cnt, > struct bpf_test_run_opts *opts) > > To be consistent with the rest of libbpf it would be better > to take int *prog_fds, but I think then the function would have to > convert the array to account for platforms where > > sizeof(int) != sizeof(__u32) > > Please let me know what your preference is. Seems reasonable to me. For the series, Acked-by: John Fastabend <john.fastabend@gmail.com>
On Tue, Feb 16, 2021 at 2:58 AM Lorenz Bauer <lmb@cloudflare.com> wrote: > > We don't have PROG_TEST_RUN support for sk_lookup programs at the > moment. So far this hasn't been a problem, since we can run our > tests in a separate network namespace. For benchmarking it's nice > to have PROG_TEST_RUN, so I've gone and implemented it. > > Multiple sk_lookup programs can be attached at once to the same > netns. This can't be expressed with the current PROG_TEST_RUN > API, so I'm proposing to extend it with an array of prog_fd. > > Patches 1-2 are clean ups. Patches 3-4 add the new UAPI and > implement PROG_TEST_RUN for sk_lookup. Patch 5 adds a new > function to libbpf to access multi prog tests. Patches 6-8 add > tests. > > Andrii, for patch 4 I decided on the following API: > > int bpf_prog_test_run_array(__u32 *prog_fds, __u32 prog_fds_cnt, > struct bpf_test_run_opts *opts) > > To be consistent with the rest of libbpf it would be better > to take int *prog_fds, but I think then the function would have to > convert the array to account for platforms where > > sizeof(int) != sizeof(__u32) Curious, is there any supported architecture where this is not the case? I think it's fine to be consistent, tbh, and use int. Worst case, in some obscure architecture we'd need to create a copy of an array. Doesn't seem like a big deal (and highly unlikely anyways). > > Please let me know what your preference is. > > Lorenz Bauer (8): > bpf: consolidate shared test timing code > bpf: add for_each_bpf_prog helper > bpf: allow multiple programs in BPF_PROG_TEST_RUN > bpf: add PROG_TEST_RUN support for sk_lookup programs > tools: libbpf: allow testing program types with multi-prog semantics > selftests: bpf: convert sk_lookup multi prog tests to PROG_TEST_RUN > selftests: bpf: convert sk_lookup ctx access tests to PROG_TEST_RUN > selftests: bpf: check that PROG_TEST_RUN repeats as requested > > include/linux/bpf-netns.h | 2 + > include/linux/bpf.h | 24 +- > include/linux/filter.h | 4 +- > include/uapi/linux/bpf.h | 11 +- > kernel/bpf/net_namespace.c | 2 +- > kernel/bpf/syscall.c | 73 +++++- > net/bpf/test_run.c | 230 +++++++++++++----- > net/core/filter.c | 1 + > tools/include/uapi/linux/bpf.h | 11 +- > tools/lib/bpf/bpf.c | 16 +- > tools/lib/bpf/bpf.h | 3 + > tools/lib/bpf/libbpf.map | 1 + > .../selftests/bpf/prog_tests/prog_run_xattr.c | 51 +++- > .../selftests/bpf/prog_tests/sk_lookup.c | 172 +++++++++---- > .../selftests/bpf/progs/test_sk_lookup.c | 62 +++-- > 15 files changed, 499 insertions(+), 164 deletions(-) > > -- > 2.27.0 >
On Tue, 23 Feb 2021 at 07:29, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > Curious, is there any supported architecture where this is not the > case? I think it's fine to be consistent, tbh, and use int. Worst > case, in some obscure architecture we'd need to create a copy of an > array. Doesn't seem like a big deal (and highly unlikely anyways). Ok, thanks! I'm not super familiar with C platform differences, so I wanted to be on the safe side. I'll take this up depending on the outcome of the conversation with Alexey, maybe I don't need to add this after all.
On 2/23/21 8:29 AM, Andrii Nakryiko wrote: > On Tue, Feb 16, 2021 at 2:58 AM Lorenz Bauer <lmb@cloudflare.com> wrote: >> >> We don't have PROG_TEST_RUN support for sk_lookup programs at the >> moment. So far this hasn't been a problem, since we can run our >> tests in a separate network namespace. For benchmarking it's nice >> to have PROG_TEST_RUN, so I've gone and implemented it. >> >> Multiple sk_lookup programs can be attached at once to the same >> netns. This can't be expressed with the current PROG_TEST_RUN >> API, so I'm proposing to extend it with an array of prog_fd. >> >> Patches 1-2 are clean ups. Patches 3-4 add the new UAPI and >> implement PROG_TEST_RUN for sk_lookup. Patch 5 adds a new >> function to libbpf to access multi prog tests. Patches 6-8 add >> tests. >> >> Andrii, for patch 4 I decided on the following API: >> >> int bpf_prog_test_run_array(__u32 *prog_fds, __u32 prog_fds_cnt, >> struct bpf_test_run_opts *opts) >> >> To be consistent with the rest of libbpf it would be better >> to take int *prog_fds, but I think then the function would have to >> convert the array to account for platforms where >> >> sizeof(int) != sizeof(__u32) > > Curious, is there any supported architecture where this is not the > case? I think it's fine to be consistent, tbh, and use int. Worst > case, in some obscure architecture we'd need to create a copy of an > array. Doesn't seem like a big deal (and highly unlikely anyways). Given __u32 are kernel UAPI exported types for user space (e.g. used in syscall APIs), you can check where / how they are defined. Mainly here: include/uapi/asm-generic/int-l64.h:27:typedef unsigned int __u32; include/uapi/asm-generic/int-ll64.h:27:typedef unsigned int __u32; Thanks, Daniel
We don't have PROG_TEST_RUN support for sk_lookup programs at the moment. So far this hasn't been a problem, since we can run our tests in a separate network namespace. For benchmarking it's nice to have PROG_TEST_RUN, so I've gone and implemented it. Multiple sk_lookup programs can be attached at once to the same netns. This can't be expressed with the current PROG_TEST_RUN API, so I'm proposing to extend it with an array of prog_fd. Patches 1-2 are clean ups. Patches 3-4 add the new UAPI and implement PROG_TEST_RUN for sk_lookup. Patch 5 adds a new function to libbpf to access multi prog tests. Patches 6-8 add tests. Andrii, for patch 4 I decided on the following API: int bpf_prog_test_run_array(__u32 *prog_fds, __u32 prog_fds_cnt, struct bpf_test_run_opts *opts) To be consistent with the rest of libbpf it would be better to take int *prog_fds, but I think then the function would have to convert the array to account for platforms where sizeof(int) != sizeof(__u32) Please let me know what your preference is. Lorenz Bauer (8): bpf: consolidate shared test timing code bpf: add for_each_bpf_prog helper bpf: allow multiple programs in BPF_PROG_TEST_RUN bpf: add PROG_TEST_RUN support for sk_lookup programs tools: libbpf: allow testing program types with multi-prog semantics selftests: bpf: convert sk_lookup multi prog tests to PROG_TEST_RUN selftests: bpf: convert sk_lookup ctx access tests to PROG_TEST_RUN selftests: bpf: check that PROG_TEST_RUN repeats as requested include/linux/bpf-netns.h | 2 + include/linux/bpf.h | 24 +- include/linux/filter.h | 4 +- include/uapi/linux/bpf.h | 11 +- kernel/bpf/net_namespace.c | 2 +- kernel/bpf/syscall.c | 73 +++++- net/bpf/test_run.c | 230 +++++++++++++----- net/core/filter.c | 1 + tools/include/uapi/linux/bpf.h | 11 +- tools/lib/bpf/bpf.c | 16 +- tools/lib/bpf/bpf.h | 3 + tools/lib/bpf/libbpf.map | 1 + .../selftests/bpf/prog_tests/prog_run_xattr.c | 51 +++- .../selftests/bpf/prog_tests/sk_lookup.c | 172 +++++++++---- .../selftests/bpf/progs/test_sk_lookup.c | 62 +++-- 15 files changed, 499 insertions(+), 164 deletions(-)