[PATCH bpf-next 0/6] use network helpers, part 4

[PATCH bpf-next 0/6] use network helpers, part 4

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patchset adds post_socket_cb pointer together with 'struct
post_socket_opts cb_opts' 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.

Geliang Tang (6):
  selftests/bpf: Add post_socket_cb for network_helper_opts
  selftests/bpf: Use start_server_addr in sockopt_inherit
  selftests/bpf: Use start_server_addr in test_tcp_check_syncookie
  selftests/bpf: Use connect_to_fd in sockopt_inherit
  selftests/bpf: Use connect_to_fd in test_tcp_check_syncookie
  selftests/bpf: Drop get_port in test_tcp_check_syncookie

 tools/testing/selftests/bpf/Makefile          |   1 +
 tools/testing/selftests/bpf/network_helpers.c |  25 ++--
 tools/testing/selftests/bpf/network_helpers.h |   4 +
 .../bpf/prog_tests/sockopt_inherit.c          |  63 ++--------
 .../bpf/test_tcp_check_syncookie_user.c       | 117 ++++--------------
 5 files changed, 61 insertions(+), 149 deletions(-)

-- 
2.43.0

[PATCH bpf-next 1/6] selftests/bpf: Add post_socket_cb for network_helper_opts

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

__start_server() sets SO_REUSPORT through setsockopt() when the parameter
'reuseport' is set. This patch makes it more flexible by adding a function
pointer post_socket_cb, together with 'struct post_socket_opts cb_opts'
for future extension into struct network_helper_opts. Then 'reuseport'
parameter can be dropped.

Now the original start_reuseport_server() can be implemented by setting a
newly defined reuseport_cb() function pointer to post_socket_cb filed of
struct network_helper_opts.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/bpf/network_helpers.c | 25 ++++++++++++-------
 tools/testing/selftests/bpf/network_helpers.h |  4 +++
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index 054d26e383e0..a7ab05baedb6 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/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,9 @@ 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, &opts->cb_opts)) {
+		log_err("Failed to call post_socket_cb");
 		goto error_close;
 	}
 
@@ -132,7 +131,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 +146,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 +163,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 +172,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 +190,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)
diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
index c62b54daa914..887075fbf6ec 100644
--- a/tools/testing/selftests/bpf/network_helpers.h
+++ b/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,8 @@ struct network_helper_opts {
 	bool noconnect;
 	int type;
 	int proto;
+	int (*post_socket_cb)(int fd, const struct post_socket_opts *opts);
+	struct post_socket_opts cb_opts;
 };
 
 /* ipv4 test vector */
-- 
2.43.0

Re: [PATCH bpf-next 1/6] selftests/bpf: Add post_socket_cb for network_helper_opts

[Martin KaFai Lau]

On 5/5/24 4:35 AM, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> __start_server() sets SO_REUSPORT through setsockopt() when the parameter
> 'reuseport' is set. This patch makes it more flexible by adding a function
> pointer post_socket_cb, together with 'struct post_socket_opts cb_opts'
> for future extension into struct network_helper_opts. Then 'reuseport'
> parameter can be dropped.
> 
> Now the original start_reuseport_server() can be implemented by setting a
> newly defined reuseport_cb() function pointer to post_socket_cb filed of
> struct network_helper_opts.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>   tools/testing/selftests/bpf/network_helpers.c | 25 ++++++++++++-------
>   tools/testing/selftests/bpf/network_helpers.h |  4 +++
>   2 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> index 054d26e383e0..a7ab05baedb6 100644
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/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,9 @@ 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, &opts->cb_opts)) {
> +		log_err("Failed to call post_socket_cb");
>   		goto error_close;
>   	}
>   
> @@ -132,7 +131,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 +146,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 +163,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 +172,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 +190,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)
> diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
> index c62b54daa914..887075fbf6ec 100644
> --- a/tools/testing/selftests/bpf/network_helpers.h
> +++ b/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,8 @@ struct network_helper_opts {
>   	bool noconnect;
>   	int type;
>   	int proto;
> +	int (*post_socket_cb)(int fd, const struct post_socket_opts *opts);
> +	struct post_socket_opts cb_opts;

I am going to remove cb_opts from network_helper_opts for now. Lets wait for the 
first use case of post_socket_opts comes up. NULL is always passed to 
post_socket_cb for now.

>   };
>   
>   /* ipv4 test vector */

[PATCH bpf-next 2/6] selftests/bpf: Use start_server_addr in sockopt_inherit

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Include network_helpers.h in prog_tests/sockopt_inherit.c, use public
helper start_server_addr() instead of the local defined function
start_server(). This can avoid duplicate code.

Add a helper custom_cb() to set SOL_CUSTOM sockopt looply, set it to
post_socket_cb pointer of struct network_helper_opts, and pass it to
start_server_addr().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../bpf/prog_tests/sockopt_inherit.c          | 32 +++++++------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
index 917f486db826..ff0694ef5286 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <test_progs.h>
 #include "cgroup_helpers.h"
