Re: [PATCH bpf-next v5 9/9] selftest/bpf: Implement tests for bpftool gen min_core_btf

On Wed, Feb 2, 2022 at 2:50 PM Andrii Nakryiko
[off-list ref] wrote:

On Fri, Jan 28, 2022 at 3:23 PM Mauricio Vásquez Bernal
[off-list ref] wrote:

quoted

On Fri, Jan 28, 2022 at 5:33 PM Mauricio Vásquez [off-list ref] wrote:

quoted

This commit implements some integration tests for "BTFGen". The goal
of such tests is to verify that the generated BTF file contains the
expected types.

This is not an exhaustive list of test cases. I'm not sure if this is
the approach we should follow to implement such tests, it seems to me
that checking each generated BTF file by hand is a lot of work but I
don't have other ideas to simplify it.

I considered different options to write these tests:
1. Use core_reloc_types.h to create a "source" BTF file with a lot of
types, then run BTFGen for all test_core_reloc_*.o files and use the
generated BTF file as btf_src_file in core_reloc.c. In other words,
re-run all test_core_reloc tests using a generated BTF file as source
instead of the "btf__core_reloc_" #name ".o" one. I think this test is
great because it tests the full functionality and actually checks that
the programs are able to run using the generated file. The problem is
how do we test that the BTFGen is creating an optimized file? Just
copying the source file without any modification will make all those
tests pass. We could check that the generated file is small (by
checking the size or the number of types) but it doesn't seem a very
reliable approach to me.

I think this second run after minimizing BTF is a good idea. I
wouldn't bother to check for "minimal BTF" for this case.

Right. Do you want this to be part of this series or can we merge later on?

quoted

2. We could write some .c files with the types we expect to have on
the generated file and compare it with the generated file. The issue
here is that comparing those BTF files doesn't seem to be too
trivial...

But I would add few realistic examples that use various combinations
of CO-RE relocations against Linux types. Then minimize BTF and
validate that BTF is what we expect.

What do you mean by "realistic examples"? Aren't the BPF programs
(that use core_reloc_types.h) I added in this commit good enough for
this test?

As for how to compare BTFs. I've been wanting to do something like
btf__normalize() API to renumber and resort all the BTF types into
some "canonical" order, so that two BTFs can be actually compared and
diffed. It might be finally the time to do that.

The big complication is your decision to dump all the fields of types
that are used by type-based relocations. I'm not convinced that's the
best way to do this. I'd keep empty struct/union for such cases,
actually. That would minimize the number of types and thus BTF in
general. It also will simplify the logic of emitting minimized BTF a
bit (all_types checks won't be necessary, I think).

As I also mentioned in previous patches, for types that are only
referenced through pointer, I'd emit FWD declaration only. Or at best
empty struct/union.

With all that, after btf__minimize() operation, comparing BTFs would
be actually pretty easy, because we'll know the order of each type,

Why do we know the order of each type? I think the order of the types
in the generated BTF files depends on:
1. The order or the relocations on the BPF object. (I'm not sure if
the compiler generates them in the same order as they appear in the
code)
2. BTFGen implementation: types are added recursively and there is
also a hashmap in between.
3. How bpftool is invoked. bpftool gen min_core_btf ... OBJ1 OBJ2 vs
bpftool gen min_core_btf ... OBJ2 OBJ1.

What I'm saying is that given a source BTF file and a BPF object I
don't know what is the order of the output BTF file. I know we could
run the test, check the generated output and use it for the test but
it seems like "cheating" to me...

Am I missing something?

so
using the
VALIDATE_RAW_BTF() (see prog_tests/btf_dedup_split.c) the tests will
be easy and clean.


One last thing, let's not add a new test binary (test_bpftool), let's
keep adding more tests into test_progs.

Will do.

quoted

Do you have any suggestions about it? Thanks!