Merge branch 'selftests/bpf: integrate test_tcp_check_syncookie.sh in…

…to test_progs'

Alexis Lothoré (eBPF Foundation) says:

====================
this series aims to bring test_tcp_check_syncookie.sh scope into
test_progs to make sure that the corresponding tests are also run
automatically in CI. This script tests for bpf_tcp_{gen,check}_syncookie
and bpf_skc_lookup_tcp, in different contexts (ipv4, v6 or dual, and
with tc and xdp programs).
Some other tests like btf_skc_cls_ingress have some overlapping tests with
test_tcp_check_syncookie.sh, so this series moves the missing bits from
test_tcp_check_syncookie.sh into btf_skc_cls_ingress, which is already
integrated into test_progs.
- the first three commits bring some minor improvements to
  btf_skc_cls_ingress without changing its testing scope
- fourth and fifth commits bring test_tcp_check_syncookie.sh features
  into btf_skc_cls_ingress
- last commit removes test_tcp_check_syncookie.sh

The only topic for which I am not sure for this integration is the
necessity or not to run the tests with different program types:
test_tcp_check_syncookie.sh runs tests with both tc and xdp programs, but
btf_skc_cls_ingress currently tests those helpers only with a tc
program. Would it make sense to also make sure that btf_skc_cls_ingress
is tested with all the programs types supported by those helpers ?

The series has been tested both in CI and in a local x86_64 qemu
environment:
  # ./test_progs -a btf_skc_cls_ingress
  torvalds#38/1    btf_skc_cls_ingress/conn_ipv4:OK
  torvalds#38/2    btf_skc_cls_ingress/conn_ipv6:OK
  torvalds#38/3    btf_skc_cls_ingress/conn_dual:OK
  torvalds#38/4    btf_skc_cls_ingress/syncookie_ipv4:OK
  torvalds#38/5    btf_skc_cls_ingress/syncookie_ipv6:OK
  torvalds#38/6    btf_skc_cls_ingress/syncookie_dual:OK
  torvalds#38      btf_skc_cls_ingress:OK
  Summary: 1/6 PASSED, 0 SKIPPED, 0 FAILED

---
Changes in v2:
- fix initial test author mail in Cc
- Fix default cases in switches: indent, action
- remove unneeded initializer
- remove duplicate interface bring-up
- remove unnecessary check and return in bpf program
- Link to v1: https://lore.kernel.org/r/20241016-syncookie-v1-0-3b7a0de12153@bootlin.com
====================

Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>

tools/testing/selftests/bpf/.gitignore

@@ -24,7 +24,6 @@ test_flow_dissector
 flow_dissector_load
 test_tcpnotify_user
 test_libbpf
-test_tcp_check_syncookie_user
 test_sysctl
 xdping
 test_cpp

tools/testing/selftests/bpf/Makefile

@@ -137,7 +137,6 @@ TEST_PROGS := test_kmod.sh \
 	test_xdp_vlan_mode_generic.sh \
 	test_xdp_vlan_mode_native.sh \
 	test_lwt_ip_encap.sh \
-	test_tcp_check_syncookie.sh \
 	test_tc_tunnel.sh \
 	test_tc_edt.sh \
 	test_xdping.sh \
@@ -154,10 +153,9 @@ TEST_PROGS_EXTENDED := with_addr.sh \
 
 # Compile but not part of 'make run_tests'
 TEST_GEN_PROGS_EXTENDED = \
-	flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
-	test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
-	xskxceiver xdp_redirect_multi xdp_synproxy veristat xdp_hw_metadata \
-	xdp_features bpf_test_no_cfi.ko
+	flow_dissector_load test_flow_dissector	test_lirc_mode2_user xdping \
+	test_cpp runqslower bench bpf_testmod.ko xskxceiver xdp_redirect_multi \
+	xdp_synproxy veristat xdp_hw_metadata xdp_features bpf_test_no_cfi.ko
 
 TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi
 
