All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/6] use network helpers, part 3
@ 2024-04-25  3:23 Geliang Tang
  2024-04-25  3:23 ` [PATCH bpf-next 1/6] selftests/bpf: Add opts argument for __start_server Geliang Tang
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Geliang Tang @ 2024-04-25  3:23 UTC (permalink / raw)
  To: Andrii Nakryiko, Eduard Zingerman, Mykola Lysenko,
	Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Shuah Khan
  Cc: Geliang Tang, bpf, linux-kselftest, Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

This patchset adds opts argument for __start_server, and adds setsockopt
pointer together with optval and optlen into struct network_helper_opts
to make start_server_addr helper more flexible. With these modifications,
many duplicate codes can be dropped.

Patch 4 addresses comments of Martin and Eduard in the previous series.

Geliang Tang (6):
  selftests/bpf: Add opts argument for __start_server
  selftests/bpf: Make start_mptcp_server static
  selftests/bpf: Drop start_server_proto helper
  selftests/bpf: Add setsockopt for network_helper_opts
  selftests/bpf: Use start_server_addr in sockopt_inherit
  selftests/bpf: Use start_server_addr in test_tcp_check_syncookie

 tools/testing/selftests/bpf/Makefile          |  1 +
 tools/testing/selftests/bpf/network_helpers.c | 53 ++++++++-------
 tools/testing/selftests/bpf/network_helpers.h |  5 +-
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 16 +++++
 .../bpf/prog_tests/sockopt_inherit.c          | 34 ++++------
 .../bpf/test_tcp_check_syncookie_user.c       | 64 ++++++-------------
 6 files changed, 78 insertions(+), 95 deletions(-)

-- 
2.40.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH bpf-next 1/6] selftests/bpf: Add opts argument for __start_server
  2024-04-25  3:23 [PATCH bpf-next 0/6] use network helpers, part 3 Geliang Tang
@ 2024-04-25  3:23 ` Geliang Tang
  2024-04-25  3:23 ` [PATCH bpf-next 2/6] selftests/bpf: Make start_mptcp_server static Geliang Tang
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Geliang Tang @ 2024-04-25  3:23 UTC (permalink / raw)
  To: Andrii Nakryiko, Eduard Zingerman, Mykola Lysenko,
	Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Shuah Khan
  Cc: Geliang Tang, bpf, linux-kselftest, Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch adds network_helper_opts parameter for __start_server()
instead of "int protocol" and "int timeout_ms". This not only reduces
the number of parameters, but also makes it more flexible.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/bpf/network_helpers.c | 27 ++++++++++---------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index c134cd740638..b9d204a1a480 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -80,19 +80,19 @@ 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, int protocol, const struct sockaddr *addr,
-			  socklen_t addrlen, int timeout_ms, bool reuseport)
+static int __start_server(int type, const struct sockaddr *addr, socklen_t addrlen,
+			  bool reuseport, const struct network_helper_opts *opts)
 {
 	int on = 1;
 	int fd;
 
-	fd = socket(addr->sa_family, type, protocol);
+	fd = socket(addr->sa_family, type, opts->proto);
 	if (fd < 0) {
 		log_err("Failed to create server socket");
 		return -1;
 	}
 
-	if (settimeo(fd, timeout_ms))
+	if (settimeo(fd, opts->timeout_ms))
 		goto error_close;
 
 	if (reuseport &&
@@ -123,14 +123,17 @@ static int __start_server(int type, int protocol, const struct sockaddr *addr,
 static int start_server_proto(int family, int type, int protocol,
 			      const char *addr_str, __u16 port, int timeout_ms)
 {
+	struct network_helper_opts opts = {
+		.timeout_ms	= timeout_ms,
+		.proto		= protocol,
+	};
 	struct sockaddr_storage addr;
 	socklen_t addrlen;
 
 	if (make_sockaddr(family, addr_str, port, &addr, &addrlen))
 		return -1;
 
-	return __start_server(type, protocol, (struct sockaddr *)&addr,
-			      addrlen, timeout_ms, false);
+	return __start_server(type, (struct sockaddr *)&addr, addrlen, false, &opts);
 }
 
 int start_server(int family, int type, const char *addr_str, __u16 port,
@@ -149,6 +152,9 @@ int start_mptcp_server(int family, const char *addr_str, __u16 port,
 int *start_reuseport_server(int family, int type, const char *addr_str,
 			    __u16 port, int timeout_ms, unsigned int nr_listens)
 {
+	struct network_helper_opts opts = {
+		.timeout_ms = timeout_ms,
+	};
 	struct sockaddr_storage addr;
 	unsigned int nr_fds = 0;
 	socklen_t addrlen;
@@ -164,8 +170,7 @@ int *start_reuseport_server(int family, int type, const char *addr_str,
 	if (!fds)
 		return NULL;
 
-	fds[0] = __start_server(type, 0, (struct sockaddr *)&addr, addrlen,
-				timeout_ms, true);
+	fds[0] = __start_server(type, (struct sockaddr *)&addr, addrlen, true, &opts);
 	if (fds[0] == -1)
 		goto close_fds;
 	nr_fds = 1;
@@ -174,8 +179,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, 0, (struct sockaddr *)&addr,
-					     addrlen, timeout_ms, true);
+		fds[nr_fds] = __start_server(type, (struct sockaddr *)&addr, addrlen, true, &opts);
 		if (fds[nr_fds] == -1)
 			goto close_fds;
 	}
@@ -193,8 +197,7 @@ int start_server_addr(int type, const struct sockaddr_storage *addr, socklen_t l
 	if (!opts)
 		opts = &default_opts;
 
-	return __start_server(type, 0, (struct sockaddr *)addr, len,
-			      opts->timeout_ms, 0);
+	return __start_server(type, (struct sockaddr *)addr, len, 0, opts);
 }
 
 void free_fds(int *fds, unsigned int nr_close_fds)
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH bpf-next 2/6] selftests/bpf: Make start_mptcp_server static
  2024-04-25  3:23 [PATCH bpf-next 0/6] use network helpers, part 3 Geliang Tang
  2024-04-25  3:23 ` [PATCH bpf-next 1/6] selftests/bpf: Add opts argument for __start_server Geliang Tang
