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

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

[Mykyta Yatsenko]

From: Mykyta Yatsenko <yatsenko@meta.com>

This patch adds support for dumping BPF program instructions directly
from veristat.
While it is already possible to inspect BPF program dump using bpftool,
it requires 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@meta.com>
---
 tools/testing/selftests/bpf/veristat.c | 34 +++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index d532dd82a3a8..a4ecbc12953e 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 {
+	NO_DUMP = 0,
+	XLATED,
+	JITED,
+};
+
 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;
+	enum dump_mode dump_mode;
 } env;
 
 static int libbpf_print_fn(enum libbpf_print_level level, const char *format, va_list args)
@@ -295,6 +302,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", 'p', "DUMP_MODE", 0, "Print BPF program dump (xlated, jited)" },
 	{},
 };
 
@@ -427,6 +435,16 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
 			return err;
 		}
 		break;
+	case 'p':
+		if (strcmp(arg, "jited") == 0) {
+			env.dump_mode = JITED;
+		} else if (strcmp(arg, "xlated") == 0) {
+			env.dump_mode = XLATED;
+		} else {
+			fprintf(stderr, "Unrecognized dump mode '%s'\n", arg);
+			return -EINVAL;
+		}
+		break;
 	default:
 		return ARGP_ERR_UNKNOWN;
 	}
@@ -1554,6 +1572,17 @@ static int parse_rvalue(const char *val, struct rvalue *rvalue)
 	return 0;
 }
 
+static void dump(__u32 prog_id, const char *prog_name)
+{
+	char command[64];
+
+	snprintf(command, sizeof(command), "bpftool prog dump %s id %u",
+		 env.dump_mode == JITED ? "jited" : "xlated", prog_id);
+	printf("Prog's '%s' dump:\n", prog_name);
+	system(command);
+	printf("\n");
+}
+
 static int process_prog(const char *filename, struct bpf_object *obj, struct bpf_program *prog)
 {
 	const char *base_filename = basename(strdupa(filename));
@@ -1630,8 +1659,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 != NO_DUMP)
+			dump(info.id, prog_name);
+	}
 
 	parse_verif_log(buf, buf_sz, stats);
 
-- 
2.50.1

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

[Andrii Nakryiko]

On Tue, Aug 19, 2025 at 4:43 AM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> From: Mykyta Yatsenko <yatsenko@meta.com>
>
> This patch adds support for dumping BPF program instructions directly
> from veristat.
> While it is already possible to inspect BPF program dump using bpftool,
> it requires multiple commands. During active development, it's common

not just that, veristat will load and unload program very fast, so you
don't have sufficient time to find prog ID and dump it. So it's not
realistically possible today, unless you artificially make veristat
pause.

> 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@meta.com>
> ---
>  tools/testing/selftests/bpf/veristat.c | 34 +++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
> index d532dd82a3a8..a4ecbc12953e 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 {
> +       NO_DUMP = 0,
> +       XLATED,
> +       JITED,
> +};

nit: DUMP_NONE, DUMP_XLATED, DUMP_JITED ?


and to think about it, why not support dumping both XLATED and
JITED?... Make this a set of bits?

> +
>  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;
> +       enum dump_mode dump_mode;
>  } env;
>
>  static int libbpf_print_fn(enum libbpf_print_level level, const char *format, va_list args)
> @@ -295,6 +302,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", 'p', "DUMP_MODE", 0, "Print BPF program dump (xlated, jited)" },

"dump" and "p" don't have an obvious connection... let's just have
long-form --dump=xlated for now (and I'd default to --dump meaning
--dump=xlated to save typing for common case)?

>         {},
>  };
>
> @@ -427,6 +435,16 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
>                         return err;
>                 }
>                 break;
> +       case 'p':
> +               if (strcmp(arg, "jited") == 0) {
> +                       env.dump_mode = JITED;
> +               } else if (strcmp(arg, "xlated") == 0) {

nit: make case-insensitive?

> +                       env.dump_mode = XLATED;
> +               } else {
> +                       fprintf(stderr, "Unrecognized dump mode '%s'\n", arg);
> +                       return -EINVAL;
> +               }
> +               break;
>         default:
>                 return ARGP_ERR_UNKNOWN;
>         }
> @@ -1554,6 +1572,17 @@ static int parse_rvalue(const char *val, struct rvalue *rvalue)
>         return 0;
>  }
>
> +static void dump(__u32 prog_id, const char *prog_name)
> +{
> +       char command[64];
> +
> +       snprintf(command, sizeof(command), "bpftool prog dump %s id %u",
> +                env.dump_mode == JITED ? "jited" : "xlated", prog_id);
> +       printf("Prog's '%s' dump:\n", prog_name);

let's make it a bit more apparent and follow PROCESSING styling:

<file>/<program> DUMP (XLATED|JITED):
...

> +       system(command);

Quick googling didn't answer this question, but with system() we make
assumption that system()'s stdout/stderr always goes into veristat's
stdout/stderr. It might be always true, but why rely on this if we can
just popen()? popen() also would allow us to do any processing we
might want to do (unlikely, but it's good to have an option, IMO).

Let's go with popen(), it's not much of a complication.

pw-bot: cr

> +       printf("\n");
> +}
> +
>  static int process_prog(const char *filename, struct bpf_object *obj, struct bpf_program *prog)
>  {
>         const char *base_filename = basename(strdupa(filename));
> @@ -1630,8 +1659,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 != NO_DUMP)
> +                       dump(info.id, prog_name);
> +       }
>
>         parse_verif_log(buf, buf_sz, stats);
>
> --
> 2.50.1
>

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

