On Sun, May 29, 2022 at 3:37 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> The BTF and func_info specification for test_verifier tests follows
> the same notation as in prog_tests/btf.c tests. E.g.:
>
> ...
> .func_info = { { 0, 6 }, { 8, 7 } },
> .func_info_cnt = 2,
> .btf_strings = "\0int\0",
> .btf_types = {
> BTF_TYPE_INT_ENC(1, BTF_INT_SIGNED, 0, 32, 4),
> BTF_PTR_ENC(1),
> },
> ...
>
> The BTF specification is loaded only when specified.
>
> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
This can be very useful! Thanks!
> ---
> tools/testing/selftests/bpf/prog_tests/btf.c | 1 -
> tools/testing/selftests/bpf/test_btf.h | 2 +
> tools/testing/selftests/bpf/test_verifier.c | 110 ++++++++++++++++---
> 3 files changed, 95 insertions(+), 18 deletions(-)
[...]
>
> -static int load_btf(void)
> +static char bpf_vlog[UINT_MAX >> 8];
> +
> +static int load_btf_spec(__u32 *types, int types_len,
> + const char *strings, int strings_len)
> {
> struct btf_header hdr = {
> .magic = BTF_MAGIC,
> .version = BTF_VERSION,
> .hdr_len = sizeof(struct btf_header),
> - .type_len = sizeof(btf_raw_types),
> - .str_off = sizeof(btf_raw_types),
> - .str_len = sizeof(btf_str_sec),
> + .type_len = types_len,
> + .str_off = types_len,
> + .str_len = strings_len,
> };
> void *ptr, *raw_btf;
> int btf_fd;
>
> - ptr = raw_btf = malloc(sizeof(hdr) + sizeof(btf_raw_types) +
> - sizeof(btf_str_sec));
> + raw_btf = malloc(sizeof(hdr) + types_len + strings_len);
>
> + ptr = raw_btf;
> memcpy(ptr, &hdr, sizeof(hdr));
> ptr += sizeof(hdr);
> - memcpy(ptr, btf_raw_types, hdr.type_len);
> + memcpy(ptr, types, hdr.type_len);
> ptr += hdr.type_len;
> - memcpy(ptr, btf_str_sec, hdr.str_len);
> + memcpy(ptr, strings, hdr.str_len);
> ptr += hdr.str_len;
>
> - btf_fd = bpf_btf_load(raw_btf, ptr - raw_btf, NULL);
> - free(raw_btf);
> + LIBBPF_OPTS(bpf_btf_load_opts, opts,
> + .log_buf = bpf_vlog,
> + .log_size = sizeof(bpf_vlog),
> + .log_level = (verbose
> + ? VERBOSE_LIBBPF_LOG_LEVEL
> + : DEFAULT_LIBBPF_LOG_LEVEL),
> + );
Please move the local variable definition to the beginning of the function.
> +
> + btf_fd = bpf_btf_load(raw_btf, ptr - raw_btf, &opts);
> if (btf_fd < 0)
> - return -1;
> - return btf_fd;
> + printf("Failed to load BTF spec: '%s'\n", strerror(errno));
> +
> + free(raw_btf);
> +
> + return btf_fd < 0 ? -1 : btf_fd;
> +}
> +
[...]
> +
> static void do_test_single(struct bpf_test *test, bool unpriv,
> int *passes, int *errors)
> {
> - int fd_prog, expected_ret, alignment_prevented_execution;
> + int fd_prog, btf_fd, expected_ret, alignment_prevented_execution;
> int prog_len, prog_type = test->prog_type;
> struct bpf_insn *prog = test->insns;
> LIBBPF_OPTS(bpf_prog_load_opts, opts);
> @@ -1366,8 +1424,10 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
> __u32 pflags;
> int i, err;
>
> + fd_prog = -1;
This is not really necessary.
> for (i = 0; i < MAX_NR_MAPS; i++)
> map_fds[i] = -1;
> + btf_fd = -1;
>
> if (!prog_type)
> prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
[...]
> @@ -1540,6 +1614,8 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
> fail_log:
> (*errors)++;
> printf("%s", bpf_vlog);
> + if (!string_ends_with_nl(bpf_vlog, sizeof(bpf_vlog)))
> + printf("\n");
This seems like overkill. Can we just print "\n" in all cases?
> goto close_fds;
> }
>
> --
> 2.25.1
>