@ 2024-04-25  3:23 ` Geliang Tang
  2024-04-25  3:23 ` [PATCH bpf-next 3/6] selftests/bpf: Drop start_server_proto helper Geliang Tang
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Geliang Tang @ 2024-04-25  3:23 UTC (permalink / raw)
  To: Andrii Nakryiko, Eduard Zingerman, Mykola Lysenko,
	Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Shuah Khan
  Cc: Geliang Tang, bpf, linux-kselftest, Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

start_mptcp_server() shouldn't be a public helper, it only be used in
MPTCP tests. This patch moves it into prog_tests/mptcp.c, and implenments
it using make_sockaddr() and start_server_addr() instead of using
start_server_proto().

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

diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index b9d204a1a480..4f16f7d6ce3f 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -142,13 +142,6 @@ int start_server(int family, int type, const char *addr_str, __u16 port,
 	return start_server_proto(family, type, 0, addr_str, port, timeout_ms);
 }
 
-int start_mptcp_server(int family, const char *addr_str, __u16 port,
-		       int timeout_ms)
-{
-	return start_server_proto(family, SOCK_STREAM, IPPROTO_MPTCP, addr_str,
-				  port, timeout_ms);
-}
-
 int *start_reuseport_server(int family, int type, const char *addr_str,
 			    __u16 port, int timeout_ms, unsigned int nr_listens)
 {
diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
index 5a8c5cf4ec1a..c62b54daa914 100644
--- a/tools/testing/selftests/bpf/network_helpers.h
+++ b/tools/testing/selftests/bpf/network_helpers.h
@@ -49,8 +49,6 @@ extern struct ipv6_packet pkt_v6;
 int settimeo(int fd, int timeout_ms);
 int start_server(int family, int type, const char *addr, __u16 port,
 		 int timeout_ms);
-int start_mptcp_server(int family, const char *addr, __u16 port,
-		       int timeout_ms);
 int *start_reuseport_server(int family, int type, const char *addr_str,
 			    __u16 port, int timeout_ms,
 			    unsigned int nr_listens);
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index f84c74b87d23..3629d17b08ee 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -104,6 +104,22 @@ static void cleanup_netns(struct nstoken *nstoken)
 	SYS_NOFAIL("ip netns del %s", NS_TEST);
 }
 
