From: Geliang Tang tanggeliang@kylinos.cn
v11: - new patches 2, 4, 6. - drop expect_errno from network_helper_opts as Eduard and Martin suggested. - drop sockmap_ktls patches from this set. - add a new helper connect_fd_to_addr_str.
v10: - a new patch 10 is added. - patches 1-6, 8-9 unchanged, only commit logs updated. - "err = -errno" is used in patches 7, 11, 12 to get the real error number before checking value of "err".
v9: - new patches 5-7, new struct member expect_errno for network_helper_opts. - patches 1-4, 8-9 unchanged. - update patches 10-11 to make sure all tests pass.
v8: - only patch 8 updated, to fix errors reported by CI.
v7: - address Martin's comments in v6. (thanks) - use MAX(opts->backlog, 0) instead of opts->backlog. - use connect_to_fd_opts instead connect_to_fd. - more ASSERT_* to check errors.
v6: - update patch 6 as Daniel suggested. (thanks)
v5: - keep make_server and make_client as Eduard suggested.
v4: - a new patch to use make_sockaddr in sockmap_ktls. - a new patch to close fd in error path in drop_on_reuseport. - drop make_server() in patch 7. - drop make_client() too in patch 9.
v3: - a new patch to add backlog for network_helper_opts. - use start_server_str in sockmap_ktls now, not start_server.
v2: - address Eduard's comments in v1. (thanks) - fix errors reported by CI.
This patch set uses network helpers in sk_lookup, and drop the local helpers inetaddr_len() and make_socket().
Geliang Tang (9): selftests/bpf: Add backlog for network_helper_opts selftests/bpf: Add ASSERT_OK_FD macro selftests/bpf: Close fd in error path in drop_on_reuseport selftests/bpf: Use start_server_str in sk_lookup selftests/bpf: Use start_server_addr in sk_lookup selftests/bpf: Use connect_fd_to_fd in sk_lookup selftests/bpf: Add connect_fd_to_addr_str helper selftests/bpf: Use connect_fd_to_addr_str in sk_lookup selftests/bpf: Drop make_socket in sk_lookup
tools/testing/selftests/bpf/network_helpers.c | 23 ++- tools/testing/selftests/bpf/network_helpers.h | 7 + .../selftests/bpf/prog_tests/sk_lookup.c | 156 ++++++------------ tools/testing/selftests/bpf/test_progs.h | 8 + 4 files changed, 92 insertions(+), 102 deletions(-)
From: Geliang Tang tanggeliang@kylinos.cn
Some callers expect __start_server() helper to pass their own "backlog" value to listen() instead of the default of 1. So this patch adds struct member "backlog" for network_helper_opts to allow callers to set "backlog" value via start_server_str() helper.
listen(fd, 0 /* backlog */) can be used to enforce syncookie. Meaning backlog 0 is a legit value.
Using 0 as a default and changing it to 1 here is fine. It makes the test program easier to write for the common case. Enforcing syncookie mode by using backlog 0 is a niche use case but it should at least have a way for the caller to do that.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/network_helpers.c | 2 +- tools/testing/selftests/bpf/network_helpers.h | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index 44c2c8fa542a..e0cba4178e41 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -106,7 +106,7 @@ static int __start_server(int type, const struct sockaddr *addr, socklen_t addrl }
if (type == SOCK_STREAM) { - if (listen(fd, 1) < 0) { + if (listen(fd, opts->backlog ? MAX(opts->backlog, 0) : 1) < 0) { log_err("Failed to listed on socket"); goto error_close; } diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h index 9ea36524b9db..4f26bfc2dbf5 100644 --- a/tools/testing/selftests/bpf/network_helpers.h +++ b/tools/testing/selftests/bpf/network_helpers.h @@ -25,6 +25,10 @@ struct network_helper_opts { int timeout_ms; bool must_fail; int proto; + /* The backlog argument for listen(), defines the maximum length to which + * the queue of pending connections for sockfd may grow. + */ + int backlog; int (*post_socket_cb)(int fd, void *opts); void *cb_opts; };
From: Geliang Tang tanggeliang@kylinos.cn
Add a new dedicated ASSERT macro ASSERT_OK_FD to test whether a socket FD is valid or not. It can be used to replace macros ASSERT_GT(fd, 0, ""), ASSERT_NEQ(fd, -1, "") or statements (fd < 0), (fd != -1).
Suggested-by: Martin KaFai Lau martin.lau@kernel.org Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/test_progs.h | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h index 0ba5a20b19ba..4f7b91c25b1e 100644 --- a/tools/testing/selftests/bpf/test_progs.h +++ b/tools/testing/selftests/bpf/test_progs.h @@ -377,6 +377,14 @@ int test__join_cgroup(const char *path); ___ok; \ })
+#define ASSERT_OK_FD(fd, name) ({ \ + static int duration = 0; \ + int ___fd = (fd); \ + bool ___ok = ___fd >= 0; \ + CHECK(!___ok, (name), "unexpected fd: %d\n", ___fd); \ + ___ok; \ +}) + #define SYS(goto_label, fmt, ...) \ ({ \ char cmd[1024]; \
On 7/9/24 2:16 AM, Geliang Tang wrote:
From: Geliang Tang tanggeliang@kylinos.cn
Add a new dedicated ASSERT macro ASSERT_OK_FD to test whether a socket FD is valid or not. It can be used to replace macros ASSERT_GT(fd, 0, ""), ASSERT_NEQ(fd, -1, "") or statements (fd < 0), (fd != -1).
Suggested-by: Martin KaFai Lau martin.lau@kernel.org Signed-off-by: Geliang Tang tanggeliang@kylinos.cn
tools/testing/selftests/bpf/test_progs.h | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h index 0ba5a20b19ba..4f7b91c25b1e 100644 --- a/tools/testing/selftests/bpf/test_progs.h +++ b/tools/testing/selftests/bpf/test_progs.h @@ -377,6 +377,14 @@ int test__join_cgroup(const char *path); ___ok; \ }) +#define ASSERT_OK_FD(fd, name) ({ \
- static int duration = 0; \
- int ___fd = (fd); \
- bool ___ok = ___fd >= 0; \
- CHECK(!___ok, (name), "unexpected fd: %d\n", ___fd); \
printing errno should be useful.
- ___ok; \
+})
- #define SYS(goto_label, fmt, ...) \ ({ \ char cmd[1024]; \
From: Geliang Tang tanggeliang@kylinos.cn
In the error path when update_lookup_map() fails in drop_on_reuseport in prog_tests/sk_lookup.c, "server1", the fd of server 1, should be closed. This patch fixes this by using "goto close_srv1" lable instead of "detach" to close "server1" in this case.
Acked-by: Eduard Zingerman eddyz87@gmail.com Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c index 597d0467a926..de2466547efe 100644 --- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c +++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c @@ -994,7 +994,7 @@ static void drop_on_reuseport(const struct test *t)
err = update_lookup_map(t->sock_map, SERVER_A, server1); if (err) - goto detach; + goto close_srv1;
/* second server on destination address we should never reach */ server2 = make_server(t->sotype, t->connect_to.ip, t->connect_to.port,
On 7/9/24 2:16 AM, Geliang Tang wrote:
From: Geliang Tang tanggeliang@kylinos.cn
In the error path when update_lookup_map() fails in drop_on_reuseport in prog_tests/sk_lookup.c, "server1", the fd of server 1, should be closed. This patch fixes this by using "goto close_srv1" lable instead of "detach" to close "server1" in this case.
A reference to the Fixes tag will be useful
Fixes: 0ab5539f8584 ("selftests/bpf: Tests for BPF_SK_LOOKUP attach point")
From: Geliang Tang tanggeliang@kylinos.cn
This patch uses public helper start_server_str() to simplify make_server() in prog_tests/sk_lookup.c.
Add a callback setsockopts() to do all sockopts, set it to post_socket_cb pointer of struct network_helper_opts. And add a new struct cb_opts to save the data needed to pass to the callback. Then pass this network_helper_opts to start_server_str().
Also use ASSERT_OK_FD() to check fd returned by start_server_str().
Acked-by: Eduard Zingerman eddyz87@gmail.com Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- .../selftests/bpf/prog_tests/sk_lookup.c | 58 +++++++++++-------- 1 file changed, 34 insertions(+), 24 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c index de2466547efe..20ee5da2c721 100644 --- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c +++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c @@ -77,6 +77,12 @@ struct test { bool reuseport_has_conns; /* Add a connected socket to reuseport group */ };
+struct cb_opts { + int family; + int sotype; + bool reuseport; +}; + static __u32 duration; /* for CHECK macro */
static bool is_ipv6(const char *ip) @@ -142,19 +148,14 @@ static int make_socket(int sotype, const char *ip, int port, return fd; }
-static int make_server(int sotype, const char *ip, int port, - struct bpf_program *reuseport_prog) +static int setsockopts(int fd, void *opts) { - struct sockaddr_storage addr = {0}; + struct cb_opts *co = (struct cb_opts *)opts; const int one = 1; - int err, fd = -1; - - fd = make_socket(sotype, ip, port, &addr); - if (fd < 0) - return -1; + int err = 0;
/* Enabled for UDPv6 sockets for IPv4-mapped IPv6 to work. */ - if (sotype == SOCK_DGRAM) { + if (co->sotype == SOCK_DGRAM) { err = setsockopt(fd, SOL_IP, IP_RECVORIGDSTADDR, &one, sizeof(one)); if (CHECK(err, "setsockopt(IP_RECVORIGDSTADDR)", "failed\n")) { @@ -163,7 +164,7 @@ static int make_server(int sotype, const char *ip, int port, } }
- if (sotype == SOCK_DGRAM && addr.ss_family == AF_INET6) { + if (co->sotype == SOCK_DGRAM && co->family == AF_INET6) { err = setsockopt(fd, SOL_IPV6, IPV6_RECVORIGDSTADDR, &one, sizeof(one)); if (CHECK(err, "setsockopt(IPV6_RECVORIGDSTADDR)", "failed\n")) { @@ -172,7 +173,7 @@ static int make_server(int sotype, const char *ip, int port, } }
- if (sotype == SOCK_STREAM) { + if (co->sotype == SOCK_STREAM) { err = setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)); if (CHECK(err, "setsockopt(SO_REUSEADDR)", "failed\n")) { @@ -181,7 +182,7 @@ static int make_server(int sotype, const char *ip, int port, } }
- if (reuseport_prog) { + if (co->reuseport) { err = setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &one, sizeof(one)); if (CHECK(err, "setsockopt(SO_REUSEPORT)", "failed\n")) { @@ -190,19 +191,28 @@ static int make_server(int sotype, const char *ip, int port, } }
- err = bind(fd, (void *)&addr, inetaddr_len(&addr)); - if (CHECK(err, "bind", "failed\n")) { - log_err("failed to bind listen socket"); - goto fail; - } +fail: + return err; +}
- if (sotype == SOCK_STREAM) { - err = listen(fd, SOMAXCONN); - if (CHECK(err, "make_server", "listen")) { - log_err("failed to listen on port %d", port); - goto fail; - } - } +static int make_server(int sotype, const char *ip, int port, + struct bpf_program *reuseport_prog) +{ + struct cb_opts cb_opts = { + .family = is_ipv6(ip) ? AF_INET6 : AF_INET, + .sotype = sotype, + .reuseport = reuseport_prog, + }; + struct network_helper_opts opts = { + .backlog = SOMAXCONN, + .post_socket_cb = setsockopts, + .cb_opts = &cb_opts, + }; + int err, fd; + + fd = start_server_str(cb_opts.family, sotype, ip, port, &opts); + if (!ASSERT_OK_FD(fd, "start_server_str")) + return -1;
/* Late attach reuseport prog so we can have one init path */ if (reuseport_prog) {
From: Geliang Tang tanggeliang@kylinos.cn
This patch uses public helper start_server_addr() in udp_recv_send() in prog_tests/sk_lookup.c to simplify the code.
And use ASSERT_OK_FD() to check fd returned by start_server_addr().
Acked-by: Eduard Zingerman eddyz87@gmail.com Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c index 20ee5da2c721..386e482be617 100644 --- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c +++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c @@ -416,18 +416,12 @@ static int udp_recv_send(int server_fd) }
/* Reply from original destination address. */ - fd = socket(dst_addr->ss_family, SOCK_DGRAM, 0); - if (CHECK(fd < 0, "socket", "failed\n")) { + fd = start_server_addr(SOCK_DGRAM, dst_addr, sizeof(*dst_addr), NULL); + if (!ASSERT_OK_FD(fd, "start_server_addr")) { log_err("failed to create tx socket"); return -1; }
- ret = bind(fd, (struct sockaddr *)dst_addr, sizeof(*dst_addr)); - if (CHECK(ret, "bind", "failed\n")) { - log_err("failed to bind tx socket"); - goto out; - } - msg.msg_control = NULL; msg.msg_controllen = 0; n = sendmsg(fd, &msg, 0);
From: Geliang Tang tanggeliang@kylinos.cn
This patch uses public helper connect_fd_to_fd() exported in network_helpers.h instead of using getsockname() + connect() in run_lookup_prog() in prog_tests/sk_lookup.c. This can simplify the code.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c index 386e482be617..ad3f943cc2bd 100644 --- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c +++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c @@ -633,9 +633,6 @@ static void run_lookup_prog(const struct test *t) * BPF socket lookup. */ if (t->reuseport_has_conns) { - struct sockaddr_storage addr = {}; - socklen_t len = sizeof(addr); - /* Add an extra socket to reuseport group */ reuse_conn_fd = make_server(t->sotype, t->listen_at.ip, t->listen_at.port, @@ -643,12 +640,8 @@ static void run_lookup_prog(const struct test *t) if (reuse_conn_fd < 0) goto close;
- /* Connect the extra socket to itself */ - err = getsockname(reuse_conn_fd, (void *)&addr, &len); - if (CHECK(err, "getsockname", "errno %d\n", errno)) - goto close; - err = connect(reuse_conn_fd, (void *)&addr, len); - if (CHECK(err, "connect", "errno %d\n", errno)) + err = connect_fd_to_fd(reuse_conn_fd, reuse_conn_fd, 0); + if (!ASSERT_OK(err, "connect_fd_to_fd")) goto close; }
On 7/9/24 2:16 AM, Geliang Tang wrote:
From: Geliang Tang tanggeliang@kylinos.cn
This patch uses public helper connect_fd_to_fd() exported in network_helpers.h instead of using getsockname() + connect() in run_lookup_prog() in prog_tests/sk_lookup.c. This can simplify the code.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn
tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c index 386e482be617..ad3f943cc2bd 100644 --- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c +++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c @@ -633,9 +633,6 @@ static void run_lookup_prog(const struct test *t) * BPF socket lookup. */ if (t->reuseport_has_conns) {
struct sockaddr_storage addr = {};socklen_t len = sizeof(addr);- /* Add an extra socket to reuseport group */ reuse_conn_fd = make_server(t->sotype, t->listen_at.ip, t->listen_at.port,
@@ -643,12 +640,8 @@ static void run_lookup_prog(const struct test *t) if (reuse_conn_fd < 0) goto close;
/* Connect the extra socket to itself */
This comment is still valid after this change.
err = getsockname(reuse_conn_fd, (void *)&addr, &len);if (CHECK(err, "getsockname", "errno %d\n", errno))goto close;err = connect(reuse_conn_fd, (void *)&addr, len);if (CHECK(err, "connect", "errno %d\n", errno))
err = connect_fd_to_fd(reuse_conn_fd, reuse_conn_fd, 0);if (!ASSERT_OK(err, "connect_fd_to_fd")) goto close;
From: Geliang Tang tanggeliang@kylinos.cn
Similar to connect_fd_to_fd() helper to connect from a client fd to a server fd, this patch adds a new helper connect_fd_to_addr_str() to connect from a client fd to a server address. It accepts the server address string "addr_str", together with the server family, type and port, as parameters instead of using a "server_fd" like connect_fd_to_fd().
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/network_helpers.c | 21 +++++++++++++++++++ tools/testing/selftests/bpf/network_helpers.h | 3 +++ 2 files changed, 24 insertions(+)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index e0cba4178e41..9758e707b859 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -388,6 +388,27 @@ int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms) return 0; }
+int connect_fd_to_addr_str(int client_fd, int family, int type, + const char *addr_str, __u16 port, + const struct network_helper_opts *opts) +{ + struct sockaddr_storage addr; + socklen_t len; + + if (!opts) + opts = &default_opts; + + if (settimeo(client_fd, opts->timeout_ms)) + return -1; + + if (make_sockaddr(family, addr_str, port, &addr, &len)) { + log_err("Failed to make server addr"); + return -1; + } + + return connect_fd_to_addr(client_fd, &addr, len, false); +} + int make_sockaddr(int family, const char *addr_str, __u16 port, struct sockaddr_storage *addr, socklen_t *len) { diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h index 4f26bfc2dbf5..43526271366f 100644 --- a/tools/testing/selftests/bpf/network_helpers.h +++ b/tools/testing/selftests/bpf/network_helpers.h @@ -67,6 +67,9 @@ 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, int type, const struct network_helper_opts *opts); int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms); +int connect_fd_to_addr_str(int client_fd, int family, int type, + const char *addr_str, __u16 port, + const struct network_helper_opts *opts); int fastopen_connect(int server_fd, const char *data, unsigned int data_len, int timeout_ms); int make_sockaddr(int family, const char *addr_str, __u16 port,
On 7/9/24 2:16 AM, Geliang Tang wrote:
From: Geliang Tang tanggeliang@kylinos.cn
Similar to connect_fd_to_fd() helper to connect from a client fd to a server fd, this patch adds a new helper connect_fd_to_addr_str() to connect from a client fd to a server address. It accepts the server address string "addr_str", together with the server family, type and port, as parameters instead of using a "server_fd" like connect_fd_to_fd().
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn
tools/testing/selftests/bpf/network_helpers.c | 21 +++++++++++++++++++ tools/testing/selftests/bpf/network_helpers.h | 3 +++ 2 files changed, 24 insertions(+)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index e0cba4178e41..9758e707b859 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -388,6 +388,27 @@ int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms) return 0; } +int connect_fd_to_addr_str(int client_fd, int family, int type,
Similar to the comment in the earlier revision on the existing connect_to_fd_opts. The "int type" is redundant of "int client_fd".
and where is the "int type" arg actually used in this new function?
Beside, is it more useful for patch 8 to add connect_to_addr_str() which calls socket()/client_socket(), connect(), and then return the client_fd instead?
Something like this?
int connect_to_addr_str(int family, int type, const char *addr_str, __u16 port, const struct network_helper_opts *opts)
Patch 1-6 is applied with the mentioned minor changes. Thanks.
const char *addr_str, __u16 port,const struct network_helper_opts *opts)+{
- struct sockaddr_storage addr;
- socklen_t len;
- if (!opts)
opts = &default_opts;- if (settimeo(client_fd, opts->timeout_ms))
return -1;- if (make_sockaddr(family, addr_str, port, &addr, &len)) {
log_err("Failed to make server addr");return -1;- }
- return connect_fd_to_addr(client_fd, &addr, len, false);
+}
- int make_sockaddr(int family, const char *addr_str, __u16 port, struct sockaddr_storage *addr, socklen_t *len) {
From: Geliang Tang tanggeliang@kylinos.cn
This patch uses the new helper connect_fd_to_addr_str() in make_client() instead of using local defined function make_socket() + connect(). This local function can be dropped latter.
A new parameter "expect_errno" is added for make_client() too to allow different "expect_errno" is passed to make_client(). It is used to check with "errno" after invoking connect_fd_to_addr_str().
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- .../testing/selftests/bpf/prog_tests/sk_lookup.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c index ad3f943cc2bd..26a1c339492e 100644 --- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c +++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c @@ -229,17 +229,17 @@ static int make_server(int sotype, const char *ip, int port, return -1; }
-static int make_client(int sotype, const char *ip, int port) +static int make_client(int sotype, const char *ip, int port, int expect_errno) { - struct sockaddr_storage addr = {0}; + int family = is_ipv6(ip) ? AF_INET6 : AF_INET; int err, fd;
- fd = make_socket(sotype, ip, port, &addr); + fd = socket(family, sotype, 0); if (fd < 0) return -1;
- err = connect(fd, (void *)&addr, inetaddr_len(&addr)); - if (CHECK(err, "make_client", "connect")) { + err = connect_fd_to_addr_str(fd, family, sotype, ip, port, NULL); + if (CHECK(err && (!expect_errno || errno != expect_errno), "make_client", "connect")) { log_err("failed to connect client socket"); goto fail; } @@ -645,7 +645,7 @@ static void run_lookup_prog(const struct test *t) goto close; }
- client_fd = make_client(t->sotype, t->connect_to.ip, t->connect_to.port); + client_fd = make_client(t->sotype, t->connect_to.ip, t->connect_to.port, 0); if (client_fd < 0) goto close;
@@ -1151,7 +1151,7 @@ static void run_sk_assign_connected(struct test_sk_lookup *skel, if (server_fd < 0) return;
- connected_fd = make_client(sotype, EXT_IP4, EXT_PORT); + connected_fd = make_client(sotype, EXT_IP4, EXT_PORT, 0); if (connected_fd < 0) goto out_close_server;
@@ -1165,7 +1165,7 @@ static void run_sk_assign_connected(struct test_sk_lookup *skel, goto out_close_connected;
/* Try to redirect TCP SYN / UDP packet to a connected socket */ - client_fd = make_client(sotype, EXT_IP4, EXT_PORT); + client_fd = make_client(sotype, EXT_IP4, EXT_PORT, 0); if (client_fd < 0) goto out_unlink_prog; if (sotype == SOCK_DGRAM) {
From: Geliang Tang tanggeliang@kylinos.cn
Use local helper make_client() in drop_on_lookup(), drop_on_reuseport() and run_multi_prog_lookup() instead of using make_socket() + connect(). Then make_socket() and inetaddr_len() can be dropped now.
make_client() returns normal "fd" instead of "-1" when connect() fails but "expect_errno" matched in it. So "err = -errno" is needed to get the real error number before checking value of "err".
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- .../selftests/bpf/prog_tests/sk_lookup.c | 59 +++---------------- 1 file changed, 8 insertions(+), 51 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c index 26a1c339492e..f4bc6f968789 100644 --- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c +++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c @@ -108,46 +108,6 @@ static int attach_reuseport(int sock_fd, struct bpf_program *reuseport_prog) return 0; }
-static socklen_t inetaddr_len(const struct sockaddr_storage *addr) -{ - return (addr->ss_family == AF_INET ? sizeof(struct sockaddr_in) : - addr->ss_family == AF_INET6 ? sizeof(struct sockaddr_in6) : 0); -} - -static int make_socket(int sotype, const char *ip, int port, - struct sockaddr_storage *addr) -{ - struct timeval timeo = { .tv_sec = IO_TIMEOUT_SEC }; - int err, family, fd; - - family = is_ipv6(ip) ? AF_INET6 : AF_INET; - err = make_sockaddr(family, ip, port, addr, NULL); - if (CHECK(err, "make_address", "failed\n")) - return -1; - - fd = socket(addr->ss_family, sotype, 0); - if (CHECK(fd < 0, "socket", "failed\n")) { - log_err("failed to make socket"); - return -1; - } - - err = setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &timeo, sizeof(timeo)); - if (CHECK(err, "setsockopt(SO_SNDTIMEO)", "failed\n")) { - log_err("failed to set SNDTIMEO"); - close(fd); - return -1; - } - - err = setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo)); - if (CHECK(err, "setsockopt(SO_RCVTIMEO)", "failed\n")) { - log_err("failed to set RCVTIMEO"); - close(fd); - return -1; - } - - return fd; -} - static int setsockopts(int fd, void *opts) { struct cb_opts *co = (struct cb_opts *)opts; @@ -861,7 +821,6 @@ static void test_redirect_lookup(struct test_sk_lookup *skel)
static void drop_on_lookup(const struct test *t) { - struct sockaddr_storage dst = {}; int client_fd, server_fd, err; struct bpf_link *lookup_link; ssize_t n; @@ -875,12 +834,12 @@ static void drop_on_lookup(const struct test *t) if (server_fd < 0) goto detach;
- client_fd = make_socket(t->sotype, t->connect_to.ip, - t->connect_to.port, &dst); + client_fd = make_client(t->sotype, t->connect_to.ip, + t->connect_to.port, ECONNREFUSED); if (client_fd < 0) goto close_srv;
- err = connect(client_fd, (void *)&dst, inetaddr_len(&dst)); + err = -errno; if (t->sotype == SOCK_DGRAM) { err = send_byte(client_fd); if (err) @@ -975,7 +934,6 @@ static void test_drop_on_lookup(struct test_sk_lookup *skel)
static void drop_on_reuseport(const struct test *t) { - struct sockaddr_storage dst = { 0 }; int client, server1, server2, err; struct bpf_link *lookup_link; ssize_t n; @@ -999,12 +957,12 @@ static void drop_on_reuseport(const struct test *t) if (server2 < 0) goto close_srv1;
- client = make_socket(t->sotype, t->connect_to.ip, - t->connect_to.port, &dst); + client = make_client(t->sotype, t->connect_to.ip, + t->connect_to.port, ECONNREFUSED); if (client < 0) goto close_srv2;
- err = connect(client, (void *)&dst, inetaddr_len(&dst)); + err = -errno; if (t->sotype == SOCK_DGRAM) { err = send_byte(client); if (err) @@ -1214,7 +1172,6 @@ struct test_multi_prog {
static void run_multi_prog_lookup(const struct test_multi_prog *t) { - struct sockaddr_storage dst = {}; int map_fd, server_fd, client_fd; struct bpf_link *link1, *link2; int prog_idx, done, err; @@ -1247,11 +1204,11 @@ static void run_multi_prog_lookup(const struct test_multi_prog *t) if (err) goto out_close_server;
- client_fd = make_socket(SOCK_STREAM, EXT_IP4, EXT_PORT, &dst); + client_fd = make_client(SOCK_STREAM, EXT_IP4, EXT_PORT, t->expect_errno); if (client_fd < 0) goto out_close_server;
- err = connect(client_fd, (void *)&dst, inetaddr_len(&dst)); + err = t->expect_errno ? -errno : 0; if (CHECK(err && !t->expect_errno, "connect", "unexpected error %d\n", errno)) goto out_close_client;
Hello:
This series was applied to bpf/bpf-next.git (master) by Martin KaFai Lau martin.lau@kernel.org:
On Tue, 9 Jul 2024 17:16:16 +0800 you wrote:
From: Geliang Tang tanggeliang@kylinos.cn
v11:
- new patches 2, 4, 6.
- drop expect_errno from network_helper_opts as Eduard and Martin suggested.
- drop sockmap_ktls patches from this set.
- add a new helper connect_fd_to_addr_str.
[...]
Here is the summary with links: - [bpf-next,v11,1/9] selftests/bpf: Add backlog for network_helper_opts https://git.kernel.org/bpf/bpf-next/c/a3016a27cea8 - [bpf-next,v11,2/9] selftests/bpf: Add ASSERT_OK_FD macro https://git.kernel.org/bpf/bpf-next/c/7046345d48ad - [bpf-next,v11,3/9] selftests/bpf: Close fd in error path in drop_on_reuseport https://git.kernel.org/bpf/bpf-next/c/adae187ebedc - [bpf-next,v11,4/9] selftests/bpf: Use start_server_str in sk_lookup https://git.kernel.org/bpf/bpf-next/c/14fc6fcd35e7 - [bpf-next,v11,5/9] selftests/bpf: Use start_server_addr in sk_lookup https://git.kernel.org/bpf/bpf-next/c/d9810c43f660 - [bpf-next,v11,6/9] selftests/bpf: Use connect_fd_to_fd in sk_lookup https://git.kernel.org/bpf/bpf-next/c/9004054b1629 - [bpf-next,v11,7/9] selftests/bpf: Add connect_fd_to_addr_str helper (no matching commit) - [bpf-next,v11,8/9] selftests/bpf: Use connect_fd_to_addr_str in sk_lookup (no matching commit) - [bpf-next,v11,9/9] selftests/bpf: Drop make_socket in sk_lookup (no matching commit)
You are awesome, thank you!
linux-kselftest-mirror@lists.linaro.org
-
Geliang Tang -
Martin KaFai Lau -
patchwork-bot+netdevbpf＠kernel.org