Re: [PATCH bpf-next v12 04/10] libbpf: Probe percpu data feature

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The feature probe `probe_kern_percpu_data` incorrectly returns a negative error code and logs a warning when the kernel lacks support for `BPF_MAP_TYPE_PERCPU_ARRAY`. Feature probes in libbpf should silently return 0 when a feature is missing to gracefully indicate its absence without polluting stderr.
--

--- Patch [4]: [PATCH bpf-next v12 04/10] libbpf: Probe percpu data feature ---
commit 783c3759ac5b8bbe1c5368930abe9953ff13c465
Author: Leon Hwang <[email protected]>

    libbpf: Probe percpu data feature
    
    This commit adds a feature probe for global percpu data support. It allows
    libbpf to detect if the kernel supports percpu data early and make capability
    decisions accordingly.

> diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c
> index b7e388f99d0bb..ef9581c113035 100644
> --- a/tools/lib/bpf/features.c
> +++ b/tools/lib/bpf/features.c
[ ... ]
> +static int probe_kern_percpu_data(int token_fd)
> +{
> +	struct bpf_insn insns[] = {
> +		BPF_LD_MAP_VALUE(BPF_REG_1, 0, 0),
> +		BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, 0),
> +		BPF_EXIT_INSN(),
> +	};
> +	LIBBPF_OPTS(bpf_map_create_opts, map_opts,
> +		.token_fd = token_fd,
> +		.map_flags = token_fd ? BPF_F_TOKEN_FD : 0,
> +	);
> +	LIBBPF_OPTS(bpf_prog_load_opts, prog_opts,
> +		.token_fd = token_fd,
> +		.prog_flags = token_fd ? BPF_F_TOKEN_FD : 0,
> +	);
> +	int ret, map, insn_cnt = ARRAY_SIZE(insns);
> +
> +	map = bpf_map_create(BPF_MAP_TYPE_PERCPU_ARRAY, "libbpf_percpu", sizeof(int), 8, 1,
> +			     &map_opts);
> +	if (map < 0) {
> +		pr_warn("Error in %s(): %s. Couldn't create simple percpu_array map.\n",
> +			__func__, errstr(map));
> +		return map;
> +	}

[Severity: Medium]
Will this cause spurious warnings when probing environments that don't support
BPF_MAP_TYPE_PERCPU_ARRAY?

Since feat_supported() expects feature probes to return 0 when a feature is
missing, returning a negative error code here causes feat_supported() to log
an additional failure warning.

Should this return 0 instead of map (and perhaps omit the pr_warn) to
gracefully indicate the feature's absence without polluting stderr?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4