+static int start_mptcp_server(int family, const char *addr_str, __u16 port,
+			      int timeout_ms)
+{
+	struct network_helper_opts opts = {
+		.timeout_ms	= timeout_ms,
+		.proto		= IPPROTO_MPTCP,
+	};
+	struct sockaddr_storage addr;
+	socklen_t addrlen;
+
+	if (make_sockaddr(family, addr_str, port, &addr, &addrlen))
+		return -1;
+
+	return start_server_addr(SOCK_STREAM, &addr, addrlen, &opts);
+}
+
 static int verify_tsk(int map_fd, int client_fd)
 {
 	int err, cfd = client_fd;
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH bpf-next 3/6] selftests/bpf: Drop start_server_proto helper
  2024-04-25  3:23 [PATCH bpf-next 0/6] use network helpers, part 3 Geliang Tang
  2024-04-25  3:23 ` [PATCH bpf-next 1/6] selftests/bpf: Add opts argument for __start_server Geliang Tang
  2024-04-25  3:23 ` [PATCH bpf-next 2/6] selftests/bpf: Make start_mptcp_server static Geliang Tang
@ 2024-04-25  3:23 ` Geliang Tang
  2024-04-25  3:23 ` [PATCH bpf-next 4/6] selftests/bpf: Add setsockopt for network_helper_opts Geliang Tang
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Geliang Tang @ 2024-04-25  3:23 UTC (permalink / raw)
  To: Andrii Nakryiko, Eduard Zingerman, Mykola Lysenko,
	Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Shuah Khan
  Cc: Geliang Tang, bpf, linux-kselftest, Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

Protocol can be set by __start_server() helper directly now, this makes
the heler start_server_proto() useless.

This patch drops it, and implenments start_server() using make_sockaddr()
and __start_server().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/bpf/network_helpers.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index 4f16f7d6ce3f..1519f02f8090 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -120,12 +120,11 @@ static int __start_server(int type, const struct sockaddr *addr, socklen_t addrl
 	return -1;
 }
 
