Messages in this thread | | | Subject | Re: [PATCH bpf-next v2] selftests/bpf: fix incorrect fcntl call (test_sockmap.c) | From | Daniel Borkmann <> | Date | Mon, 26 Sep 2022 17:54:19 +0200 |
| |
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@gmail.com> wrote: >>> Alexei Starovoitov wrote: >>>> On Wed, Aug 24, 2022 at 7:11 PM Qiao Ma <mqaio@linux.alibaba.com> 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@linux.alibaba.com> >>>>> --- >>>>> 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
| |