Re: [PATCH bpf-next v2 21/21] selftests/bpf: Benchmark tracing_multi bpf progs attachment

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] bpf: bench fails with EMFILE due to unraised RLIMIT_NOFILE
- [Low] bpf: benchmark time overflow on 32-bit due to long truncation
--

commit d6a7c4ba0d33ae305098f82b092a318e73ea06fb
Author: Leon Hwang <leon.hwang@linux.dev>

selftests/bpf: Benchmark tracing_multi bpf progs attachment

This commit adds an x86-64 one-shot generic benchmark to compare the
performance of attaching 1000 individual fentry links versus a single
fentry.multi link for the same 1000 BPF program targets.

> diff --git a/tools/testing/selftests/bpf/benchs/bench_tracing_multi_attach_progs.c b/tools/testing/selftests/bpf/benchs/bench_tracing_multi_attach_progs.c
> new file mode 100644
> index 0000000000000..6cc6851eb488e
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/benchs/bench_tracing_multi_attach_progs.c
[ ... ]
> +static long create_and_attach_fentry(void)
> +{
> +	int link_fds[TARGET_CNT];
> +	long start_ns, end_ns;

[Severity: Low]
Will this truncate the 64-bit nanosecond timestamps returned by get_time_ns()
on 32-bit architectures?

get_time_ns() returns a __u64, and storing it in a 32-bit long drops the upper
32 bits. Also, if the elapsed time (end_ns - start_ns) exceeds roughly 2.14
seconds, the signed 32-bit long will overflow into a negative value, resulting
in negative benchmark times and malformed speedup calculations.

Loading 1000 targets and links could easily exceed 2.14s on slow or emulated
32-bit hosts.

> +	int err, i, j;
> +
> +	start_ns = get_time_ns();
> +
> +	/* An ordinary fentry program is bound to one target when it is loaded. */
[ ... ]
> +	for (i = 0; i < TARGET_CNT; i++) {
> +		link_fds[i] = bpf_link_create(ctx.fentry_prog_fds[i], 0, BPF_TRACE_FENTRY, NULL);
> +		if (link_fds[i] < 0) {

[Severity: Medium]
Will this benchmark predictably fail with EMFILE due to exceeding the default
1024 file descriptor limit?

The benchmark attempts to load 1000 target programs, 1000 fentry programs, and
1000 links (over 3000 FDs total) sequentially without raising RLIMIT_NOFILE in
setup_libbpf() or bench.c. Running this locally without manually increasing
the open file limit via ulimit -n will cause bpf_link_create() or program
loading to fail.

> +			err = errno;
> +			for (j = 0; j < i; j++)
> +				close(link_fds[j]);
> +			fprintf(stderr, "failed to create fentry link %d: %s\n",
> +				i, strerror(err));
> +			exit(1);
> +		}
> +	}
> +	end_ns = get_time_ns();
> +
> +	for (i = 0; i < TARGET_CNT; i++)
> +		close(link_fds[i]);
> +
> +	return end_ns - start_ns;
> +}
[ ... ]
> +static void setup(void)
> +{
> +	double fentry_multi_ms, fentry_ms;
> +	long fentry_multi_ns, fentry_ns;

[Severity: Low]
Similar to the issue in create_and_attach_fentry() above, will these signed
long values overflow and yield negative durations if the elapsed benchmark
time exceeds 2.14 seconds on 32-bit architectures?

> +
> +	prepare();
> +	fentry_ns = create_and_attach_fentry();
> +	fentry_multi_ns = create_and_attach_fentry_multi();

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012154024.29423-1-leon.hwang@linux.dev?part=21