On Fri, 2025-09-05 at 12:14 -0700, Andrii Nakryiko wrote:
> 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?
Should be fine, I guess:
$ rpm -qf /usr/sbin/command
bash-5.2.37-1.fc42.x86_64
> >
> > > + 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).
+1
>
> 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!
Yeap, nice little feature.
I was doing bogus __xlated("foo") before in tests,
just to see how assembly looks like.
