netdev - Re: [PATCH v2 bpf-next] selftests/bpf: merge most of test_btf into test

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzaQdbR8BMRTOJpQtNc2oe77F9Uu2=EmJXXP5fQjXY414w@mail.gmail.com>
Date:   Tue, 15 Sep 2020 17:14:16 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>, Yonghong Song <yhs@...com>,
        Martin Lau <kafai@...com>
Subject: Re: [PATCH v2 bpf-next] selftests/bpf: merge most of test_btf into test_progs

On Tue, Sep 15, 2020 at 4:37 PM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Mon, Sep 14, 2020 at 06:43:41PM -0700, Andrii Nakryiko wrote:
> > Move almost 200 tests from test_btf into test_progs framework to be exercised
> > regularly. Pretty-printing tests were left alone and renamed into
> > test_btf_pprint because they are very slow and were not even executed by
> > default with test_btf.
>
> I think would be good to run them by default.
> The following trivial tweak makes them fast:

Sounds good, I'll debug why it was failing now that I can run it reasonably.

> diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
> index c75fc6447186..589afd4f0e47 100644
> --- a/tools/testing/selftests/bpf/test_btf.c
> +++ b/tools/testing/selftests/bpf/test_btf.c
> @@ -4428,7 +4428,7 @@ static struct btf_raw_test pprint_test_template[] = {
>         .value_size = sizeof(struct pprint_mapv),
>         .key_type_id = 3,       /* unsigned int */
>         .value_type_id = 16,    /* struct pprint_mapv */
> -       .max_entries = 128 * 1024,
> +       .max_entries = 128,
>  },
>
>  {
> @@ -4493,7 +4493,7 @@ static struct btf_raw_test pprint_test_template[] = {
>         .value_size = sizeof(struct pprint_mapv),
>         .key_type_id = 3,       /* unsigned int */
>         .value_type_id = 16,    /* struct pprint_mapv */
> -       .max_entries = 128 * 1024,
> +       .max_entries = 128,
>  },
>
>  {
> @@ -4564,7 +4564,7 @@ static struct btf_raw_test pprint_test_template[] = {
>         .value_size = sizeof(struct pprint_mapv),
>         .key_type_id = 3,       /* unsigned int */
>         .value_type_id = 16,    /* struct pprint_mapv */
> -       .max_entries = 128 * 1024,
> +       .max_entries = 128,
>  },
>
> Martin,
> do you remember why you picked such large numbers of entries
> for the test?
> If I read the code correctly smaller number doesn't reduce the test coverage.
>
> > All the test_btf tests that were moved are modeled as proper sub-tests in
> > test_progs framework for ease of debugging and reporting.
> >
> > No functional or behavioral changes were intended, I tried to preserve
> > original behavior as close to the original as possible. `test_progs -v` will
> > activate "always_log" flag to emit BTF validation log.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@...com>
> > ---
> >
> > v1->v2:
> >  - pretty-print BTF tests were renamed test_btf -> test_btf_pprint, which
> >    allowed GIT to detect that majority of  test_btf code was moved into
> >    prog_tests/btf.c; so diff is much-much smaller;
>
> Thanks. I hope with addition to pprint test the diff will be even smaller.
> I think it's worth to investigate why they're failing if moved to test_progs.
> I think they're the only tests that exercise seq_read logic.
> Clearly the bug:
> [   25.960993] WARNING: CPU: 2 PID: 1995 at kernel/bpf/hashtab.c:717 htab_map_get_next_key+0x7fc/0xab0
> is still there.
> If pprint tests were part of test_progs we would have caught that earlier.

yep, with pretty-printing tests in test_btf diff should be very small

>
> Yonghong,
> please take a look at that issue.