From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 531A080BE0 for ; Wed, 17 Apr 2024 08:14:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713341680; cv=none; b=Qslr7EdEedfCKJ0In+NHD/KD/iZEOnbW+baxM9aZ2Hu6BU4As82k/798VBSZ1RFs3xfsFhIJAwLyMNe2e8gEkijb84eBa1+jnxW/jb0233bo3zvYLhVuORgaT85FT6VQXqdqfzI6qI4dtHH9vQdpLpqJWsqtfST9nrpqeDwyZ0c= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713341680; c=relaxed/simple; bh=IkUqyaRgQgMXzADsv4EzlRn6kcDKXTLdvqhGH+8RBp4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Aws18j2QfB33/q+6oCINSElc/zhTB+OF6vVb6Pl53Z9sHwxoIgEMQXKIkMQWVdClqILlAJtz+2HfWAD5nWtvCWAHg6zOPpuo91lLU9LgVPXxTixgIhJXDm0JYkgPZ4+qV6NwoJaXAF4oeotZuDDbrlk39iCvgG8WHrf2pIq5HWc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=LrCARCl3; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="LrCARCl3" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0646FC2BD10; Wed, 17 Apr 2024 08:14:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1713341679; bh=IkUqyaRgQgMXzADsv4EzlRn6kcDKXTLdvqhGH+8RBp4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=LrCARCl3UWgxyoAK2NmfILZD/eLwCgSHFqDXB7/p9cl8FWd6Sv95rsFyRiHElyHJZ C2QPDolYtucxfwpfgcQUV54MQBf0FMSkrn4hB5t3vIami2aIckHVVAP8hPaeuDj94w ZZP/RxjhMxoa7gWCUaX77Fc46pO3Opnj/v4AXWZl8MQYEeI7kPUVbKmqyY/pszH9VF FKOoTkg9W/wHpNRXg9QW2nF3t3+7UjLBPKHDZN9sKOPgcvL2xUO25Z6KidfoTmj15Y x5ncTkSMOnKcWzZsncmwiuPFAlcBOHY8bSi4PTL2UrgqcPeIHlU/v/zSSLvaXt6y8K iyrjzDSbARwXQ== Date: Wed, 17 Apr 2024 16:14:28 +0800 From: Geliang Tang To: Martin KaFai Lau Cc: Andrii Nakryiko , Eduard Zingerman , Mykola Lysenko , Alexei Starovoitov , Daniel Borkmann , Song Liu , Yonghong Song , John Fastabend , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa , Shuah Khan , Jakub Sitnicki , bpf@vger.kernel.org, Geliang Tang Subject: Re: [PATCH bpf v4 1/2] selftests/bpf: Add F_SETFL for fcntl in test_sockmap Message-ID: References: Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="1xliXYEKyRZdwTar" Content-Disposition: inline In-Reply-To: --1xliXYEKyRZdwTar Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Martin, On Thu, Apr 11, 2024 at 11:10:49AM -0700, Martin KaFai Lau wrote: > On 4/8/24 10:18 PM, Geliang Tang wrote: > > From: Geliang Tang > > > > 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. > > > > In nonblock mode, if EWOULDBLOCK is received, continue receiving, otherwise > > some subtests of test_sockmap fail. > > > > Fixes: 16962b2404ac ("bpf: sockmap, add selftests") > > Signed-off-by: Geliang Tang > > Acked-by: Yonghong Song > > --- > > tools/testing/selftests/bpf/test_sockmap.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c > > index 024a0faafb3b..4feed253fca2 100644 > > --- a/tools/testing/selftests/bpf/test_sockmap.c > > +++ b/tools/testing/selftests/bpf/test_sockmap.c > > @@ -603,7 +603,9 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt, > > struct timeval timeout; > > fd_set w; > > - fcntl(fd, fd_flags); > > + if (fcntl(fd, F_SETFL, fd_flags)) > > + goto out_errno; > > + > > /* 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 > > @@ -678,6 +680,7 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt, > > perror("recv failed()"); > > goto out_errno; > > } > > + continue; > > From looking at it again, there is a select() earlier, so it should not hit > EWOULDBLOCK. Can the patch in the attachment be accepted? It can work, but I'm not sure if it has changed the behavior of this test. Anyway, I would like to hear your opinion. Thanks, -Geliang > > Patch 2 looks good. Only patch 2 is applied. Thanks. > > > } > > s->bytes_recvd += recv; --1xliXYEKyRZdwTar Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-selftests-bpf-Add-F_SETFL-for-fcntl-in-test_sockmap.patch" >From d20ac7e06d9e869094f452d8c2dcdc316508dcc8 Mon Sep 17 00:00:00 2001 Message-Id: From: Geliang Tang Date: Wed, 3 Apr 2024 16:08:21 +0800 Subject: [PATCH] selftests/bpf: Add F_SETFL for fcntl in test_sockmap 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. In nonblock mode, if EWOULDBLOCK is received, continue receiving, otherwise some subtests of test_sockmap fail. Fixes: 16962b2404ac ("bpf: sockmap, add selftests") Signed-off-by: Geliang Tang Acked-by: Yonghong Song --- tools/testing/selftests/bpf/test_sockmap.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c index 024a0faafb3b..8130f465afb9 100644 --- a/tools/testing/selftests/bpf/test_sockmap.c +++ b/tools/testing/selftests/bpf/test_sockmap.c @@ -603,7 +603,9 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt, struct timeval timeout; fd_set w; - fcntl(fd, fd_flags); + if (fcntl(fd, F_SETFL, fd_flags)) + goto out_errno; + /* 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 @@ -1531,10 +1533,10 @@ static void test_txmsg_skb(int cgrp, struct sockmap_options *opt) txmsg_ktls_skb_drop = 1; test_exec(cgrp, opt); - txmsg_ktls_skb_drop = 0; txmsg_ktls_skb_redir = 1; test_exec(cgrp, opt); txmsg_ktls_skb_redir = 0; + txmsg_ktls_skb_drop = 0; /* Tests that omit skb_parser */ txmsg_omit_skb_parser = 1; -- 2.40.1 --1xliXYEKyRZdwTar--