* [PATCH bpf-next v11 1/9] selftests/bpf: Add backlog for network_helper_opts
2024-07-09 9:16 [PATCH bpf-next v11 0/9] use network helpers, part 8 Geliang Tang
@ 2024-07-09 9:16 ` Geliang Tang
2024-07-09 9:16 ` [PATCH bpf-next v11 2/9] selftests/bpf: Add ASSERT_OK_FD macro Geliang Tang
` (8 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2024-07-09 9:16 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
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;
};
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH bpf-next v11 2/9] selftests/bpf: Add ASSERT_OK_FD macro
2024-07-09 9:16 [PATCH bpf-next v11 0/9] use network helpers, part 8 Geliang Tang
2024-07-09 9:16 ` [PATCH bpf-next v11 1/9] selftests/bpf: Add backlog for network_helper_opts Geliang Tang
@ 2024-07-09 9:16 ` Geliang Tang
2024-07-10 19:00 ` Martin KaFai Lau
2024-07-09 9:16 ` [PATCH bpf-next v11 3/9] selftests/bpf: Close fd in error path in drop_on_reuseport Geliang Tang
` (7 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Geliang Tang @ 2024-07-09 9:16 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, Martin KaFai Lau
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]; \
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next v11 2/9] selftests/bpf: Add ASSERT_OK_FD macro
2024-07-09 9:16 ` [PATCH bpf-next v11 2/9] selftests/bpf: Add ASSERT_OK_FD macro Geliang Tang
@ 2024-07-10 19:00 ` Martin KaFai Lau
0 siblings, 0 replies; 15+ messages in thread
From: Martin KaFai Lau @ 2024-07-10 19:00 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, Hao Luo, Jiri Olsa, Shuah Khan,
Geliang Tang, bpf, linux-kselftest, Martin KaFai Lau,
Stanislav Fomichev
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]; \
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH bpf-next v11 3/9] selftests/bpf: Close fd in error path in drop_on_reuseport
2024-07-09 9:16 [PATCH bpf-next v11 0/9] use network helpers, part 8 Geliang Tang
2024-07-09 9:16 ` [PATCH bpf-next v11 1/9] selftests/bpf: Add backlog for network_helper_opts Geliang Tang
2024-07-09 9:16 ` [PATCH bpf-next v11 2/9] selftests/bpf: Add ASSERT_OK_FD macro Geliang Tang
@ 2024-07-09 9:16 ` Geliang Tang
2024-07-10 19:02 ` Martin KaFai Lau
2024-07-09 9:16 ` [PATCH bpf-next v11 4/9] selftests/bpf: Use start_server_str in sk_lookup Geliang Tang
` (6 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Geliang Tang @ 2024-07-09 9:16 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
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,
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next v11 3/9] selftests/bpf: Close fd in error path in drop_on_reuseport
2024-07-09 9:16 ` [PATCH bpf-next v11 3/9] selftests/bpf: Close fd in error path in drop_on_reuseport Geliang Tang
@ 2024-07-10 19:02 ` Martin KaFai Lau
0 siblings, 0 replies; 15+ messages in thread
From: Martin KaFai Lau @ 2024-07-10 19:02 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, Hao Luo, Jiri Olsa, Shuah Khan,
Geliang Tang, bpf, linux-kselftest, Stanislav Fomichev
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")
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH bpf-next v11 4/9] selftests/bpf: Use start_server_str in sk_lookup
2024-07-09 9:16 [PATCH bpf-next v11 0/9] use network helpers, part 8 Geliang Tang
` (2 preceding siblings ...)
2024-07-09 9:16 ` [PATCH bpf-next v11 3/9] selftests/bpf: Close fd in error path in drop_on_reuseport Geliang Tang
@ 2024-07-09 9:16 ` Geliang Tang
2024-07-09 9:16 ` [PATCH bpf-next v11 5/9] selftests/bpf: Use start_server_addr " Geliang Tang
` (5 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2024-07-09 9:16 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
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) {
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH bpf-next v11 5/9] selftests/bpf: Use start_server_addr in sk_lookup
2024-07-09 9:16 [PATCH bpf-next v11 0/9] use network helpers, part 8 Geliang Tang
` (3 preceding siblings ...)
2024-07-09 9:16 ` [PATCH bpf-next v11 4/9] selftests/bpf: Use start_server_str in sk_lookup Geliang Tang
@ 2024-07-09 9:16 ` Geliang Tang
2024-07-09 9:16 ` [PATCH bpf-next v11 6/9] selftests/bpf: Use connect_fd_to_fd " Geliang Tang
` (4 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2024-07-09 9:16 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
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);
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH bpf-next v11 6/9] selftests/bpf: Use connect_fd_to_fd in sk_lookup
2024-07-09 9:16 [PATCH bpf-next v11 0/9] use network helpers, part 8 Geliang Tang
` (4 preceding siblings ...)
2024-07-09 9:16 ` [PATCH bpf-next v11 5/9] selftests/bpf: Use start_server_addr " Geliang Tang
@ 2024-07-09 9:16 ` Geliang Tang
2024-07-10 19:04 ` Martin KaFai Lau
2024-07-09 9:16 ` [PATCH bpf-next v11 7/9] selftests/bpf: Add connect_fd_to_addr_str helper Geliang Tang
` (3 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Geliang Tang @ 2024-07-09 9:16 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
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;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next v11 6/9] selftests/bpf: Use connect_fd_to_fd in sk_lookup
2024-07-09 9:16 ` [PATCH bpf-next v11 6/9] selftests/bpf: Use connect_fd_to_fd " Geliang Tang
@ 2024-07-10 19:04 ` Martin KaFai Lau
0 siblings, 0 replies; 15+ messages in thread
From: Martin KaFai Lau @ 2024-07-10 19:04 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, Hao Luo, Jiri Olsa, Shuah Khan,
Geliang Tang, bpf, linux-kselftest, Stanislav Fomichev
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;
> }
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH bpf-next v11 7/9] selftests/bpf: Add connect_fd_to_addr_str helper
2024-07-09 9:16 [PATCH bpf-next v11 0/9] use network helpers, part 8 Geliang Tang
` (5 preceding siblings ...)
2024-07-09 9:16 ` [PATCH bpf-next v11 6/9] selftests/bpf: Use connect_fd_to_fd " Geliang Tang
@ 2024-07-09 9:16 ` Geliang Tang
2024-07-10 19:20 ` Martin KaFai Lau
2024-07-09 9:16 ` [PATCH bpf-next v11 8/9] selftests/bpf: Use connect_fd_to_addr_str in sk_lookup Geliang Tang
` (2 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Geliang Tang @ 2024-07-09 9:16 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
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,
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next v11 7/9] selftests/bpf: Add connect_fd_to_addr_str helper
2024-07-09 9:16 ` [PATCH bpf-next v11 7/9] selftests/bpf: Add connect_fd_to_addr_str helper Geliang Tang
@ 2024-07-10 19:20 ` Martin KaFai Lau
0 siblings, 0 replies; 15+ messages in thread
From: Martin KaFai Lau @ 2024-07-10 19:20 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, Hao Luo, Jiri Olsa, Shuah Khan,
Geliang Tang, bpf, linux-kselftest, Stanislav Fomichev
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)
> {
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH bpf-next v11 8/9] selftests/bpf: Use connect_fd_to_addr_str in sk_lookup
2024-07-09 9:16 [PATCH bpf-next v11 0/9] use network helpers, part 8 Geliang Tang
` (6 preceding siblings ...)
2024-07-09 9:16 ` [PATCH bpf-next v11 7/9] selftests/bpf: Add connect_fd_to_addr_str helper Geliang Tang
@ 2024-07-09 9:16 ` Geliang Tang
2024-07-09 9:16 ` [PATCH bpf-next v11 9/9] selftests/bpf: Drop make_socket " Geliang Tang
2024-07-10 19:00 ` [PATCH bpf-next v11 0/9] use network helpers, part 8 patchwork-bot+netdevbpf
9 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2024-07-09 9:16 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
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) {
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH bpf-next v11 9/9] selftests/bpf: Drop make_socket in sk_lookup
2024-07-09 9:16 [PATCH bpf-next v11 0/9] use network helpers, part 8 Geliang Tang
` (7 preceding siblings ...)
2024-07-09 9:16 ` [PATCH bpf-next v11 8/9] selftests/bpf: Use connect_fd_to_addr_str in sk_lookup Geliang Tang
@ 2024-07-09 9:16 ` Geliang Tang
2024-07-10 19:00 ` [PATCH bpf-next v11 0/9] use network helpers, part 8 patchwork-bot+netdevbpf
9 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2024-07-09 9:16 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
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;
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next v11 0/9] use network helpers, part 8
2024-07-09 9:16 [PATCH bpf-next v11 0/9] use network helpers, part 8 Geliang Tang
` (8 preceding siblings ...)
2024-07-09 9:16 ` [PATCH bpf-next v11 9/9] selftests/bpf: Drop make_socket " Geliang Tang
@ 2024-07-10 19:00 ` patchwork-bot+netdevbpf
9 siblings, 0 replies; 15+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-07-10 19:00 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 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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 15+ messages in thread