On Mon, 2025-09-08 at 16:36 +0000, Puranjay Mohan wrote:
[...]
> diff --git a/tools/testing/selftests/bpf/prog_tests/stream.c b/tools/testing/selftests/bpf/prog_tests/stream.c
> index 9d0e5d93edee7..61ab1da9b189b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/stream.c
> +++ b/tools/testing/selftests/bpf/prog_tests/stream.c
> @@ -18,29 +18,9 @@ void test_stream_success(void)
> return;
> }
>
> -struct {
> - int prog_off;
> - const char *errstr;
> -} stream_error_arr[] = {
> - {
> - offsetof(struct stream, progs.stream_cond_break),
> - "ERROR: Timeout detected for may_goto instruction\n"
> - "CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
> - "Call trace:\n"
> - "([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
> - "|[ \t]+[^\n]+\n)*",
> - },
> - {
> - offsetof(struct stream, progs.stream_deadlock),
> - "ERROR: AA or ABBA deadlock detected for bpf_res_spin_lock\n"
> - "Attempted lock = (0x[0-9a-fA-F]+)\n"
> - "Total held locks = 1\n"
> - "Held lock\\[ 0\\] = \\1\n" // Lock address must match
> - "CPU: [0-9]+ UID: 0 PID: [0-9]+ Comm: .*\n"
> - "Call trace:\n"
> - "([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
> - "|[ \t]+[^\n]+\n)*",
> - },
Nit: maybe put an update of the old tests to a separate commit?
> +int prog_off[] = {
> + offsetof(struct stream, progs.stream_arena_read_fault),
> + offsetof(struct stream, progs.stream_arena_write_fault),
> };
>
> static int match_regex(const char *pattern, const char *string)
> @@ -56,34 +36,33 @@ static int match_regex(const char *pattern, const char *string)
> return rc == 0 ? 1 : 0;
> }
>
> -void test_stream_errors(void)
> +void test_stream_arena_fault_address(void)
> {
> LIBBPF_OPTS(bpf_test_run_opts, opts);
> LIBBPF_OPTS(bpf_prog_stream_read_opts, ropts);
> struct stream *skel;
> int ret, prog_fd;
> char buf[1024];
> + char fault_addr[64];
>
> skel = stream__open_and_load();
> if (!ASSERT_OK_PTR(skel, "stream__open_and_load"))
> return;
>
> - for (int i = 0; i < ARRAY_SIZE(stream_error_arr); i++) {
> + for (int i = 0; i < ARRAY_SIZE(prog_off); i++) {
Nit: start a sub-test for each i?
> struct bpf_program **prog;
>
> - prog = (struct bpf_program **)(((char *)skel) + stream_error_arr[i].prog_off);
> + prog = (struct bpf_program **)(((char *)skel) + prog_off[i]);
> prog_fd = bpf_program__fd(*prog);
> ret = bpf_prog_test_run_opts(prog_fd, &opts);
> ASSERT_OK(ret, "ret");
> ASSERT_OK(opts.retval, "retval");
>
> -#if !defined(__x86_64__) && !defined(__s390x__) && !defined(__aarch64__)
> - ASSERT_TRUE(1, "Timed may_goto unsupported, skip.");
> - if (i == 0) {
> - ret = bpf_prog_stream_read(prog_fd, 2, buf, sizeof(buf), &ropts);
> - ASSERT_EQ(ret, 0, "stream read");
> - continue;
> - }
> +#if !defined(__x86_64__) && !defined(__aarch64__)
> + ASSERT_TRUE(1, "Arena fault reporting unsupported, skip.");
> + ret = bpf_prog_stream_read(prog_fd, 2, buf, sizeof(buf), &ropts);
> + ASSERT_EQ(ret, 0, "stream read");
> + continue;
> #endif
Nit: move this `#if !defined` to the beginning of the function and add
test__skip() call there?
>
> ret = bpf_prog_stream_read(prog_fd, BPF_STREAM_STDERR, buf, sizeof(buf), &ropts);
> @@ -91,9 +70,13 @@ void test_stream_errors(void)
> ASSERT_LE(ret, 1023, "len for buf");
> buf[ret] = '\0';
>
> - ret = match_regex(stream_error_arr[i].errstr, buf);
> - if (!ASSERT_TRUE(ret == 1, "regex match"))
> + sprintf(fault_addr, "0x%lx", skel->bss->fault_addr);
> + ret = match_regex(fault_addr, buf);
> +
> + if (!ASSERT_TRUE(ret == 1, "regex match")) {
> fprintf(stderr, "Output from stream:\n%s\n", buf);
> + fprintf(stderr, "Fault Addr: 0x%lx\n", skel->bss->fault_addr);
> + }
> }
>
> stream__destroy(skel);
