public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
* [LTP] [PATCH] epoll_pwait06: Skip BPF map file descriptor
@ 2025-09-23 15:48 Martin Doucha
  2025-09-24  8:56 ` Cyril Hrubis
  2025-09-26 14:07 ` Cyril Hrubis
  0 siblings, 2 replies; 9+ messages in thread
From: Martin Doucha @ 2025-09-23 15:48 UTC (permalink / raw)
  To: ltp

Older kernels don't support waiting for BPF map file descriptors using
epoll. Skip the subtest, other file descriptor types are sufficient.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---

epoll_ctl(EPOLL_CTL_ADD) returns EPERM for BPF map file descriptor
on kernel v4.12.

 testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c b/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c
index 3bedc2cf5..d47327bed 100644
--- a/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c
+++ b/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c
@@ -36,6 +36,7 @@ static void run(void)
 		case TST_FD_DIR:
 		case TST_FD_DEV_ZERO:
 		case TST_FD_PROC_MAPS:
+		case TST_FD_BPF_MAP:
 		case TST_FD_FSOPEN:
 		case TST_FD_FSPICK:
 		case TST_FD_OPEN_TREE:
-- 
2.51.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [LTP] [PATCH] epoll_pwait06: Skip BPF map file descriptor
  2025-09-23 15:48 [LTP] [PATCH] epoll_pwait06: Skip BPF map file descriptor Martin Doucha
@ 2025-09-24  8:56 ` Cyril Hrubis
  2025-09-24 11:06   ` Martin Doucha
  2025-09-26 14:07 ` Cyril Hrubis
  1 sibling, 1 reply; 9+ messages in thread
From: Cyril Hrubis @ 2025-09-24  8:56 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

Hi!
> Older kernels don't support waiting for BPF map file descriptors using
> epoll. Skip the subtest, other file descriptor types are sufficient.
> 
> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> ---
> 
> epoll_ctl(EPOLL_CTL_ADD) returns EPERM for BPF map file descriptor
> on kernel v4.12.
> 
>  testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c b/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c
> index 3bedc2cf5..d47327bed 100644
> --- a/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c
> +++ b/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c
> @@ -36,6 +36,7 @@ static void run(void)
>  		case TST_FD_DIR:
>  		case TST_FD_DEV_ZERO:
>  		case TST_FD_PROC_MAPS:
> +		case TST_FD_BPF_MAP:
>  		case TST_FD_FSOPEN:
>  		case TST_FD_FSPICK:
>  		case TST_FD_OPEN_TREE:

Can we make this kernel version dependent? I do not like disabling tests
that work on newer kernels just because it does not work on something
that is eight years old.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [LTP] [PATCH] epoll_pwait06: Skip BPF map file descriptor
  2025-09-24  8:56 ` Cyril Hrubis
@ 2025-09-24 11:06   ` Martin Doucha
  2025-09-24 11:52     ` Cyril Hrubis
  0 siblings, 1 reply; 9+ messages in thread
From: Martin Doucha @ 2025-09-24 11:06 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

On 9/24/25 10:56, Cyril Hrubis wrote:
> Hi!
>> diff --git a/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c b/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c
>> index 3bedc2cf5..d47327bed 100644
>> --- a/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c
>> +++ b/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c
>> @@ -36,6 +36,7 @@ static void run(void)
>>   		case TST_FD_DIR:
>>   		case TST_FD_DEV_ZERO:
>>   		case TST_FD_PROC_MAPS:
>> +		case TST_FD_BPF_MAP:
>>   		case TST_FD_FSOPEN:
>>   		case TST_FD_FSPICK:
>>   		case TST_FD_OPEN_TREE:
> 
> Can we make this kernel version dependent? I do not like disabling tests
> that work on newer kernels just because it does not work on something
> that is eight years old.