+#include "network_helpers.h"
 
 #include "sockopt_inherit.skel.h"
 
@@ -98,23 +99,12 @@ 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);
@@ -125,20 +115,21 @@ static int start_server(void)
 		}
 	}
 
-	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 +151,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;
 
-- 
2.43.0

Re: [PATCH bpf-next 2/6] selftests/bpf: Use start_server_addr in sockopt_inherit

[Martin KaFai Lau]

On 5/5/24 4:35 AM, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Include network_helpers.h in prog_tests/sockopt_inherit.c, use public
> helper start_server_addr() instead of the local defined function
> start_server(). This can avoid duplicate code.
> 
> Add a helper custom_cb() to set SOL_CUSTOM sockopt looply, set it to
> post_socket_cb pointer of struct network_helper_opts, and pass it to
> start_server_addr().
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>   .../bpf/prog_tests/sockopt_inherit.c          | 32 +++++++------------
>   1 file changed, 12 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
> index 917f486db826..ff0694ef5286 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
> @@ -1,6 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0
>   #include <test_progs.h>
>   #include "cgroup_helpers.h"
> +#include "network_helpers.h"
>   
>   #include "sockopt_inherit.skel.h"
>   
> @@ -98,23 +99,12 @@ 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);

There is a close(fd) on the error case a few lines below (not in this diff 
context). The caller (network_helpers.c) will also close it. I removed the 
close(fd) from the custom_cb() here.

Thanks for the patches. Applied.

> @@ -125,20 +115,21 @@ static int start_server(void)
>   		}
>   	}
>   
> -	if (bind(fd, (const struct sockaddr *)&addr, sizeof(addr)) < 0) {
> -		log_err("Failed to bind socket");
> -		close(fd);
> -		return -1;
> -	}
> -
> -	return fd;
> +	return 0;
>   }
>   

[PATCH bpf-next 3/6] selftests/bpf: Use start_server_addr in test_tcp_check_syncookie

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Include network_helpers.h in test_tcp_check_syncookie_user.c, use
public helper start_server_addr() in it instead of the local defined
function start_server(). This can avoid duplicate code.

Add two helpers v6only_true() and v6only_false() to set IPV6_V6ONLY
sockopt to true or false, set them to post_socket_cb pointer of struct
network_helper_opts, and pass it to start_server_setsockopt().

In order to use functions defined in network_helpers.c, Makefile needs
to be updated too.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/bpf/Makefile          |  1 +
 .../bpf/test_tcp_check_syncookie_user.c       | 68 +++++++------------
 2 files changed, 25 insertions(+), 44 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 82247aeef857..b90c718218ae 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -297,6 +297,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)    \
diff --git a/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c b/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
index 32df93747095..bf60bc267cbc 100644
--- a/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
+++ b/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
@@ -16,44 +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;
-}
+#include "network_helpers.h"
 
 static int connect_to_server(const struct sockaddr *addr, socklen_t len)
 {
@@ -216,8 +179,23 @@ static bool get_port(int server_fd, in_port_t *port)
 	return true;
 }
 
+static int v6only_true(int fd, const struct post_socket_opts *opts)
+{
+	int mode = true;
+
+	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;
+
+	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,18 +237,20 @@ 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);
+	server = start_server_addr(SOCK_STREAM, (struct sockaddr_storage *)&addr4,
+				   sizeof(addr4), NULL);
 	if (server == -1 || !get_port(server, &addr4.sin_port))
 		goto err;
 
-	server_v6 = start_server((const struct sockaddr *)&addr6,
-				 sizeof(addr6), false);
+	opts.post_socket_cb = v6only_true;
+	server_v6 = start_server_addr(SOCK_STREAM, (struct sockaddr_storage *)&addr6,
+				      sizeof(addr6), &opts);
 	if (server_v6 == -1 || !get_port(server_v6, &addr6.sin6_port))
 		goto err;
 
-	server_dual = start_server((const struct sockaddr *)&addr6dual,
-				   sizeof(addr6dual), true);
+	opts.post_socket_cb = v6only_false;
+	server_dual = start_server_addr(SOCK_STREAM, (struct sockaddr_storage *)&addr6dual,
+					sizeof(addr6dual), &opts);
 	if (server_dual == -1 || !get_port(server_dual, &addr4dual.sin_port))
 		goto err;
 
-- 
2.43.0

[PATCH bpf-next 4/6] selftests/bpf: Use connect_to_fd in sockopt_inherit

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch uses public helper connect_to_fd() exported in network_helpers.h
instead of the local defined function connect_to_server() in
prog_tests/sockopt_inherit.c. This can avoid duplicate code.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../bpf/prog_tests/sockopt_inherit.c          | 31 +------------------
 1 file changed, 1 insertion(+), 30 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