@@ -347,7 +345,6 @@ $(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)
 $(OUTPUT)/test_maps: $(TESTING_HELPERS)
 $(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS) $(UNPRIV_HELPERS)
 $(OUTPUT)/xsk.o: $(BPFOBJ)
-$(OUTPUT)/test_tcp_check_syncookie_user: $(NETWORK_HELPERS)
 
 BPFTOOL ?= $(DEFAULT_BPFTOOL)
 $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile)    \

tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c

@@ -17,104 +17,180 @@
 #include "test_progs.h"
 #include "test_btf_skc_cls_ingress.skel.h"
 
-static struct test_btf_skc_cls_ingress *skel;
-static struct sockaddr_in6 srv_sa6;
-static __u32 duration;
+#define TEST_NS "skc_cls_ingress"
 
-static int prepare_netns(void)
+#define BIT(n)		(1 << (n))
+#define TEST_MODE_IPV4	BIT(0)
+#define TEST_MODE_IPV6	BIT(1)
+#define TEST_MODE_DUAL	(TEST_MODE_IPV4 | TEST_MODE_IPV6)
+
+#define SERVER_ADDR_IPV4	"127.0.0.1"
+#define SERVER_ADDR_IPV6	"::1"
+#define SERVER_ADDR_DUAL	"::0"
+/* RFC791, 576 for minimal IPv4 datagram, minus 40 bytes of TCP header */
+#define MIN_IPV4_MSS		536
+
+static struct netns_obj *prepare_netns(struct test_btf_skc_cls_ingress *skel)
 {
 	LIBBPF_OPTS(bpf_tc_hook, qdisc_lo, .attach_point = BPF_TC_INGRESS);
 	LIBBPF_OPTS(bpf_tc_opts, tc_attach,
 		    .prog_fd = bpf_program__fd(skel->progs.cls_ingress));
+	struct netns_obj *ns = NULL;
 
-	if (CHECK(unshare(CLONE_NEWNET), "create netns",
-		  "unshare(CLONE_NEWNET): %s (%d)",
-		  strerror(errno), errno))
-		return -1;
-
-	if (CHECK(system("ip link set dev lo up"),
-		  "ip link set dev lo up", "failed\n"))
-		return -1;
+	ns = netns_new(TEST_NS, true);
+	if (!ASSERT_OK_PTR(ns, "create and join netns"))
+		return ns;
 
 	qdisc_lo.ifindex = if_nametoindex("lo");
 	if (!ASSERT_OK(bpf_tc_hook_create(&qdisc_lo), "qdisc add dev lo clsact"))
-		return -1;
+		goto free_ns;
 
 	if (!ASSERT_OK(bpf_tc_attach(&qdisc_lo, &tc_attach),
 		       "filter add dev lo ingress"))
-		return -1;
+		goto free_ns;
 
 	/* Ensure 20 bytes options (i.e. in total 40 bytes tcp header) for the
 	 * bpf_tcp_gen_syncookie() helper.
 	 */
 	if (write_sysctl("/proc/sys/net/ipv4/tcp_window_scaling", "1") ||
 	    write_sysctl("/proc/sys/net/ipv4/tcp_timestamps", "1") ||
 	    write_sysctl("/proc/sys/net/ipv4/tcp_sack", "1"))
-		return -1;
+		goto free_ns;
+
+	return ns;
 
-	return 0;
+free_ns:
+	netns_free(ns);
+	return NULL;
 }
 
-static void reset_test(void)
+static void reset_test(struct test_btf_skc_cls_ingress *skel)
 {
+	memset(&skel->bss->srv_sa4, 0, sizeof(skel->bss->srv_sa4));
 	memset(&skel->bss->srv_sa6, 0, sizeof(skel->bss->srv_sa6));
 	skel->bss->listen_tp_sport = 0;
 	skel->bss->req_sk_sport = 0;
 	skel->bss->recv_cookie = 0;
 	skel->bss->gen_cookie = 0;
 	skel->bss->linum = 0;
+	skel->bss->mss = 0;
 }
 