I like kernel version checks even less. I could call epoll_ctl() 
directly without the safe macro instead and check for EPERM. That's the 
appropriate feature check.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [LTP] [PATCH] epoll_pwait06: Skip BPF map file descriptor
  2025-09-24 11:06   ` Martin Doucha
@ 2025-09-24 11:52     ` Cyril Hrubis
  2025-09-24 11:59       ` Martin Doucha
  0 siblings, 1 reply; 9+ messages in thread
From: Cyril Hrubis @ 2025-09-24 11:52 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

Hi!
> >> diff --git a/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c b/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c
> >> index 3bedc2cf5..d47327bed 100644
> >> --- a/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c
> >> +++ b/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c
> >> @@ -36,6 +36,7 @@ static void run(void)
> >>   		case TST_FD_DIR:
> >>   		case TST_FD_DEV_ZERO:
> >>   		case TST_FD_PROC_MAPS:
> >> +		case TST_FD_BPF_MAP:
> >>   		case TST_FD_FSOPEN:
> >>   		case TST_FD_FSPICK:
> >>   		case TST_FD_OPEN_TREE:
> > 
> > Can we make this kernel version dependent? I do not like disabling tests
> > that work on newer kernels just because it does not work on something
> > that is eight years old.
> 
> I like kernel version checks even less. I could call epoll_ctl() 
> directly without the safe macro instead and check for EPERM. That's the 
> appropriate feature check.

That does not work either. We had patches that were misapplied and broke
kernel so that it wrongly returned error instead of the expected
operation. Just checking for EPERM would silence such bugs.

In the end I came to a conclusion that the only way how to make sure
things are not broken is to expect that certain functionality is present
either on CONFIG_ options or if that is not possible on kernel version.
It's ugly but that's how things are.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [LTP] [PATCH] epoll_pwait06: Skip BPF map file descriptor
  2025-09-24 11:52     ` Cyril Hrubis
@ 2025-09-24 11:59       ` Martin Doucha
  2025-09-24 12:08         ` Cyril Hrubis
  0 siblings, 1 reply; 9+ messages in thread
From: Martin Doucha @ 2025-09-24 11:59 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

On 9/24/25 13:52, Cyril Hrubis wrote:
> Hi!
>> I like kernel version checks even less. I could call epoll_ctl()
>> directly without the safe macro instead and check for EPERM. That's the
>> appropriate feature check.
> 
> That does not work either. We had patches that were misapplied and broke
> kernel so that it wrongly returned error instead of the expected
> operation. Just checking for EPERM would silence such bugs.
> 
> In the end I came to a conclusion that the only way how to make sure
> things are not broken is to expect that certain functionality is present
> either on CONFIG_ options or if that is not possible on kernel version.
> It's ugly but that's how things are.

I'll accept EPERM only for the file descriptor types which are now 
unconditionally skipped. The only file descriptor type which could then 
get skipped incorrectly will be BPF. But that's not a problem because 
verifying epoll support is out of scope of this test. The primary 
purpose is to verify that small epoll_pwait() timeouts won't get 
misinterpreted as infinity. In theory, verifying that on a single file 
descriptor type should be sufficient.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [LTP] [PATCH] epoll_pwait06: Skip BPF map file descriptor
  2025-09-24 11:59       ` Martin Doucha
@ 2025-09-24 12:08         ` Cyril Hrubis
  2025-09-24 13:06           ` Martin Doucha
  0 siblings, 1 reply; 9+ messages in thread
From: Cyril Hrubis @ 2025-09-24 12:08 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

Hi!
> I'll accept EPERM only for the file descriptor types which are now 
> unconditionally skipped. The only file descriptor type which could then 
> get skipped incorrectly will be BPF. But that's not a problem because 
> verifying epoll support is out of scope of this test. The primary 
> purpose is to verify that small epoll_pwait() timeouts won't get 
> misinterpreted as infinity. In theory, verifying that on a single file 
> descriptor type should be sufficient.

Ah, I had no idea that this is a regression test for a generic epoll
code, it never occured to me that we run something like that for all
types of file descriptors. For that test it makes sense to just skip
unsupported fds.

I guess that we are actually mixing two tests and it would make sense to
separate these two. I.e. one that is a regression test for the epoll bug
and second, probably called epoll_ctl06.c that would hammer the
EPOLL_CTL_ADD with all kinds of fds and expect either success or error.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [LTP] [PATCH] epoll_pwait06: Skip BPF map file descriptor
  2025-09-24 12:08         ` Cyril Hrubis
