On Tue, Sep 15, 2020 at 04:37:50PM -0700, Alexei Starovoitov 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:
> 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?
It has been a while. iirc, I think there was no particular reason but
there was only one pprint test and then a few had been added since
then.
> If I read the code correctly smaller number doesn't reduce the test coverage.
Indeed. Please go ahead to reduce the max_entries. Sorry for the pain.
>
> > 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@xxxxxx>
> > ---
> >
> > 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.