-static int start_server_proto(int family, int type, int protocol,
-			      const char *addr_str, __u16 port, int timeout_ms)
+int start_server(int family, int type, const char *addr_str, __u16 port,
+		 int timeout_ms)
 {
 	struct network_helper_opts opts = {
 		.timeout_ms	= timeout_ms,
-		.proto		= protocol,
 	};
 	struct sockaddr_storage addr;
 	socklen_t addrlen;
@@ -136,12 +135,6 @@ static int start_server_proto(int family, int type, int protocol,
 	return __start_server(type, (struct sockaddr *)&addr, addrlen, false, &opts);
 }
 
-int start_server(int family, int type, const char *addr_str, __u16 port,
-		 int timeout_ms)
-{
-	return start_server_proto(family, type, 0, addr_str, port, timeout_ms);
-}
-
 int *start_reuseport_server(int family, int type, const char *addr_str,
 			    __u16 port, int timeout_ms, unsigned int nr_listens)
 {
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH bpf-next 4/6] selftests/bpf: Add setsockopt for network_helper_opts
  2024-04-25  3:23 [PATCH bpf-next 0/6] use network helpers, part 3 Geliang Tang
                   ` (2 preceding siblings ...)
  2024-04-25  3:23 ` [PATCH bpf-next 3/6] selftests/bpf: Drop start_server_proto helper Geliang Tang
@ 2024-04-25  3:23 ` Geliang Tang
  2024-05-01  0:40   ` Martin KaFai Lau
  2024-04-25  3:23 ` [PATCH bpf-next 5/6] selftests/bpf: Use start_server_addr in sockopt_inherit Geliang Tang
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 9+ messages in thread
From: Geliang Tang @ 2024-04-25  3:23 UTC (permalink / raw)
  To: Andrii Nakryiko, Eduard Zingerman, Mykola Lysenko,
	Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Shuah Khan
  Cc: Geliang Tang, bpf, linux-kselftest, 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 setsockopt
function pointer, together with optval and optlen fields 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 setsockopt_reuseport() function pointer to setsockopt
filed of struct network_helper_opts.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/bpf/network_helpers.c | 26 ++++++++++++-------
 tools/testing/selftests/bpf/network_helpers.h |  3 +++
 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 1519f02f8090..baeb4acf5fae 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->setsockopt &&
+	    opts->setsockopt(fd, opts->optval, opts->optlen)) {
+		log_err("Failed to set sockopt");
 		goto error_close;
 	}
 
@@ -132,14 +131,23 @@ 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 setsockopt_reuseport(int fd, const void *optval, socklen_t optlen)
+{
+	return setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, optval, optlen);
 }
 
 int *start_reuseport_server(int family, int type, const char *addr_str,
 			    __u16 port, int timeout_ms, unsigned int nr_listens)
 {
+	int on = 1;
 	struct network_helper_opts opts = {
 		.timeout_ms = timeout_ms,
+		.setsockopt = setsockopt_reuseport,
+		.optval = &on,
+		.optlen = sizeof(on),
 	};
 	struct sockaddr_storage addr;
 	unsigned int nr_fds = 0;
@@ -156,7 +164,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 +173,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 +191,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..540ecfc52bd7 100644
--- a/tools/testing/selftests/bpf/network_helpers.h
+++ b/tools/testing/selftests/bpf/network_helpers.h
@@ -28,6 +28,9 @@ struct network_helper_opts {
 	bool noconnect;
 	int type;
 	int proto;
+	int (*setsockopt)(int fd, const void *optval, socklen_t optlen);
+	const void *optval;
+	socklen_t optlen;
 };
 
 /* ipv4 test vector */
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH bpf-next 5/6] selftests/bpf: Use start_server_addr in sockopt_inherit
  2024-04-25  3:23 [PATCH bpf-next 0/6] use network helpers, part 3 Geliang Tang
                   ` (3 preceding siblings ...)
  2024-04-25  3:23 ` [PATCH bpf-next 4/6] selftests/bpf: Add setsockopt for network_helper_opts Geliang Tang
@ 2024-04-25  3:23 ` Geliang Tang
  2024-04-25  3:23 ` [PATCH bpf-next 6/6] selftests/bpf: Use start_server_addr in test_tcp_check_syncookie Geliang Tang
  2024-05-01  0:20 ` [PATCH bpf-next 0/6] use network helpers, part 3 patchwork-bot+netdevbpf
  6 siblings, 0 replies; 9+ messages in thread
From: Geliang Tang @ 2024-04-25  3:23 UTC (permalink / raw)
  To: Andrii Nakryiko, Eduard Zingerman, Mykola Lysenko,
	Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Shuah Khan
  Cc: Geliang Tang, bpf, linux-kselftest, 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 setsockopt_custom() to set SOL_CUSTOM sockopt looply, set
it to setsockopt 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          | 34 +++++++------------
 1 file changed, 13 insertions(+), 21 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..31cf7e9075da 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 setsockopt_custom(int fd, const void *optval, socklen_t optlen)
 {
-	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 = {
+		.setsockopt = setsockopt_custom,
+	};
 	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,8 +151,9 @@ static void run_test(int cgroup_fd)
 	if (!ASSERT_OK_PTR(link_setsockopt, "cg-attach-setsockopt"))
 		goto close_bpf_object;
 
-	server_fd = start_server();
-	if (!ASSERT_GE(server_fd, 0, "start_server"))
+	server_fd = start_server_addr(SOCK_STREAM, (struct sockaddr_storage *)&addr,
+				      sizeof(addr), &opts);
+	if (!ASSERT_GE(server_fd, 0, "start_server_addr"))
 		goto close_bpf_object;
 
 	pthread_mutex_lock(&server_started_mtx);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH bpf-next 6/6] selftests/bpf: Use start_server_addr in test_tcp_check_syncookie
  2024-04-25  3:23 [PATCH bpf-next 0/6] use network helpers, part 3 Geliang Tang
                   ` (4 preceding siblings ...)
  2024-04-25  3:23 ` [PATCH bpf-next 5/6] selftests/bpf: Use start_server_addr in sockopt_inherit Geliang Tang
@ 2024-04-25  3:23 ` Geliang Tang
  2024-05-01  0:20 ` [PATCH bpf-next 0/6] use network helpers, part 3 patchwork-bot+netdevbpf
  6 siblings, 0 replies; 9+ messages in thread
From: Geliang Tang @ 2024-04-25  3:23 UTC (permalink / raw)
  To: Andrii Nakryiko, Eduard Zingerman, Mykola Lysenko,
	Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Shuah Khan
  Cc: Geliang Tang, bpf, linux-kselftest, 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 a helper setsockopt_v6only() to set IPV6_V6ONLY sockopt, set it
to setsockopt 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       | 64 ++++++-------------
 2 files changed, 21 insertions(+), 44 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 2182a5af68c4..b5496ebec594 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..bf60d529d0e8 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,19 @@ static bool get_port(int server_fd, in_port_t *port)
 	return true;
 }
 