-static void print_err_line(void)
+static void print_err_line(struct test_btf_skc_cls_ingress *skel)
 {
 	if (skel->bss->linum)
 		printf("bpf prog error at line %u\n", skel->bss->linum);
 }
 
-static void test_conn(void)
+static int v6only_true(int fd, void *opts)
+{
+	int mode = true;
+
+	return setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &mode, sizeof(mode));
+}
+
+static int v6only_false(int fd, void *opts)
 {
+	int mode = false;
+
+	return setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &mode, sizeof(mode));
+}
+
+static void run_test(struct test_btf_skc_cls_ingress *skel, bool gen_cookies,
+		     int ip_mode)
+{
+	const char *tcp_syncookies = gen_cookies ? "2" : "1";
 	int listen_fd = -1, cli_fd = -1, srv_fd = -1, err;
-	socklen_t addrlen = sizeof(srv_sa6);
+	struct network_helper_opts opts = { 0 };
+	struct sockaddr_storage *addr;
+	struct sockaddr_in6 srv_sa6;
+	struct sockaddr_in srv_sa4;
+	socklen_t addr_len;
+	int sock_family;
+	char *srv_addr;
 	int srv_port;
 
-	if (write_sysctl("/proc/sys/net/ipv4/tcp_syncookies", "1"))
+	switch (ip_mode) {
+	case TEST_MODE_IPV4:
+		sock_family = AF_INET;
+		srv_addr = SERVER_ADDR_IPV4;
+		addr = (struct sockaddr_storage *)&srv_sa4;
+		addr_len = sizeof(srv_sa4);
+		break;
+	case TEST_MODE_IPV6:
+		opts.post_socket_cb = v6only_true;
+		sock_family = AF_INET6;
+		srv_addr = SERVER_ADDR_IPV6;
+		addr = (struct sockaddr_storage *)&srv_sa6;
+		addr_len = sizeof(srv_sa6);
+		break;
+	case TEST_MODE_DUAL:
+		opts.post_socket_cb = v6only_false;
+		sock_family = AF_INET6;
+		srv_addr = SERVER_ADDR_DUAL;
+		addr = (struct sockaddr_storage *)&srv_sa6;
+		addr_len = sizeof(srv_sa6);
+		break;
+	default:
+		PRINT_FAIL("Unknown IP mode %d", ip_mode);
 		return;
+	}
 
-	listen_fd = start_server(AF_INET6, SOCK_STREAM, "::1", 0, 0);
-	if (CHECK_FAIL(listen_fd == -1))
+	if (write_sysctl("/proc/sys/net/ipv4/tcp_syncookies", tcp_syncookies))
 		return;
 
-	err = getsockname(listen_fd, (struct sockaddr *)&srv_sa6, &addrlen);
-	if (CHECK(err, "getsockname(listen_fd)", "err:%d errno:%d\n", err,
-		  errno))
-		goto done;
-	memcpy(&skel->bss->srv_sa6, &srv_sa6, sizeof(srv_sa6));
-	srv_port = ntohs(srv_sa6.sin6_port);
+	listen_fd = start_server_str(sock_family, SOCK_STREAM, srv_addr,  0,
+				     &opts);
+	if (!ASSERT_OK_FD(listen_fd, "start server"))
+		return;
 
-	cli_fd = connect_to_fd(listen_fd, 0);
-	if (CHECK_FAIL(cli_fd == -1))
+	err = getsockname(listen_fd, (struct sockaddr *)addr, &addr_len);
+	if (!ASSERT_OK(err, "getsockname(listen_fd)"))
 		goto done;
 
-	srv_fd = accept(listen_fd, NULL, NULL);
-	if (CHECK_FAIL(srv_fd == -1))
+	switch (ip_mode) {
+	case TEST_MODE_IPV4:
+		memcpy(&skel->bss->srv_sa4, &srv_sa4, sizeof(srv_sa4));
+		srv_port = ntohs(srv_sa4.sin_port);
+		break;
+	case TEST_MODE_IPV6:
+	case TEST_MODE_DUAL:
+		memcpy(&skel->bss->srv_sa6, &srv_sa6, sizeof(srv_sa6));
+		srv_port = ntohs(srv_sa6.sin6_port);
+		break;
+	default:
 		goto done;
+	}
 
-	if (CHECK(skel->bss->listen_tp_sport != srv_port ||
-		  skel->bss->req_sk_sport != srv_port,
-		  "Unexpected sk src port",
-		  "listen_tp_sport:%u req_sk_sport:%u expected:%u\n",
-		  skel->bss->listen_tp_sport, skel->bss->req_sk_sport,
-		  srv_port))
+	cli_fd = connect_to_fd(listen_fd, 0);
+	if (!ASSERT_OK_FD(cli_fd, "connect client"))
 		goto done;
 
-	if (CHECK(skel->bss->gen_cookie || skel->bss->recv_cookie,
-		  "Unexpected syncookie states",
-		  "gen_cookie:%u recv_cookie:%u\n",
-		  skel->bss->gen_cookie, skel->bss->recv_cookie))
+	srv_fd = accept(listen_fd, NULL, NULL);
+	if (!ASSERT_OK_FD(srv_fd, "accept connection"))
 		goto done;
 
-	CHECK(skel->bss->linum, "bpf prog detected error", "at line %u\n",
-	      skel->bss->linum);
+	ASSERT_EQ(skel->bss->listen_tp_sport, srv_port, "listen tp src port");
+
+	if (!gen_cookies) {
+		ASSERT_EQ(skel->bss->req_sk_sport, srv_port,
+			  "request socket source port with syncookies disabled");
+		ASSERT_EQ(skel->bss->gen_cookie, 0,
+			  "generated syncookie with syncookies disabled");
+		ASSERT_EQ(skel->bss->recv_cookie, 0,
+			  "received syncookie with syncookies disabled");
+	} else {
+		ASSERT_EQ(skel->bss->req_sk_sport, 0,
+			  "request socket source port with syncookies enabled");
+		ASSERT_NEQ(skel->bss->gen_cookie, 0,
+			   "syncookie properly generated");
+		ASSERT_EQ(skel->bss->gen_cookie, skel->bss->recv_cookie,
+			  "matching syncookies on client and server");
+		ASSERT_GT(skel->bss->mss, MIN_IPV4_MSS,
+			  "MSS in cookie min value");
+		ASSERT_LT(skel->bss->mss, USHRT_MAX,
+			  "MSS in cookie max value");
+	}
 
 done:
 	if (listen_fd != -1)
@@ -125,96 +201,74 @@ static void test_conn(void)
 		close(srv_fd);
 }
 