index ff0694ef5286..17a015a34d15 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
@@ -10,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;
@@ -165,7 +136,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;
 
-- 
2.43.0

[PATCH bpf-next 5/6] selftests/bpf: Use connect_to_fd in test_tcp_check_syncookie

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch uses public helper connect_to_fd() exported in network_helpers.h
instead of the local defined function connect_to_server() in
test_tcp_check_syncookie_user.c. This can avoid duplicate code.

Then the arguments "addr" and "len" of run_test() become useless, drop them
too.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../bpf/test_tcp_check_syncookie_user.c       | 38 +++----------------
 1 file changed, 5 insertions(+), 33 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c b/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
index bf60bc267cbc..b928474f3bf9 100644
--- a/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
+++ b/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
@@ -18,30 +18,6 @@
 #include "cgroup_helpers.h"
 #include "network_helpers.h"
 
-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;
-}
-
 static int get_map_fd_by_prog_id(int prog_id, bool *xdp)
 {
 	struct bpf_prog_info info = {};
@@ -80,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;
@@ -107,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;
 
@@ -254,16 +229,13 @@ int main(int argc, char **argv)
 	if (server_dual == -1 || !get_port(server_dual, &addr4dual.sin_port))
 		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");
-- 
2.43.0

[PATCH bpf-next 6/6] selftests/bpf: Drop get_port in test_tcp_check_syncookie

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

The arguments "addr" and "len" of run_test() have dropped. This makes
function get_port() useless. Drop it from test_tcp_check_syncookie_user.c.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../bpf/test_tcp_check_syncookie_user.c       | 21 +++----------------
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c b/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
index b928474f3bf9..7b5fc98838cd 100644
--- a/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
+++ b/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
@@ -139,21 +139,6 @@ 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)
-{
-	struct sockaddr_in addr;
-	socklen_t len = sizeof(addr);
-
-	if (getsockname(server_fd, (struct sockaddr *)&addr, &len)) {
-		log_err("Failed to get server addr");
-		return false;
-	}
-
-	/* sin_port and sin6_port are located at the same offset. */
-	*port = addr.sin_port;
-	return true;
-}
-
 static int v6only_true(int fd, const struct post_socket_opts *opts)
 {
 	int mode = true;
@@ -214,19 +199,19 @@ int main(int argc, char **argv)
 
 	server = start_server_addr(SOCK_STREAM, (struct sockaddr_storage *)&addr4,
 				   sizeof(addr4), NULL);
-	if (server == -1 || !get_port(server, &addr4.sin_port))
+	if (server == -1)
 		goto err;
 
 	opts.post_socket_cb = v6only_true;
 	server_v6 = start_server_addr(SOCK_STREAM, (struct sockaddr_storage *)&addr6,
 				      sizeof(addr6), &opts);
-	if (server_v6 == -1 || !get_port(server_v6, &addr6.sin6_port))
+	if (server_v6 == -1)
 		goto err;
 
 	opts.post_socket_cb = v6only_false;
 	server_dual = start_server_addr(SOCK_STREAM, (struct sockaddr_storage *)&addr6dual,
 					sizeof(addr6dual), &opts);
-	if (server_dual == -1 || !get_port(server_dual, &addr4dual.sin_port))
+	if (server_dual == -1)
 		goto err;
 
 	if (run_test(server, results, xdp))
-- 
2.43.0

Re: [PATCH bpf-next 0/6] use network helpers, part 4

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Martin KaFai Lau <martin.lau@kernel.org>:

On Sun,  5 May 2024 19:35:07 +0800 you wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patchset adds post_socket_cb pointer together with 'struct
> post_socket_opts cb_opts' into struct network_helper_opts to make
> start_server_addr() helper more flexible. With these modifications,
> many duplicate codes can be dropped.
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/6] selftests/bpf: Add post_socket_cb for network_helper_opts
    https://git.kernel.org/bpf/bpf-next/c/20434d2d896f
  - [bpf-next,2/6] selftests/bpf: Use start_server_addr in sockopt_inherit
    https://git.kernel.org/bpf/bpf-next/c/5166b3e3e30a
  - [bpf-next,3/6] selftests/bpf: Use start_server_addr in test_tcp_check_syncookie
    https://git.kernel.org/bpf/bpf-next/c/49e1fa8dbd81
  - [bpf-next,4/6] selftests/bpf: Use connect_to_fd in sockopt_inherit
    https://git.kernel.org/bpf/bpf-next/c/5059c73eca67
  - [bpf-next,5/6] selftests/bpf: Use connect_to_fd in test_tcp_check_syncookie
    https://git.kernel.org/bpf/bpf-next/c/65a3f0df44dd
  - [bpf-next,6/6] selftests/bpf: Drop get_port in test_tcp_check_syncookie
    https://git.kernel.org/bpf/bpf-next/c/7abbf38cd8ed

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html