+static int setsockopt_v6only(int fd, const void *optval, socklen_t optlen)
+{
+	return setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, optval, optlen);
+}
+
 int main(int argc, char **argv)
 {
+	int mode = false;
+	struct network_helper_opts opts = {
+		.setsockopt	= setsockopt_v6only,
+		.optval		= &mode,
+		.optlen		= sizeof(mode),
+	};
 	struct sockaddr_in addr4;
 	struct sockaddr_in6 addr6;
 	struct sockaddr_in addr4dual;
@@ -259,18 +233,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);
+	mode = 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);
+	mode = 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.40.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH bpf-next 0/6] use network helpers, part 3
  2024-04-25  3:23 [PATCH bpf-next 0/6] use network helpers, part 3 Geliang Tang
                   ` (5 preceding siblings ...)
  2024-04-25  3:23 ` [PATCH bpf-next 6/6] selftests/bpf: Use start_server_addr in test_tcp_check_syncookie Geliang Tang
@ 2024-05-01  0:20 ` patchwork-bot+netdevbpf
  6 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-05-01  0:20 UTC (permalink / raw)
  To: Geliang Tang
  Cc: andrii, eddyz87, mykolal, ast, daniel, martin.lau, song,
	yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa,
	shuah, tanggeliang, bpf, linux-kselftest

Hello:

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

On Thu, 25 Apr 2024 11:23:40 +0800 you wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patchset adds opts argument for __start_server, and adds setsockopt
> pointer together with optval and optlen 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 opts argument for __start_server
    https://git.kernel.org/bpf/bpf-next/c/95b88500b97c
  - [bpf-next,2/6] selftests/bpf: Make start_mptcp_server static
    https://git.kernel.org/bpf/bpf-next/c/044032ee6c4e
  - [bpf-next,3/6] selftests/bpf: Drop start_server_proto helper
    https://git.kernel.org/bpf/bpf-next/c/8405e6980f21
  - [bpf-next,4/6] selftests/bpf: Add setsockopt for network_helper_opts
    (no matching commit)
  - [bpf-next,5/6] selftests/bpf: Use start_server_addr in sockopt_inherit
    (no matching commit)
  - [bpf-next,6/6] selftests/bpf: Use start_server_addr in test_tcp_check_syncookie
    (no matching commit)

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH bpf-next 4/6] selftests/bpf: Add setsockopt for network_helper_opts
  2024-04-25  3:23 ` [PATCH bpf-next 4/6] selftests/bpf: Add setsockopt for network_helper_opts Geliang Tang
