From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-171.mta1.migadu.com (out-171.mta1.migadu.com [95.215.58.171]) (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 3889410E6 for ; Thu, 18 Apr 2024 00:29:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.171 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713400147; cv=none; b=ue9oMO7wTI6KQi+VHvn0sFdEE/bytw0FgMkYs+Ulby6VKwKqywfrPD0rQkdj2WadmPZuH4zQ28fjYOdK/APl4ZQkE6sOyzJnBmeNUbCWKFRZ7/JkjJ9wfF/6VOT0FMVbRrNR4Bq9CWpTWRu3iw+W2kxDIUQw/F+BtjAqOJP6Y0M= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713400147; c=relaxed/simple; bh=GdggoZ1K60FrBpgzx76J4MEqArzYmkNfQKJR2YwNJ+E=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=gCNyQEtixBQDgOAhQmZf0oHUg3H0QBGnxpg6dRCUv2QjxW7VfwdBxXkUImdkxlvZv8U78WsjAgOpRdi2J9Lh4amOHjpuvnCX8D6ccn6v/kkprOb+YMjmtsT+dFmccrbrdI0Wxe8Yuz1c+aLkOFfZ0Y6f+9vw4l6/yy++QcDRzuQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=QuBm9MOT; arc=none smtp.client-ip=95.215.58.171 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="QuBm9MOT" Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1713400144; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=UfHNlhqRN7By/vmcB62J/IvapY2Y6zwl/VeN5q7bk08=; b=QuBm9MOTxx7wh8bAZC0nUNqpBFOifJvkES3LY/KhExjGmWDKIGxbwz+06VQcLsoD9VqdEv 35LAR/8z6LEAcwHacElaCbuF5TR0c+3taQA067F/abUQgvh8IiLs5oaZ0nlToIEbtQRDK6 C/H20xtMWM5AWfnDOz+kb7gGK3yUPBM= Date: Wed, 17 Apr 2024 17:28:58 -0700 Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH bpf v4 1/2] selftests/bpf: Add F_SETFL for fcntl in test_sockmap To: Geliang Tang , Jakub Sitnicki , John Fastabend Cc: Andrii Nakryiko , Eduard Zingerman , Mykola Lysenko , Alexei Starovoitov , Daniel Borkmann , Song Liu , Yonghong Song , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa , Shuah Khan , bpf@vger.kernel.org References: Content-Language: en-US X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Martin KaFai Lau In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT On 4/17/24 1:14 AM, Geliang Tang wrote: > 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. I don't know what is the correct expectation also. John and JakubS, can you take a look?