From: Geliang Tang tanggeliang@kylinos.cn
v5: - address Martin's comments for v4. (thanks) - drop start_server_addr_opts, add opts as a argument of start_server_addr. - add opts argument for connect_to_addr too. - move some patches out of this set, stay with start_server_addr() and connect_to_addr() only in it.
v4: - add more patches using make_sockaddr and get_socket_local_port helpers.
v3: - address comments of Martin and Eduard in v2. (thanks) - move "int type" to the first argument of start_server_addr and connect_to_addr. - add start_server_addr_opts. - using "sockaddr_storage" instead of "sockaddr". - move start_server_setsockopt patches out of this series.
v2: - update patch 6 only, fix errors reported by CI.
This patchset uses public helpers start_server_* and connect_to_* defined in network_helpers.c to drop duplicate code.
Geliang Tang (6): selftests/bpf: Add start_server_addr helper selftests/bpf: Use start_server_addr in cls_redirect selftests/bpf: Use start_server_addr in sk_assign selftests/bpf: Update arguments of connect_to_addr selftests/bpf: Use connect_to_addr in cls_redirect selftests/bpf: Use connect_to_addr in sk_assign
tools/testing/selftests/bpf/network_helpers.c | 27 +++++++-- tools/testing/selftests/bpf/network_helpers.h | 5 +- .../selftests/bpf/prog_tests/cls_redirect.c | 38 +------------ .../selftests/bpf/prog_tests/sk_assign.c | 55 ++----------------- .../selftests/bpf/prog_tests/sock_addr.c | 6 +- 5 files changed, 37 insertions(+), 94 deletions(-)
From: Geliang Tang tanggeliang@kylinos.cn
In order to pair up with connect_to_addr(), this patch adds a new helper start_server_addr(), which is a wrapper of __start_server(). It accepts an argument 'addr' of 'struct sockaddr_storage' type instead of a string type argument like start_server(), and a network_helper_opts argument as the last one.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/network_helpers.c | 14 ++++++++++++-- tools/testing/selftests/bpf/network_helpers.h | 2 ++ 2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index dc1fd7af9c7a..28fe8367451b 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -52,6 +52,8 @@ struct ipv6_packet pkt_v6 = { .tcp.doff = 5, };
+static const struct network_helper_opts default_opts; + int settimeo(int fd, int timeout_ms) { struct timeval timeout = { .tv_sec = 3 }; @@ -185,6 +187,16 @@ int *start_reuseport_server(int family, int type, const char *addr_str, return NULL; }
+int start_server_addr(int type, const struct sockaddr_storage *addr, socklen_t len, + const struct network_helper_opts *opts) +{ + if (!opts) + opts = &default_opts; + + return __start_server(type, 0, (struct sockaddr *)addr, len, + opts->timeout_ms, 0); +} + void free_fds(int *fds, unsigned int nr_close_fds) { if (fds) { @@ -278,8 +290,6 @@ int connect_to_addr(const struct sockaddr_storage *addr, socklen_t addrlen, int return -1; }
-static const struct network_helper_opts default_opts; - int connect_to_fd_opts(int server_fd, const struct network_helper_opts *opts) { struct sockaddr_storage addr; diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h index 70f4e4c92733..414ea50bb3fc 100644 --- a/tools/testing/selftests/bpf/network_helpers.h +++ b/tools/testing/selftests/bpf/network_helpers.h @@ -53,6 +53,8 @@ int start_mptcp_server(int family, const char *addr, __u16 port, int *start_reuseport_server(int family, int type, const char *addr_str, __u16 port, int timeout_ms, unsigned int nr_listens); +int start_server_addr(int type, const struct sockaddr_storage *addr, socklen_t len, + const struct network_helper_opts *opts); void free_fds(int *fds, unsigned int nr_close_fds); int connect_to_addr(const struct sockaddr_storage *addr, socklen_t len, int type); int connect_to_fd(int server_fd, int timeout_ms);
From: Geliang Tang tanggeliang@kylinos.cn
Include network_helpers.h in prog_tests/cls_redirect.c, use the newly added public helper start_server_addr() instead of the local defined function start_server(). This can avoid duplicate code.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- .../selftests/bpf/prog_tests/cls_redirect.c | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/cls_redirect.c b/tools/testing/selftests/bpf/prog_tests/cls_redirect.c index 2a55f717fc07..68cb93106658 100644 --- a/tools/testing/selftests/bpf/prog_tests/cls_redirect.c +++ b/tools/testing/selftests/bpf/prog_tests/cls_redirect.c @@ -10,6 +10,7 @@ #include <netinet/tcp.h>
#include <test_progs.h> +#include "network_helpers.h"
#include "progs/test_cls_redirect.h" #include "test_cls_redirect.skel.h" @@ -35,23 +36,6 @@ struct tuple { struct addr_port dst; };
-static int start_server(const struct sockaddr *addr, socklen_t len, int type) -{ - int fd = socket(addr->sa_family, type, 0); - if (CHECK_FAIL(fd == -1)) - return -1; - if (CHECK_FAIL(bind(fd, addr, len) == -1)) - goto err; - if (type == SOCK_STREAM && CHECK_FAIL(listen(fd, 128) == -1)) - goto err; - - return fd; - -err: - close(fd); - return -1; -} - static int connect_to_server(const struct sockaddr *addr, socklen_t len, int type) { @@ -98,7 +82,7 @@ static bool set_up_conn(const struct sockaddr *addr, socklen_t len, int type, socklen_t slen = sizeof(ss); struct sockaddr *sa = (struct sockaddr *)&ss;
- *server = start_server(addr, len, type); + *server = start_server_addr(type, (struct sockaddr_storage *)addr, len, NULL); if (*server < 0) return false;
From: Geliang Tang tanggeliang@kylinos.cn
Include network_helpers.h in prog_tests/sk_assign.c, use the newly added public helper start_server_addr() instead of the local defined function start_server(). This can avoid duplicate code.
The code that sets SO_RCVTIMEO timeout as timeo_sec (3s) can be dropped, since start_server_addr() sets default timeout as 3s.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- .../selftests/bpf/prog_tests/sk_assign.c | 29 +++---------------- 1 file changed, 4 insertions(+), 25 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_assign.c b/tools/testing/selftests/bpf/prog_tests/sk_assign.c index 1374b626a985..b066b6b88d7c 100644 --- a/tools/testing/selftests/bpf/prog_tests/sk_assign.c +++ b/tools/testing/selftests/bpf/prog_tests/sk_assign.c @@ -15,6 +15,7 @@ #include <unistd.h>
#include "test_progs.h" +#include "network_helpers.h"
#define BIND_PORT 1234 #define CONNECT_PORT 4321 @@ -73,30 +74,6 @@ configure_stack(void) return true; }
-static int -start_server(const struct sockaddr *addr, socklen_t len, int type) -{ - int fd; - - fd = socket(addr->sa_family, type, 0); - if (CHECK_FAIL(fd == -1)) - goto out; - if (CHECK_FAIL(setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeo_sec, - timeo_optlen))) - goto close_out; - if (CHECK_FAIL(bind(fd, addr, len) == -1)) - goto close_out; - if (type == SOCK_STREAM && CHECK_FAIL(listen(fd, 128) == -1)) - 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 type) { @@ -310,7 +287,9 @@ void test_sk_assign(void) continue; prepare_addr(test->addr, test->family, BIND_PORT, false); addr = (const struct sockaddr *)test->addr; - server = start_server(addr, test->len, test->type); + server = start_server_addr(test->type, + (const struct sockaddr_storage *)addr, + test->len, NULL); if (server == -1) goto close;
From: Geliang Tang tanggeliang@kylinos.cn
Move the third argument "int type" of connect_to_addr() to the first one which is closer to how the socket syscall is doing it. And add a network_helper_opts argument as the fourth one. Then change its usages in sock_addr.c too.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/network_helpers.c | 13 ++++++++++--- tools/testing/selftests/bpf/network_helpers.h | 3 ++- tools/testing/selftests/bpf/prog_tests/sock_addr.c | 6 +++--- 3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index 28fe8367451b..9d63d2ac13d8 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -270,17 +270,24 @@ static int connect_fd_to_addr(int fd, return 0; }
-int connect_to_addr(const struct sockaddr_storage *addr, socklen_t addrlen, int type) +int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t addrlen, + const struct network_helper_opts *opts) { int fd;
- fd = socket(addr->ss_family, type, 0); + if (!opts) + opts = &default_opts; + + fd = socket(addr->ss_family, type, opts->proto); if (fd < 0) { log_err("Failed to create client socket"); return -1; }
- if (connect_fd_to_addr(fd, addr, addrlen, false)) + if (settimeo(fd, opts->timeout_ms)) + goto error_close; + + if (connect_fd_to_addr(fd, addr, addrlen, opts->must_fail)) goto error_close;
return fd; diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h index 414ea50bb3fc..aef297dfa6ca 100644 --- a/tools/testing/selftests/bpf/network_helpers.h +++ b/tools/testing/selftests/bpf/network_helpers.h @@ -56,7 +56,8 @@ int *start_reuseport_server(int family, int type, const char *addr_str, int start_server_addr(int type, const struct sockaddr_storage *addr, socklen_t len, const struct network_helper_opts *opts); void free_fds(int *fds, unsigned int nr_close_fds); -int connect_to_addr(const struct sockaddr_storage *addr, socklen_t len, int type); +int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t len, + const struct network_helper_opts *opts); int connect_to_fd(int server_fd, int timeout_ms); int connect_to_fd_opts(int server_fd, const struct network_helper_opts *opts); int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms); diff --git a/tools/testing/selftests/bpf/prog_tests/sock_addr.c b/tools/testing/selftests/bpf/prog_tests/sock_addr.c index 5fd617718991..61668e0f11b0 100644 --- a/tools/testing/selftests/bpf/prog_tests/sock_addr.c +++ b/tools/testing/selftests/bpf/prog_tests/sock_addr.c @@ -328,7 +328,7 @@ static void test_bind(struct sock_addr_test *test) goto cleanup;
/* Try to connect to server just in case */ - client = connect_to_addr(&expected_addr, expected_addr_len, test->socket_type); + client = connect_to_addr(test->socket_type, &expected_addr, expected_addr_len, NULL); if (!ASSERT_GE(client, 0, "connect_to_addr")) goto cleanup;
@@ -357,7 +357,7 @@ static void test_connect(struct sock_addr_test *test) if (!ASSERT_EQ(err, 0, "make_sockaddr")) goto cleanup;
- client = connect_to_addr(&addr, addr_len, test->socket_type); + client = connect_to_addr(test->socket_type, &addr, addr_len, NULL); if (!ASSERT_GE(client, 0, "connect_to_addr")) goto cleanup;
@@ -538,7 +538,7 @@ static void test_getpeername(struct sock_addr_test *test) if (!ASSERT_EQ(err, 0, "make_sockaddr")) goto cleanup;
- client = connect_to_addr(&addr, addr_len, test->socket_type); + client = connect_to_addr(test->socket_type, &addr, addr_len, NULL); if (!ASSERT_GE(client, 0, "connect_to_addr")) goto cleanup;
From: Geliang Tang tanggeliang@kylinos.cn
This patch uses public helper connect_to_addr() exported in network_helpers.h instead of the local defined function connect_to_server() in prog_tests/cls_redirect.c. This can avoid duplicate code.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- .../selftests/bpf/prog_tests/cls_redirect.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/cls_redirect.c b/tools/testing/selftests/bpf/prog_tests/cls_redirect.c index 68cb93106658..34b59f6baca1 100644 --- a/tools/testing/selftests/bpf/prog_tests/cls_redirect.c +++ b/tools/testing/selftests/bpf/prog_tests/cls_redirect.c @@ -36,22 +36,6 @@ struct tuple { struct addr_port dst; };
-static int connect_to_server(const struct sockaddr *addr, socklen_t len, - int type) -{ - int fd = socket(addr->sa_family, type, 0); - if (CHECK_FAIL(fd == -1)) - return -1; - if (CHECK_FAIL(connect(fd, addr, len))) - goto err; - - return fd; - -err: - close(fd); - return -1; -} - static bool fill_addr_port(const struct sockaddr *sa, struct addr_port *ap) { const struct sockaddr_in6 *in6; @@ -89,7 +73,7 @@ static bool set_up_conn(const struct sockaddr *addr, socklen_t len, int type, if (CHECK_FAIL(getsockname(*server, sa, &slen))) goto close_server;
- *conn = connect_to_server(sa, slen, type); + *conn = connect_to_addr(type, (struct sockaddr_storage *)sa, slen, NULL); if (*conn < 0) goto close_server;
From: Geliang Tang tanggeliang@kylinos.cn
This patch uses public helper connect_to_addr() exported in network_helpers.h instead of the local defined function connect_to_server() in prog_tests/sk_assign.c. This can avoid duplicate code.
The code that sets SO_SNDTIMEO timeout as timeo_sec (3s) can be dropped, since connect_to_addr() sets default timeout as 3s.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- .../selftests/bpf/prog_tests/sk_assign.c | 26 +------------------ 1 file changed, 1 insertion(+), 25 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_assign.c b/tools/testing/selftests/bpf/prog_tests/sk_assign.c index b066b6b88d7c..0b9bd1d6f7cc 100644 --- a/tools/testing/selftests/bpf/prog_tests/sk_assign.c +++ b/tools/testing/selftests/bpf/prog_tests/sk_assign.c @@ -23,8 +23,6 @@ #define NS_SELF "/proc/self/ns/net" #define SERVER_MAP_PATH "/sys/fs/bpf/tc/globals/server_map"
-static const struct timeval timeo_sec = { .tv_sec = 3 }; -static const size_t timeo_optlen = sizeof(timeo_sec); static int stop, duration;
static bool @@ -74,28 +72,6 @@ configure_stack(void) return true; }
-static int -connect_to_server(const struct sockaddr *addr, socklen_t len, int type) -{ - int fd = -1; - - fd = socket(addr->sa_family, type, 0); - if (CHECK_FAIL(fd == -1)) - goto out; - if (CHECK_FAIL(setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &timeo_sec, - timeo_optlen))) - goto close_out; - if (CHECK_FAIL(connect(fd, addr, len))) - goto close_out; - - goto out; -close_out: - close(fd); - fd = -1; -out: - return fd; -} - static in_port_t get_port(int fd) { @@ -138,7 +114,7 @@ run_test(int server_fd, const struct sockaddr *addr, socklen_t len, int type) in_port_t port; int ret = 1;
- client = connect_to_server(addr, len, type); + client = connect_to_addr(type, (struct sockaddr_storage *)addr, len, NULL); if (client == -1) { perror("Cannot connect to server"); goto out;
Hello:
This series was applied to bpf/bpf-next.git (master) by Martin KaFai Lau martin.lau@kernel.org:
On Thu, 18 Apr 2024 16:09:06 +0800 you wrote:
From: Geliang Tang tanggeliang@kylinos.cn
v5:
- address Martin's comments for v4. (thanks)
- drop start_server_addr_opts, add opts as a argument of start_server_addr.
- add opts argument for connect_to_addr too.
- move some patches out of this set, stay with start_server_addr() and connect_to_addr() only in it.
[...]
Here is the summary with links: - [bpf-next,v5,1/6] selftests/bpf: Add start_server_addr helper https://git.kernel.org/bpf/bpf-next/c/9c598a83b7ea - [bpf-next,v5,2/6] selftests/bpf: Use start_server_addr in cls_redirect https://git.kernel.org/bpf/bpf-next/c/9851382fb369 - [bpf-next,v5,3/6] selftests/bpf: Use start_server_addr in sk_assign https://git.kernel.org/bpf/bpf-next/c/a2e4979536c4 - [bpf-next,v5,4/6] selftests/bpf: Update arguments of connect_to_addr https://git.kernel.org/bpf/bpf-next/c/db9994d022ec - [bpf-next,v5,5/6] selftests/bpf: Use connect_to_addr in cls_redirect https://git.kernel.org/bpf/bpf-next/c/805b4d90c0df - [bpf-next,v5,6/6] selftests/bpf: Use connect_to_addr in sk_assign https://git.kernel.org/bpf/bpf-next/c/63a51820d29b
You are awesome, thank you!
linux-kselftest-mirror@lists.linaro.org
-
Geliang Tang -
patchwork-bot+netdevbpf＠kernel.org