diff mbox series

[bpf] bpf: selftests: Fix racing issue in btf_skc_cls_ingress test

Message ID 20211216191630.466151-1-kafai@fb.com (mailing list archive)
State Accepted
Commit c2fcbf81c332b42382a0c439bfe2414a241e4f5b
Delegated to: BPF
Headers show
Series [bpf] bpf: selftests: Fix racing issue in btf_skc_cls_ingress test | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 7 maintainers not CCed: netdev@vger.kernel.org shuah@kernel.org songliubraving@fb.com john.fastabend@gmail.com kpsingh@kernel.org yhs@fb.com linux-kselftest@vger.kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 50 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf fail VM_Test
bpf/vmtest-bpf-PR fail PR summary

Commit Message

Martin KaFai Lau Dec. 16, 2021, 7:16 p.m. UTC 
The libbpf CI reported occasional failure in btf_skc_cls_ingress:

test_syncookie:FAIL:Unexpected syncookie states gen_cookie:80326634 recv_cookie:0
bpf prog error at line 97

"error at line 97" means the bpf prog cannot find the listening socket
when the final ack is received.  It then skipped processing
the syncookie in the final ack which then led to "recv_cookie:0".

The problem is the userspace program did not do accept() and went ahead
to close(listen_fd) before the kernel (and the bpf prog) had
a chance to process the final ack.

The fix is to add accept() call so that the userspace
will wait for the kernel to finish processing the final
ack first before close()-ing everything.

Fixes: 9a856cae2217 ("bpf: selftest: Add test_btf_skc_cls_ingress")
Reported-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 .../bpf/prog_tests/btf_skc_cls_ingress.c         | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Comments

patchwork-bot+netdevbpf@kernel.org Dec. 16, 2021, 8:43 p.m. UTC | #1 
Hello:

This patch was applied to bpf/bpf.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Thu, 16 Dec 2021 11:16:30 -0800 you wrote:
> The libbpf CI reported occasional failure in btf_skc_cls_ingress:
> 
> test_syncookie:FAIL:Unexpected syncookie states gen_cookie:80326634 recv_cookie:0
> bpf prog error at line 97
> 
> "error at line 97" means the bpf prog cannot find the listening socket
> when the final ack is received.  It then skipped processing
> the syncookie in the final ack which then led to "recv_cookie:0".
> 
> [...]

Here is the summary with links:
  - [bpf] bpf: selftests: Fix racing issue in btf_skc_cls_ingress test
    https://git.kernel.org/bpf/bpf/c/c2fcbf81c332

You are awesome, thank you!
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c
index 762f6a9da8b5..664ffc0364f4 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c
@@ -90,7 +90,7 @@  static void print_err_line(void)
 
 static void test_conn(void)
 {
-	int listen_fd = -1, cli_fd = -1, err;
+	int listen_fd = -1, cli_fd = -1, srv_fd = -1, err;
 	socklen_t addrlen = sizeof(srv_sa6);
 	int srv_port;
 
@@ -112,6 +112,10 @@  static void test_conn(void)
 	if (CHECK_FAIL(cli_fd == -1))
 		goto done;
 
+	srv_fd = accept(listen_fd, NULL, NULL);
+	if (CHECK_FAIL(srv_fd == -1))
+		goto done;
+
 	if (CHECK(skel->bss->listen_tp_sport != srv_port ||
 		  skel->bss->req_sk_sport != srv_port,
 		  "Unexpected sk src port",
@@ -134,11 +138,13 @@  static void test_conn(void)
 		close(listen_fd);
 	if (cli_fd != -1)
 		close(cli_fd);
+	if (srv_fd != -1)
+		close(srv_fd);
 }
 
 static void test_syncookie(void)
 {
-	int listen_fd = -1, cli_fd = -1, err;
+	int listen_fd = -1, cli_fd = -1, srv_fd = -1, err;
 	socklen_t addrlen = sizeof(srv_sa6);
 	int srv_port;
 
@@ -161,6 +167,10 @@  static void test_syncookie(void)
 	if (CHECK_FAIL(cli_fd == -1))
 		goto done;
 
+	srv_fd = accept(listen_fd, NULL, NULL);
+	if (CHECK_FAIL(srv_fd == -1))
+		goto done;
+
 	if (CHECK(skel->bss->listen_tp_sport != srv_port,
 		  "Unexpected tp src port",
 		  "listen_tp_sport:%u expected:%u\n",
@@ -188,6 +198,8 @@  static void test_syncookie(void)
 		close(listen_fd);
 	if (cli_fd != -1)
 		close(cli_fd);
+	if (srv_fd != -1)
+		close(srv_fd);
 }
 
 struct test {