Re: [PATCH bpf-next v5 4/4] selftests/bpf: Add tests for arena fault reporting

[Puranjay Mohan <puranjay@kernel.org>]

Eduard Zingerman <eddyz87@gmail.com> writes:

> On Mon, 2025-09-01 at 19:37 +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..b2a85364e3c4f 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/stream.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/stream.c
>> @@ -41,6 +41,22 @@ struct {
>>  		"([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_arena_read_fault),
>> +		"ERROR: Arena READ access at unmapped address 0x.*\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_arena_write_fault),
>> +		"ERROR: Arena WRITE access at unmapped address 0x.*\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)*",
>> +	},
>
> I commented when prog_tests/stream.c was first introduced but it was
> decided to postpone the change back then.
> It would be nice to have the above expressed in terms similar to
> bpf_misc.h:__msg() macro. E.g. name it __bpf_{stdout,stderr} and
> have something like this in the progs/stream.c:
>
>   SEC("syscall")
>   __success __retval(0)
>   __bpf_stderr("ERROR: Arena WRITE access at unmapped address 0x{{.*}}")

I would prefer naming them __stderr and __stdout

>   __bpf_stderr("CPU: {{[0-9]+}} UID: 0 PID: {{[0-9]+}} Comm: {{.*}}")
>   ...
>   int stream_arena_write_fault(void *ctx)
>   {
> 	...
>   }
>
> Now that more tests are added, what do you think about such extension?

I tried implementing this and it looks nice, so I think we
should have it.
Will add it with the next version.

Thanks,
Puranjay