@ 2024-05-01  0:40   ` Martin KaFai Lau
  0 siblings, 0 replies; 9+ messages in thread
From: Martin KaFai Lau @ 2024-05-01  0:40 UTC (permalink / raw)
  To: Geliang Tang
  Cc: Andrii Nakryiko, Eduard Zingerman, Mykola Lysenko,
	Alexei Starovoitov, Daniel Borkmann, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan, Geliang Tang, bpf, linux-kselftest

On 4/24/24 8:23 PM, Geliang Tang wrote:
> +static int setsockopt_reuseport(int fd, const void *optval, socklen_t optlen)
> +{
> +	return setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, optval, optlen);
>   }
>   

[ ... ]

>   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..540ecfc52bd7 100644
> --- a/tools/testing/selftests/bpf/network_helpers.h
> +++ b/tools/testing/selftests/bpf/network_helpers.h
> @@ -28,6 +28,9 @@ struct network_helper_opts {
>   	bool noconnect;
>   	int type;
>   	int proto;
> +	int (*setsockopt)(int fd, const void *optval, socklen_t optlen);
> +	const void *optval;
> +	socklen_t optlen;

optval and optlen could be in the stack of the (*setsockopt) callback.
e.g. the "int on;" could be local to the setsockopt_reuseport() instead of 
adding optval/len to the network_helper_opts. Passing one optval in 
network_helper_opts could be less flexible when we want to do multiple 
setsockopt() after socket().

Another nit I would like to make, rename this from (*setsockopt) to 
(*post_socket_cb) because this callback could do more than setsockopt, e.g. 
adding a sk local storage to a socket fd before bind(). Also, add a "const 
struct post_socket_opts *opts" for future extension, Like:

struct post_socket_opts {};

int (*post_socket_cb)(int fd, const struct post_socket_opts *opts);

Patch 6 will need two setsockopt cb functions because of different optval but I 
believe the tradeoff is worth it for this callback doing more than just one 
setsockopt.

Patch 1 to 3 have been applied. Thanks.


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-05-01  0:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-25  3:23 [PATCH bpf-next 0/6] use network helpers, part 3 Geliang Tang
2024-04-25  3:23 ` [PATCH bpf-next 1/6] selftests/bpf: Add opts argument for __start_server Geliang Tang
2024-04-25  3:23 ` [PATCH bpf-next 2/6] selftests/bpf: Make start_mptcp_server static Geliang Tang
2024-04-25  3:23 ` [PATCH bpf-next 3/6] selftests/bpf: Drop start_server_proto helper Geliang Tang
2024-04-25  3:23 ` [PATCH bpf-next 4/6] selftests/bpf: Add setsockopt for network_helper_opts Geliang Tang
2024-05-01  0:40   ` Martin KaFai Lau
2024-04-25  3:23 ` [PATCH bpf-next 5/6] selftests/bpf: Use start_server_addr in sockopt_inherit Geliang Tang
2024-04-25  3:23 ` [PATCH bpf-next 6/6] selftests/bpf: Use start_server_addr in test_tcp_check_syncookie Geliang Tang
2024-05-01  0:20 ` [PATCH bpf-next 0/6] use network helpers, part 3 patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.