On Fri, Sep 5, 2025 at 12:00 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Fri, 2025-09-05 at 15:08 +0100, Mykyta Yatsenko wrote:
> > From: Mykyta Yatsenko <yatsenko@xxxxxxxx>
> >
> > Add the ability to dump BPF program instructions directly from veristat.
> > Previously, inspecting a program required separate bpftool invocations:
> > one to load and another to dump it, which meant running multiple
> > commands.
> > During active development, it's common for developers to use veristat
> > for testing verification. Integrating instruction dumping into veristat
> > reduces the need to switch tools and simplifies the workflow.
> > By making this information more readily accessible, this change aims
> > to streamline the BPF development cycle and improve usability for
> > developers.
> > This implementation leverages bpftool, by running it directly via popen
> > to avoid any code duplication and keep veristat simple.
> >
> > Signed-off-by: Mykyta Yatsenko <yatsenko@xxxxxxxx>
> > ---
>
> Lgtm with a small nit.
>
> Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
>
> > @@ -1554,6 +1573,35 @@ static int parse_rvalue(const char *val, struct rvalue *rvalue)
> > return 0;
> > }
> >
> > +static void dump(__u32 prog_id, enum dump_mode mode, const char *file_name, const char *prog_name)
> > +{
> > + char command[64], buf[4096];
> > + FILE *fp;
> > + int status;
> > +
> > + status = system("which bpftool > /dev/null 2>&1");
>
> Fun fact: if you do a minimal Fedora install (dnf group install core)
> "which" is not installed by default o.O
> (not suggesting any changes).
I switched to `command -v bpftool` for now, is there any gotcha with
that one as well?
>
> > + if (status != 0) {
> > + fprintf(stderr, "bpftool is not available, can't print program dump\n");
> > + return;
> > + }
>
> [...]
>
> > @@ -1630,8 +1678,13 @@ static int process_prog(const char *filename, struct bpf_object *obj, struct bpf
> >
> > memset(&info, 0, info_len);
> > fd = bpf_program__fd(prog);
> > - if (fd > 0 && bpf_prog_get_info_by_fd(fd, &info, &info_len) == 0)
> > + if (fd > 0 && bpf_prog_get_info_by_fd(fd, &info, &info_len) == 0) {
> > stats->stats[JITED_SIZE] = info.jited_prog_len;
> > + if (env.dump_mode & DUMP_JITED)
> > + dump(info.id, DUMP_JITED, base_filename, prog_name);
> > + if (env.dump_mode & DUMP_XLATED)
> > + dump(info.id, DUMP_XLATED, base_filename, prog_name);
>
> Nit: if you do `./veristat --dump=jited iters.bpf.o` there would be an empty line
> after dump for each program, but not for --dump=xlated.
>
Yeah, bpftool's output isn't consistent. I just added an extra empty
line, that makes dump a bit more clean (and I didn't mind two empty
lines, whatever).
I was also finding it hard to notice where the dump for a given
program starts, so I reformatted header a bit. Overall, applied the
following changes and pushed to bpf-next, thanks for a useful feature!
diff --git a/tools/testing/selftests/bpf/veristat.c
b/tools/testing/selftests/bpf/veristat.c
index 85ae7f6fee90..e962f133250c 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -1579,7 +1579,7 @@ static void dump(__u32 prog_id, enum dump_mode
mode, const char *file_name, cons
FILE *fp;
int status;
- status = system("which bpftool > /dev/null 2>&1");
+ status = system("command -v bpftool > /dev/null 2>&1");
if (status != 0) {
fprintf(stderr, "bpftool is not available, can't print
program dump\n");
return;
@@ -1592,9 +1592,10 @@ static void dump(__u32 prog_id, enum dump_mode
mode, const char *file_name, cons
return;
}
- printf("%s/%s DUMP %s:\n", file_name, prog_name, mode ==
DUMP_JITED ? "JITED" : "XLATED");
+ printf("DUMP (%s) %s/%s:\n", mode == DUMP_JITED ? "JITED" :
"XLATED", file_name, prog_name);
while (fgets(buf, sizeof(buf), fp))
fputs(buf, stdout);
+ fprintf(stdout, "\n");
if (ferror(fp))
fprintf(stderr, "Failed to dump BPF prog with error:
%d\n", errno);
> > + }
> >
> > parse_verif_log(buf, buf_sz, stats);
> >
