On 8/19/25 01:18, Eduard Zingerman wrote:
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?
yes, I guess it will be the same result.
+ if (!fp) { + fprintf(stderr, "Can't run bpftool\n"); + return; + } +Could you please insert some header (program name)/footer (newline)?
yes, sounds like a good idea.
The header is printed with the verifier log. I think dump should go before it, probably with its own header,+ 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 bpfmemset(&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.
as its probably not often used along with log.
