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 <[email protected]> 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/[email protected]?part=21
