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 for Android: free password hash cracker in your pocket

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

Message-ID: <20200915233750.imml2qj6p72olga4@ast-mbp.dhcp.thefacebook.com>
Date:   Tue, 15 Sep 2020 16:37:50 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Andrii Nakryiko <andriin@...com>
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org, ast@...com,
        daniel@...earbox.net, andrii.nakryiko@...il.com,
        kernel-team@...com, yhs@...com, kafai@...com
Subject: Re: [PATCH v2 bpf-next] selftests/bpf: merge most of test_btf into
 test_progs

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:
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.

Yonghong,
please take a look at that issue.