bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Sitnicki <jakub@cloudflare.com>
To: bpf@vger.kernel.org
Cc: netdev@vger.kernel.org, kernel-team@cloudflare.com,
	Daniel Borkmann <daniel@iogearbox.net>,
	John Fastabend <john.fastabend@gmail.com>
Subject: [PATCH bpf-next 3/3] selftests/bpf: Test unhashing kTLS socket after removing from map
Date: Mon, 17 Feb 2020 12:15:30 +0000	[thread overview]
Message-ID: <20200217121530.754315-4-jakub@cloudflare.com> (raw)
In-Reply-To: <20200217121530.754315-1-jakub@cloudflare.com>

When a TCP socket gets inserted into a sockmap, its sk_prot callbacks get
replaced with tcp_bpf callbacks built from regular tcp callbacks. If TLS
gets enabled on the same socket, sk_prot callbacks get replaced once again,
this time with kTLS callbacks built from tcp_bpf callbacks.

Now, we allow removing a socket from a sockmap that has kTLS enabled. After
removal, socket remains with kTLS configured. This is where things things
get tricky.

Since the socket has a set of sk_prot callbacks that are a mix of kTLS and
tcp_bpf callbacks, we need to restore just the tcp_bpf callbacks to the
original ones. At the moment, it comes down to the the unhash operation.

We had a regression recently because tcp_bpf callbacks were not cleared in
this particular scenario of removing a kTLS socket from a sockmap. It got
fixed in commit 4da6a196f93b ("bpf: Sockmap/tls, during free we may call
tcp_bpf_unhash() in loop").

Add a test that triggers the regression so that we don't reintroduce it in
the future.

Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
---
 .../selftests/bpf/prog_tests/sockmap_ktls.c   | 123 ++++++++++++++++++
 1 file changed, 123 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c b/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c
new file mode 100644
index 000000000000..589b50c91b96
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2020 Cloudflare
+/*
+ * Tests for sockmap/sockhash holding kTLS sockets.
+ */
+
+#include "test_progs.h"
+
+#define MAX_TEST_NAME 80
+
+static int tcp_server(int family)
+{
+	int err, s;
+
+	s = socket(family, SOCK_STREAM, 0);
+	if (CHECK_FAIL(s == -1)) {
+		perror("socket");
+		return -1;
+	}
+
+	err = listen(s, SOMAXCONN);
+	if (CHECK_FAIL(err)) {
+		perror("listen");
+		return -1;
+	}
+
+	return s;
+}
+
+static int disconnect(int fd)
+{
+	struct sockaddr unspec = { AF_UNSPEC };
+
+	return connect(fd, &unspec, sizeof(unspec));
+}
+
+/* Disconnect (unhash) a kTLS socket after removing it from sockmap. */
+static void test_sockmap_ktls_disconnect_after_delete(int family, int map)
+{
+	struct sockaddr_storage addr = {0};
+	socklen_t len = sizeof(addr);
+	int err, cli, srv, zero = 0;
+
+	srv = tcp_server(family);
+	if (srv == -1)
+		return;
+
+	err = getsockname(srv, (struct sockaddr *)&addr, &len);
+	if (CHECK_FAIL(err)) {
+		perror("getsockopt");
+		goto close_srv;
+	}
+
+	cli = socket(family, SOCK_STREAM, 0);
+	if (CHECK_FAIL(cli == -1)) {
+		perror("socket");
+		goto close_srv;
+	}
+
+	err = connect(cli, (struct sockaddr *)&addr, len);
+	if (CHECK_FAIL(err)) {
+		perror("connect");
+		goto close_cli;
+	}
+
+	err = bpf_map_update_elem(map, &zero, &cli, 0);
+	if (CHECK_FAIL(err)) {
+		perror("bpf_map_update_elem");
+		goto close_cli;
+	}
+
+	err = setsockopt(cli, IPPROTO_TCP, TCP_ULP, "tls", strlen("tls"));
+	if (CHECK_FAIL(err)) {
+		perror("setsockopt(TCP_ULP)");
+		goto close_cli;
+	}
+
+	err = bpf_map_delete_elem(map, &zero);
+	if (CHECK_FAIL(err)) {
+		perror("bpf_map_delete_elem");
+		goto close_cli;
+	}
+
+	err = disconnect(cli);
+	if (CHECK_FAIL(err))
+		perror("disconnect");
+
+close_cli:
+	close(cli);
+close_srv:
+	close(srv);
+}
+
+static void run_tests(int family, enum bpf_map_type map_type)
+{
+	char test_name[MAX_TEST_NAME];
+	int map;
+
+	map = bpf_create_map(map_type, sizeof(int), sizeof(int), 1, 0);
+	if (CHECK_FAIL(map == -1)) {
+		perror("bpf_map_create");
+		return;
+	}
+
+	snprintf(test_name, MAX_TEST_NAME,
+		 "sockmap_ktls disconnect_after_delete %s %s",
+		 family == AF_INET ? "IPv4" : "IPv6",
+		 map_type == BPF_MAP_TYPE_SOCKMAP ? "SOCKMAP" : "SOCKHASH");
+	if (!test__start_subtest(test_name))
+		return;
+
+	test_sockmap_ktls_disconnect_after_delete(family, map);
+
+	close(map);
+}
+
+void test_sockmap_ktls(void)
+{
+	run_tests(AF_INET, BPF_MAP_TYPE_SOCKMAP);
+	run_tests(AF_INET, BPF_MAP_TYPE_SOCKHASH);
+	run_tests(AF_INET6, BPF_MAP_TYPE_SOCKMAP);
+	run_tests(AF_INET6, BPF_MAP_TYPE_SOCKHASH);
+}
-- 
2.24.1


  parent reply	other threads:[~2020-02-17 12:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-17 12:15 [PATCH bpf-next 0/3] sockmap/ktls: Simplify how we restore sk_prot callbacks Jakub Sitnicki
2020-02-17 12:15 ` [PATCH bpf-next 1/3] bpf, sk_msg: Let ULP restore sk_proto and write_space callback Jakub Sitnicki
2020-02-19  6:09   ` John Fastabend
2020-02-17 12:15 ` [PATCH bpf-next 2/3] bpf, sk_msg: Don't clear saved sock proto on restore Jakub Sitnicki
2020-02-19  6:19   ` John Fastabend
2020-02-17 12:15 ` Jakub Sitnicki [this message]
2020-02-19  6:22   ` [PATCH bpf-next 3/3] selftests/bpf: Test unhashing kTLS socket after removing from map John Fastabend
2020-02-19 17:36 ` [PATCH bpf-next 0/3] sockmap/ktls: Simplify how we restore sk_prot callbacks Daniel Borkmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200217121530.754315-4-jakub@cloudflare.com \
    --to=jakub@cloudflare.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=john.fastabend@gmail.com \
    --cc=kernel-team@cloudflare.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).