* [PATCH bpf-next 0/3] Fix BTF-to-C converter's padding generation @ 2019-10-08 23:10 Andrii Nakryiko 2019-10-08 23:10 ` [PATCH bpf-next 1/3] libbpf: fix struct end padding in btf_dump Andrii Nakryiko ` (3 more replies) 0 siblings, 4 replies; 7+ messages in thread From: Andrii Nakryiko @ 2019-10-08 23:10 UTC (permalink / raw) To: bpf, netdev, ast, daniel, john.fastabend Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko Fix BTF-to-C logic of handling padding at the end of a struct. Fix existing test that should have captured this. Also move test_btf_dump into a test_progs test to leverage common infrastructure. Andrii Nakryiko (3): libbpf: fix struct end padding in btf_dump selftests/bpf: convert test_btf_dump into test_progs test selftests/bpf: fix btf_dump padding test case tools/lib/bpf/btf_dump.c | 8 +- tools/testing/selftests/bpf/Makefile | 2 +- .../btf_dump.c} | 88 +++++++------------ .../bpf/progs/btf_dump_test_case_padding.c | 5 +- 4 files changed, 46 insertions(+), 57 deletions(-) rename tools/testing/selftests/bpf/{test_btf_dump.c => prog_tests/btf_dump.c} (51%) -- 2.17.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf-next 1/3] libbpf: fix struct end padding in btf_dump 2019-10-08 23:10 [PATCH bpf-next 0/3] Fix BTF-to-C converter's padding generation Andrii Nakryiko @ 2019-10-08 23:10 ` Andrii Nakryiko 2019-10-09 4:11 ` John Fastabend 2019-10-08 23:10 ` [PATCH bpf-next 2/3] selftests/bpf: convert test_btf_dump into test_progs test Andrii Nakryiko ` (2 subsequent siblings) 3 siblings, 1 reply; 7+ messages in thread From: Andrii Nakryiko @ 2019-10-08 23:10 UTC (permalink / raw) To: bpf, netdev, ast, daniel, john.fastabend Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko Fix a case where explicit padding at the end of a struct is necessary due to non-standart alignment requirements of fields (which BTF doesn't capture explicitly). Fixes: 351131b51c7a ("libbpf: add btf_dump API for BTF-to-C conversion") Reported-by: John Fastabend <john.fastabend@gmail.com> Signed-off-by: Andrii Nakryiko <andriin@fb.com> --- tools/lib/bpf/btf_dump.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c index ede55fec3618..87f27e2664c5 100644 --- a/tools/lib/bpf/btf_dump.c +++ b/tools/lib/bpf/btf_dump.c @@ -876,7 +876,6 @@ static void btf_dump_emit_struct_def(struct btf_dump *d, __u16 vlen = btf_vlen(t); packed = is_struct ? btf_is_struct_packed(d->btf, id, t) : 0; - align = packed ? 1 : btf_align_of(d->btf, id); btf_dump_printf(d, "%s%s%s {", is_struct ? "struct" : "union", @@ -906,6 +905,13 @@ static void btf_dump_emit_struct_def(struct btf_dump *d, btf_dump_printf(d, ";"); } + /* pad at the end, if necessary */ + if (is_struct) { + align = packed ? 1 : btf_align_of(d->btf, id); + btf_dump_emit_bit_padding(d, off, t->size * 8, 0, align, + lvl + 1); + } + if (vlen) btf_dump_printf(d, "\n"); btf_dump_printf(d, "%s}", pfx(lvl)); -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH bpf-next 1/3] libbpf: fix struct end padding in btf_dump 2019-10-08 23:10 ` [PATCH bpf-next 1/3] libbpf: fix struct end padding in btf_dump Andrii Nakryiko @ 2019-10-09 4:11 ` John Fastabend 0 siblings, 0 replies; 7+ messages in thread From: John Fastabend @ 2019-10-09 4:11 UTC (permalink / raw) To: Andrii Nakryiko, bpf, netdev, ast, daniel, john.fastabend Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko Andrii Nakryiko wrote: > Fix a case where explicit padding at the end of a struct is necessary > due to non-standart alignment requirements of fields (which BTF doesn't > capture explicitly). > > Fixes: 351131b51c7a ("libbpf: add btf_dump API for BTF-to-C conversion") > Reported-by: John Fastabend <john.fastabend@gmail.com> > Signed-off-by: Andrii Nakryiko <andriin@fb.com> > --- Tested a few different kernels and seems to have resolved the issue I was seeing. Tested-by: John Fastabend <john.fastabend@gmail.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf-next 2/3] selftests/bpf: convert test_btf_dump into test_progs test 2019-10-08 23:10 [PATCH bpf-next 0/3] Fix BTF-to-C converter's padding generation Andrii Nakryiko 2019-10-08 23:10 ` [PATCH bpf-next 1/3] libbpf: fix struct end padding in btf_dump Andrii Nakryiko @ 2019-10-08 23:10 ` Andrii Nakryiko 2019-10-08 23:10 ` [PATCH bpf-next 3/3] selftests/bpf: fix btf_dump padding test case Andrii Nakryiko 2019-10-09 22:48 ` [PATCH bpf-next 0/3] Fix BTF-to-C converter's padding generation Alexei Starovoitov 3 siblings, 0 replies; 7+ messages in thread From: Andrii Nakryiko @ 2019-10-08 23:10 UTC (permalink / raw) To: bpf, netdev, ast, daniel, john.fastabend Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko Convert test_btf_dump into a part of test_progs, instead of a stand-alone test binary. Signed-off-by: Andrii Nakryiko <andriin@fb.com> --- tools/testing/selftests/bpf/Makefile | 2 +- .../btf_dump.c} | 88 +++++++------------ 2 files changed, 35 insertions(+), 55 deletions(-) rename tools/testing/selftests/bpf/{test_btf_dump.c => prog_tests/btf_dump.c} (51%) diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 771a4e82128b..0de5d6eb62de 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -29,7 +29,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \ test_cgroup_storage test_select_reuseport test_section_names \ test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \ - test_btf_dump test_cgroup_attach xdping + test_cgroup_attach xdping BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c))) TEST_GEN_FILES = $(BPF_OBJ_FILES) diff --git a/tools/testing/selftests/bpf/test_btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c similarity index 51% rename from tools/testing/selftests/bpf/test_btf_dump.c rename to tools/testing/selftests/bpf/prog_tests/btf_dump.c index 6e75dd3cb14f..7390d3061065 100644 --- a/tools/testing/selftests/bpf/test_btf_dump.c +++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c @@ -1,36 +1,26 @@ -#include <stdio.h> -#include <stdlib.h> -#include <string.h> -#include <unistd.h> -#include <errno.h> -#include <linux/err.h> -#include <btf.h> - -#define CHECK(condition, format...) ({ \ - int __ret = !!(condition); \ - if (__ret) { \ - fprintf(stderr, "%s:%d:FAIL ", __func__, __LINE__); \ - fprintf(stderr, format); \ - } \ - __ret; \ -}) +// SPDX-License-Identifier: GPL-2.0 +#include <test_progs.h> +#include <bpf/btf.h> + +static int duration = 0; void btf_dump_printf(void *ctx, const char *fmt, va_list args) { vfprintf(ctx, fmt, args); } -struct btf_dump_test_case { +static struct btf_dump_test_case { const char *name; + const char *file; struct btf_dump_opts opts; } btf_dump_test_cases[] = { - {.name = "btf_dump_test_case_syntax", .opts = {}}, - {.name = "btf_dump_test_case_ordering", .opts = {}}, - {.name = "btf_dump_test_case_padding", .opts = {}}, - {.name = "btf_dump_test_case_packing", .opts = {}}, - {.name = "btf_dump_test_case_bitfields", .opts = {}}, - {.name = "btf_dump_test_case_multidim", .opts = {}}, - {.name = "btf_dump_test_case_namespacing", .opts = {}}, + {"btf_dump: syntax", "btf_dump_test_case_syntax", {}}, + {"btf_dump: ordering", "btf_dump_test_case_ordering", {}}, + {"btf_dump: padding", "btf_dump_test_case_padding", {}}, + {"btf_dump: packing", "btf_dump_test_case_packing", {}}, + {"btf_dump: bitfields", "btf_dump_test_case_bitfields", {}}, + {"btf_dump: multidim", "btf_dump_test_case_multidim", {}}, + {"btf_dump: namespacing", "btf_dump_test_case_namespacing", {}}, }; static int btf_dump_all_types(const struct btf *btf, @@ -55,55 +45,51 @@ static int btf_dump_all_types(const struct btf *btf, return err; } -int test_btf_dump_case(int n, struct btf_dump_test_case *test_case) +static int test_btf_dump_case(int n, struct btf_dump_test_case *t) { char test_file[256], out_file[256], diff_cmd[1024]; struct btf *btf = NULL; int err = 0, fd = -1; FILE *f = NULL; - fprintf(stderr, "Test case #%d (%s): ", n, test_case->name); - - snprintf(test_file, sizeof(test_file), "%s.o", test_case->name); + snprintf(test_file, sizeof(test_file), "%s.o", t->file); btf = btf__parse_elf(test_file, NULL); - if (CHECK(IS_ERR(btf), + if (CHECK(IS_ERR(btf), "btf_parse_elf", "failed to load test BTF: %ld\n", PTR_ERR(btf))) { err = -PTR_ERR(btf); btf = NULL; goto done; } - snprintf(out_file, sizeof(out_file), - "/tmp/%s.output.XXXXXX", test_case->name); + snprintf(out_file, sizeof(out_file), "/tmp/%s.output.XXXXXX", t->file); fd = mkstemp(out_file); - if (CHECK(fd < 0, "failed to create temp output file: %d\n", fd)) { + if (CHECK(fd < 0, "create_tmp", "failed to create file: %d\n", fd)) { err = fd; goto done; } f = fdopen(fd, "w"); - if (CHECK(f == NULL, "failed to open temp output file: %s(%d)\n", + if (CHECK(f == NULL, "open_tmp", "failed to open file: %s(%d)\n", strerror(errno), errno)) { close(fd); goto done; } - test_case->opts.ctx = f; - err = btf_dump_all_types(btf, &test_case->opts); + t->opts.ctx = f; + err = btf_dump_all_types(btf, &t->opts); fclose(f); close(fd); - if (CHECK(err, "failure during C dumping: %d\n", err)) { + if (CHECK(err, "btf_dump", "failure during C dumping: %d\n", err)) { goto done; } - snprintf(test_file, sizeof(test_file), "progs/%s.c", test_case->name); + snprintf(test_file, sizeof(test_file), "progs/%s.c", t->file); if (access(test_file, R_OK) == -1) /* * When the test is run with O=, kselftest copies TEST_FILES * without preserving the directory structure. */ - snprintf(test_file, sizeof(test_file), "%s.c", - test_case->name); + snprintf(test_file, sizeof(test_file), "%s.c", t->file); /* * Diff test output and expected test output, contained between * START-EXPECTED-OUTPUT and END-EXPECTED-OUTPUT lines in test case. @@ -118,33 +104,27 @@ int test_btf_dump_case(int n, struct btf_dump_test_case *test_case) "out {sub(/^[ \\t]*\\*/, \"\"); print}' '%s' | diff -u - '%s'", test_file, out_file); err = system(diff_cmd); - if (CHECK(err, + if (CHECK(err, "diff", "differing test output, output=%s, err=%d, diff cmd:\n%s\n", out_file, err, diff_cmd)) goto done; remove(out_file); - fprintf(stderr, "OK\n"); done: btf__free(btf); return err; } -int main() { - int test_case_cnt, i, err, failed = 0; - - test_case_cnt = sizeof(btf_dump_test_cases) / - sizeof(btf_dump_test_cases[0]); +void test_btf_dump() { + int i; - for (i = 0; i < test_case_cnt; i++) { - err = test_btf_dump_case(i, &btf_dump_test_cases[i]); - if (err) - failed++; - } + for (i = 0; i < ARRAY_SIZE(btf_dump_test_cases); i++) { + struct btf_dump_test_case *t = &btf_dump_test_cases[i]; - fprintf(stderr, "%d tests succeeded, %d tests failed.\n", - test_case_cnt - failed, failed); + if (!test__start_subtest(t->name)) + continue; - return failed; + test_btf_dump_case(i, &btf_dump_test_cases[i]); + } } -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf-next 3/3] selftests/bpf: fix btf_dump padding test case 2019-10-08 23:10 [PATCH bpf-next 0/3] Fix BTF-to-C converter's padding generation Andrii Nakryiko 2019-10-08 23:10 ` [PATCH bpf-next 1/3] libbpf: fix struct end padding in btf_dump Andrii Nakryiko 2019-10-08 23:10 ` [PATCH bpf-next 2/3] selftests/bpf: convert test_btf_dump into test_progs test Andrii Nakryiko @ 2019-10-08 23:10 ` Andrii Nakryiko 2019-10-09 22:48 ` [PATCH bpf-next 0/3] Fix BTF-to-C converter's padding generation Alexei Starovoitov 3 siblings, 0 replies; 7+ messages in thread From: Andrii Nakryiko @ 2019-10-08 23:10 UTC (permalink / raw) To: bpf, netdev, ast, daniel, john.fastabend Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko Existing padding test case for btf_dump has a good test that was supposed to test padding generation at the end of a struct, but its expected output was specified incorrectly. Fix this. Fixes: 2d2a3ad872f8 ("selftests/bpf: add btf_dump BTF-to-C conversion tests") Reported-by: John Fastabend <john.fastabend@gmail.com> Signed-off-by: Andrii Nakryiko <andriin@fb.com> --- .../testing/selftests/bpf/progs/btf_dump_test_case_padding.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c index 3a62119c7498..35c512818a56 100644 --- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c +++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c @@ -62,6 +62,10 @@ struct padded_a_lot { * long: 64; * long: 64; * int b; + * long: 32; + * long: 64; + * long: 64; + * long: 64; *}; * */ @@ -95,7 +99,6 @@ struct zone_padding { struct zone { int a; short b; - short: 16; struct zone_padding __pad__; }; -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next 0/3] Fix BTF-to-C converter's padding generation 2019-10-08 23:10 [PATCH bpf-next 0/3] Fix BTF-to-C converter's padding generation Andrii Nakryiko ` (2 preceding siblings ...) 2019-10-08 23:10 ` [PATCH bpf-next 3/3] selftests/bpf: fix btf_dump padding test case Andrii Nakryiko @ 2019-10-09 22:48 ` Alexei Starovoitov 2019-10-09 23:11 ` Andrii Nakryiko 3 siblings, 1 reply; 7+ messages in thread From: Alexei Starovoitov @ 2019-10-09 22:48 UTC (permalink / raw) To: Andrii Nakryiko Cc: bpf, Network Development, Alexei Starovoitov, Daniel Borkmann, John Fastabend, Andrii Nakryiko, Kernel Team On Tue, Oct 8, 2019 at 4:12 PM Andrii Nakryiko <andriin@fb.com> wrote: > > Fix BTF-to-C logic of handling padding at the end of a struct. Fix existing > test that should have captured this. Also move test_btf_dump into a test_progs > test to leverage common infrastructure. Applied. But I see some build weirdness. Probably due to some previous patches. Parallel build in selftests/bpf/ is not always succeeding. For this particular set. New progs/*.c files failed to build the first time. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next 0/3] Fix BTF-to-C converter's padding generation 2019-10-09 22:48 ` [PATCH bpf-next 0/3] Fix BTF-to-C converter's padding generation Alexei Starovoitov @ 2019-10-09 23:11 ` Andrii Nakryiko 0 siblings, 0 replies; 7+ messages in thread From: Andrii Nakryiko @ 2019-10-09 23:11 UTC (permalink / raw) To: Alexei Starovoitov Cc: Andrii Nakryiko, bpf, Network Development, Alexei Starovoitov, Daniel Borkmann, John Fastabend, Kernel Team On Wed, Oct 9, 2019 at 3:49 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Tue, Oct 8, 2019 at 4:12 PM Andrii Nakryiko <andriin@fb.com> wrote: > > > > Fix BTF-to-C logic of handling padding at the end of a struct. Fix existing > > test that should have captured this. Also move test_btf_dump into a test_progs > > test to leverage common infrastructure. > > Applied. > But I see some build weirdness. > Probably due to some previous patches. > Parallel build in selftests/bpf/ is not always succeeding. > For this particular set. New progs/*.c files failed to build the first time. I think it's another problem with test_attach_probe.c - it depends on libbpf's auto-generated bpf_helper_defs.h, which is not enforced to happen before progs/*.c are built. I think it's time to do some selftests/bpf/Makefile overhaul, honestly. But I'll try to come up with a quick fix for this particular issue as well. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-10-09 23:11 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-08 23:10 [PATCH bpf-next 0/3] Fix BTF-to-C converter's padding generation Andrii Nakryiko 2019-10-08 23:10 ` [PATCH bpf-next 1/3] libbpf: fix struct end padding in btf_dump Andrii Nakryiko 2019-10-09 4:11 ` John Fastabend 2019-10-08 23:10 ` [PATCH bpf-next 2/3] selftests/bpf: convert test_btf_dump into test_progs test Andrii Nakryiko 2019-10-08 23:10 ` [PATCH bpf-next 3/3] selftests/bpf: fix btf_dump padding test case Andrii Nakryiko 2019-10-09 22:48 ` [PATCH bpf-next 0/3] Fix BTF-to-C converter's padding generation Alexei Starovoitov 2019-10-09 23:11 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).