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

[Eduard Zingerman <eddyz87@gmail.com>]

On Fri, 2025-09-05 at 15:08 +0100, Mykyta Yatsenko wrote:
> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> 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@meta.com>
> ---

Lgtm with a small nit.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

> @@ -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).

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

> +	}
>  
>  	parse_verif_log(buf, buf_sz, stats);
>