> diff --git a/tools/testing/selftests/bpf/prog_tests/bpftool_ringbuf.c
> b/tools/testing/selftests/bpf/prog_tests/bpftool_ringbuf.c
> new file mode 100644
> index 000000000000..1a4ae88c05a0
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/bpftool_ringbuf.c
> @@ -0,0 +1,425 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <ctype.h>
> +#include <fcntl.h>
> +#include <poll.h>
> +#include <signal.h>
> +#include <sys/mman.h>
> +#include <sys/wait.h>
> +#include <test_progs.h>
> +#include "bpftool_ringbuf.skel.h"
> +
> +#define WAIT_STEPS 500
> +#define WAIT_US 10000
> +
> +struct consumer {
> + pid_t pid;
> + int fd;
> +};
> +
> +static const char *bpftool_path(void)
> +{
> + const char *path = getenv("BPFTOOL");
> +
> + if (path)
> + return access(path, X_OK) ? NULL : path;
> + if (!access("./tools/sbin/bpftool", X_OK))
> + return "./tools/sbin/bpftool";
> + if (!access("../tools/sbin/bpftool", X_OK))
> + return "../tools/sbin/bpftool";
> + return NULL;
> +}
Does bpftool_path() need to duplicate the path discovery logic that
already exists in bpftool_helpers.c?
tools/testing/selftests/bpf/bpftool_helpers.c:detect_bpftool_path()
performs the identical three-step search:
bpftool_helpers.c:detect_bpftool_path() {
...
#define BPFTOOL_DEFAULT_PATH "tools/sbin/bpftool"
env_path = getenv("BPFTOOL");
if (env_path && access(env_path, X_OK) == 0) ...
snprintf(tmp, sizeof(tmp), "./%s", BPFTOOL_DEFAULT_PATH);
if (access(tmp, X_OK) == 0) ...
snprintf(tmp, sizeof(tmp), "../s", BPFTOOL_DEFAULT_PATH);
if (access(tmp, X_OK) == 0) ...
}
The existing implementation is already linked into test_progs via
TRUNNER_EXTRA_SOURCES, and it includes diagnostics that this copy drops:
"bpftool '%s' doesn't exist or is not executable" and "Failed to detect
bpftool path, use BPFTOOL env var to override", plus comments explaining
why the '../' fallback exists (for running non-default test_progs
flavors like no_alu32 or cpuv4).
The public API (run_bpftool_command/get_bpftool_command_output) uses
popen(), which doesn't work here since this test needs fork/exec with a
controllable child pid for kill/waitpid and a readable fd for
poll/read. Could the existing detect_bpftool_path() be exported in
bpftool_helpers.h to provide just the path lookup without the popen
machinery?
> +
> +static void consumer_cleanup(struct consumer *child)
> +{
> + if (child->pid > 0) {
> + kill(child->pid, SIGKILL);
> + while (waitpid(child->pid, NULL, 0) < 0 && errno == EINTR)
> + ;
> + child->pid = -1;
> + }
> + if (child->fd >= 0) {
> + close(child->fd);
> + child->fd = -1;
> + }
> +}
> +
> +static bool consumer_start(struct consumer *child, int map_fd, const char
> *format,
> + const char *pin_path, const char *option,
> + bool pair, bool capture_errors)
> +{
This isn't a bug, but would a small options struct make the call sites
more readable than the trailing positional booleans?
Most call sites pass NULL and false for the last few parameters:
consumer_start(&child, fd, format, pinned ? pin_path : NULL,
NULL, false, false)
consumer_start(&child, bpf_map__fd(skel->maps.perfbuf), NULL,
NULL, NULL, false, false)
At these call sites nothing indicates which NULL is option and which
false is pair versus capture_errors. Only the reject path ever passes
non-NULL option or true pair/capture_errors, so those flags could be
isolated (perhaps a wrapper for the reject case) so the common call
sites don't have to carry them.
[ ... ]
> diff --git a/tools/testing/selftests/bpf/progs/bpftool_ringbuf.c
> b/tools/testing/selftests/bpf/progs/bpftool_ringbuf.c
> new file mode 100644
> index 000000000000..bbbc93c30a83
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/bpftool_ringbuf.c
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34557186664
