Re: [PATCH bpf-next v5] selftests/bpf: add BPF program dump in veristat

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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)

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
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
>





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux