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

From: Quentin Monnet <quentin@isovalent.com>
To: "Mauricio Vásquez Bernal" <mauricio@kinvolk.io>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Rafael David Tinoco <rafaeldtinoco@gmail.com>,
	Lorenzo Fontana <lorenzo.fontana@elastic.co>,
	Leonardo Di Donato <leonardo.didonato@elastic.co>
Subject: Re: [PATCH bpf-next v5 9/9] selftest/bpf: Implement tests for bpftool gen min_core_btf
Date: Tue, 1 Feb 2022 20:58:09 +0000	[thread overview]
Message-ID: <30675965-71c5-e0e7-d3f3-8022bf41b764@isovalent.com> (raw)
In-Reply-To: <CAHap4zsWqpTezbzZn7TOWvFA4c2PbSum4vY1_9YB+XSfFor21g@mail.gmail.com>

2022-01-28 18:23 UTC-0500 ~ Mauricio Vásquez Bernal <mauricio@kinvolk.io>
> On Fri, Jan 28, 2022 at 5:33 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>>
>> 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.

To check that the resulting BTF is optimised, one idea maybe would be to
first produce such a minimal BTF file for the program (with a manual
check) and then to expand it with additional symbols that you know are
all unnecessary to the program. Then for the test you can run bpftool to
produce the minimal BTF again and can check if any of the definitions
known as superfluous are still present.

Another solution could be to attempt to load the BTF and program by
removing any of the info from the produced BTF file, and see if the
program still loads.

Not sure if any of those solutions is easy to implement, though.

> 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...
> 
> Do you have any suggestions about it? Thanks!

I'm not familiar enough with BTF to have some great suggestion here,
maybe Andrii can help.

As a side note, it's already good to have some testing for the new
feature. The CI tests are pretty limited for bpftool at the moment and
we don't test much of it, so even basic tests to make sure that the
feature is not completely broken is a good start. Then the more we
cover, the safer we are of course :).

Thanks for this work!
Quentin