Merge branch 'use network helpers, part 4'

Geliang Tang says:

====================
From: Geliang Tang <tanggeliang@kylinos.cn>

This patchset adds post_socket_cb pointer into
struct network_helper_opts to make start_server_addr() helper
more flexible. With these modifications, many duplicate codes
can be dropped.

Patches 1-3 address Martin's comments in the previous series.
====================
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>

tools/testing/selftests/bpf/Makefile

@@ -310,6 +310,7 @@ $(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/network_helpers.c

@@ -81,9 +81,8 @@ int settimeo(int fd, int timeout_ms)
 #define save_errno_close(fd) ({ int __save = errno; close(fd); errno = __save; })
 
 static int __start_server(int type, const struct sockaddr *addr, socklen_t addrlen,
-			  bool reuseport, const struct network_helper_opts *opts)
+			  const struct network_helper_opts *opts)
 {
-	int on = 1;
 	int fd;
 
 	fd = socket(addr->sa_family, type, opts->proto);
@@ -95,9 +94,8 @@ static int __start_server(int type, const struct sockaddr *addr, socklen_t addrl
 	if (settimeo(fd, opts->timeout_ms))
 		goto error_close;
 
-	if (reuseport &&
-	    setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &on, sizeof(on))) {
-		log_err("Failed to set SO_REUSEPORT");
+	if (opts->post_socket_cb && opts->post_socket_cb(fd, NULL)) {
+		log_err("Failed to call post_socket_cb");
 		goto error_close;
 	}
 
@@ -132,7 +130,14 @@ int start_server(int family, int type, const char *addr_str, __u16 port,
 	if (make_sockaddr(family, addr_str, port, &addr, &addrlen))
 		return -1;
 
-	return __start_server(type, (struct sockaddr *)&addr, addrlen, false, &opts);
+	return __start_server(type, (struct sockaddr *)&addr, addrlen, &opts);
+}
+
+static int reuseport_cb(int fd, const struct post_socket_opts *opts)
+{
+	int on = 1;
+
+	return setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &on, sizeof(on));
 }
 
 int *start_reuseport_server(int family, int type, const char *addr_str,
@@ -140,6 +145,7 @@ int *start_reuseport_server(int family, int type, const char *addr_str,
 {
 	struct network_helper_opts opts = {
 		.timeout_ms = timeout_ms,
+		.post_socket_cb = reuseport_cb,
 	};
 	struct sockaddr_storage addr;
 	unsigned int nr_fds = 0;
@@ -156,7 +162,7 @@ int *start_reuseport_server(int family, int type, const char *addr_str,
 	if (!fds)
 		return NULL;
 
-	fds[0] = __start_server(type, (struct sockaddr *)&addr, addrlen, true, &opts);
+	fds[0] = __start_server(type, (struct sockaddr *)&addr, addrlen, &opts);
 	if (fds[0] == -1)
 		goto close_fds;
 	nr_fds = 1;
@@ -165,7 +171,7 @@ int *start_reuseport_server(int family, int type, const char *addr_str,
 		goto close_fds;
 
 	for (; nr_fds < nr_listens; nr_fds++) {
-		fds[nr_fds] = __start_server(type, (struct sockaddr *)&addr, addrlen, true, &opts);
+		fds[nr_fds] = __start_server(type, (struct sockaddr *)&addr, addrlen, &opts);
 		if (fds[nr_fds] == -1)
 			goto close_fds;
 	}
@@ -183,7 +189,7 @@ int start_server_addr(int type, const struct sockaddr_storage *addr, socklen_t l
 	if (!opts)
 		opts = &default_opts;
 
-	return __start_server(type, (struct sockaddr *)addr, len, 0, opts);
+	return __start_server(type, (struct sockaddr *)addr, len, opts);
 }
 
 void free_fds(int *fds, unsigned int nr_close_fds)

tools/testing/selftests/bpf/network_helpers.h

@@ -21,6 +21,8 @@ typedef __u16 __sum16;
 #define VIP_NUM 5
 #define MAGIC_BYTES 123
 
+struct post_socket_opts {};
+
 struct network_helper_opts {
 	const char *cc;
 	int timeout_ms;
@@ -28,6 +30,7 @@ struct network_helper_opts {
 	bool noconnect;
 	int type;
 	int proto;
+	int (*post_socket_cb)(int fd, const struct post_socket_opts *opts);
 };
 
 /* ipv4 test vector */

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

 // SPDX-License-Identifier: GPL-2.0
 #include <test_progs.h>
 #include "cgroup_helpers.h"
+#include "network_helpers.h"
 
 #include "sockopt_inherit.skel.h"
 
@@ -9,35 +10,6 @@
 #define CUSTOM_INHERIT2			1
 #define CUSTOM_LISTENER			2
 
-static int connect_to_server(int server_fd)
-{
-	struct sockaddr_storage addr;
-	socklen_t len = sizeof(addr);
-	int fd;
-
-	fd = socket(AF_INET, SOCK_STREAM, 0);
-	if (fd < 0) {
-		log_err("Failed to create client socket");
-		return -1;
-	}
-
-	if (getsockname(server_fd, (struct sockaddr *)&addr, &len)) {
-		log_err("Failed to get server addr");
-		goto out;
-	}
-
-	if (connect(fd, (const struct sockaddr *)&addr, len) < 0) {
-		log_err("Fail to connect to server");
-		goto out;
-	}
-
-	return fd;
-
-out:
-	close(fd);
-	return -1;
-}
-
 static int verify_sockopt(int fd, int optname, const char *msg, char expected)
 {
 	socklen_t optlen = 1;
@@ -98,47 +70,36 @@ static void *server_thread(void *arg)
 	return (void *)(long)err;
 }
 
-static int start_server(void)
+static int custom_cb(int fd, const struct post_socket_opts *opts)
 {
-	struct sockaddr_in addr = {
-		.sin_family = AF_INET,
-		.sin_addr.s_addr = htonl(INADDR_LOOPBACK),
-	};
 	char buf;
 	int err;
-	int fd;
 	int i;
 
-	fd = socket(AF_INET, SOCK_STREAM, 0);
-	if (fd < 0) {
-		log_err("Failed to create server socket");
-		return -1;
-	}
-
 	for (i = CUSTOM_INHERIT1; i <= CUSTOM_LISTENER; i++) {
 		buf = 0x01;
 		err = setsockopt(fd, SOL_CUSTOM, i, &buf, 1);
 		if (err) {
 			log_err("Failed to call setsockopt(%d)", i);
-			close(fd);
 			return -1;
 		}
 	}
 
-	if (bind(fd, (const struct sockaddr *)&addr, sizeof(addr)) < 0) {
-		log_err("Failed to bind socket");
-		close(fd);
-		return -1;
-	}
-
-	return fd;
+	return 0;
 }
 
 static void run_test(int cgroup_fd)
 {
 	struct bpf_link *link_getsockopt = NULL;
 	struct bpf_link *link_setsockopt = NULL;
+	struct network_helper_opts opts = {
+		.post_socket_cb = custom_cb,
+	};
 	int server_fd = -1, client_fd;
+	struct sockaddr_in addr = {
+		.sin_family = AF_INET,
+		.sin_addr.s_addr = htonl(INADDR_LOOPBACK),
+	};
 	struct sockopt_inherit *obj;
 	void *server_err;
 	pthread_t tid;
@@ -160,7 +121,8 @@ static void run_test(int cgroup_fd)
 	if (!ASSERT_OK_PTR(link_setsockopt, "cg-attach-setsockopt"))
 		goto close_bpf_object;
 
-	server_fd = start_server();
+	server_fd = start_server_addr(SOCK_STREAM, (struct sockaddr_storage *)&addr,
+				      sizeof(addr), &opts);
 	if (!ASSERT_GE(server_fd, 0, "start_server"))
 		goto close_bpf_object;
 
@@ -173,7 +135,7 @@ static void run_test(int cgroup_fd)
 	pthread_cond_wait(&server_started, &server_started_mtx);
 	pthread_mutex_unlock(&server_started_mtx);
 
-	client_fd = connect_to_server(server_fd);
+	client_fd = connect_to_fd(server_fd, 0);
 	if (!ASSERT_GE(client_fd, 0, "connect_to_server"))
 		goto close_server_fd;
 

tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c

@@ -16,68 +16,7 @@
 #include <bpf/libbpf.h>
 
 #include "cgroup_helpers.h"
-
-static int start_server(const struct sockaddr *addr, socklen_t len, bool dual)
-{
-	int mode = !dual;
-	int fd;
-
-	fd = socket(addr->sa_family, SOCK_STREAM, 0);
-	if (fd == -1) {
-		log_err("Failed to create server socket");
-		goto out;
-	}
-
-	if (addr->sa_family == AF_INET6) {
-		if (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (char *)&mode,
-			       sizeof(mode)) == -1) {
-			log_err("Failed to set the dual-stack mode");
-			goto close_out;
-		}
-	}
-
-	if (bind(fd, addr, len) == -1) {
-		log_err("Failed to bind server socket");
-		goto close_out;
-	}
-
-	if (listen(fd, 128) == -1) {
-		log_err("Failed to listen on server socket");
-		goto close_out;
-	}
-
-	goto out;
-
-close_out:
-	close(fd);
-	fd = -1;
-out:
-	return fd;
-}
-
-static int connect_to_server(const struct sockaddr *addr, socklen_t len)
-{
-	int fd = -1;
-
-	fd = socket(addr->sa_family, SOCK_STREAM, 0);
-	if (fd == -1) {
-		log_err("Failed to create client socket");
-		goto out;
-	}
-
-	if (connect(fd, (const struct sockaddr *)addr, len) == -1) {
-		log_err("Fail to connect to server");
-		goto close_out;
-	}
-
-	goto out;
-
-close_out:
-	close(fd);
-	fd = -1;
-out:
-	return fd;
-}
+#include "network_helpers.h"
 
 static int get_map_fd_by_prog_id(int prog_id, bool *xdp)
 {
@@ -117,8 +56,7 @@ static int get_map_fd_by_prog_id(int prog_id, bool *xdp)
 	return map_fd;
 }
 
-static int run_test(int server_fd, int results_fd, bool xdp,
-		    const struct sockaddr *addr, socklen_t len)
+static int run_test(int server_fd, int results_fd, bool xdp)
 {
 	int client = -1, srv_client = -1;
 	int ret = 0;
@@ -144,7 +82,7 @@ static int run_test(int server_fd, int results_fd, bool xdp,
 		goto err;
 	}
 
-	client = connect_to_server(addr, len);
+	client = connect_to_fd(server_fd, 0);
 	if (client == -1)
 		goto err;
 
@@ -201,23 +139,23 @@ static int run_test(int server_fd, int results_fd, bool xdp,
 	return ret;
 }
 
-static bool get_port(int server_fd, in_port_t *port)
+static int v6only_true(int fd, const struct post_socket_opts *opts)
 {
-	struct sockaddr_in addr;
-	socklen_t len = sizeof(addr);
+	int mode = true;
 
-	if (getsockname(server_fd, (struct sockaddr *)&addr, &len)) {
-		log_err("Failed to get server addr");
-		return false;
-	}
+	return setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &mode, sizeof(mode));
+}
+
+static int v6only_false(int fd, const struct post_socket_opts *opts)
+{
+	int mode = false;
 
-	/* sin_port and sin6_port are located at the same offset. */
-	*port = addr.sin_port;
-	return true;
+	return setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &mode, sizeof(mode));
 }
 
 int main(int argc, char **argv)
 {
+	struct network_helper_opts opts = { 0 };
 	struct sockaddr_in addr4;
 	struct sockaddr_in6 addr6;
 	struct sockaddr_in addr4dual;
@@ -259,31 +197,30 @@ int main(int argc, char **argv)
 	addr6dual.sin6_addr = in6addr_any;
 	addr6dual.sin6_port = 0;
 
-	server = start_server((const struct sockaddr *)&addr4, sizeof(addr4),
-			      false);
-	if (server == -1 || !get_port(server, &addr4.sin_port))
+	server = start_server_addr(SOCK_STREAM, (struct sockaddr_storage *)&addr4,
+				   sizeof(addr4), NULL);
+	if (server == -1)
 		goto err;
 
-	server_v6 = start_server((const struct sockaddr *)&addr6,
-				 sizeof(addr6), false);
-	if (server_v6 == -1 || !get_port(server_v6, &addr6.sin6_port))
+	opts.post_socket_cb = v6only_true;
+	server_v6 = start_server_addr(SOCK_STREAM, (struct sockaddr_storage *)&addr6,
+				      sizeof(addr6), &opts);
+	if (server_v6 == -1)
 		goto err;
 
-	server_dual = start_server((const struct sockaddr *)&addr6dual,
-				   sizeof(addr6dual), true);
-	if (server_dual == -1 || !get_port(server_dual, &addr4dual.sin_port))
+	opts.post_socket_cb = v6only_false;
+	server_dual = start_server_addr(SOCK_STREAM, (struct sockaddr_storage *)&addr6dual,
+					sizeof(addr6dual), &opts);
+	if (server_dual == -1)
 		goto err;
 
-	if (run_test(server, results, xdp,
-		     (const struct sockaddr *)&addr4, sizeof(addr4)))
+	if (run_test(server, results, xdp))
 		goto err;
 
-	if (run_test(server_v6, results, xdp,
-		     (const struct sockaddr *)&addr6, sizeof(addr6)))
+	if (run_test(server_v6, results, xdp))
 		goto err;
 
-	if (run_test(server_dual, results, xdp,
-		     (const struct sockaddr *)&addr4dual, sizeof(addr4dual)))
+	if (run_test(server_dual, results, xdp))
 		goto err;
 
 	printf("ok\n");