@ 2025-09-24 13:06           ` Martin Doucha
  2025-09-24 13:16             ` Cyril Hrubis
  0 siblings, 1 reply; 9+ messages in thread
From: Martin Doucha @ 2025-09-24 13:06 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

On 9/24/25 14:08, Cyril Hrubis wrote:
> Hi!
>> I'll accept EPERM only for the file descriptor types which are now
>> unconditionally skipped. The only file descriptor type which could then
>> get skipped incorrectly will be BPF. But that's not a problem because
>> verifying epoll support is out of scope of this test. The primary
>> purpose is to verify that small epoll_pwait() timeouts won't get
>> misinterpreted as infinity. In theory, verifying that on a single file
>> descriptor type should be sufficient.
> 
> Ah, I had no idea that this is a regression test for a generic epoll
> code, it never occured to me that we run something like that for all
> types of file descriptors. For that test it makes sense to just skip
> unsupported fds.
> 
> I guess that we are actually mixing two tests and it would make sense to
> separate these two. I.e. one that is a regression test for the epoll bug
> and second, probably called epoll_ctl06.c that would hammer the
> EPOLL_CTL_ADD with all kinds of fds and expect either success or error.

Sure, we can add another test for epoll_ctl() later. I wrote 
epoll_pwait06 because I actually ran into the timeout regression while 
playing a game at home. I decided to loop over multiple file descriptor 
types because it's simple enough and provides better coverage for edge 
cases.

Now, should I resubmit with that EPERM check or is the one-line fix good 
enough as is?

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [LTP] [PATCH] epoll_pwait06: Skip BPF map file descriptor
  2025-09-24 13:06           ` Martin Doucha
@ 2025-09-24 13:16             ` Cyril Hrubis
  0 siblings, 0 replies; 9+ messages in thread
From: Cyril Hrubis @ 2025-09-24 13:16 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

Hi!
> Sure, we can add another test for epoll_ctl() later. I wrote 
> epoll_pwait06 because I actually ran into the timeout regression while 
> playing a game at home. I decided to loop over multiple file descriptor 
> types because it's simple enough and provides better coverage for edge 
> cases.
> 
> Now, should I resubmit with that EPERM check or is the one-line fix good 
> enough as is?

Let's go with that for the release as long as you promise to add more
tests later.

For the original patch:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [LTP] [PATCH] epoll_pwait06: Skip BPF map file descriptor
  2025-09-23 15:48 [LTP] [PATCH] epoll_pwait06: Skip BPF map file descriptor Martin Doucha
  2025-09-24  8:56 ` Cyril Hrubis
@ 2025-09-26 14:07 ` Cyril Hrubis
  1 sibling, 0 replies; 9+ messages in thread
From: Cyril Hrubis @ 2025-09-26 14:07 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-09-26 14:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-23 15:48 [LTP] [PATCH] epoll_pwait06: Skip BPF map file descriptor Martin Doucha
2025-09-24  8:56 ` Cyril Hrubis
2025-09-24 11:06   ` Martin Doucha
2025-09-24 11:52     ` Cyril Hrubis
2025-09-24 11:59       ` Martin Doucha
2025-09-24 12:08         ` Cyril Hrubis
2025-09-24 13:06           ` Martin Doucha
2025-09-24 13:16             ` Cyril Hrubis
2025-09-26 14:07 ` Cyril Hrubis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox