| Message ID | 20200801084721.1812607-4-songliubraving@fb.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | introduce BPF_PROG_TYPE_USER | expand |
On Sat, Aug 1, 2020 at 1:50 AM Song Liu <songliubraving@fb.com> wrote: > > This test checks the correctness of BPF_PROG_TYPE_USER program, including: > running on the right cpu, passing in correct args, returning retval, and > being able to call bpf_get_stack|stackid. > > Signed-off-by: Song Liu <songliubraving@fb.com> > --- > .../selftests/bpf/prog_tests/user_prog.c | 52 +++++++++++++++++ > tools/testing/selftests/bpf/progs/user_prog.c | 56 +++++++++++++++++++ > 2 files changed, 108 insertions(+) > create mode 100644 tools/testing/selftests/bpf/prog_tests/user_prog.c > create mode 100644 tools/testing/selftests/bpf/progs/user_prog.c > > diff --git a/tools/testing/selftests/bpf/prog_tests/user_prog.c b/tools/testing/selftests/bpf/prog_tests/user_prog.c > new file mode 100644 > index 0000000000000..416707b3bff01 > --- /dev/null > +++ b/tools/testing/selftests/bpf/prog_tests/user_prog.c > @@ -0,0 +1,52 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (c) 2020 Facebook */ > +#include <test_progs.h> > +#include "user_prog.skel.h" > + > +static int duration; > + > +void test_user_prog(void) > +{ > + struct bpf_user_prog_args args = {{0, 1, 2, 3, 4}}; > + struct bpf_prog_test_run_attr attr = {}; > + struct user_prog *skel; > + int i, numcpu, ret; > + > + skel = user_prog__open_and_load(); > + > + if (CHECK(!skel, "user_prog__open_and_load", > + "skeleton open_and_laod failed\n")) > + return; > + > + numcpu = libbpf_num_possible_cpus(); nit: possible doesn't mean online right now, so it will fail on offline or non-present CPUs > + > + attr.prog_fd = bpf_program__fd(skel->progs.user_func); > + attr.data_size_in = sizeof(args); > + attr.data_in = &args; > + > + /* start from -1, so we test cpu_plus == 0 */ > + for (i = -1; i < numcpu; i++) { > + args.args[0] = i + 1; > + attr.cpu_plus = i + 1; > + ret = bpf_prog_test_run_xattr(&attr); > + CHECK(ret, "bpf_prog_test_run_xattr", "returns error\n"); > + > + /* skip two tests for i == -1 */ > + if (i == -1) > + continue; > + CHECK(attr.retval != i + 2, "bpf_prog_test_run_xattr", > + "doesn't get expected retval\n"); > + CHECK(skel->data->sum != 11 + i, "user_prog_args_test", > + "sum of args doesn't match\n"); > + } > + > + CHECK(skel->data->cpu_match == 0, "cpu_match_test", "failed\n"); > + CHECK(skel->bss->get_stack_success != numcpu + 1, "test_bpf_get_stack", > + "failed on %d cores\n", numcpu - skel->bss->get_stack_success); > + CHECK(skel->bss->get_stackid_success != numcpu + 1, > + "test_bpf_get_stackid", > + "failed on %d cores\n", > + numcpu + 1 - skel->bss->get_stackid_success); > + > + user_prog__destroy(skel); > +} > diff --git a/tools/testing/selftests/bpf/progs/user_prog.c b/tools/testing/selftests/bpf/progs/user_prog.c > new file mode 100644 > index 0000000000000..cf320e97f107a > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/user_prog.c > @@ -0,0 +1,56 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (c) 2020 Facebook */ > +#include "vmlinux.h" > +#include <bpf/bpf_helpers.h> > +#include <bpf/bpf_tracing.h> > + > +char _license[] SEC("license") = "GPL"; > + > +#ifndef PERF_MAX_STACK_DEPTH > +#define PERF_MAX_STACK_DEPTH 127 > +#endif > + > +typedef __u64 stack_trace_t[PERF_MAX_STACK_DEPTH]; > + > +struct { > + __uint(type, BPF_MAP_TYPE_STACK_TRACE); > + __uint(max_entries, 16384); > + __uint(key_size, sizeof(__u32)); > + __uint(value_size, sizeof(stack_trace_t)); > +} stackmap SEC(".maps"); > + > +volatile int cpu_match = 1; > +volatile __u64 sum = 1; > +volatile int get_stack_success = 0; > +volatile int get_stackid_success = 0; > +volatile __u64 stacktrace[PERF_MAX_STACK_DEPTH]; nit: no need for volatile for non-static variables > + > +SEC("user") > +int user_func(struct bpf_user_prog_ctx *ctx) If you put args in bpf_user_prog_ctx as a first field, you should be able to re-use the BPF_PROG macro to access those arguments in a more user-friendly way. > +{ > + int cpu = bpf_get_smp_processor_id(); > + __u32 key = cpu; > + long stackid, err; > + > + /* check the program runs on the right cpu */ > + if (ctx->args[0] && ctx->args[0] != cpu + 1) > + cpu_match = 0; > + > + /* check the sum of arguments are correct */ > + sum = ctx->args[0] + ctx->args[1] + ctx->args[2] + > + ctx->args[3] + ctx->args[4]; > + > + /* check bpf_get_stackid works */ > + stackid = bpf_get_stackid(ctx, &stackmap, 0); > + if (stackid >= 0) > + get_stackid_success++; > + > + /* check bpf_get_stack works */ > + err = bpf_get_stack(ctx, (void *)stacktrace, > + PERF_MAX_STACK_DEPTH * sizeof(__u64), > + BPF_F_USER_STACK); > + if (err >= 0) > + get_stack_success++; > + > + return cpu + 2; > +} > -- > 2.24.1 >
> On Aug 2, 2020, at 6:43 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Sat, Aug 1, 2020 at 1:50 AM Song Liu <songliubraving@fb.com> wrote: >> >> This test checks the correctness of BPF_PROG_TYPE_USER program, including: >> running on the right cpu, passing in correct args, returning retval, and >> being able to call bpf_get_stack|stackid. >> >> Signed-off-by: Song Liu <songliubraving@fb.com> >> --- >> .../selftests/bpf/prog_tests/user_prog.c | 52 +++++++++++++++++ >> tools/testing/selftests/bpf/progs/user_prog.c | 56 +++++++++++++++++++ >> 2 files changed, 108 insertions(+) >> create mode 100644 tools/testing/selftests/bpf/prog_tests/user_prog.c >> create mode 100644 tools/testing/selftests/bpf/progs/user_prog.c >> >> diff --git a/tools/testing/selftests/bpf/prog_tests/user_prog.c b/tools/testing/selftests/bpf/prog_tests/user_prog.c >> new file mode 100644 >> index 0000000000000..416707b3bff01 >> --- /dev/null >> +++ b/tools/testing/selftests/bpf/prog_tests/user_prog.c >> @@ -0,0 +1,52 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* Copyright (c) 2020 Facebook */ >> +#include <test_progs.h> >> +#include "user_prog.skel.h" >> + >> +static int duration; >> + >> +void test_user_prog(void) >> +{ >> + struct bpf_user_prog_args args = {{0, 1, 2, 3, 4}}; >> + struct bpf_prog_test_run_attr attr = {}; >> + struct user_prog *skel; >> + int i, numcpu, ret; >> + >> + skel = user_prog__open_and_load(); >> + >> + if (CHECK(!skel, "user_prog__open_and_load", >> + "skeleton open_and_laod failed\n")) >> + return; >> + >> + numcpu = libbpf_num_possible_cpus(); > > nit: possible doesn't mean online right now, so it will fail on > offline or non-present CPUs Just found parse_cpu_mask_file(), will use it to fix this. [...] >> + >> +volatile int cpu_match = 1; >> +volatile __u64 sum = 1; >> +volatile int get_stack_success = 0; >> +volatile int get_stackid_success = 0; >> +volatile __u64 stacktrace[PERF_MAX_STACK_DEPTH]; > > nit: no need for volatile for non-static variables > >> + >> +SEC("user") >> +int user_func(struct bpf_user_prog_ctx *ctx) > > If you put args in bpf_user_prog_ctx as a first field, you should be > able to re-use the BPF_PROG macro to access those arguments in a more > user-friendly way. I am not sure I am following here. Do you mean something like: struct bpf_user_prog_ctx { __u64 args[BPF_USER_PROG_MAX_ARGS]; struct pt_regs *regs; }; (swap args and regs)? Thanks, Song
On Sun, Aug 2, 2020 at 9:33 PM Song Liu <songliubraving@fb.com> wrote: > > > > > On Aug 2, 2020, at 6:43 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > > > On Sat, Aug 1, 2020 at 1:50 AM Song Liu <songliubraving@fb.com> wrote: > >> > >> This test checks the correctness of BPF_PROG_TYPE_USER program, including: > >> running on the right cpu, passing in correct args, returning retval, and > >> being able to call bpf_get_stack|stackid. > >> > >> Signed-off-by: Song Liu <songliubraving@fb.com> > >> --- > >> .../selftests/bpf/prog_tests/user_prog.c | 52 +++++++++++++++++ > >> tools/testing/selftests/bpf/progs/user_prog.c | 56 +++++++++++++++++++ > >> 2 files changed, 108 insertions(+) > >> create mode 100644 tools/testing/selftests/bpf/prog_tests/user_prog.c > >> create mode 100644 tools/testing/selftests/bpf/progs/user_prog.c > >> > >> diff --git a/tools/testing/selftests/bpf/prog_tests/user_prog.c b/tools/testing/selftests/bpf/prog_tests/user_prog.c > >> new file mode 100644 > >> index 0000000000000..416707b3bff01 > >> --- /dev/null > >> +++ b/tools/testing/selftests/bpf/prog_tests/user_prog.c > >> @@ -0,0 +1,52 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +/* Copyright (c) 2020 Facebook */ > >> +#include <test_progs.h> > >> +#include "user_prog.skel.h" > >> + > >> +static int duration; > >> + > >> +void test_user_prog(void) > >> +{ > >> + struct bpf_user_prog_args args = {{0, 1, 2, 3, 4}}; > >> + struct bpf_prog_test_run_attr attr = {}; > >> + struct user_prog *skel; > >> + int i, numcpu, ret; > >> + > >> + skel = user_prog__open_and_load(); > >> + > >> + if (CHECK(!skel, "user_prog__open_and_load", > >> + "skeleton open_and_laod failed\n")) > >> + return; > >> + > >> + numcpu = libbpf_num_possible_cpus(); > > > > nit: possible doesn't mean online right now, so it will fail on > > offline or non-present CPUs > > Just found parse_cpu_mask_file(), will use it to fix this. > > [...] > > >> + > >> +volatile int cpu_match = 1; > >> +volatile __u64 sum = 1; > >> +volatile int get_stack_success = 0; > >> +volatile int get_stackid_success = 0; > >> +volatile __u64 stacktrace[PERF_MAX_STACK_DEPTH]; > > > > nit: no need for volatile for non-static variables > > > >> + > >> +SEC("user") > >> +int user_func(struct bpf_user_prog_ctx *ctx) > > > > If you put args in bpf_user_prog_ctx as a first field, you should be > > able to re-use the BPF_PROG macro to access those arguments in a more > > user-friendly way. > > I am not sure I am following here. Do you mean something like: > > struct bpf_user_prog_ctx { > __u64 args[BPF_USER_PROG_MAX_ARGS]; > struct pt_regs *regs; > }; > > (swap args and regs)? > Yes, BPF_PROG assumes that context is a plain u64[5] array, so if you put args at the beginning, it will work nicely with BPF_PROG. > Thanks, > Song > >
diff --git a/tools/testing/selftests/bpf/prog_tests/user_prog.c b/tools/testing/selftests/bpf/prog_tests/user_prog.c new file mode 100644 index 0000000000000..416707b3bff01 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/user_prog.c @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2020 Facebook */ +#include <test_progs.h> +#include "user_prog.skel.h" + +static int duration; + +void test_user_prog(void) +{ + struct bpf_user_prog_args args = {{0, 1, 2, 3, 4}}; + struct bpf_prog_test_run_attr attr = {}; + struct user_prog *skel; + int i, numcpu, ret; + + skel = user_prog__open_and_load(); + + if (CHECK(!skel, "user_prog__open_and_load", + "skeleton open_and_laod failed\n")) + return; + + numcpu = libbpf_num_possible_cpus(); + + attr.prog_fd = bpf_program__fd(skel->progs.user_func); + attr.data_size_in = sizeof(args); + attr.data_in = &args; + + /* start from -1, so we test cpu_plus == 0 */ + for (i = -1; i < numcpu; i++) { + args.args[0] = i + 1; + attr.cpu_plus = i + 1; + ret = bpf_prog_test_run_xattr(&attr); + CHECK(ret, "bpf_prog_test_run_xattr", "returns error\n"); + + /* skip two tests for i == -1 */ + if (i == -1) + continue; + CHECK(attr.retval != i + 2, "bpf_prog_test_run_xattr", + "doesn't get expected retval\n"); + CHECK(skel->data->sum != 11 + i, "user_prog_args_test", + "sum of args doesn't match\n"); + } + + CHECK(skel->data->cpu_match == 0, "cpu_match_test", "failed\n"); + CHECK(skel->bss->get_stack_success != numcpu + 1, "test_bpf_get_stack", + "failed on %d cores\n", numcpu - skel->bss->get_stack_success); + CHECK(skel->bss->get_stackid_success != numcpu + 1, + "test_bpf_get_stackid", + "failed on %d cores\n", + numcpu + 1 - skel->bss->get_stackid_success); + + user_prog__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/user_prog.c b/tools/testing/selftests/bpf/progs/user_prog.c new file mode 100644 index 0000000000000..cf320e97f107a --- /dev/null +++ b/tools/testing/selftests/bpf/progs/user_prog.c @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2020 Facebook */ +#include "vmlinux.h" +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> + +char _license[] SEC("license") = "GPL"; + +#ifndef PERF_MAX_STACK_DEPTH +#define PERF_MAX_STACK_DEPTH 127 +#endif + +typedef __u64 stack_trace_t[PERF_MAX_STACK_DEPTH]; + +struct { + __uint(type, BPF_MAP_TYPE_STACK_TRACE); + __uint(max_entries, 16384); + __uint(key_size, sizeof(__u32)); + __uint(value_size, sizeof(stack_trace_t)); +} stackmap SEC(".maps"); + +volatile int cpu_match = 1; +volatile __u64 sum = 1; +volatile int get_stack_success = 0; +volatile int get_stackid_success = 0; +volatile __u64 stacktrace[PERF_MAX_STACK_DEPTH]; + +SEC("user") +int user_func(struct bpf_user_prog_ctx *ctx) +{ + int cpu = bpf_get_smp_processor_id(); + __u32 key = cpu; + long stackid, err; + + /* check the program runs on the right cpu */ + if (ctx->args[0] && ctx->args[0] != cpu + 1) + cpu_match = 0; + + /* check the sum of arguments are correct */ + sum = ctx->args[0] + ctx->args[1] + ctx->args[2] + + ctx->args[3] + ctx->args[4]; + + /* check bpf_get_stackid works */ + stackid = bpf_get_stackid(ctx, &stackmap, 0); + if (stackid >= 0) + get_stackid_success++; + + /* check bpf_get_stack works */ + err = bpf_get_stack(ctx, (void *)stacktrace, + PERF_MAX_STACK_DEPTH * sizeof(__u64), + BPF_F_USER_STACK); + if (err >= 0) + get_stack_success++; + + return cpu + 2; +}
This test checks the correctness of BPF_PROG_TYPE_USER program, including: running on the right cpu, passing in correct args, returning retval, and being able to call bpf_get_stack|stackid. Signed-off-by: Song Liu <songliubraving@fb.com> --- .../selftests/bpf/prog_tests/user_prog.c | 52 +++++++++++++++++ tools/testing/selftests/bpf/progs/user_prog.c | 56 +++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/user_prog.c create mode 100644 tools/testing/selftests/bpf/progs/user_prog.c