Re: [PATCH bpf-next v2 2/2] selftests/bpf: Use bpf_program__clone() in veristat

["Emil Tsalapatis" <emil@etsalapatis.com>]

On Fri Feb 20, 2026 at 2:18 PM EST, Mykyta Yatsenko wrote:
> From: Mykyta Yatsenko <yatsenko@meta.com>
>
> Replace veristat's per-program object re-opening with
> bpf_program__clone().
>
> Previously, veristat opened a separate bpf_object for every program in a
> multi-program object file, iterated all programs to enable only the
> target one, and then loaded the entire object.
>
> Use bpf_object__prepare() once, then call bpf_program__clone() for each
> program individually. This lets veristat load programs one at a time
> from a single prepared object.
>
> The caller now owns the returned fd and closes it after collecting stats.
> Remove the special single-program fast path and the per-file early exit
> in handle_verif_mode() so all files are always processed.
>
> Split fixup_obj() into fixup_obj_maps() for object-wide map fixups that
> must run before bpf_object__prepare(), and fixup_obj() for per-program
> fixups (struct_ops masking, freplace type guessing) that run before each
> bpf_program__clone() call.
>
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
>  tools/testing/selftests/bpf/veristat.c | 104 ++++++++++++++-------------------
>  1 file changed, 44 insertions(+), 60 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
> index 1be1e353d40a..7d7eb56e04d0 100644
> --- a/tools/testing/selftests/bpf/veristat.c
> +++ b/tools/testing/selftests/bpf/veristat.c
> @@ -1236,7 +1236,7 @@ static void mask_unrelated_struct_ops_progs(struct bpf_object *obj,
>  	}
>  }
>  
> -static void fixup_obj(struct bpf_object *obj, struct bpf_program *prog, const char *filename)
> +static void fixup_obj_maps(struct bpf_object *obj)
>  {
>  	struct bpf_map *map;
>  
> @@ -1251,15 +1251,23 @@ static void fixup_obj(struct bpf_object *obj, struct bpf_program *prog, const ch
>  		case BPF_MAP_TYPE_INODE_STORAGE:
>  		case BPF_MAP_TYPE_CGROUP_STORAGE:
>  		case BPF_MAP_TYPE_CGRP_STORAGE:
> -			break;
>  		case BPF_MAP_TYPE_STRUCT_OPS:
> -			mask_unrelated_struct_ops_progs(obj, map, prog);
>  			break;
>  		default:
>  			if (bpf_map__max_entries(map) == 0)
>  				bpf_map__set_max_entries(map, 1);
>  		}
>  	}
> +}
> +
> +static void fixup_obj(struct bpf_object *obj, struct bpf_program *prog, const char *filename)
> +{
> +	struct bpf_map *map;
> +
> +	bpf_object__for_each_map(map, obj) {
> +		if (bpf_map__type(map) == BPF_MAP_TYPE_STRUCT_OPS)
> +			mask_unrelated_struct_ops_progs(obj, map, prog);
> +	}
>  
>  	/* SEC(freplace) programs can't be loaded with veristat as is,
>  	 * but we can try guessing their target program's expected type by
> @@ -1608,6 +1616,7 @@ static int process_prog(const char *filename, struct bpf_object *obj, struct bpf
>  	const char *base_filename = basename(strdupa(filename));
>  	const char *prog_name = bpf_program__name(prog);
>  	long mem_peak_a, mem_peak_b, mem_peak = -1;
> +	LIBBPF_OPTS(bpf_prog_load_opts, opts);
>  	char *buf;
>  	int buf_sz, log_level;
>  	struct verif_stats *stats;
> @@ -1647,9 +1656,6 @@ static int process_prog(const char *filename, struct bpf_object *obj, struct bpf
>  	}
>  	verif_log_buf[0] = '\0';
>  
> -	bpf_program__set_log_buf(prog, buf, buf_sz);
> -	bpf_program__set_log_level(prog, log_level);
> -
>  	/* increase chances of successful BPF object loading */
>  	fixup_obj(obj, prog, base_filename);
>  
> @@ -1658,15 +1664,21 @@ static int process_prog(const char *filename, struct bpf_object *obj, struct bpf
>  	if (env.force_reg_invariants)
>  		bpf_program__set_flags(prog, bpf_program__flags(prog) | BPF_F_TEST_REG_INVARIANTS);
>  
> -	err = bpf_object__prepare(obj);
> -	if (!err) {
> -		cgroup_err = reset_stat_cgroup();
> -		mem_peak_a = cgroup_memory_peak();
> -		err = bpf_object__load(obj);
> -		mem_peak_b = cgroup_memory_peak();
> -		if (!cgroup_err && mem_peak_a >= 0 && mem_peak_b >= 0)
> -			mem_peak = mem_peak_b - mem_peak_a;
> +	opts.log_buf = buf;
> +	opts.log_size = buf_sz;
> +	opts.log_level = log_level;
> +
> +	cgroup_err = reset_stat_cgroup();
> +	mem_peak_a = cgroup_memory_peak();
> +	fd = bpf_program__clone(prog, &opts);
> +	if (fd < 0) {
> +		err = fd;
> +		fprintf(stderr, "Failed to load program %s %d\n", prog_name, err);
>  	}
> +	mem_peak_b = cgroup_memory_peak();
> +	if (!cgroup_err && mem_peak_a >= 0 && mem_peak_b >= 0)
> +		mem_peak = mem_peak_b - mem_peak_a;
> +

AFAICT the meaning of mem_peak changes with this patch, right? Before it was the diff between 
prepare() and load(), with the patch as it is now it is max(prepare,
load) since mem_peak_a is reset right above. If we do not automatically
load OBJ_PREPARED programs (see the review for patch 1/2) we can keep
the old behavior by storing mem_peak_a after the first prepare and
reading mem_peak_b after clone+load.

>  	env.progs_processed++;
>  
>  	stats->file_name = strdup(base_filename);
> @@ -1678,7 +1690,6 @@ static int process_prog(const char *filename, struct bpf_object *obj, struct bpf
>  	stats->stats[MEMORY_PEAK] = mem_peak < 0 ? -1 : mem_peak / (1024 * 1024);
>  
>  	memset(&info, 0, info_len);
> -	fd = bpf_program__fd(prog);
>  	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)
> @@ -1699,7 +1710,8 @@ static int process_prog(const char *filename, struct bpf_object *obj, struct bpf
>  
>  	if (verif_log_buf != buf)
>  		free(buf);
> -
> +	if (fd > 0)
> +		close(fd);
>  	return 0;
>  }
>  
> @@ -2182,8 +2194,8 @@ static int set_global_vars(struct bpf_object *obj, struct var_preset *presets, i
>  static int process_obj(const char *filename)
>  {
>  	const char *base_filename = basename(strdupa(filename));
> -	struct bpf_object *obj = NULL, *tobj;
> -	struct bpf_program *prog, *tprog, *lprog;
> +	struct bpf_object *obj = NULL;
> +	struct bpf_program *prog;
>  	libbpf_print_fn_t old_libbpf_print_fn;
>  	LIBBPF_OPTS(bpf_object_open_opts, opts);
>  	int err = 0, prog_cnt = 0;
> @@ -2222,51 +2234,26 @@ static int process_obj(const char *filename)
>  	env.files_processed++;
>  
>  	bpf_object__for_each_program(prog, obj) {
> +		bpf_program__set_autoload(prog, true);
>  		prog_cnt++;
>  	}
>  
> -	if (prog_cnt == 1) {
> -		prog = bpf_object__next_program(obj, NULL);
> -		bpf_program__set_autoload(prog, true);
> -		err = set_global_vars(obj, env.presets, env.npresets);
> -		if (err) {
> -			fprintf(stderr, "Failed to set global variables %d\n", err);
> -			goto cleanup;
> -		}
> -		process_prog(filename, obj, prog);
> +	fixup_obj_maps(obj);
> +
> +	err = set_global_vars(obj, env.presets, env.npresets);
> +	if (err) {
> +		fprintf(stderr, "Failed to set global variables %d\n", err);
>  		goto cleanup;
>  	}
>  
> -	bpf_object__for_each_program(prog, obj) {
> -		const char *prog_name = bpf_program__name(prog);
> -
> -		tobj = bpf_object__open_file(filename, &opts);
> -		if (!tobj) {
> -			err = -errno;
> -			fprintf(stderr, "Failed to open '%s': %d\n", filename, err);
> -			goto cleanup;
> -		}
> -
> -		err = set_global_vars(tobj, env.presets, env.npresets);
> -		if (err) {
> -			fprintf(stderr, "Failed to set global variables %d\n", err);
> -			goto cleanup;
> -		}
> -
> -		lprog = NULL;
> -		bpf_object__for_each_program(tprog, tobj) {
> -			const char *tprog_name = bpf_program__name(tprog);
> -
> -			if (strcmp(prog_name, tprog_name) == 0) {
> -				bpf_program__set_autoload(tprog, true);
> -				lprog = tprog;
> -			} else {
> -				bpf_program__set_autoload(tprog, false);
> -			}
> -		}
> +	err = bpf_object__prepare(obj);
> +	if (err) {
> +		fprintf(stderr, "Failed to prepare BPF object for loading %d\n", err);
> +		goto cleanup;
> +	}
>  
> -		process_prog(filename, tobj, lprog);
> -		bpf_object__close(tobj);
> +	bpf_object__for_each_program(prog, obj) {
> +		process_prog(filename, obj, prog);
>  	}
>  
>  cleanup:
> @@ -3264,17 +3251,14 @@ static int handle_verif_mode(void)
>  	create_stat_cgroup();
>  	for (i = 0; i < env.filename_cnt; i++) {
>  		err = process_obj(env.filenames[i]);
> -		if (err) {
> +		if (err)
>  			fprintf(stderr, "Failed to process '%s': %d\n", env.filenames[i], err);
> -			goto out;
> -		}

This means we now do not stop on the first failure - this I assume is
deliberate since it is an improvement over existing behavior imo, but
still pointing it out in case it isn't.

>  	}
>  
>  	qsort(env.prog_stats, env.prog_stat_cnt, sizeof(*env.prog_stats), cmp_prog_stats);
>  
>  	output_prog_stats();
>  
> -out:
>  	destroy_stat_cgroup();
>  	return err;
>  }