On Thu, Sep 4, 2025 at 8:57 AM Mykyta Yatsenko
<mykyta.yatsenko5@xxxxxxxxx> wrote:
>
>
>
> On 9/4/25 01:06, Andrii Nakryiko wrote:
> > On Tue, Sep 2, 2025 at 4:35 PM Mykyta Yatsenko
> > <mykyta.yatsenko5@xxxxxxxxx> 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>
> >> ---
> >> tools/testing/selftests/bpf/veristat.c | 69 +++++++++++++++++++++++++-
> >> 1 file changed, 68 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
> >> index d532dd82a3a8..e27893863400 100644
> >> --- a/tools/testing/selftests/bpf/veristat.c
> >> +++ b/tools/testing/selftests/bpf/veristat.c
> >> @@ -181,6 +181,12 @@ struct var_preset {
> >> bool applied;
> >> };
> >>
> >> +enum dump_mode {
> >> + DUMP_NONE = 0,
> >> + DUMP_XLATED = 1,
> >> + DUMP_JITED = 2,
> >> +};
> >> +
> >> static struct env {
> >> char **filenames;
> >> int filename_cnt;
> >> @@ -227,6 +233,7 @@ static struct env {
> >> char orig_cgroup[PATH_MAX];
> >> char stat_cgroup[PATH_MAX];
> >> int memory_peak_fd;
> >> + __u32 dump_mode;
> >> } env;
> >>
> >> static int libbpf_print_fn(enum libbpf_print_level level, const char *format, va_list args)
> >> @@ -271,6 +278,7 @@ const char argp_program_doc[] =
> >> enum {
> >> OPT_LOG_FIXED = 1000,
> >> OPT_LOG_SIZE = 1001,
> >> + OPT_DUMP = 1002,
> >> };
> >>
> >> static const struct argp_option opts[] = {
> >> @@ -295,6 +303,7 @@ static const struct argp_option opts[] = {
> >> "Force BPF verifier failure on register invariant violation (BPF_F_TEST_REG_INVARIANTS program flag)" },
> >> { "top-src-lines", 'S', "N", 0, "Emit N most frequent source code lines" },
> >> { "set-global-vars", 'G', "GLOBAL", 0, "Set global variables provided in the expression, for example \"var1 = 1\"" },
> >> + { "dump", OPT_DUMP, "DUMP_MODE", OPTION_ARG_OPTIONAL, "Print BPF program dump (xlated, jited)" },
> >> {},
> >> };
> >>
> >> @@ -427,6 +436,16 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
> >> return err;
> >> }
> >> break;
> >> + case OPT_DUMP:
> >> + if (!arg || strcasecmp(arg, "xlated") == 0) {
> >> + env.dump_mode |= DUMP_XLATED;
> >> + } else if (strcasecmp(arg, "jited") == 0) {
> >> + env.dump_mode |= DUMP_JITED;
> >> + } else {
> >> + fprintf(stderr, "Unrecognized dump mode '%s'\n", arg);
> >> + return -EINVAL;
> >> + }
> >> + break;
> >> default:
> >> return ARGP_ERR_UNKNOWN;
> >> }
> >> @@ -1554,6 +1573,49 @@ 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];
> >> + ssize_t len, wrote, off;
> >> + FILE *fp;
> >> + int status;
> >> +
> >> + status = system("which bpftool > /dev/null 2>&1");
> >> + if (status != 0) {
> >> + fprintf(stderr, "bpftool is not available, can't print program dump\n");
> >> + return;
> >> + }
> >> + snprintf(command, sizeof(command), "bpftool prog dump %s id %u",
> >> + mode == DUMP_JITED ? "jited" : "xlated", prog_id);
> >> + fp = popen(command, "r");
> >> + if (!fp) {
> >> + fprintf(stderr, "Can't run bpftool\n");
> > maybe "bpftool failed with error: %d\n" and pass errno?
> >
> >> + return;
> >> + }
> >> +
> >> + printf("%s/%s DUMP %s:\n", file_name, prog_name, mode == DUMP_JITED ? "JITED" : "XLATED");
> >> + fflush(stdout);
> >> + do {
> >> + len = read(fileno(fp), buf, sizeof(buf));
> >> + if (len < 0)
> >> + goto error;
> >> +
> >> + for (off = 0; off < len;) {
> >> + wrote = write(STDOUT_FILENO, buf + off, len - off);
> >> + if (wrote <= 0)
> >> + goto error;
> >> + off += wrote;
> >> + }
> >> + } while (len > 0);
> >> + write(STDOUT_FILENO, "\n", 1);
> > Given we have FILE abstraction, wouldn't it be more natural to use
> > fread()/fwrite()/feof()?
> >
> > this also doesn't handle interrupted syscalls (-EINTR)
> I did that initially, but then removed, is there a relevant scenario
> where veristat handles signals, is it
> SIGSTOP/SIGCONT ? Otherwise it's going to terminate anyway, isn't it?
there are a bunch of signals that can be sent to veristat (e.g.,
SIGCHLD), that wouldn't kill the process. I guess why not handle that
if that's part of syscall handling protocol?
> >
> > pw-bot: cr
> >
> >> + goto out;
> >> +error:
> >> + fprintf(stderr, "Could not write BPF prog dump. Error: %s (errno=%d)\n", strerror(errno),
> > why so specific, "write", if it could be an error during reading from
> By write I meant "output" or "print" in a sense that we
> can't print dump any further, because there is some error.
> I'll send the next version.
> > bpftool? And note a more or less consistent "Failed to ..." wording,
> > there is not a single "Could not" in veristat.c. So something generic
> > like "Failed to fetch BPF program dump" or something?
> >
> >> + errno);
> >> +out:
> >> + pclose(fp);
> >> +}
> >> +
> >> static int process_prog(const char *filename, struct bpf_object *obj, struct bpf_program *prog)
> >> {
> >> const char *base_filename = basename(strdupa(filename));
> >> @@ -1630,8 +1692,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);
> >> + }
> >>
> >> parse_verif_log(buf, buf_sz, stats);
> >>
> >> --
> >> 2.51.0
> >>
>
