Re: [PATCH bpf-next v2 0/5] bpftool: Add inline annotations when dumping program CFGs

From: Quentin Monnet <quentin@isovalent.com>
To: Eduard Zingerman <eddyz87@gmail.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>
Cc: Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>,
	bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v2 0/5] bpftool: Add inline annotations when dumping program CFGs
Date: Fri, 31 Mar 2023 15:53:05 +0100	[thread overview]
Message-ID: <7cb20aa0-fd1c-d459-5250-82843b65c37e@isovalent.com> (raw)
In-Reply-To: <cdd67fd11f71210b75e48a848ade42f545cddf8f.camel@gmail.com>

2023-03-27 20:56 UTC+0300 ~ Eduard Zingerman <eddyz87@gmail.com>
> On Mon, 2023-03-27 at 12:06 +0100, Quentin Monnet wrote:
>> This set contains some improvements for bpftool's "visual" program dump
>> option, which produces the control flow graph in a DOT format. The main
>> objective is to add support for inline annotations on such graphs, so that
>> we can have the C source code for the program showing up alongside the
>> instructions, when available. The last commits also make it possible to
>> display the line numbers or the bare opcodes in the graph, as supported by
>> regular program dumps.
>>
>> v2: Replace fputc(..., stdout) with putchar(...) in dotlabel_puts().
> 
> Hi Quentin,
> 
> It looks like currently there are no test cases for bpftool prog dump.
> Borrowing an idea to mix bpf program with comments parsed by awk from
> prog_tests/btf_dump.c it is possible to put together something like
> below (although, would be much simpler as a bash script). Is it worth
> the effort or dump format is too unstable?
> 
> Thanks,
> Eduard

Correct, I don't think we have tests for that at the moment.

But yes, I would love to get a test like this (and more bpftool tests in
general, when I can find cycles). I don't mind a bash script,
personally; your bpftool_cfg.c is mostly a succession of commands, and
the other tests we have for bpftool are in bash or Python anyway
(test_bpftool*). We could use bpftool to load the program (I suppose
it's debatable whether it's good to use bpftool itself to set up a test
for bpftool; on the other hand, it's heavily based on libbpf, so using
libbpf directly doesn't seem to make that much of a difference).

As for the stability of the output, generally the produced BPF bytecode
is obviously subject to change, but for a program so simple as "return
0;" we should be mostly good. Just make sure you specify the ISA version
when you compile the program, with < v3 we would load 64 bits instead of
32 in the first instruction. On bpftool's side, I don't expect the CFG
output to change much, so no problem.

Thanks a lot for your review, and for working on this test!

I'll send a v3, probably sometime next week.
Quentin