| Message ID | 20210423002646.35043-6-alexei.starovoitov@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: syscall program, FD array, loader program, light skeleton. | expand |
On Thu, Apr 22, 2021 at 5:26 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > From: Alexei Starovoitov <ast@kernel.org> > > bpf_prog_type_syscall is a program that creates a bpf map, > updates it, and loads another bpf program using bpf_sys_bpf() helper. > > Signed-off-by: Alexei Starovoitov <ast@kernel.org> > --- > tools/testing/selftests/bpf/Makefile | 1 + > .../selftests/bpf/prog_tests/syscall.c | 53 ++++++++++++++ > tools/testing/selftests/bpf/progs/syscall.c | 73 +++++++++++++++++++ > 3 files changed, 127 insertions(+) > create mode 100644 tools/testing/selftests/bpf/prog_tests/syscall.c > create mode 100644 tools/testing/selftests/bpf/progs/syscall.c > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index c5bcdb3d4b12..9fdfdbc61857 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -278,6 +278,7 @@ MENDIAN=$(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian) > CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG)) > BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \ > -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR) \ > + -I$(TOOLSINCDIR) \ is this for filter.h? also, please align \ with the previous line > -I$(abspath $(OUTPUT)/../usr/include) > > CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \ > diff --git a/tools/testing/selftests/bpf/prog_tests/syscall.c b/tools/testing/selftests/bpf/prog_tests/syscall.c > new file mode 100644 > index 000000000000..e550e36bb5da > --- /dev/null > +++ b/tools/testing/selftests/bpf/prog_tests/syscall.c > @@ -0,0 +1,53 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (c) 2021 Facebook */ > +#include <test_progs.h> > +#include "syscall.skel.h" > + > +struct args { > + __u64 log_buf; > + __u32 log_size; > + int max_entries; > + int map_fd; > + int prog_fd; > +}; > + > +void test_syscall(void) > +{ > + static char verifier_log[8192]; > + struct args ctx = { > + .max_entries = 1024, > + .log_buf = (uintptr_t) verifier_log, > + .log_size = sizeof(verifier_log), > + }; > + struct bpf_prog_test_run_attr tattr = { > + .ctx_in = &ctx, > + .ctx_size_in = sizeof(ctx), > + }; > + struct syscall *skel = NULL; > + __u64 key = 12, value = 0; > + __u32 duration = 0; > + int err; > + > + skel = syscall__open_and_load(); > + if (CHECK(!skel, "skel_load", "syscall skeleton failed\n")) > + goto cleanup; > + > + tattr.prog_fd = bpf_program__fd(skel->progs.bpf_prog); > + err = bpf_prog_test_run_xattr(&tattr); > + if (CHECK(err || tattr.retval != 1, "test_run sys_bpf", > + "err %d errno %d retval %d duration %d\n", > + err, errno, tattr.retval, tattr.duration)) > + goto cleanup; > + > + CHECK(ctx.map_fd <= 0, "map_fd", "fd = %d\n", ctx.map_fd); > + CHECK(ctx.prog_fd <= 0, "prog_fd", "fd = %d\n", ctx.prog_fd); please use ASSERT_xxx() macros everywhere. I've just added ASSERT_GT(), so once that patch set lands you should have all the variants you need. > + CHECK(memcmp(verifier_log, "processed", sizeof("processed") - 1) != 0, > + "verifier_log", "%s\n", verifier_log); > + > + err = bpf_map_lookup_elem(ctx.map_fd, &key, &value); > + CHECK(err, "map_lookup", "map_lookup failed\n"); > + CHECK(value != 34, "invalid_value", > + "got value %llu expected %u\n", value, 34); > +cleanup: > + syscall__destroy(skel); > +} > diff --git a/tools/testing/selftests/bpf/progs/syscall.c b/tools/testing/selftests/bpf/progs/syscall.c > new file mode 100644 > index 000000000000..01476f88e45f > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/syscall.c > @@ -0,0 +1,73 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (c) 2021 Facebook */ > +#include <linux/stddef.h> > +#include <linux/bpf.h> > +#include <bpf/bpf_helpers.h> > +#include <bpf/bpf_tracing.h> > +#include <../../tools/include/linux/filter.h> with TOOLSINCDIR shouldn't this be just <linux/fiter.h>? > + > +volatile const int workaround = 1; not needed anymore? > + > +char _license[] SEC("license") = "GPL"; > + > +struct args { > + __u64 log_buf; > + __u32 log_size; > + int max_entries; > + int map_fd; > + int prog_fd; > +}; > + [...]
On Mon, Apr 26, 2021 at 10:02:59AM -0700, Andrii Nakryiko wrote: > > +/* Copyright (c) 2021 Facebook */ > > +#include <linux/stddef.h> > > +#include <linux/bpf.h> > > +#include <bpf/bpf_helpers.h> > > +#include <bpf/bpf_tracing.h> > > +#include <../../tools/include/linux/filter.h> > > with TOOLSINCDIR shouldn't this be just <linux/fiter.h>? sadly no. There is uapi/linux/filter.h that gets included first. And changing the order of -Is brings the whole set of other issues. I couldn't come up with anything better unfortunately.
On Mon, Apr 26, 2021 at 7:43 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Mon, Apr 26, 2021 at 10:02:59AM -0700, Andrii Nakryiko wrote: > > > +/* Copyright (c) 2021 Facebook */ > > > +#include <linux/stddef.h> > > > +#include <linux/bpf.h> > > > +#include <bpf/bpf_helpers.h> > > > +#include <bpf/bpf_tracing.h> > > > +#include <../../tools/include/linux/filter.h> > > > > with TOOLSINCDIR shouldn't this be just <linux/fiter.h>? > > sadly no. There is uapi/linux/filter.h that gets included first. > And changing the order of -Is brings the whole set of other issues. > I couldn't come up with anything better unfortunately. Then let's at least drop TOOLSINCDIR for now, if it's not really used?
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index c5bcdb3d4b12..9fdfdbc61857 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -278,6 +278,7 @@ MENDIAN=$(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian) CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG)) BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \ -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR) \ + -I$(TOOLSINCDIR) \ -I$(abspath $(OUTPUT)/../usr/include) CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \ diff --git a/tools/testing/selftests/bpf/prog_tests/syscall.c b/tools/testing/selftests/bpf/prog_tests/syscall.c new file mode 100644 index 000000000000..e550e36bb5da --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/syscall.c @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2021 Facebook */ +#include <test_progs.h> +#include "syscall.skel.h" + +struct args { + __u64 log_buf; + __u32 log_size; + int max_entries; + int map_fd; + int prog_fd; +}; + +void test_syscall(void) +{ + static char verifier_log[8192]; + struct args ctx = { + .max_entries = 1024, + .log_buf = (uintptr_t) verifier_log, + .log_size = sizeof(verifier_log), + }; + struct bpf_prog_test_run_attr tattr = { + .ctx_in = &ctx, + .ctx_size_in = sizeof(ctx), + }; + struct syscall *skel = NULL; + __u64 key = 12, value = 0; + __u32 duration = 0; + int err; + + skel = syscall__open_and_load(); + if (CHECK(!skel, "skel_load", "syscall skeleton failed\n")) + goto cleanup; + + tattr.prog_fd = bpf_program__fd(skel->progs.bpf_prog); + err = bpf_prog_test_run_xattr(&tattr); + if (CHECK(err || tattr.retval != 1, "test_run sys_bpf", + "err %d errno %d retval %d duration %d\n", + err, errno, tattr.retval, tattr.duration)) + goto cleanup; + + CHECK(ctx.map_fd <= 0, "map_fd", "fd = %d\n", ctx.map_fd); + CHECK(ctx.prog_fd <= 0, "prog_fd", "fd = %d\n", ctx.prog_fd); + CHECK(memcmp(verifier_log, "processed", sizeof("processed") - 1) != 0, + "verifier_log", "%s\n", verifier_log); + + err = bpf_map_lookup_elem(ctx.map_fd, &key, &value); + CHECK(err, "map_lookup", "map_lookup failed\n"); + CHECK(value != 34, "invalid_value", + "got value %llu expected %u\n", value, 34); +cleanup: + syscall__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/syscall.c b/tools/testing/selftests/bpf/progs/syscall.c new file mode 100644 index 000000000000..01476f88e45f --- /dev/null +++ b/tools/testing/selftests/bpf/progs/syscall.c @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2021 Facebook */ +#include <linux/stddef.h> +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> +#include <../../tools/include/linux/filter.h> + +volatile const int workaround = 1; + +char _license[] SEC("license") = "GPL"; + +struct args { + __u64 log_buf; + __u32 log_size; + int max_entries; + int map_fd; + int prog_fd; +}; + +SEC("syscall") +int bpf_prog(struct args *ctx) +{ + static char license[] = "GPL"; + static struct bpf_insn insns[] = { + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0), + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), + BPF_LD_MAP_FD(BPF_REG_1, 0), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem), + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }; + static union bpf_attr map_create_attr = { + .map_type = BPF_MAP_TYPE_HASH, + .key_size = 8, + .value_size = 8, + }; + static union bpf_attr map_update_attr = { .map_fd = 1, }; + static __u64 key = 12; + static __u64 value = 34; + static union bpf_attr prog_load_attr = { + .prog_type = BPF_PROG_TYPE_XDP, + .insn_cnt = sizeof(insns) / sizeof(insns[0]), + }; + int ret; + + map_create_attr.max_entries = ctx->max_entries; + prog_load_attr.license = (long) license; + prog_load_attr.insns = (long) insns; + prog_load_attr.log_buf = ctx->log_buf; + prog_load_attr.log_size = ctx->log_size; + prog_load_attr.log_level = 1; + + ret = bpf_sys_bpf(BPF_MAP_CREATE, &map_create_attr, sizeof(map_create_attr)); + if (ret <= 0) + return ret; + ctx->map_fd = ret; + insns[3].imm = ret; + + map_update_attr.map_fd = ret; + map_update_attr.key = (long) &key; + map_update_attr.value = (long) &value; + ret = bpf_sys_bpf(BPF_MAP_UPDATE_ELEM, &map_update_attr, sizeof(map_update_attr)); + if (ret < 0) + return ret; + + ret = bpf_sys_bpf(BPF_PROG_LOAD, &prog_load_attr, sizeof(prog_load_attr)); + if (ret <= 0) + return ret; + ctx->prog_fd = ret; + return 1; +}