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

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

 



On Mon, 2025-08-18 at 19:04 +0100, Mykyta Yatsenko wrote:

[...]

> diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
> index d532dd82a3a8..3ba06f532bfa 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", 0, "Print BPF program dump" },

Nit: describe that it should be either '-p xlated' or '-p 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,26 @@ static int parse_rvalue(const char *val, struct rvalue *rvalue)
>  	return 0;
>  }
>  
> +static void dump(int prog_fd)
                    ^^^^^^^^^^^
		    Nit: prog_id

> +{
> +	char command[512];
> +	char buf[1024];
> +	FILE *fp;
> +
> +	snprintf(command, sizeof(command), "bpftool prog dump %s id %d",
> +		 env.dump_mode == JITED ? "jited" : "xlated", prog_fd);
> +	fp = popen(command, "r");

Silly question, would it be sufficient to just do "system()" and forgo
the loop below?

> +	if (!fp) {
> +		fprintf(stderr, "Can't run bpftool\n");
> +		return;
> +	}
> +

Could you please insert some header (program name)/footer (newline)?

> +	while (fgets(buf, sizeof(buf), fp))
> +		printf("%s", buf);
> +
> +	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 +1668,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);
> +	}
>  
>  	parse_verif_log(buf, buf_sz, stats);
>  

Note: below this hunk there is the following code:

          if (env.verbose) {
                  printf(format: "PROCESSING %s/%s, DURATION US: %ld, VERDICT: %s, VERIFIER LOG:\n%s\n",
                         filename, prog_name, stats->stats[DURATION],
                         err ? "failure" : "success", buf);
          }

It looks a bit strange, that program is printed before the above
header is printed.





[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