Re: [PATCH bpf-next v1] selftests/bpf: Add benchmark for tail call performance

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Memory leak on the error path in `tailcall_measure`.
- [Medium] Hardcoded CPU affinity causes test failure on systems with fewer than 3 CPUs.
- [Low] Redundant file descriptor validation for BPF skeleton-generated maps.
--

commit f098091b64abfdc8c42b360eefbec9dae2a20a03
Author: Tiezhu Yang <yangtiezhu@loongson.cn>

selftests/bpf: Add benchmark for tail call performance

This commit introduces a new benchmark to measure the runtime performance and
overhead of BPF tail calls. It uses a tracepoint on sys_enter and toggles
between direct and bpf2bpf tail calls, along with a shell script for automated
testing.

> diff --git a/tools/testing/selftests/bpf/benchs/bench_tailcall.c b/tools/testing/selftests/bpf/benchs/bench_tailcall.c
> new file mode 100644
> index 0000000000000..a203017f5e282
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/benchs/bench_tailcall.c

[ ... ]

> +static void tailcall_measure(struct bench_res *res)
> +{
> +	__u32 key = 0;
> +	__u8 *values;
> +	__u64 total_hits = 0;
> +
> +	values = calloc(ctx.ncpus, ctx.percpu_size);
> +	if (!values)
> +		return;
> +
> +	if (bpf_map_lookup_elem(ctx.map_fd, &key, values) != 0)
> +		return;

[Severity: Medium]
Does this error path leak the memory allocated for values? If
bpf_map_lookup_elem() fails, the function returns immediately without calling
free(values).

> +
> +	for (int i = 0; i < ctx.ncpus; i++)
> +		total_hits += *(__u64 *)(values + i * ctx.percpu_size);
> +
> +	res->hits = total_hits;
> +	free(values);
> +}

[ ... ]

> +static void tailcall_setup(void)
> +{
> +	int main_fd, target_fd, jmp_map_fd;
> +	__u32 key1 = 1;
> +
> +	ctx.skel = tailcall_bench__open();
> +	if (!ctx.skel)
> +		exit(1);
> +
> +	ctx.skel->data->my_pid = getpid();
> +	ctx.ncpus = libbpf_num_possible_cpus();
> +
> +	if (tailcall_bench__load(ctx.skel))
> +		exit(1);
> +
> +	jmp_map_fd = bpf_map__fd(ctx.skel->maps.jmp_table);
> +	ctx.map_fd = bpf_map__fd(ctx.skel->maps.pcpu_hits_map);
> +	ctx.percpu_size = bpf_map__value_size(ctx.skel->maps.pcpu_hits_map);
> +
> +	if (ctx.map_fd < 0 || jmp_map_fd < 0)
> +		exit(1);

[Severity: Low]
This isn't a bug, but are these file descriptor checks necessary? After a
successful skeleton load, fields like skel->maps are guaranteed to be valid
and bpf_map__fd() cannot return a negative value.

> +
> +	main_fd = bpf_program__fd(ctx.skel->progs.tailcall_bench_main);
> +	target_fd = bpf_program__fd(ctx.skel->progs.tailcall_bench_target);

[ ... ]

> diff --git a/tools/testing/selftests/bpf/benchs/run_bench_tailcall.sh b/tools/testing/selftests/bpf/benchs/run_bench_tailcall.sh
> new file mode 100755
> index 0000000000000..c687f34455e8e
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/benchs/run_bench_tailcall.sh

[ ... ]

> +# 2. Strict error handling configurations
> +set -eufo pipefail
> +
> +# 3. Use default bench binary path if not exported by the framework
> +BENCH_BIN=${BENCH:-./bench}
> +
> +# 4. Run with strict core affinity and isolation for reliable profiling
> +RUN_BENCH="numactl --physcpubind=0,2 --membind=0 nice -n -20 $BENCH_BIN -w5 -d20 -a"

[Severity: Medium]
Will this hardcoded CPU affinity fail on systems with fewer than 3 CPUs?
If CPU 2 does not exist, numactl might fail and immediately abort the script
due to the set -e configuration.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630022306.10017-1-yangtiezhu@loongson.cn?part=1