Re: [PATCH bpf-next] selftests/bpf: Add F_SETFL for fcntl

[Jakub Sitnicki <jakub@cloudflare.com>]

Hi Geliang,

On Wed, Apr 03, 2024 at 04:32 PM +08, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Incorrect arguments are passed to fcntl() in test_sockmap.c when invoking
> it to set file status flags. If O_NONBLOCK is used as 2nd argument and
> passed into fcntl, -EINVAL will be returned (See do_fcntl() in fs/fcntl.c).
> The correct approach is to use F_SETFL as 2nd argument, and O_NONBLOCK as
> 3rd one.
>
> Fixes: 16962b2404ac ("bpf: sockmap, add selftests")
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  tools/testing/selftests/bpf/test_sockmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
> index 024a0faafb3b..34d6a1e6f664 100644
> --- a/tools/testing/selftests/bpf/test_sockmap.c
> +++ b/tools/testing/selftests/bpf/test_sockmap.c
> @@ -603,7 +603,7 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
>  		struct timeval timeout;
>  		fd_set w;
>  
> -		fcntl(fd, fd_flags);
> +		fcntl(fd, F_SETFL, fd_flags);
>  		/* Account for pop bytes noting each iteration of apply will
>  		 * call msg_pop_data helper so we need to account for this
>  		 * by calculating the number of apply iterations. Note user

Good catch. But we also need to figure out why some tests failing with
this patch applied and fix them in one go:

# 6/ 7  sockmap::txmsg test skb:FAIL
#21/ 7 sockhash::txmsg test skb:FAIL
#36/ 7 sockhash:ktls:txmsg test skb:FAIL
Pass: 42 Fail: 3

I'm seeing this error message when running `test_sockmap`:

detected skb data error with skb ingress update @iov[0]:0 "00 00 00 00" != "PASS"
data verify msg failed: Unknown error -5
rx thread exited with err 1.

I'd also:
- add an error check for fnctl, so we don't regress,
- get rid of fd_flags, pass O_NONBLOCK flag directly to fnctl.

Thanks,
-jkbs