-static void test_syncookie(void)
+static void test_conn_ipv4(struct test_btf_skc_cls_ingress *skel)
 {
-	int listen_fd = -1, cli_fd = -1, srv_fd = -1, err;
-	socklen_t addrlen = sizeof(srv_sa6);
-	int srv_port;
-
-	/* Enforce syncookie mode */
-	if (write_sysctl("/proc/sys/net/ipv4/tcp_syncookies", "2"))
-		return;
-
-	listen_fd = start_server(AF_INET6, SOCK_STREAM, "::1", 0, 0);
-	if (CHECK_FAIL(listen_fd == -1))
-		return;
-
-	err = getsockname(listen_fd, (struct sockaddr *)&srv_sa6, &addrlen);
-	if (CHECK(err, "getsockname(listen_fd)", "err:%d errno:%d\n", err,
-		  errno))
-		goto done;
-	memcpy(&skel->bss->srv_sa6, &srv_sa6, sizeof(srv_sa6));
-	srv_port = ntohs(srv_sa6.sin6_port);
-
-	cli_fd = connect_to_fd(listen_fd, 0);
-	if (CHECK_FAIL(cli_fd == -1))
-		goto done;
-
-	srv_fd = accept(listen_fd, NULL, NULL);
-	if (CHECK_FAIL(srv_fd == -1))
-		goto done;
+	run_test(skel, false, TEST_MODE_IPV4);
+}
 