[Eduard Zingerman]

On Wed, 2025-08-20 at 14:34 -0700, Andrii Nakryiko wrote:

[...]

> > +       system(command);
> 
> Quick googling didn't answer this question, but with system() we make
> assumption that system()'s stdout/stderr always goes into veristat's
> stdout/stderr. It might be always true, but why rely on this if we can
> just popen()? popen() also would allow us to do any processing we
> might want to do (unlikely, but it's good to have an option, IMO).
> 
> Let's go with popen(), it's not much of a complication.

I actually double-checked this yesterday:

man system:

> The system() library function behaves as if it used fork(2) to
> create a child process that executed the shell command specified in
> command using execl(3) as follows:
>
>     execl("/bin/sh", "sh", "-c", command, (char *) NULL);

man execl:

> The exec() family of functions replaces the current process image
> with a new process image.  The functions described in this manual
> page are lay‐ ered on top of execve(2).

max execve:

> By default, file descriptors remain open across an execve().  File
> descriptors that are marked close-on-exec are closed;

So, stdout/stderr is guaranteed to be shared.

(and on a different topic we discussed, 'popen' is documented as doing
 "sh -c command > pipe", so it differs from 'system' only in that it
 creates an additional pipe and adds redirection to sh invocation).

But there is a different complication, if one tests this with
iters.bpf.o, it becomes clear that the following is necessary:

  @@ -1579,7 +1579,9 @@ static void dump(__u32 prog_id, const char *prog_name)
          snprintf(command, sizeof(command), "bpftool prog dump %s id %u",
                   env.dump_mode == JITED ? "jited" : "xlated", prog_id);
          printf("Prog's '%s' dump:\n", prog_name);
  +       fflush(stdout);
          system(command);
  +       fflush(stdout);
          printf("\n");
   }

So, whatever.

[...]

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

[Andrii Nakryiko]

On Wed, Aug 20, 2025 at 2:52 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Wed, 2025-08-20 at 14:34 -0700, Andrii Nakryiko wrote:
>
> [...]
>
> > > +       system(command);
> >
> > Quick googling didn't answer this question, but with system() we make
> > assumption that system()'s stdout/stderr always goes into veristat's
> > stdout/stderr. It might be always true, but why rely on this if we can
> > just popen()? popen() also would allow us to do any processing we
> > might want to do (unlikely, but it's good to have an option, IMO).
> >
> > Let's go with popen(), it's not much of a complication.
>
> I actually double-checked this yesterday:
>
> man system:
>
> > The system() library function behaves as if it used fork(2) to
> > create a child process that executed the shell command specified in
> > command using execl(3) as follows:
> >
> >     execl("/bin/sh", "sh", "-c", command, (char *) NULL);
>
> man execl:
>
> > The exec() family of functions replaces the current process image
> > with a new process image.  The functions described in this manual
> > page are lay‐ ered on top of execve(2).
>
> max execve:
>
> > By default, file descriptors remain open across an execve().  File
> > descriptors that are marked close-on-exec are closed;
>
> So, stdout/stderr is guaranteed to be shared.
>
> (and on a different topic we discussed, 'popen' is documented as doing
>  "sh -c command > pipe", so it differs from 'system' only in that it
>  creates an additional pipe and adds redirection to sh invocation).
>
> But there is a different complication, if one tests this with
> iters.bpf.o, it becomes clear that the following is necessary:
>
>   @@ -1579,7 +1579,9 @@ static void dump(__u32 prog_id, const char *prog_name)
>           snprintf(command, sizeof(command), "bpftool prog dump %s id %u",
>                    env.dump_mode == JITED ? "jited" : "xlated", prog_id);
>           printf("Prog's '%s' dump:\n", prog_name);
>   +       fflush(stdout);
>           system(command);
>   +       fflush(stdout);
>           printf("\n");
>    }
>
> So, whatever.

What's the concern with popen()? That we have to copy one stream into another?

I didn't like (instinctively) the implicitness of system() adding
something to veristat's output, and you just proved that instinct
correct with that fflush()... ;)

It's like an argument of passing data through explicit function
arguments vs some global variable, IMO. Both can work given enough
care (assuming single-threaded execution), but explicit beats implicit
most of the time. And if we ever want to parallelize, then the winner
is obvious.

>
> [...]

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

[Eduard Zingerman]

On Wed, 2025-08-20 at 15:02 -0700, Andrii Nakryiko wrote:

[...]

> What's the concern with popen()? That we have to copy one stream into another?

No concern, just see an additional loop as unnecessary, this does not
matter much.

> 
> I didn't like (instinctively) the implicitness of system() adding
> something to veristat's output, and you just proved that instinct
> correct with that fflush()... ;)

You asked a question off-list on Tuesday whether system() and popen()
behave the same, here is an answer (both are wrappers for 'sh -c').

[...]