Re: [RFC PATCH bpf-next v3 2/2] selftests/bpf: add tests for selective kmod BTF loading
Fuyu Zhao <[email protected]>
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On 8/19/2026 5:12 PM, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Medium] The custom libbpf print callback silently drops all non-matching libbpf logs, hiding crucial diagnostic information if the test fails. > -- > > --- Patch [2]: [RFC PATCH bpf-next v3 2/2] selftests/bpf: add tests for selective kmod BTF loading --- > commit 17f3a2340dd7be0947e3baabf30a8c7722ec91fe > Author: Fuyu Zhao <[email protected]> > > selftests/bpf: add tests for selective kmod BTF loading > > This commit adds tests for the selective kmod BTF loading feature. It > covers scenarios such as valid modules, non-existent modules, duplicate > names, unneeded modules, and missing count properties. > >> diff --git a/tools/testing/selftests/bpf/prog_tests/kmod_btfs.c b/tools/testing/selftests/bpf/prog_tests/kmod_btfs.c >> new file mode 100644 >> index 0000000000000..61d2845a731ff >> --- /dev/null >> +++ b/tools/testing/selftests/bpf/prog_tests/kmod_btfs.c >> @@ -0,0 +1,196 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +#include <test_progs.h> >> +#include "kmod_btfs.skel.h" > > [ ... ] > >> +static int libbpf_print_cb(enum libbpf_print_level level, const char *fmt, >> + va_list args) >> +{ >> + if (!strcmp(fmt, btf_skip_fmt)) { >> + if (!strcmp(va_arg(args, char *), "bpf_test_no_cfi")) >> + btf_skipped = true; >> + } else if (!strcmp(fmt, dup_ignore_fmt)) { >> + if (!strcmp(va_arg(args, char *), "bpf_testmod")) >> + dup_ignored = true; >> + } >> + >> + return 0; >> +} > > [Severity: Medium] > Does this unconditionally swallow all other libbpf debug, info, and error logs? > > Since old_print_cb is stored as a local variable in the subtests (like > kmod_btfs_dup and kmod_btfs_skip) rather than globally, this callback has no > way to forward unmatched logs to the original handler. If a BPF program fails > to load during these tests, will the underlying verifier or libbpf errors be > completely hidden, making CI failures harder to debug? > I don't consider this a problem since the test runs serially and the callback only affects the kmod_btfs test. However, I can forward unmatched logs to the original callback through old_print_cb to preserve the original logging behavior.