Re: [PATCH bpf-next v2] selftests/bpf: fix incorrect fcntl call (test_sockmap.c)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/22/22 8:46 PM, John Fastabend wrote:
Andrii Nakryiko wrote:
On Fri, Aug 26, 2022 at 9:24 AM John Fastabend <john.fastabend@xxxxxxxxx> wrote:
Alexei Starovoitov wrote:
On Wed, Aug 24, 2022 at 7:11 PM Qiao Ma <mqaio@xxxxxxxxxxxxxxxxx> wrote:

In test_sockmap.c, the testcase sets socket nonblock first, and then
calls select() and recvmsg() to receive data.
If some error occur, nonblock setting will make recvmsg() return
immediately, rather than blocking forever.

However, the way to call fcntl() to set nonblock is wrong.
To set socket noblock, we need to use
fcntl(fd, F_SETFL, O_NONBLOCK);
rather than:
fcntl(fd, O_NONBLOCK);

Signed-off-by: Qiao Ma <mqaio@xxxxxxxxxxxxxxxxx>
---
  tools/testing/selftests/bpf/test_sockmap.c | 7 ++++++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
index 0fbaccdc8861..abb4102f33b0 100644
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -598,7 +598,12 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
                 struct timeval timeout;
                 fd_set w;

-               fcntl(fd, fd_flags);
+               err = fcntl(fd, F_SETFL, fd_flags);
+               if (err < 0) {
+                       perror("fcntl failed");
+                       goto out_errno;
+               }
+

John, Jakub,

Please review this.
Unfortunately test_sockmap (and sockmap kernel) is broken
before and after this patch,
so I'm hesitant to apply it not to make thing harder to debug.
Here is what I see:
# ./test_sockmap

[...]

and test_sockmap 'hangs' (or doing something for long time) after
#31/ 6 sockhash:ktls:txmsg test drop:OK

Thanks for spotting I'll take a look.

Friendly ping. John, did you get a chance to look at this? This patch
is still marked as "Needs ACK" in Patchworks.

Yep thanks. We are tracking a couple fixes internally around this so should
have something pop out soon. I think we want the fix and test to go in at
the same time.

Ok, I'll mark it as 'awaiting upstream' assuming that you carry this fix forward
together with your series then.

Thanks,
Daniel



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux