From: Geliang Tang tanggeliang@kylinos.cn
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 (14): selftests/bpf: Update arguments of connect_to_addr selftests/bpf: Add start_server_addr* helpers selftests/bpf: Use start_server_addr in cls_redirect selftests/bpf: Use connect_to_addr in cls_redirect selftests/bpf: Use start_server_addr in sk_assign selftests/bpf: Use connect_to_addr in sk_assign selftests/bpf: Use get_socket_local_port in sk_assign selftests/bpf: Use make_sockaddr in sk_assign selftests/bpf: Use log_err in network_helpers selftests/bpf: Use start_server_addr in test_sock_addr selftests/bpf: Use connect_to_addr in test_sock_addr selftests/bpf: Use make_sockaddr in test_sock_addr selftests/bpf: Use make_sockaddr in test_sock selftests/bpf: Use make_sockaddr in ip_check_defrag
tools/testing/selftests/bpf/Makefile | 4 +- tools/testing/selftests/bpf/network_helpers.c | 37 ++++- tools/testing/selftests/bpf/network_helpers.h | 6 +- .../selftests/bpf/prog_tests/cls_redirect.c | 38 +---- .../selftests/bpf/prog_tests/empty_skb.c | 2 + .../bpf/prog_tests/ip_check_defrag.c | 20 +-- .../selftests/bpf/prog_tests/sk_assign.c | 92 ++---------- .../selftests/bpf/prog_tests/sock_addr.c | 6 +- .../selftests/bpf/prog_tests/tc_redirect.c | 2 +- .../selftests/bpf/prog_tests/test_tunnel.c | 4 + .../selftests/bpf/prog_tests/xdp_metadata.c | 16 +++ tools/testing/selftests/bpf/test_sock.c | 28 +--- tools/testing/selftests/bpf/test_sock_addr.c | 136 +++--------------- 13 files changed, 104 insertions(+), 287 deletions(-)
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 change its usages in sock_addr.c too.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/network_helpers.c | 2 +- tools/testing/selftests/bpf/network_helpers.h | 2 +- tools/testing/selftests/bpf/prog_tests/sock_addr.c | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index dc1fd7af9c7a..563dde8617dd 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -258,7 +258,7 @@ 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) { int fd;
diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h index 70f4e4c92733..ac4da5fdcc95 100644 --- a/tools/testing/selftests/bpf/network_helpers.h +++ b/tools/testing/selftests/bpf/network_helpers.h @@ -54,7 +54,7 @@ int *start_reuseport_server(int family, int type, const char *addr_str, __u16 port, int timeout_ms, unsigned int nr_listens); 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); 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..b182667e78c9 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); 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); 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); if (!ASSERT_GE(client, 0, "connect_to_addr")) goto cleanup;
From: Geliang Tang tanggeliang@kylinos.cn
In order to pair up with connect_to_addr(), this patch adds a new helper start_server_addr(), and another one start_server_addr_opts(), which is a wrapper of __start_server(), only added a network_helper_opts arg at the end.
They all accept an argument 'addr' of 'struct sockaddr_storage' type instead of a string type argument like start_server().
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/network_helpers.c | 16 ++++++++++++++++ tools/testing/selftests/bpf/network_helpers.h | 3 +++ 2 files changed, 19 insertions(+)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index 563dde8617dd..836436688ca6 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -185,6 +185,22 @@ int *start_reuseport_server(int family, int type, const char *addr_str, return NULL; }
+int start_server_addr_opts(int type, const struct sockaddr_storage *addr, socklen_t len, + const struct network_helper_opts *opts) +{ + return __start_server(type, 0, (struct sockaddr *)addr, len, + opts->timeout_ms, 0); +} + +int start_server_addr(int type, const struct sockaddr_storage *addr, socklen_t len) +{ + struct network_helper_opts opts = { + .timeout_ms = 0, + }; + + return start_server_addr_opts(type, addr, len, &opts); +} + void free_fds(int *fds, unsigned int nr_close_fds) { if (fds) { diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h index ac4da5fdcc95..9e6fcc89a8d0 100644 --- a/tools/testing/selftests/bpf/network_helpers.h +++ b/tools/testing/selftests/bpf/network_helpers.h @@ -53,6 +53,9 @@ 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_opts(int type, const struct sockaddr_storage *addr, socklen_t len, + const struct network_helper_opts *opts); +int start_server_addr(int type, const struct sockaddr_storage *addr, socklen_t len); void free_fds(int *fds, unsigned int nr_close_fds); int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t len); int connect_to_fd(int server_fd, int timeout_ms);
On 4/16/24 3:13 AM, Geliang Tang wrote:
From: Geliang Tang tanggeliang@kylinos.cn
In order to pair up with connect_to_addr(), this patch adds a new helper start_server_addr(), and another one start_server_addr_opts(), which is a wrapper of __start_server(), only added a network_helper_opts arg at the end.
They all accept an argument 'addr' of 'struct sockaddr_storage' type instead of a string type argument like start_server().
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn
tools/testing/selftests/bpf/network_helpers.c | 16 ++++++++++++++++ tools/testing/selftests/bpf/network_helpers.h | 3 +++ 2 files changed, 19 insertions(+)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index 563dde8617dd..836436688ca6 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -185,6 +185,22 @@ int *start_reuseport_server(int family, int type, const char *addr_str, return NULL; } +int start_server_addr_opts(int type, const struct sockaddr_storage *addr, socklen_t len,
const struct network_helper_opts *opts)
I meant to only add one helper with the "opts" arg in v2 instead of adding two helpers. I want to minimize the number of new helpers and each just has different variants of args. timeout_ms usually makes sense, so opts is usually required.
Thinking a bit more. A small nit from my v2 comment. Name this as start_server_addr() without the "_opts" part. Keep it short. I want the default helper has the opts arg from now other than a few exceptions like close_netns(), get_socket_local_port()...etc. No need to change other existing helpers. Stay with start_server_addr() and connect_to_addr() in this set.
so,
int start_server_addr(int type, const struct sockaddr_storage *addr, socklen_t len, const struct network_helper_opts *opts);
opts could be NULL.
pw-bot: cr
+{
- return __start_server(type, 0, (struct sockaddr *)addr, len,
opts->timeout_ms, 0);+}
+int start_server_addr(int type, const struct sockaddr_storage *addr, socklen_t len) +{
- struct network_helper_opts opts = {
.timeout_ms = 0,- };
- return start_server_addr_opts(type, addr, len, &opts);
+}
- void free_fds(int *fds, unsigned int nr_close_fds) { if (fds) {
diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h index ac4da5fdcc95..9e6fcc89a8d0 100644 --- a/tools/testing/selftests/bpf/network_helpers.h +++ b/tools/testing/selftests/bpf/network_helpers.h @@ -53,6 +53,9 @@ 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_opts(int type, const struct sockaddr_storage *addr, socklen_t len,
const struct network_helper_opts *opts);+int start_server_addr(int type, const struct sockaddr_storage *addr, socklen_t len); void free_fds(int *fds, unsigned int nr_close_fds); int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t len); 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..4050d470d2a2 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); if (*server < 0) return false;
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 4050d470d2a2..9aa2f3b12884 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); if (*conn < 0) goto close_server;
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 | 28 ++----------------- 1 file changed, 3 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..fa8f757c0edd 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,8 @@ 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); if (server == -1) goto close;
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 fa8f757c0edd..766fc56f5fc7 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); if (client == -1) { perror("Cannot connect to server"); goto out;
On 4/16/24 3:13 AM, Geliang Tang wrote:
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 fa8f757c0edd..766fc56f5fc7 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)))
I think Eduard has also pointed out in v2 that the existing connect_to_addr() does not set the timeout and suggested to add the opts arg also.
One option is to just modify the connect_to_addr() to set the default timeout (3s) without needing to add the opts arg. Since we are changing the signature of connect_to_addr() anyway, lets just add the opts argument also. When opts is NULL or opts->timeout_ms is 0, then use the default 3s timeout like other helpers do.
something like,
int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t addrlen, const struct network_helper_opts *opts);
From: Geliang Tang tanggeliang@kylinos.cn
This patch uses public helper get_socket_local_port() exported in network_helpers.h instead of the local defined function get_port() in prog_tests/sk_assign.c. This can avoid duplicate code.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- .../selftests/bpf/prog_tests/sk_assign.c | 25 +------------------ 1 file changed, 1 insertion(+), 24 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_assign.c b/tools/testing/selftests/bpf/prog_tests/sk_assign.c index 766fc56f5fc7..c0ef210f7622 100644 --- a/tools/testing/selftests/bpf/prog_tests/sk_assign.c +++ b/tools/testing/selftests/bpf/prog_tests/sk_assign.c @@ -72,29 +72,6 @@ configure_stack(void) return true; }
-static in_port_t -get_port(int fd) -{ - struct sockaddr_storage ss; - socklen_t slen = sizeof(ss); - in_port_t port = 0; - - if (CHECK_FAIL(getsockname(fd, (struct sockaddr *)&ss, &slen))) - return port; - - switch (ss.ss_family) { - case AF_INET: - port = ((struct sockaddr_in *)&ss)->sin_port; - break; - case AF_INET6: - port = ((struct sockaddr_in6 *)&ss)->sin6_port; - break; - default: - CHECK(1, "Invalid address family", "%d\n", ss.ss_family); - } - return port; -} - static ssize_t rcv_msg(int srv_client, int type) { @@ -138,7 +115,7 @@ run_test(int server_fd, const struct sockaddr *addr, socklen_t len, int type) goto out; }
- port = get_port(srv_client); + port = get_socket_local_port(srv_client); if (CHECK_FAIL(!port)) goto out; /* SOCK_STREAM is connected via accept(), so the server's local address
From: Geliang Tang tanggeliang@kylinos.cn
This patch uses public helper make_sockaddr() exported in network_helpers.h instead of open-coding it prepare_addr() in prog_tests/sk_assign.c. This can avoid duplicate code.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/prog_tests/sk_assign.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_assign.c b/tools/testing/selftests/bpf/prog_tests/sk_assign.c index c0ef210f7622..67daa2268cdc 100644 --- a/tools/testing/selftests/bpf/prog_tests/sk_assign.c +++ b/tools/testing/selftests/bpf/prog_tests/sk_assign.c @@ -148,24 +148,19 @@ prepare_addr(struct sockaddr *addr, int family, __u16 port, bool rewrite_addr) { struct sockaddr_in *addr4; struct sockaddr_in6 *addr6; + socklen_t len; + + make_sockaddr(family, family == AF_INET ? "127.0.0.1" : "::1", + port, (struct sockaddr_storage *)addr, &len);
switch (family) { case AF_INET: addr4 = (struct sockaddr_in *)addr; - memset(addr4, 0, sizeof(*addr4)); - addr4->sin_family = family; - addr4->sin_port = htons(port); if (rewrite_addr) addr4->sin_addr.s_addr = htonl(TEST_DADDR); - else - addr4->sin_addr.s_addr = htonl(INADDR_LOOPBACK); break; case AF_INET6: addr6 = (struct sockaddr_in6 *)addr; - memset(addr6, 0, sizeof(*addr6)); - addr6->sin6_family = family; - addr6->sin6_port = htons(port); - addr6->sin6_addr = in6addr_loopback; if (rewrite_addr) addr6->sin6_addr.s6_addr32[3] = htonl(TEST_DADDR); break;
From: Geliang Tang tanggeliang@kylinos.cn
The helpers ASSERT_OK/GE/OK_PTR should avoid using in public functions. This patch uses log_err() to replace them in network_helpers.c, then uses ASSERT_OK_PTR() to check the return values of all open_netns().
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/network_helpers.c | 19 ++++++++++++++----- .../selftests/bpf/prog_tests/empty_skb.c | 2 ++ .../bpf/prog_tests/ip_check_defrag.c | 2 ++ .../selftests/bpf/prog_tests/tc_redirect.c | 2 +- .../selftests/bpf/prog_tests/test_tunnel.c | 4 ++++ .../selftests/bpf/prog_tests/xdp_metadata.c | 16 ++++++++++++++++ 6 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index 836436688ca6..4fd3ab4fa72c 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -458,22 +458,30 @@ struct nstoken *open_netns(const char *name) struct nstoken *token;
token = calloc(1, sizeof(struct nstoken)); - if (!ASSERT_OK_PTR(token, "malloc token")) + if (!token) { + log_err("Failed to malloc token"); return NULL; + }
token->orig_netns_fd = open("/proc/self/ns/net", O_RDONLY); - if (!ASSERT_GE(token->orig_netns_fd, 0, "open /proc/self/ns/net")) + if (token->orig_netns_fd <= 0) { + log_err("Failed to open /proc/self/ns/net"); goto fail; + }
snprintf(nspath, sizeof(nspath), "%s/%s", "/var/run/netns", name); nsfd = open(nspath, O_RDONLY | O_CLOEXEC); - if (!ASSERT_GE(nsfd, 0, "open netns fd")) + if (nsfd <= 0) { + log_err("Failed to open netns fd"); goto fail; + }
err = setns(nsfd, CLONE_NEWNET); close(nsfd); - if (!ASSERT_OK(err, "setns")) + if (err) { + log_err("Failed to setns"); goto fail; + }
return token; fail: @@ -486,7 +494,8 @@ void close_netns(struct nstoken *token) if (!token) return;
- ASSERT_OK(setns(token->orig_netns_fd, CLONE_NEWNET), "setns"); + if (setns(token->orig_netns_fd, CLONE_NEWNET)) + log_err("Failed to setns"); close(token->orig_netns_fd); free(token); } diff --git a/tools/testing/selftests/bpf/prog_tests/empty_skb.c b/tools/testing/selftests/bpf/prog_tests/empty_skb.c index 261228eb68e8..438583e1f2d1 100644 --- a/tools/testing/selftests/bpf/prog_tests/empty_skb.c +++ b/tools/testing/selftests/bpf/prog_tests/empty_skb.c @@ -94,6 +94,8 @@ void test_empty_skb(void)
SYS(out, "ip netns add empty_skb"); tok = open_netns("empty_skb"); + if (!ASSERT_OK_PTR(tok, "setns")) + goto out; SYS(out, "ip link add veth0 type veth peer veth1"); SYS(out, "ip link set dev veth0 up"); SYS(out, "ip link set dev veth1 up"); diff --git a/tools/testing/selftests/bpf/prog_tests/ip_check_defrag.c b/tools/testing/selftests/bpf/prog_tests/ip_check_defrag.c index 8dd2af9081f4..284764e7179f 100644 --- a/tools/testing/selftests/bpf/prog_tests/ip_check_defrag.c +++ b/tools/testing/selftests/bpf/prog_tests/ip_check_defrag.c @@ -88,6 +88,8 @@ static int attach(struct ip_check_defrag *skel, bool ipv6) int err = -1;
nstoken = open_netns(NS1); + if (!ASSERT_OK_PTR(nstoken, "setns")) + goto out;
skel->links.defrag = bpf_program__attach_netfilter(skel->progs.defrag, &opts); if (!ASSERT_OK_PTR(skel->links.defrag, "program attach")) diff --git a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c index dbe06aeaa2b2..b1073d36d77a 100644 --- a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c +++ b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c @@ -530,7 +530,7 @@ static int wait_netstamp_needed_key(void) __u64 tstamp = 0;
nstoken = open_netns(NS_DST); - if (!nstoken) + if (!ASSERT_OK_PTR(nstoken, "setns dst")) return -1;
srv_fd = start_server(AF_INET6, SOCK_DGRAM, "::1", 0, 0); diff --git a/tools/testing/selftests/bpf/prog_tests/test_tunnel.c b/tools/testing/selftests/bpf/prog_tests/test_tunnel.c index 5f1fb0a2ea56..cec746e77cd3 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_tunnel.c +++ b/tools/testing/selftests/bpf/prog_tests/test_tunnel.c @@ -612,6 +612,8 @@ static void test_ipip_tunnel(enum ipip_encap encap)
/* ping from at_ns0 namespace test */ nstoken = open_netns("at_ns0"); + if (!ASSERT_OK_PTR(nstoken, "setns")) + goto done; err = test_ping(AF_INET, IP4_ADDR_TUNL_DEV1); if (!ASSERT_OK(err, "test_ping")) goto done; @@ -666,6 +668,8 @@ static void test_xfrm_tunnel(void)
/* ping from at_ns0 namespace test */ nstoken = open_netns("at_ns0"); + if (!ASSERT_OK_PTR(nstoken, "setns")) + goto done; err = test_ping(AF_INET, IP4_ADDR_TUNL_DEV1); close_netns(nstoken); if (!ASSERT_OK(err, "test_ping")) diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c index 05edcf32f528..f76b5d67a3ee 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c @@ -384,6 +384,8 @@ void test_xdp_metadata(void) SYS(out, "ip netns add " RX_NETNS_NAME);
tok = open_netns(TX_NETNS_NAME); + if (!ASSERT_OK_PTR(tok, "setns")) + goto out; SYS(out, "ip link add numtxqueues 1 numrxqueues 1 " TX_NAME " type veth peer " RX_NAME " numtxqueues 1 numrxqueues 1"); SYS(out, "ip link set " RX_NAME " netns " RX_NETNS_NAME); @@ -400,6 +402,8 @@ void test_xdp_metadata(void) SYS(out, "ip -4 neigh add " RX_ADDR " lladdr " RX_MAC " dev " TX_NAME_VLAN);
switch_ns_to_rx(&tok); + if (!ASSERT_OK_PTR(tok, "setns rx")) + goto out;
SYS(out, "ip link set dev " RX_NAME " address " RX_MAC); SYS(out, "ip link set dev " RX_NAME " up"); @@ -449,6 +453,8 @@ void test_xdp_metadata(void) goto out;
switch_ns_to_tx(&tok); + if (!ASSERT_OK_PTR(tok, "setns tx")) + goto out;
/* Setup separate AF_XDP for TX interface nad send packet to the RX socket. */ tx_ifindex = if_nametoindex(TX_NAME); @@ -461,6 +467,8 @@ void test_xdp_metadata(void) goto out;
switch_ns_to_rx(&tok); + if (!ASSERT_OK_PTR(tok, "setns rx")) + goto out;
/* Verify packet sent from AF_XDP has proper metadata. */ if (!ASSERT_GE(verify_xsk_metadata(&rx_xsk, true), 0, @@ -468,6 +476,8 @@ void test_xdp_metadata(void) goto out;
switch_ns_to_tx(&tok); + if (!ASSERT_OK_PTR(tok, "setns tx")) + goto out; complete_tx(&tx_xsk);
/* Now check metadata of packet, generated with network stack */ @@ -475,6 +485,8 @@ void test_xdp_metadata(void) goto out;
switch_ns_to_rx(&tok); + if (!ASSERT_OK_PTR(tok, "setns rx")) + goto out;
if (!ASSERT_GE(verify_xsk_metadata(&rx_xsk, false), 0, "verify_xsk_metadata")) @@ -498,6 +510,8 @@ void test_xdp_metadata(void) goto out;
switch_ns_to_tx(&tok); + if (!ASSERT_OK_PTR(tok, "setns tx")) + goto out;
/* Send packet to trigger . */ if (!ASSERT_GE(generate_packet(&tx_xsk, AF_XDP_CONSUMER_PORT), 0, @@ -505,6 +519,8 @@ void test_xdp_metadata(void) goto out;
switch_ns_to_rx(&tok); + if (!ASSERT_OK_PTR(tok, "setns rx")) + goto out;
while (!retries--) { if (bpf_obj2->bss->called)
On 4/16/24 3:13 AM, Geliang Tang wrote:
From: Geliang Tang tanggeliang@kylinos.cn
The helpers ASSERT_OK/GE/OK_PTR should avoid using in public functions. This patch uses log_err() to replace them in network_helpers.c, then uses ASSERT_OK_PTR() to check the return values of all open_netns().
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn
tools/testing/selftests/bpf/network_helpers.c | 19 ++++++++++++++----- .../selftests/bpf/prog_tests/empty_skb.c | 2 ++ .../bpf/prog_tests/ip_check_defrag.c | 2 ++ .../selftests/bpf/prog_tests/tc_redirect.c | 2 +- .../selftests/bpf/prog_tests/test_tunnel.c | 4 ++++ .../selftests/bpf/prog_tests/xdp_metadata.c | 16 ++++++++++++++++ 6 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index 836436688ca6..4fd3ab4fa72c 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -458,22 +458,30 @@ struct nstoken *open_netns(const char *name) struct nstoken *token; token = calloc(1, sizeof(struct nstoken));
- if (!ASSERT_OK_PTR(token, "malloc token"))
- if (!token) {
log_err("Failed to malloc token");- }
token->orig_netns_fd = open("/proc/self/ns/net", O_RDONLY);
- if (!ASSERT_GE(token->orig_netns_fd, 0, "open /proc/self/ns/net"))
- if (token->orig_netns_fd <= 0) {
The test is incorrect. just test == -1.
Also, I think there is an existing bug that orig_netns_fd will be leaked in the later "goto fail;" case.
log_err("Failed to open /proc/self/ns/net");- }
snprintf(nspath, sizeof(nspath), "%s/%s", "/var/run/netns", name); nsfd = open(nspath, O_RDONLY | O_CLOEXEC);
- if (!ASSERT_GE(nsfd, 0, "open netns fd"))
- if (nsfd <= 0) {
Same here.
log_err("Failed to open netns fd");
I meant here regarding to the existing bug that leaks orig_netns_fd.
From: Geliang Tang tanggeliang@kylinos.cn
Include network_helpers.h in test_sock_addr.c, use the newly added public helper start_server_addr() instead of the local defined function start_server(). This can avoid duplicate code.
In order to use functions defined in network_helpers.c, Makefile needs to be updated and <Linux/err.h> needs to be included in network_helpers.h to avoid compilation errors.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/Makefile | 3 +- tools/testing/selftests/bpf/network_helpers.h | 1 + tools/testing/selftests/bpf/test_sock_addr.c | 38 ++----------------- 3 files changed, 7 insertions(+), 35 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index b0be07f29dde..53713f4495dd 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -289,11 +289,12 @@ UNPRIV_HELPERS := $(OUTPUT)/unpriv_helpers.o TRACE_HELPERS := $(OUTPUT)/trace_helpers.o JSON_WRITER := $(OUTPUT)/json_writer.o CAP_HELPERS := $(OUTPUT)/cap_helpers.o +NETWORK_HELPERS := $(OUTPUT)/network_helpers.o
$(OUTPUT)/test_dev_cgroup: $(CGROUP_HELPERS) $(TESTING_HELPERS) $(OUTPUT)/test_skb_cgroup_id_user: $(CGROUP_HELPERS) $(TESTING_HELPERS) $(OUTPUT)/test_sock: $(CGROUP_HELPERS) $(TESTING_HELPERS) -$(OUTPUT)/test_sock_addr: $(CGROUP_HELPERS) $(TESTING_HELPERS) +$(OUTPUT)/test_sock_addr: $(CGROUP_HELPERS) $(TESTING_HELPERS) $(NETWORK_HELPERS) $(OUTPUT)/test_sockmap: $(CGROUP_HELPERS) $(TESTING_HELPERS) $(OUTPUT)/test_tcpnotify_user: $(CGROUP_HELPERS) $(TESTING_HELPERS) $(TRACE_HELPERS) $(OUTPUT)/get_cgroup_id_user: $(CGROUP_HELPERS) $(TESTING_HELPERS) diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h index 9e6fcc89a8d0..5e0806a3d745 100644 --- a/tools/testing/selftests/bpf/network_helpers.h +++ b/tools/testing/selftests/bpf/network_helpers.h @@ -11,6 +11,7 @@ typedef __u16 __sum16; #include <linux/ipv6.h> #include <linux/ethtool.h> #include <linux/sockios.h> +#include <linux/err.h> #include <netinet/tcp.h> #include <bpf/bpf_endian.h> #include <net/if.h> diff --git a/tools/testing/selftests/bpf/test_sock_addr.c b/tools/testing/selftests/bpf/test_sock_addr.c index 80c42583f597..9ce5dad7468b 100644 --- a/tools/testing/selftests/bpf/test_sock_addr.c +++ b/tools/testing/selftests/bpf/test_sock_addr.c @@ -19,6 +19,7 @@ #include <bpf/libbpf.h>
#include "cgroup_helpers.h" +#include "network_helpers.h" #include "testing_helpers.h" #include "bpf_util.h"
@@ -939,37 +940,6 @@ static int cmp_peer_addr(int sock1, const struct sockaddr_storage *addr2) return cmp_sock_addr(getpeername, sock1, addr2, /*cmp_port*/ 1); }
-static int start_server(int type, const struct sockaddr_storage *addr, - socklen_t addr_len) -{ - int fd; - - fd = socket(addr->ss_family, type, 0); - if (fd == -1) { - log_err("Failed to create server socket"); - goto out; - } - - if (bind(fd, (const struct sockaddr *)addr, addr_len) == -1) { - log_err("Failed to bind server socket"); - goto close_out; - } - - if (type == SOCK_STREAM) { - 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(int type, const struct sockaddr_storage *addr, socklen_t addr_len) { @@ -1178,7 +1148,7 @@ static int run_bind_test_case(const struct sock_addr_test *test) if (init_addrs(test, &requested_addr, &expected_addr, NULL)) goto err;
- servfd = start_server(test->type, &requested_addr, addr_len); + servfd = start_server_addr(test->type, &requested_addr, addr_len); if (servfd == -1) goto err;
@@ -1214,7 +1184,7 @@ static int run_connect_test_case(const struct sock_addr_test *test) goto err;
/* Prepare server to connect to */ - servfd = start_server(test->type, &expected_addr, addr_len); + servfd = start_server_addr(test->type, &expected_addr, addr_len); if (servfd == -1) goto err;
@@ -1271,7 +1241,7 @@ static int run_xmsg_test_case(const struct sock_addr_test *test, int max_cmsg) goto err;
/* Prepare server to sendmsg to */ - servfd = start_server(test->type, &server_addr, addr_len); + servfd = start_server_addr(test->type, &server_addr, addr_len); if (servfd == -1) goto err;
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 test_sock_addr.c. This can avoid duplicate code.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/test_sock_addr.c | 36 ++------------------ 1 file changed, 2 insertions(+), 34 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_sock_addr.c b/tools/testing/selftests/bpf/test_sock_addr.c index 9ce5dad7468b..422255c8575e 100644 --- a/tools/testing/selftests/bpf/test_sock_addr.c +++ b/tools/testing/selftests/bpf/test_sock_addr.c @@ -940,38 +940,6 @@ static int cmp_peer_addr(int sock1, const struct sockaddr_storage *addr2) return cmp_sock_addr(getpeername, sock1, addr2, /*cmp_port*/ 1); }
-static int connect_to_server(int type, const struct sockaddr_storage *addr, - socklen_t addr_len) -{ - int domain; - int fd = -1; - - domain = addr->ss_family; - - if (domain != AF_INET && domain != AF_INET6) { - log_err("Unsupported address family"); - goto err; - } - - fd = socket(domain, type, 0); - if (fd == -1) { - log_err("Failed to create client socket"); - goto err; - } - - if (connect(fd, (const struct sockaddr *)addr, addr_len) == -1) { - log_err("Fail to connect to server"); - goto err; - } - - goto out; -err: - close(fd); - fd = -1; -out: - return fd; -} - int init_pktinfo(int domain, struct cmsghdr *cmsg) { struct in6_pktinfo *pktinfo6; @@ -1156,7 +1124,7 @@ static int run_bind_test_case(const struct sock_addr_test *test) goto err;
/* Try to connect to server just in case */ - clientfd = connect_to_server(test->type, &expected_addr, addr_len); + clientfd = connect_to_addr(test->type, &expected_addr, addr_len); if (clientfd == -1) goto err;
@@ -1188,7 +1156,7 @@ static int run_connect_test_case(const struct sock_addr_test *test) if (servfd == -1) goto err;
- clientfd = connect_to_server(test->type, &requested_addr, addr_len); + clientfd = connect_to_addr(test->type, &requested_addr, addr_len); if (clientfd == -1) goto err;
From: Geliang Tang tanggeliang@kylinos.cn
This patch uses public helper make_sockaddr() exported in network_helpers.h instead of the local defined function mk_sockaddr() in test_sock_addr.c. This can avoid duplicate code.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/test_sock_addr.c | 62 ++++---------------- 1 file changed, 12 insertions(+), 50 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_sock_addr.c b/tools/testing/selftests/bpf/test_sock_addr.c index 422255c8575e..09503a9b4bec 100644 --- a/tools/testing/selftests/bpf/test_sock_addr.c +++ b/tools/testing/selftests/bpf/test_sock_addr.c @@ -605,44 +605,6 @@ static struct sock_addr_test tests[] = { }, };
-static int mk_sockaddr(int domain, const char *ip, unsigned short port, - struct sockaddr *addr, socklen_t addr_len) -{ - struct sockaddr_in6 *addr6; - struct sockaddr_in *addr4; - - if (domain != AF_INET && domain != AF_INET6) { - log_err("Unsupported address family"); - return -1; - } - - memset(addr, 0, addr_len); - - if (domain == AF_INET) { - if (addr_len < sizeof(struct sockaddr_in)) - return -1; - addr4 = (struct sockaddr_in *)addr; - addr4->sin_family = domain; - addr4->sin_port = htons(port); - if (inet_pton(domain, ip, (void *)&addr4->sin_addr) != 1) { - log_err("Invalid IPv4: %s", ip); - return -1; - } - } else if (domain == AF_INET6) { - if (addr_len < sizeof(struct sockaddr_in6)) - return -1; - addr6 = (struct sockaddr_in6 *)addr; - addr6->sin6_family = domain; - addr6->sin6_port = htons(port); - if (inet_pton(domain, ip, (void *)&addr6->sin6_addr) != 1) { - log_err("Invalid IPv6: %s", ip); - return -1; - } - } - - return 0; -} - static int load_insns(const struct sock_addr_test *test, const struct bpf_insn *insns, size_t insns_cnt) { @@ -749,6 +711,7 @@ static int recvmsg_deny_prog_load(const struct sock_addr_test *test)
static int sendmsg4_rw_asm_prog_load(const struct sock_addr_test *test) { + socklen_t addr_len = sizeof(struct sockaddr_storage); struct sockaddr_in dst4_rw_addr; struct in_addr src4_rw_ip;
@@ -757,9 +720,8 @@ static int sendmsg4_rw_asm_prog_load(const struct sock_addr_test *test) return -1; }
- if (mk_sockaddr(AF_INET, SERV4_REWRITE_IP, SERV4_REWRITE_PORT, - (struct sockaddr *)&dst4_rw_addr, - sizeof(dst4_rw_addr)) == -1) + if (make_sockaddr(AF_INET, SERV4_REWRITE_IP, SERV4_REWRITE_PORT, + (struct sockaddr_storage *)&dst4_rw_addr, &addr_len) == -1) return -1;
struct bpf_insn insns[] = { @@ -812,6 +774,7 @@ static int sendmsg4_rw_c_prog_load(const struct sock_addr_test *test) static int sendmsg6_rw_dst_asm_prog_load(const struct sock_addr_test *test, const char *rw_dst_ip) { + socklen_t addr_len = sizeof(struct sockaddr_storage); struct sockaddr_in6 dst6_rw_addr; struct in6_addr src6_rw_ip;
@@ -820,9 +783,8 @@ static int sendmsg6_rw_dst_asm_prog_load(const struct sock_addr_test *test, return -1; }
- if (mk_sockaddr(AF_INET6, rw_dst_ip, SERV6_REWRITE_PORT, - (struct sockaddr *)&dst6_rw_addr, - sizeof(dst6_rw_addr)) == -1) + if (make_sockaddr(AF_INET6, rw_dst_ip, SERV6_REWRITE_PORT, + (struct sockaddr_storage *)&dst6_rw_addr, &addr_len) == -1) return -1;
struct bpf_insn insns[] = { @@ -1086,17 +1048,17 @@ static int init_addrs(const struct sock_addr_test *test, { socklen_t addr_len = sizeof(struct sockaddr_storage);
- if (mk_sockaddr(test->domain, test->expected_ip, test->expected_port, - (struct sockaddr *)expected_addr, addr_len) == -1) + if (make_sockaddr(test->domain, test->expected_ip, test->expected_port, + expected_addr, &addr_len) == -1) goto err;
- if (mk_sockaddr(test->domain, test->requested_ip, test->requested_port, - (struct sockaddr *)requested_addr, addr_len) == -1) + if (make_sockaddr(test->domain, test->requested_ip, test->requested_port, + requested_addr, &addr_len) == -1) goto err;
if (test->expected_src_ip && - mk_sockaddr(test->domain, test->expected_src_ip, 0, - (struct sockaddr *)expected_src_addr, addr_len) == -1) + make_sockaddr(test->domain, test->expected_src_ip, 0, + expected_src_addr, &addr_len) == -1) goto err;
return 0;
From: Geliang Tang tanggeliang@kylinos.cn
This patch uses public helper make_sockaddr() exported in network_helpers.h instead of open-coding in bind_sock() in test_sock.c. This can avoid duplicate code.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/Makefile | 1 + tools/testing/selftests/bpf/test_sock.c | 28 ++++--------------------- 2 files changed, 5 insertions(+), 24 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 53713f4495dd..017fe30f49a8 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -308,6 +308,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_sock: $(NETWORK_HELPERS)
BPFTOOL ?= $(DEFAULT_BPFTOOL) $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \ diff --git a/tools/testing/selftests/bpf/test_sock.c b/tools/testing/selftests/bpf/test_sock.c index 810c3740b2cc..ac00f598ff8e 100644 --- a/tools/testing/selftests/bpf/test_sock.c +++ b/tools/testing/selftests/bpf/test_sock.c @@ -13,6 +13,7 @@ #include <bpf/bpf.h>
#include "cgroup_helpers.h" +#include "network_helpers.h" #include <bpf/bpf_endian.h> #include "bpf_util.h"
@@ -414,8 +415,6 @@ static int bind_sock(int domain, int type, const char *ip, unsigned short port, unsigned short port_retry) { struct sockaddr_storage addr; - struct sockaddr_in6 *addr6; - struct sockaddr_in *addr4; int sockfd = -1; socklen_t len; int res = SUCCESS; @@ -424,25 +423,8 @@ static int bind_sock(int domain, int type, const char *ip, if (sockfd < 0) goto err;
- memset(&addr, 0, sizeof(addr)); - - if (domain == AF_INET) { - len = sizeof(struct sockaddr_in); - addr4 = (struct sockaddr_in *)&addr; - addr4->sin_family = domain; - addr4->sin_port = htons(port); - if (inet_pton(domain, ip, (void *)&addr4->sin_addr) != 1) - goto err; - } else if (domain == AF_INET6) { - len = sizeof(struct sockaddr_in6); - addr6 = (struct sockaddr_in6 *)&addr; - addr6->sin6_family = domain; - addr6->sin6_port = htons(port); - if (inet_pton(domain, ip, (void *)&addr6->sin6_addr) != 1) - goto err; - } else { + if (make_sockaddr(domain, ip, port, &addr, &len)) goto err; - }
if (bind(sockfd, (const struct sockaddr *)&addr, len) == -1) { /* sys_bind() may fail for different reasons, errno has to be @@ -458,10 +440,8 @@ static int bind_sock(int domain, int type, const char *ip,
goto out; retry: - if (domain == AF_INET) - addr4->sin_port = htons(port_retry); - else - addr6->sin6_port = htons(port_retry); + if (make_sockaddr(domain, ip, port_retry, &addr, &len)) + goto err; if (bind(sockfd, (const struct sockaddr *)&addr, len) == -1) { if (errno != EPERM) goto err;
From: Geliang Tang tanggeliang@kylinos.cn
This patch uses public helper make_sockaddr() exported in network_helpers.h instead of open-coding in test_bpf_ip_check_defrag_ok() in prog_tests/ip_check_defrag.c. This can avoid duplicate code.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- .../selftests/bpf/prog_tests/ip_check_defrag.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/ip_check_defrag.c b/tools/testing/selftests/bpf/prog_tests/ip_check_defrag.c index 284764e7179f..aafadb0ca666 100644 --- a/tools/testing/selftests/bpf/prog_tests/ip_check_defrag.c +++ b/tools/testing/selftests/bpf/prog_tests/ip_check_defrag.c @@ -220,21 +220,9 @@ void test_bpf_ip_check_defrag_ok(bool ipv6) nstoken = open_netns(NS0); if (!ASSERT_OK_PTR(nstoken, "setns ns0")) goto out; - if (ipv6) { - struct sockaddr_in6 *c = (struct sockaddr_in6 *)&caddr; - - c->sin6_family = AF_INET6; - inet_pton(AF_INET6, VETH0_ADDR6, &c->sin6_addr); - c->sin6_port = htons(CLIENT_PORT); - err = bind(client_rx_fd, (struct sockaddr *)c, sizeof(*c)); - } else { - struct sockaddr_in *c = (struct sockaddr_in *)&caddr; - - c->sin_family = AF_INET; - inet_pton(AF_INET, VETH0_ADDR, &c->sin_addr); - c->sin_port = htons(CLIENT_PORT); - err = bind(client_rx_fd, (struct sockaddr *)c, sizeof(*c)); - } + make_sockaddr(ipv6 ? AF_INET6 : AF_INET, ipv6 ? VETH0_ADDR6 : VETH0_ADDR, + CLIENT_PORT, &caddr, &caddr_len); + err = bind(client_rx_fd, (struct sockaddr *)&caddr, sizeof(caddr)); close_netns(nstoken); if (!ASSERT_OK(err, "bind")) goto out;
linux-kselftest-mirror@lists.linaro.org
-
Geliang Tang -
Martin KaFai Lau