-	if (CHECK(skel->bss->listen_tp_sport != srv_port,
-		  "Unexpected tp src port",
-		  "listen_tp_sport:%u expected:%u\n",
-		  skel->bss->listen_tp_sport, srv_port))
-		goto done;
+static void test_conn_ipv6(struct test_btf_skc_cls_ingress *skel)
+{
+	run_test(skel, false, TEST_MODE_IPV6);
+}
 
-	if (CHECK(skel->bss->req_sk_sport,
-		  "Unexpected req_sk src port",
-		  "req_sk_sport:%u expected:0\n",
-		   skel->bss->req_sk_sport))
-		goto done;
+static void test_conn_dual(struct test_btf_skc_cls_ingress *skel)
+{
+	run_test(skel, false, TEST_MODE_DUAL);
+}
 
-	if (CHECK(!skel->bss->gen_cookie ||
-		  skel->bss->gen_cookie != skel->bss->recv_cookie,
-		  "Unexpected syncookie states",
-		  "gen_cookie:%u recv_cookie:%u\n",
-		  skel->bss->gen_cookie, skel->bss->recv_cookie))
-		goto done;
+static void test_syncookie_ipv4(struct test_btf_skc_cls_ingress *skel)
+{
+	run_test(skel, true, TEST_MODE_IPV4);
+}
 
-	CHECK(skel->bss->linum, "bpf prog detected error", "at line %u\n",
-	      skel->bss->linum);
+static void test_syncookie_ipv6(struct test_btf_skc_cls_ingress *skel)
+{
+	run_test(skel, true, TEST_MODE_IPV6);
+}
 
-done:
-	if (listen_fd != -1)
-		close(listen_fd);
-	if (cli_fd != -1)
-		close(cli_fd);
-	if (srv_fd != -1)
-		close(srv_fd);
+static void test_syncookie_dual(struct test_btf_skc_cls_ingress *skel)
+{
+	run_test(skel, true, TEST_MODE_DUAL);
 }
 
 struct test {
 	const char *desc;
-	void (*run)(void);
+	void (*run)(struct test_btf_skc_cls_ingress *skel);
 };
 
 #define DEF_TEST(name) { #name, test_##name }
 static struct test tests[] = {
-	DEF_TEST(conn),
-	DEF_TEST(syncookie),
+	DEF_TEST(conn_ipv4),
+	DEF_TEST(conn_ipv6),
+	DEF_TEST(conn_dual),
+	DEF_TEST(syncookie_ipv4),
+	DEF_TEST(syncookie_ipv6),
+	DEF_TEST(syncookie_dual),
 };
 
 void test_btf_skc_cls_ingress(void)
 {
+	struct test_btf_skc_cls_ingress *skel;
+	struct netns_obj *ns;
 	int i;
 
 	skel = test_btf_skc_cls_ingress__open_and_load();
-	if (CHECK(!skel, "test_btf_skc_cls_ingress__open_and_load", "failed\n"))
+	if (!ASSERT_OK_PTR(skel, "test_btf_skc_cls_ingress__open_and_load"))
 		return;
 
 	for (i = 0; i < ARRAY_SIZE(tests); i++) {
 		if (!test__start_subtest(tests[i].desc))
 			continue;
 
-		if (prepare_netns())
+		ns = prepare_netns(skel);
+		if (!ns)
 			break;
 
-		tests[i].run();
+		tests[i].run(skel);
 
-		print_err_line();
-		reset_test();
+		print_err_line(skel);
+		reset_test(skel);
+		netns_free(ns);
 	}
 
 	test_btf_skc_cls_ingress__destroy(skel);