| Message ID | 20200703095111.3268961-10-jolsa@kernel.org |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: Add d_path helper - preparation changes | expand |
On Fri, Jul 3, 2020 at 2:54 AM Jiri Olsa <jolsa@kernel.org> wrote: > > Adding resolve_btfids test under test_progs suite. > > It's possible to use btf_ids.h header and its logic in > user space application, so we can add easy test for it. > > The test defines BTF_ID_LIST and checks it gets properly > resolved. > > For this reason the test_progs binary (and other binaries > that use TRUNNER* macros) is processed with resolve_btfids > tool, which resolves BTF IDs in .BTF.ids section. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > tools/testing/selftests/bpf/Makefile | 22 ++- > .../selftests/bpf/prog_tests/resolve_btfids.c | 170 ++++++++++++++++++ > 2 files changed, 190 insertions(+), 2 deletions(-) > create mode 100644 tools/testing/selftests/bpf/prog_tests/resolve_btfids.c > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index 1f9c696b3edf..b47a685d12bd 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -190,6 +190,16 @@ else > cp "$(VMLINUX_H)" $@ > endif > > +$(SCRATCH_DIR)/resolve_btfids: $(BPFOBJ) \ > + $(TOOLSDIR)/bpf/resolve_btfids/main.c \ > + $(TOOLSDIR)/lib/rbtree.c \ > + $(TOOLSDIR)/lib/zalloc.c \ > + $(TOOLSDIR)/lib/string.c \ > + $(TOOLSDIR)/lib/ctype.c \ > + $(TOOLSDIR)/lib/str_error_r.c > + $(Q)$(MAKE) $(submake_extras) -C $(TOOLSDIR)/bpf/resolve_btfids \ > + OUTPUT=$(SCRATCH_DIR)/ BPFOBJ=$(BPFOBJ) > + please indent OUTPUT, so it doesn't look like it's a separate command > # Get Clang's default includes on this system, as opposed to those seen by > # '-target bpf'. This fixes "missing" files on some architectures/distros, > # such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc. > @@ -333,7 +343,8 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \ > $(TRUNNER_BPF_SKELS) \ > $$(BPFOBJ) | $(TRUNNER_OUTPUT) > $$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@) > - cd $$(@D) && $$(CC) -I. $$(CFLAGS) -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F) > + cd $$(@D) && $$(CC) -I. $$(CFLAGS) $(TRUNNER_EXTRA_CFLAGS) \ > + -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F) > > $(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \ > %.c \ > @@ -355,6 +366,7 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \ > | $(TRUNNER_BINARY)-extras > $$(call msg,BINARY,,$$@) > $$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) -o $$@ > + $(TRUNNER_BINARY_EXTRA_CMD) no need to make this generic, just write out resolve_btfids here explicitly > > endef > > @@ -365,7 +377,10 @@ TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \ > network_helpers.c testing_helpers.c \ > flow_dissector_load.h > TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \ > - $(wildcard progs/btf_dump_test_case_*.c) > + $(wildcard progs/btf_dump_test_case_*.c) \ > + $(SCRATCH_DIR)/resolve_btfids > +TRUNNER_EXTRA_CFLAGS := -D"BUILD_STR(s)=\#s" -DVMLINUX_BTF="BUILD_STR($(VMLINUX_BTF))" > +TRUNNER_BINARY_EXTRA_CMD := $(SCRATCH_DIR)/resolve_btfids --btf $(VMLINUX_BTF) test_progs I hope we can get rid of this, see suggestion below. > TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE > TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) > TRUNNER_BPF_LDFLAGS := -mattr=+alu32 > @@ -373,6 +388,7 @@ $(eval $(call DEFINE_TEST_RUNNER,test_progs)) > [...] > + > +static int duration; > + > +static struct btf *btf__parse_raw(const char *file) another copy here... > +{ > + struct btf *btf; > + struct stat st; > + __u8 *buf; > + FILE *f; > + [...] > + > +BTF_ID_LIST(test_list) > +BTF_ID_UNUSED > +BTF_ID(typedef, pid_t) > +BTF_ID(struct, sk_buff) > +BTF_ID(union, thread_union) > +BTF_ID(func, memcpy) > + > +struct symbol { > + const char *name; > + int type; > + int id; > +}; > + > +struct symbol test_symbols[] = { > + { "unused", -1, 0 }, could use BTF_KIND_UNKN here instead of -1 > + { "pid_t", BTF_KIND_TYPEDEF, -1 }, > + { "sk_buff", BTF_KIND_STRUCT, -1 }, > + { "thread_union", BTF_KIND_UNION, -1 }, > + { "memcpy", BTF_KIND_FUNC, -1 }, > +}; > + [...] > + > +static int resolve_symbols(void) > +{ > + const char *path = VMLINUX_BTF; This build-time parameter passing to find the original VMLINUX_BTF really sucks, IMO. Why not use the btf_dump tests approach and have our own small "vmlinux BTF", which resolve_btfids would use to resolve these IDs? See how btf_dump_xxx.c files define BTFs that are used in tests. You can do something similar here, and use a well-known BPF object file as a source of BTF, both here in a test and in Makefile for --btf param to resolve_btfids? > + struct btf *btf; > + int type_id; > + __u32 nr; > + [...]
On Mon, Jul 06, 2020 at 06:26:28PM -0700, Andrii Nakryiko wrote: > On Fri, Jul 3, 2020 at 2:54 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > Adding resolve_btfids test under test_progs suite. > > > > It's possible to use btf_ids.h header and its logic in > > user space application, so we can add easy test for it. > > > > The test defines BTF_ID_LIST and checks it gets properly > > resolved. > > > > For this reason the test_progs binary (and other binaries > > that use TRUNNER* macros) is processed with resolve_btfids > > tool, which resolves BTF IDs in .BTF.ids section. > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > tools/testing/selftests/bpf/Makefile | 22 ++- > > .../selftests/bpf/prog_tests/resolve_btfids.c | 170 ++++++++++++++++++ > > 2 files changed, 190 insertions(+), 2 deletions(-) > > create mode 100644 tools/testing/selftests/bpf/prog_tests/resolve_btfids.c > > > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > > index 1f9c696b3edf..b47a685d12bd 100644 > > --- a/tools/testing/selftests/bpf/Makefile > > +++ b/tools/testing/selftests/bpf/Makefile > > @@ -190,6 +190,16 @@ else > > cp "$(VMLINUX_H)" $@ > > endif > > > > +$(SCRATCH_DIR)/resolve_btfids: $(BPFOBJ) \ > > + $(TOOLSDIR)/bpf/resolve_btfids/main.c \ > > + $(TOOLSDIR)/lib/rbtree.c \ > > + $(TOOLSDIR)/lib/zalloc.c \ > > + $(TOOLSDIR)/lib/string.c \ > > + $(TOOLSDIR)/lib/ctype.c \ > > + $(TOOLSDIR)/lib/str_error_r.c > > + $(Q)$(MAKE) $(submake_extras) -C $(TOOLSDIR)/bpf/resolve_btfids \ > > + OUTPUT=$(SCRATCH_DIR)/ BPFOBJ=$(BPFOBJ) > > + > > please indent OUTPUT, so it doesn't look like it's a separate command ok > > > # Get Clang's default includes on this system, as opposed to those seen by > > # '-target bpf'. This fixes "missing" files on some architectures/distros, > > # such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc. > > @@ -333,7 +343,8 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \ > > $(TRUNNER_BPF_SKELS) \ > > $$(BPFOBJ) | $(TRUNNER_OUTPUT) > > $$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@) > > - cd $$(@D) && $$(CC) -I. $$(CFLAGS) -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F) > > + cd $$(@D) && $$(CC) -I. $$(CFLAGS) $(TRUNNER_EXTRA_CFLAGS) \ > > + -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F) > > > > $(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \ > > %.c \ > > @@ -355,6 +366,7 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \ > > | $(TRUNNER_BINARY)-extras > > $$(call msg,BINARY,,$$@) > > $$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) -o $$@ > > + $(TRUNNER_BINARY_EXTRA_CMD) > > no need to make this generic, just write out resolve_btfids here explicitly currently resolve_btfids fails if there's no .BTF.ids section found, but we can make it silently pass i nthis case and then we can invoke it for all the binaries > > > > > endef > > > > @@ -365,7 +377,10 @@ TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \ > > network_helpers.c testing_helpers.c \ > > flow_dissector_load.h > > TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \ > > - $(wildcard progs/btf_dump_test_case_*.c) > > + $(wildcard progs/btf_dump_test_case_*.c) \ > > + $(SCRATCH_DIR)/resolve_btfids > > +TRUNNER_EXTRA_CFLAGS := -D"BUILD_STR(s)=\#s" -DVMLINUX_BTF="BUILD_STR($(VMLINUX_BTF))" > > +TRUNNER_BINARY_EXTRA_CMD := $(SCRATCH_DIR)/resolve_btfids --btf $(VMLINUX_BTF) test_progs > > I hope we can get rid of this, see suggestion below. > > > TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE > > TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) > > TRUNNER_BPF_LDFLAGS := -mattr=+alu32 > > @@ -373,6 +388,7 @@ $(eval $(call DEFINE_TEST_RUNNER,test_progs)) > > > > [...] > > > + > > +static int duration; > > + > > +static struct btf *btf__parse_raw(const char *file) > > another copy here... ok > > > +{ > > + struct btf *btf; > > + struct stat st; > > + __u8 *buf; > > + FILE *f; > > + > > [...] > > > + > > +BTF_ID_LIST(test_list) > > +BTF_ID_UNUSED > > +BTF_ID(typedef, pid_t) > > +BTF_ID(struct, sk_buff) > > +BTF_ID(union, thread_union) > > +BTF_ID(func, memcpy) > > + > > +struct symbol { > > + const char *name; > > + int type; > > + int id; > > +}; > > + > > +struct symbol test_symbols[] = { > > + { "unused", -1, 0 }, > > could use BTF_KIND_UNKN here instead of -1 ok > > > + { "pid_t", BTF_KIND_TYPEDEF, -1 }, > > + { "sk_buff", BTF_KIND_STRUCT, -1 }, > > + { "thread_union", BTF_KIND_UNION, -1 }, > > + { "memcpy", BTF_KIND_FUNC, -1 }, > > +}; > > + > > [...] > > > + > > +static int resolve_symbols(void) > > +{ > > + const char *path = VMLINUX_BTF; > > > This build-time parameter passing to find the original VMLINUX_BTF > really sucks, IMO. > > Why not use the btf_dump tests approach and have our own small > "vmlinux BTF", which resolve_btfids would use to resolve these IDs? > See how btf_dump_xxx.c files define BTFs that are used in tests. You > can do something similar here, and use a well-known BPF object file as > a source of BTF, both here in a test and in Makefile for --btf param > to resolve_btfids? well VMLINUX_BTF is there and those types are used are not going away any time soon ;-) but yea, we can do that.. we do this also for bpftrace, it's nicer jirka
On Tue, Jul 7, 2020 at 8:57 AM Jiri Olsa <jolsa@redhat.com> wrote: > > On Mon, Jul 06, 2020 at 06:26:28PM -0700, Andrii Nakryiko wrote: > > On Fri, Jul 3, 2020 at 2:54 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > Adding resolve_btfids test under test_progs suite. > > > > > > It's possible to use btf_ids.h header and its logic in > > > user space application, so we can add easy test for it. > > > > > > The test defines BTF_ID_LIST and checks it gets properly > > > resolved. > > > > > > For this reason the test_progs binary (and other binaries > > > that use TRUNNER* macros) is processed with resolve_btfids > > > tool, which resolves BTF IDs in .BTF.ids section. > > > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > --- > > > tools/testing/selftests/bpf/Makefile | 22 ++- > > > .../selftests/bpf/prog_tests/resolve_btfids.c | 170 ++++++++++++++++++ > > > 2 files changed, 190 insertions(+), 2 deletions(-) > > > create mode 100644 tools/testing/selftests/bpf/prog_tests/resolve_btfids.c > > > > > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > > > index 1f9c696b3edf..b47a685d12bd 100644 > > > --- a/tools/testing/selftests/bpf/Makefile > > > +++ b/tools/testing/selftests/bpf/Makefile > > > @@ -190,6 +190,16 @@ else > > > cp "$(VMLINUX_H)" $@ > > > endif > > > > > > +$(SCRATCH_DIR)/resolve_btfids: $(BPFOBJ) \ > > > + $(TOOLSDIR)/bpf/resolve_btfids/main.c \ > > > + $(TOOLSDIR)/lib/rbtree.c \ > > > + $(TOOLSDIR)/lib/zalloc.c \ > > > + $(TOOLSDIR)/lib/string.c \ > > > + $(TOOLSDIR)/lib/ctype.c \ > > > + $(TOOLSDIR)/lib/str_error_r.c > > > + $(Q)$(MAKE) $(submake_extras) -C $(TOOLSDIR)/bpf/resolve_btfids \ > > > + OUTPUT=$(SCRATCH_DIR)/ BPFOBJ=$(BPFOBJ) > > > + > > > > please indent OUTPUT, so it doesn't look like it's a separate command > > ok > > > > > > # Get Clang's default includes on this system, as opposed to those seen by > > > # '-target bpf'. This fixes "missing" files on some architectures/distros, > > > # such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc. > > > @@ -333,7 +343,8 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \ > > > $(TRUNNER_BPF_SKELS) \ > > > $$(BPFOBJ) | $(TRUNNER_OUTPUT) > > > $$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@) > > > - cd $$(@D) && $$(CC) -I. $$(CFLAGS) -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F) > > > + cd $$(@D) && $$(CC) -I. $$(CFLAGS) $(TRUNNER_EXTRA_CFLAGS) \ > > > + -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F) > > > > > > $(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \ > > > %.c \ > > > @@ -355,6 +366,7 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \ > > > | $(TRUNNER_BINARY)-extras > > > $$(call msg,BINARY,,$$@) > > > $$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) -o $$@ > > > + $(TRUNNER_BINARY_EXTRA_CMD) > > > > no need to make this generic, just write out resolve_btfids here explicitly > > currently resolve_btfids fails if there's no .BTF.ids section found, > but we can make it silently pass i nthis case and then we can invoke > it for all the binaries ah, I see. Yeah, either we can add an option to resolve_btfids to not error when .BTF_ids is missing (probably best), or we can check whether the test has .BTF_ids section, and if it does - run resolve_btfids on it. Just ignoring errors always is more error-prone, because we won't know if it's a real problem we are ignoring, or missing .BTF_ids. > > > > > > > > > endef > > > > > > @@ -365,7 +377,10 @@ TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \ > > > network_helpers.c testing_helpers.c \ > > > flow_dissector_load.h > > > TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \ > > > - $(wildcard progs/btf_dump_test_case_*.c) > > > + $(wildcard progs/btf_dump_test_case_*.c) \ > > > + $(SCRATCH_DIR)/resolve_btfids > > > +TRUNNER_EXTRA_CFLAGS := -D"BUILD_STR(s)=\#s" -DVMLINUX_BTF="BUILD_STR($(VMLINUX_BTF))" > > > +TRUNNER_BINARY_EXTRA_CMD := $(SCRATCH_DIR)/resolve_btfids --btf $(VMLINUX_BTF) test_progs > > > > I hope we can get rid of this, see suggestion below. > > > > > TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE > > > TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) > > > TRUNNER_BPF_LDFLAGS := -mattr=+alu32 > > > @@ -373,6 +388,7 @@ $(eval $(call DEFINE_TEST_RUNNER,test_progs)) > > > > > > > [...] > > > > > + > > > +static int duration; > > > + > > > +static struct btf *btf__parse_raw(const char *file) > > > > another copy here... > > ok > > > > > > +{ > > > + struct btf *btf; > > > + struct stat st; > > > + __u8 *buf; > > > + FILE *f; > > > + > > > > [...] > > > > > + > > > +BTF_ID_LIST(test_list) > > > +BTF_ID_UNUSED > > > +BTF_ID(typedef, pid_t) > > > +BTF_ID(struct, sk_buff) > > > +BTF_ID(union, thread_union) > > > +BTF_ID(func, memcpy) > > > + > > > +struct symbol { > > > + const char *name; > > > + int type; > > > + int id; > > > +}; > > > + > > > +struct symbol test_symbols[] = { > > > + { "unused", -1, 0 }, > > > > could use BTF_KIND_UNKN here instead of -1 > > ok > > > > > > + { "pid_t", BTF_KIND_TYPEDEF, -1 }, > > > + { "sk_buff", BTF_KIND_STRUCT, -1 }, > > > + { "thread_union", BTF_KIND_UNION, -1 }, > > > + { "memcpy", BTF_KIND_FUNC, -1 }, > > > +}; > > > + > > > > [...] > > > > > + > > > +static int resolve_symbols(void) > > > +{ > > > + const char *path = VMLINUX_BTF; > > > > > > This build-time parameter passing to find the original VMLINUX_BTF > > really sucks, IMO. > > > > Why not use the btf_dump tests approach and have our own small > > "vmlinux BTF", which resolve_btfids would use to resolve these IDs? > > See how btf_dump_xxx.c files define BTFs that are used in tests. You > > can do something similar here, and use a well-known BPF object file as > > a source of BTF, both here in a test and in Makefile for --btf param > > to resolve_btfids? > > well VMLINUX_BTF is there and those types are used are not going > away any time soon ;-) but yea, we can do that.. we do this also > for bpftrace, it's nicer "VMLINUX_BTF is there" is not really true in a lot of more complicated setups, which is why I'd like to avoid that assumption. E.g., for libbpf Travis CI, we build self-tests in one VM, but run the binary in a different VM. So either vmlinux itself or the path to it might change. Also, having full control over **small** BTF allows to create various test situations that might be harder to pinpoint in real vmlinux BTF, e.g., same-named entities with different KINDS (typedef vs struct, etc). Then if that fails, debugging this on a small BTF is much-much easier than on a real thing. Real vmlinux BTF is being tested each time you build a kernel and run selftests inside VM either way, so I don't think we lose anything in terms of coverage. > > jirka >
On Tue, Jul 07, 2020 at 10:49:22AM -0700, Andrii Nakryiko wrote: SNIP > > > > # Get Clang's default includes on this system, as opposed to those seen by > > > > # '-target bpf'. This fixes "missing" files on some architectures/distros, > > > > # such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc. > > > > @@ -333,7 +343,8 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \ > > > > $(TRUNNER_BPF_SKELS) \ > > > > $$(BPFOBJ) | $(TRUNNER_OUTPUT) > > > > $$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@) > > > > - cd $$(@D) && $$(CC) -I. $$(CFLAGS) -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F) > > > > + cd $$(@D) && $$(CC) -I. $$(CFLAGS) $(TRUNNER_EXTRA_CFLAGS) \ > > > > + -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F) > > > > > > > > $(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \ > > > > %.c \ > > > > @@ -355,6 +366,7 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \ > > > > | $(TRUNNER_BINARY)-extras > > > > $$(call msg,BINARY,,$$@) > > > > $$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) -o $$@ > > > > + $(TRUNNER_BINARY_EXTRA_CMD) > > > > > > no need to make this generic, just write out resolve_btfids here explicitly > > > > currently resolve_btfids fails if there's no .BTF.ids section found, > > but we can make it silently pass i nthis case and then we can invoke > > it for all the binaries > > ah, I see. Yeah, either we can add an option to resolve_btfids to not > error when .BTF_ids is missing (probably best), or we can check > whether the test has .BTF_ids section, and if it does - run > resolve_btfids on it. Just ignoring errors always is more error-prone, > because we won't know if it's a real problem we are ignoring, or > missing .BTF_ids. ok, sounds good > > > > +static int resolve_symbols(void) > > > > +{ > > > > + const char *path = VMLINUX_BTF; > > > > > > > > > This build-time parameter passing to find the original VMLINUX_BTF > > > really sucks, IMO. > > > > > > Why not use the btf_dump tests approach and have our own small > > > "vmlinux BTF", which resolve_btfids would use to resolve these IDs? > > > See how btf_dump_xxx.c files define BTFs that are used in tests. You > > > can do something similar here, and use a well-known BPF object file as > > > a source of BTF, both here in a test and in Makefile for --btf param > > > to resolve_btfids? > > > > well VMLINUX_BTF is there and those types are used are not going > > away any time soon ;-) but yea, we can do that.. we do this also > > for bpftrace, it's nicer > > > "VMLINUX_BTF is there" is not really true in a lot of more complicated > setups, which is why I'd like to avoid that assumption. E.g., for > libbpf Travis CI, we build self-tests in one VM, but run the binary in > a different VM. So either vmlinux itself or the path to it might > change. ok > > Also, having full control over **small** BTF allows to create various > test situations that might be harder to pinpoint in real vmlinux BTF, > e.g., same-named entities with different KINDS (typedef vs struct, > etc). Then if that fails, debugging this on a small BTF is much-much > easier than on a real thing. Real vmlinux BTF is being tested each > time you build a kernel and run selftests inside VM either way, so I > don't think we lose anything in terms of coverage. agreed, will add that thanks, jirka
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 1f9c696b3edf..b47a685d12bd 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -190,6 +190,16 @@ else cp "$(VMLINUX_H)" $@ endif +$(SCRATCH_DIR)/resolve_btfids: $(BPFOBJ) \ + $(TOOLSDIR)/bpf/resolve_btfids/main.c \ + $(TOOLSDIR)/lib/rbtree.c \ + $(TOOLSDIR)/lib/zalloc.c \ + $(TOOLSDIR)/lib/string.c \ + $(TOOLSDIR)/lib/ctype.c \ + $(TOOLSDIR)/lib/str_error_r.c + $(Q)$(MAKE) $(submake_extras) -C $(TOOLSDIR)/bpf/resolve_btfids \ + OUTPUT=$(SCRATCH_DIR)/ BPFOBJ=$(BPFOBJ) + # Get Clang's default includes on this system, as opposed to those seen by # '-target bpf'. This fixes "missing" files on some architectures/distros, # such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc. @@ -333,7 +343,8 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \ $(TRUNNER_BPF_SKELS) \ $$(BPFOBJ) | $(TRUNNER_OUTPUT) $$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@) - cd $$(@D) && $$(CC) -I. $$(CFLAGS) -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F) + cd $$(@D) && $$(CC) -I. $$(CFLAGS) $(TRUNNER_EXTRA_CFLAGS) \ + -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F) $(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \ %.c \ @@ -355,6 +366,7 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \ | $(TRUNNER_BINARY)-extras $$(call msg,BINARY,,$$@) $$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) -o $$@ + $(TRUNNER_BINARY_EXTRA_CMD) endef @@ -365,7 +377,10 @@ TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \ network_helpers.c testing_helpers.c \ flow_dissector_load.h TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \ - $(wildcard progs/btf_dump_test_case_*.c) + $(wildcard progs/btf_dump_test_case_*.c) \ + $(SCRATCH_DIR)/resolve_btfids +TRUNNER_EXTRA_CFLAGS := -D"BUILD_STR(s)=\#s" -DVMLINUX_BTF="BUILD_STR($(VMLINUX_BTF))" +TRUNNER_BINARY_EXTRA_CMD := $(SCRATCH_DIR)/resolve_btfids --btf $(VMLINUX_BTF) test_progs TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) TRUNNER_BPF_LDFLAGS := -mattr=+alu32 @@ -373,6 +388,7 @@ $(eval $(call DEFINE_TEST_RUNNER,test_progs)) # Define test_progs-no_alu32 test runner. TRUNNER_BPF_BUILD_RULE := CLANG_NOALU32_BPF_BUILD_RULE +TRUNNER_BINARY_EXTRA_CMD := $(SCRATCH_DIR)/resolve_btfids --btf $(VMLINUX_BTF) test_progs-no_alu32 TRUNNER_BPF_LDFLAGS := $(eval $(call DEFINE_TEST_RUNNER,test_progs,no_alu32)) @@ -392,6 +408,8 @@ TRUNNER_EXTRA_FILES := TRUNNER_BPF_BUILD_RULE := $$(error no BPF objects should be built) TRUNNER_BPF_CFLAGS := TRUNNER_BPF_LDFLAGS := +TRUNNER_EXTRA_CFLAGS := +TRUNNER_BINARY_EXTRA_CMD := $(eval $(call DEFINE_TEST_RUNNER,test_maps)) # Define test_verifier test runner. diff --git a/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c new file mode 100644 index 000000000000..6b7b5f736181 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c @@ -0,0 +1,170 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <string.h> +#include <stdio.h> +#include <sys/stat.h> +#include <stdio.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <unistd.h> +#include <linux/err.h> +#include <stdlib.h> +#include <bpf/btf.h> +#include <bpf/libbpf.h> +#include <linux/btf.h> +#include <linux/kernel.h> +#include <linux/btf_ids.h> +#include "test_progs.h" + +static int duration; + +static struct btf *btf__parse_raw(const char *file) +{ + struct btf *btf; + struct stat st; + __u8 *buf; + FILE *f; + + if (stat(file, &st)) + return NULL; + + f = fopen(file, "rb"); + if (!f) + return NULL; + + buf = malloc(st.st_size); + if (!buf) { + btf = ERR_PTR(-ENOMEM); + goto exit_close; + } + + if ((size_t) st.st_size != fread(buf, 1, st.st_size, f)) { + btf = ERR_PTR(-EINVAL); + goto exit_free; + } + + btf = btf__new(buf, st.st_size); + +exit_free: + free(buf); +exit_close: + fclose(f); + return btf; +} + +static bool is_btf_raw(const char *file) +{ + __u16 magic = 0; + int fd, nb_read; + + fd = open(file, O_RDONLY); + if (fd < 0) + return false; + + nb_read = read(fd, &magic, sizeof(magic)); + close(fd); + return nb_read == sizeof(magic) && magic == BTF_MAGIC; +} + +static struct btf *btf_open(const char *path) +{ + if (is_btf_raw(path)) + return btf__parse_raw(path); + else + return btf__parse_elf(path, NULL); +} + +BTF_ID_LIST(test_list) +BTF_ID_UNUSED +BTF_ID(typedef, pid_t) +BTF_ID(struct, sk_buff) +BTF_ID(union, thread_union) +BTF_ID(func, memcpy) + +struct symbol { + const char *name; + int type; + int id; +}; + +struct symbol test_symbols[] = { + { "unused", -1, 0 }, + { "pid_t", BTF_KIND_TYPEDEF, -1 }, + { "sk_buff", BTF_KIND_STRUCT, -1 }, + { "thread_union", BTF_KIND_UNION, -1 }, + { "memcpy", BTF_KIND_FUNC, -1 }, +}; + +static int +__resolve_symbol(struct btf *btf, int type_id) +{ + const struct btf_type *type; + const char *str; + unsigned int i; + + type = btf__type_by_id(btf, type_id); + if (!type) { + PRINT_FAIL("Failed to get type for ID %d\n", type_id); + return -1; + } + + for (i = 0; i < ARRAY_SIZE(test_symbols); i++) { + if (test_symbols[i].id != -1) + continue; + + if (BTF_INFO_KIND(type->info) != test_symbols[i].type) + continue; + + str = btf__name_by_offset(btf, type->name_off); + if (!str) { + PRINT_FAIL("Failed to get name for BTF ID %d\n", type_id); + return -1; + } + + if (!strcmp(str, test_symbols[i].name)) + test_symbols[i].id = type_id; + } + + return 0; +} + +static int resolve_symbols(void) +{ + const char *path = VMLINUX_BTF; + struct btf *btf; + int type_id; + __u32 nr; + + btf = btf_open(path); + if (CHECK(libbpf_get_error(btf), "resolve", + "Failed to load BTF from %s\n", path)) + return -1; + + nr = btf__get_nr_types(btf); + + for (type_id = 1; type_id <= nr; type_id++) { + if (__resolve_symbol(btf, type_id)) + break; + } + + btf__free(btf); + return 0; +} + +int test_resolve_btfids(void) +{ + unsigned int i; + int ret = 0; + + if (resolve_symbols()) + return -1; + + /* Check BTF_ID_LIST(test_list) IDs */ + for (i = 0; i < ARRAY_SIZE(test_symbols) && !ret; i++) { + ret = CHECK(test_list[i] != test_symbols[i].id, + "id_check", + "wrong ID for %s (%d != %d)\n", test_symbols[i].name, + test_list[i], test_symbols[i].id); + } + + return 0; +}
Adding resolve_btfids test under test_progs suite. It's possible to use btf_ids.h header and its logic in user space application, so we can add easy test for it. The test defines BTF_ID_LIST and checks it gets properly resolved. For this reason the test_progs binary (and other binaries that use TRUNNER* macros) is processed with resolve_btfids tool, which resolves BTF IDs in .BTF.ids section. Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- tools/testing/selftests/bpf/Makefile | 22 ++- .../selftests/bpf/prog_tests/resolve_btfids.c | 170 ++++++++++++++++++ 2 files changed, 190 insertions(+), 2 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/resolve_btfids.c