Re: [PATCH bpf-next v3 1/5] libbpf: Count entry programs for bpf_gen__init()

[bot+bpf-ci]

> libbpf: Count entry programs for bpf_gen__init()

TL;DR: Nit only: the comment in bpf_gen__init() in gen_loader.c still says
nr_progs includes subprogs, which is now stale after this change. Suggests
updating it.

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c

[ ... ]

> @@ -9815,13 +9815,16 @@ static int bpf_object_load(struct bpf_object *obj, int extra_log_level, const ch
>  			if (prog_is_subprog(obj, &obj->programs[i]) &&
>  			    prog_has_callx(&obj->programs[i]))
>  				text_has_callx = true;
> -		for (i = 0; i < obj->nr_programs; i++)
> +		for (i = 0; i < obj->nr_programs; i++) {
> +			if (prog_is_subprog(obj, &obj->programs[i]))
> +				continue;
> +			nr_obj_progs++;
>  			if (obj->programs[i].load_strategy != BPF_PROG_LOAD_STRATEGY_DISABLED &&
> -			    !prog_is_subprog(obj, &obj->programs[i]) &&
>  			    (text_has_callx || prog_has_callx(&obj->programs[i])))
>  				nr_progs++;
> +		}
>  		bpf_gen__init(obj->gen_loader, obj->log_level | extra_log_level,
> -			      obj->nr_programs, obj->nr_maps, nr_func_ptr_maps * nr_progs);
> +			      nr_obj_progs, obj->nr_maps, nr_func_ptr_maps * nr_progs);

This isn't a bug, but the comment in bpf_gen__init() now looks stale:

tools/lib/bpf/gen_loader.c:bpf_gen__init() {
    ...
	/* nr_progs includes subprogs and progs that are not loaded */
	nr_progs = min(nr_progs, MAX_USED_PROGS);
    ...
}

With this change, the nr_progs argument no longer includes subprogs.  It is
the number of entry programs, which is the same count that
bpf_object__for_each_program() and the skeleton expose, and it still
includes entry programs that are not loaded.  The comment came from
259172bb6514 ("libbpf: Fix gen_loader assumption on number of programs."),
which the commit message cites.

None of the later patches in the series (276212ecac70, 28aa06d2c9ca,
c9412c70e104, 5b9c2e5eed2a) update it.  276212ecac70 ("libbpf: Track
generated loader link FDs") also sizes link_fds from this same nr_progs
value, so the comment now misdescribes a value that later code depends on.

Should the comment be changed to something like "nr_progs includes progs
that are not loaded, but not subprogs"?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/41519963115