On Fri, Aug 29, 2025 at 6:28 AM 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 | 75 +++++++++++++++++++++++++-
> 1 file changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
> index d532dd82a3a8..c824f6e72e2f 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,10 +303,12 @@ 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)" },
> {},
> };
>
> static int parse_stats(const char *stats_str, struct stat_specs *specs);
> +static int parse_dump_mode(char *mode_str, __u32 *dump_mode);
> static int append_filter(struct filter **filters, int *cnt, const char *str);
> static int append_filter_file(const char *path);
> static int append_var_preset(struct var_preset **presets, int *cnt, const char *expr);
> @@ -427,6 +437,11 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
> return err;
> }
> break;
> + case OPT_DUMP:
> + err = parse_dump_mode(arg, &env.dump_mode);
> + if (err)
> + return err;
> + break;
> default:
> return ARGP_ERR_UNKNOWN;
> }
> @@ -956,6 +971,32 @@ static int parse_stats(const char *stats_str, struct stat_specs *specs)
> return 0;
> }
>
> +static int parse_dump_mode(char *mode_str, __u32 *dump_mode)
> +{
> + char *state = NULL, *cur;
> + int cnt = 0;
> +
> + if (!mode_str) {
> + env.dump_mode = DUMP_XLATED;
> + return 0;
> + }
> +
> + for (cur = mode_str; *cur; ++cur)
> + *cur = tolower(*cur);
> +
> + while ((cur = strtok_r(cnt++ ? NULL : mode_str, ",", &state))) {
there is no need to parse comma-separated list, it is allowed to
specify the same CLI flag multiple times, so we can use that to
accumulate multiple modes:
--dump=xlated --dump=jited
this will do the trick, so keep it simple.
And then you don't need to do tolower(), you can just strcasecmp() against arg
pw-bot: cr
> + if (strcmp(cur, "jited") == 0) {
> + env.dump_mode |= DUMP_JITED;
> + } else if (strcmp(cur, "xlated") == 0) {
> + env.dump_mode |= DUMP_XLATED;
> + } else {
> + fprintf(stderr, "Unrecognized dump mode '%s'\n", cur);
> + return -EINVAL;
> + }
> + }
> + return 0;
> +}
> +
> static void free_verif_stats(struct verif_stats *stats, size_t stat_cnt)
> {
> int i;
> @@ -1554,6 +1595,35 @@ static int parse_rvalue(const char *val, struct rvalue *rvalue)
> return 0;
> }
>
> +static void dump(__u32 prog_id, const char *file_name, const char *prog_name)
> +{
> + char command[64], buf[1024];
> + enum dump_mode modes[2] = { DUMP_XLATED, DUMP_JITED };
> + const char *mode_lower[2] = { "xlated", "jited" };
> + const char *mode_upper[2] = { "XLATED", "JITED" };
Instead of this, why don't we just pass desired mode as an argument,
and then just call dump(DUMP_XLATED) if env.dump_mode & DUMP_XLATED,
and similarly for DUMP_JITED. It's not like we'll have tons of
different dump modes, right?
> + FILE *fp;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(modes); ++i) {
> + if (!(env.dump_mode & modes[i]))
> + continue;
> + snprintf(command, sizeof(command), "bpftool prog dump %s id %u",
> + mode_lower[i], prog_id);
should we check if bpftool is installed first? I.e., run `which
bpftool` and check if that's successful? And if not, print some
user-friendly warning or error?
> +
> + fp = popen(command, "r");
> + if (!fp) {
> + fprintf(stderr, "Can't run bpftool\n");
> + return;
> + }
> +
> + printf("%s/%s DUMP %s:\n", file_name, prog_name, mode_upper[i]);
> + while (fgets(buf, sizeof(buf), fp))
> + printf("%s", buf);
nit: is there any advantage to using fgets+printf vs just binary-based
read/write? (and I'd probably make buf page sized)
> + printf("\n");
> + 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 +1700,11 @@ 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_NONE)
> + dump(info.id, base_filename, prog_name);
> + }
>
> parse_verif_log(buf, buf_sz, stats);
>
> --
> 2.51.0
>
