On Fri, Sep 5, 2025 at 12:19 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> 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
command is actually a shell built-in ([0]). At least for Bourne shells, I think.
[0] https://pubs.opengroup.org/onlinepubs/009695399/utilities/command.html
>
> > >
> > > > + 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.
