* [PATCH bpf-next v2 01/14] selftests/bpf: Add start_server_addr helper
2024-04-11 1:03 [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Geliang Tang
@ 2024-04-11 1:03 ` Geliang Tang
2024-04-11 22:06 ` Martin KaFai Lau
2024-04-11 1:03 ` [PATCH bpf-next v2 02/14] selftests/bpf: Use start_server_addr in cls_redirect Geliang Tang
` (13 subsequent siblings)
14 siblings, 1 reply; 21+ messages in thread
From: Geliang Tang @ 2024-04-11 1:03 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 order to pair up with connect_to addr(), this patch adds a new helper
start_server_addr(), which is a wrapper of __start_server(), and accepts an
argument 'addr' of 'struct sockaddr' 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 | 5 +++++
tools/testing/selftests/bpf/network_helpers.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index ca16ef2b648e..7ddeb6698ec7 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -185,6 +185,11 @@ int *start_reuseport_server(int family, int type, const char *addr_str,
return NULL;
}
+int start_server_addr(const struct sockaddr *addr, socklen_t addrlen, int type)
+{
+ return __start_server(type, 0, addr, addrlen, 0, 0);
+}
+
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 70f4e4c92733..89f59b65ce76 100644
--- a/tools/testing/selftests/bpf/network_helpers.h
+++ b/tools/testing/selftests/bpf/network_helpers.h
@@ -53,6 +53,7 @@ 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(const struct sockaddr *addr, socklen_t addrlen, int type);
void free_fds(int *fds, unsigned int nr_close_fds);
int connect_to_addr(const struct sockaddr_storage *addr, socklen_t len, int type);
int connect_to_fd(int server_fd, int timeout_ms);
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next v2 01/14] selftests/bpf: Add start_server_addr helper
2024-04-11 1:03 ` [PATCH bpf-next v2 01/14] selftests/bpf: Add start_server_addr helper Geliang Tang
@ 2024-04-11 22:06 ` Martin KaFai Lau
0 siblings, 0 replies; 21+ messages in thread
From: Martin KaFai Lau @ 2024-04-11 22:06 UTC (permalink / raw)
To: Geliang Tang
Cc: Andrii Nakryiko, Eduard Zingerman, Mykola Lysenko,
Alexei Starovoitov, Daniel Borkmann, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Shuah Khan, Geliang Tang, bpf, linux-kselftest
On 4/10/24 6:03 PM, 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(), which is a wrapper of __start_server(), and accepts an
> argument 'addr' of 'struct sockaddr' type instead of a string type argument
> like start_server().
Thanks for the cleanup work in the set.
>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> tools/testing/selftests/bpf/network_helpers.c | 5 +++++
> tools/testing/selftests/bpf/network_helpers.h | 1 +
> 2 files changed, 6 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> index ca16ef2b648e..7ddeb6698ec7 100644
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/tools/testing/selftests/bpf/network_helpers.c
> @@ -185,6 +185,11 @@ int *start_reuseport_server(int family, int type, const char *addr_str,
> return NULL;
> }
>
> +int start_server_addr(const struct sockaddr *addr, socklen_t addrlen, int type)
nit. Move "int type" to the first argument which is closer to how the socket
syscall is doing it. It is unfortunate that the existing connect_to_addr() has
it at the last arg but its usage seems to be limited to sock_addr.c, so should
be an easy change.
Although there is an "addrlen", connect_to_addr() and some other helpers are
using "sockaddr_storage" instead of "sockaddr", so may as well use that to have
a consistent usage.
Also add a network_helper_opts arg at the end for the future needs (e.g.
timeout), so something like this:
int start_server_addr_opts(int type, const struct sockaddr_storage *addr,
socklen_t addrlen,
const struct network_helper_opts *opts);
pw-bot: cr
> +{
> + return __start_server(type, 0, addr, addrlen, 0, 0);
> +}
> +
> 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 70f4e4c92733..89f59b65ce76 100644
> --- a/tools/testing/selftests/bpf/network_helpers.h
> +++ b/tools/testing/selftests/bpf/network_helpers.h
> @@ -53,6 +53,7 @@ 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(const struct sockaddr *addr, socklen_t addrlen, int type);
> void free_fds(int *fds, unsigned int nr_close_fds);
> int connect_to_addr(const struct sockaddr_storage *addr, socklen_t len, int type);
> int connect_to_fd(int server_fd, int timeout_ms);
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH bpf-next v2 02/14] selftests/bpf: Use start_server_addr in cls_redirect
2024-04-11 1:03 [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Geliang Tang
2024-04-11 1:03 ` [PATCH bpf-next v2 01/14] selftests/bpf: Add start_server_addr helper Geliang Tang
@ 2024-04-11 1:03 ` Geliang Tang
2024-04-11 1:03 ` [PATCH bpf-next v2 03/14] selftests/bpf: Use connect_to_addr " Geliang Tang
` (12 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Geliang Tang @ 2024-04-11 1:03 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>
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..013051555ce6 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(addr, len, type);
if (*server < 0)
return false;
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH bpf-next v2 03/14] selftests/bpf: Use connect_to_addr in cls_redirect
2024-04-11 1:03 [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Geliang Tang
2024-04-11 1:03 ` [PATCH bpf-next v2 01/14] selftests/bpf: Add start_server_addr helper Geliang Tang
2024-04-11 1:03 ` [PATCH bpf-next v2 02/14] selftests/bpf: Use start_server_addr in cls_redirect Geliang Tang
@ 2024-04-11 1:03 ` Geliang Tang
2024-04-11 1:03 ` [PATCH bpf-next v2 04/14] selftests/bpf: Use start_server_addr in sk_assign Geliang Tang
` (11 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Geliang Tang @ 2024-04-11 1:03 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_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 013051555ce6..4b185b608fae 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((struct sockaddr_storage *)sa, slen, type);
if (*conn < 0)
goto close_server;
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH bpf-next v2 04/14] selftests/bpf: Use start_server_addr in sk_assign
2024-04-11 1:03 [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Geliang Tang
` (2 preceding siblings ...)
2024-04-11 1:03 ` [PATCH bpf-next v2 03/14] selftests/bpf: Use connect_to_addr " Geliang Tang
@ 2024-04-11 1:03 ` Geliang Tang
2024-04-11 1:03 ` [PATCH bpf-next v2 05/14] selftests/bpf: Use connect_to_addr " Geliang Tang
` (10 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Geliang Tang @ 2024-04-11 1:03 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>
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 | 27 ++-----------------
1 file changed, 2 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..130aafe8cff6 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,7 @@ 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(addr, test->len, test->type);
if (server == -1)
goto close;
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH bpf-next v2 05/14] selftests/bpf: Use connect_to_addr in sk_assign
2024-04-11 1:03 [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Geliang Tang
` (3 preceding siblings ...)
2024-04-11 1:03 ` [PATCH bpf-next v2 04/14] selftests/bpf: Use start_server_addr in sk_assign Geliang Tang
@ 2024-04-11 1:03 ` Geliang Tang
2024-04-12 16:38 ` Eduard Zingerman
2024-04-11 1:03 ` [PATCH bpf-next v2 06/14] selftests/bpf: Use log_err in network_helpers Geliang Tang
` (9 subsequent siblings)
14 siblings, 1 reply; 21+ messages in thread
From: Geliang Tang @ 2024-04-11 1:03 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_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 130aafe8cff6..b574e162ce6e 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((struct sockaddr_storage *)addr, len, type);
if (client == -1) {
perror("Cannot connect to server");
goto out;
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next v2 05/14] selftests/bpf: Use connect_to_addr in sk_assign
2024-04-11 1:03 ` [PATCH bpf-next v2 05/14] selftests/bpf: Use connect_to_addr " Geliang Tang
@ 2024-04-12 16:38 ` Eduard Zingerman
0 siblings, 0 replies; 21+ messages in thread
From: Eduard Zingerman @ 2024-04-12 16:38 UTC (permalink / raw)
To: Geliang Tang, Andrii Nakryiko, 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
On Thu, 2024-04-11 at 09:03 +0800, 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.
Hi Geliang,
Thank you for this cleanup patch-set!
I think there is a mistake regarding connect_to_addr(),
as it does not set any timeouts (while start_server_addr() does):
-------------------------------------------------------- >8 ---
int connect_to_addr(...)
{
...
fd = socket(addr->ss_family, type, 0);
if (fd < 0) { ... }
if (connect_fd_to_addr(fd, addr, addrlen, false))
goto error_close;
return fd;
error_close:
...
}
static int connect_fd_to_addr(int fd,
const struct sockaddr_storage *addr,
socklen_t addrlen, const bool must_fail)
{
...
ret = connect(fd, (const struct sockaddr *)addr, addrlen);
if (must_fail) { ... error handling ... }
else { ... error handling ... }
return 0;
}
--- 8< --------------------------------------------------------
Maybe add a flavor like connect_to_addr_opt() with an additional
flag or options parameter?
Thanks,
Eduard
[...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH bpf-next v2 06/14] selftests/bpf: Use log_err in network_helpers
2024-04-11 1:03 [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Geliang Tang
` (4 preceding siblings ...)
2024-04-11 1:03 ` [PATCH bpf-next v2 05/14] selftests/bpf: Use connect_to_addr " Geliang Tang
@ 2024-04-11 1:03 ` Geliang Tang
2024-04-11 1:03 ` [PATCH bpf-next v2 07/14] selftests/bpf: Use start_server_addr in test_sock_addr Geliang Tang
` (8 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Geliang Tang @ 2024-04-11 1:03 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>
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.
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
tools/testing/selftests/bpf/network_helpers.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index 7ddeb6698ec7..91b014784dd9 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -447,22 +447,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("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("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("open netns fd");
goto fail;
+ }
err = setns(nsfd, CLONE_NEWNET);
close(nsfd);
- if (!ASSERT_OK(err, "setns"))
+ if (err) {
+ log_err("setns");
goto fail;
+ }
return token;
fail:
@@ -475,7 +483,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("setns");
close(token->orig_netns_fd);
free(token);
}
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH bpf-next v2 07/14] selftests/bpf: Use start_server_addr in test_sock_addr
2024-04-11 1:03 [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Geliang Tang
` (5 preceding siblings ...)
2024-04-11 1:03 ` [PATCH bpf-next v2 06/14] selftests/bpf: Use log_err in network_helpers Geliang Tang
@ 2024-04-11 1:03 ` Geliang Tang
2024-04-11 1:03 ` [PATCH bpf-next v2 08/14] selftests/bpf: Use connect_to_addr " Geliang Tang
` (7 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Geliang Tang @ 2024-04-11 1:03 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>
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 3b9eb40d6343..c8049d1d13b5 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -290,11 +290,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 89f59b65ce76..738b1764f562 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..8fa6bec8f7a5 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((struct sockaddr *)&requested_addr, addr_len, test->type);
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((struct sockaddr *)&expected_addr, addr_len, test->type);
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((struct sockaddr *)&server_addr, addr_len, test->type);
if (servfd == -1)
goto err;
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH bpf-next v2 08/14] selftests/bpf: Use connect_to_addr in test_sock_addr
2024-04-11 1:03 [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Geliang Tang
` (6 preceding siblings ...)
2024-04-11 1:03 ` [PATCH bpf-next v2 07/14] selftests/bpf: Use start_server_addr in test_sock_addr Geliang Tang
@ 2024-04-11 1:03 ` Geliang Tang
2024-04-11 1:03 ` [PATCH bpf-next v2 09/14] selftests/bpf: Add function pointer for __start_server Geliang Tang
` (6 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Geliang Tang @ 2024-04-11 1:03 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_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 8fa6bec8f7a5..743ff05e7755 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(&expected_addr, addr_len, test->type);
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(&requested_addr, addr_len, test->type);
if (clientfd == -1)
goto err;
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH bpf-next v2 09/14] selftests/bpf: Add function pointer for __start_server
2024-04-11 1:03 [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Geliang Tang
` (7 preceding siblings ...)
2024-04-11 1:03 ` [PATCH bpf-next v2 08/14] selftests/bpf: Use connect_to_addr " Geliang Tang
@ 2024-04-11 1:03 ` Geliang Tang
2024-04-12 17:23 ` Eduard Zingerman
2024-04-11 1:03 ` [PATCH bpf-next v2 10/14] selftests/bpf: Add start_server_setsockopt helper Geliang Tang
` (5 subsequent siblings)
14 siblings, 1 reply; 21+ messages in thread
From: Geliang Tang @ 2024-04-11 1:03 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>
__start_server() sets SO_REUSPORT through setsockopt() when the parameter
'reuseport' is set. This patch makes it more flexible by accepting a
function pointer named setsockopt.
Then the original start_reuseport_server() can be implemented by passing
in a newly defined setsockopt_reuse() function pointer to __start_server().
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
tools/testing/selftests/bpf/network_helpers.c | 22 +++++++++++--------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index 91b014784dd9..2a389e8c2503 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -79,9 +79,9 @@ int settimeo(int fd, int timeout_ms)
#define save_errno_close(fd) ({ int __save = errno; close(fd); errno = __save; })
static int __start_server(int type, int protocol, const struct sockaddr *addr,
- socklen_t addrlen, int timeout_ms, bool reuseport)
+ socklen_t addrlen, int timeout_ms,
+ int *(*setsockopt)(int fd, int val), int val)
{
- int on = 1;
int fd;
fd = socket(addr->sa_family, type, protocol);
@@ -93,9 +93,8 @@ static int __start_server(int type, int protocol, const struct sockaddr *addr,
if (settimeo(fd, timeout_ms))
goto error_close;
- if (reuseport &&
- setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &on, sizeof(on))) {
- log_err("Failed to set SO_REUSEPORT");
+ if (setsockopt && setsockopt(fd, val)) {
+ log_err("Failed to set sockopt");
goto error_close;
}
@@ -128,7 +127,7 @@ static int start_server_proto(int family, int type, int protocol,
return -1;
return __start_server(type, protocol, (struct sockaddr *)&addr,
- addrlen, timeout_ms, false);
+ addrlen, timeout_ms, NULL, 0);
}
int start_server(int family, int type, const char *addr_str, __u16 port,
@@ -144,6 +143,11 @@ int start_mptcp_server(int family, const char *addr_str, __u16 port,
port, timeout_ms);
}
+static int setsockopt_reuse(int fd, int on)
+{
+ return setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &on, sizeof(on));
+}
+
int *start_reuseport_server(int family, int type, const char *addr_str,
__u16 port, int timeout_ms, unsigned int nr_listens)
{
@@ -163,7 +167,7 @@ int *start_reuseport_server(int family, int type, const char *addr_str,
return NULL;
fds[0] = __start_server(type, 0, (struct sockaddr *)&addr, addrlen,
- timeout_ms, true);
+ timeout_ms, (void *)setsockopt_reuse, 1);
if (fds[0] == -1)
goto close_fds;
nr_fds = 1;
@@ -173,7 +177,7 @@ int *start_reuseport_server(int family, int type, const char *addr_str,
for (; nr_fds < nr_listens; nr_fds++) {
fds[nr_fds] = __start_server(type, 0, (struct sockaddr *)&addr,
- addrlen, timeout_ms, true);
+ addrlen, timeout_ms, (void *)setsockopt_reuse, 1);
if (fds[nr_fds] == -1)
goto close_fds;
}
@@ -187,7 +191,7 @@ int *start_reuseport_server(int family, int type, const char *addr_str,
int start_server_addr(const struct sockaddr *addr, socklen_t addrlen, int type)
{
- return __start_server(type, 0, addr, addrlen, 0, 0);
+ return __start_server(type, 0, addr, addrlen, 0, NULL, 0);
}
void free_fds(int *fds, unsigned int nr_close_fds)
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next v2 09/14] selftests/bpf: Add function pointer for __start_server
2024-04-11 1:03 ` [PATCH bpf-next v2 09/14] selftests/bpf: Add function pointer for __start_server Geliang Tang
@ 2024-04-12 17:23 ` Eduard Zingerman
0 siblings, 0 replies; 21+ messages in thread
From: Eduard Zingerman @ 2024-04-12 17:23 UTC (permalink / raw)
To: Geliang Tang, Andrii Nakryiko, 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
On Thu, 2024-04-11 at 09:03 +0800, Geliang Tang wrote:
[...]
> static int __start_server(int type, int protocol, const struct sockaddr *addr,
> - socklen_t addrlen, int timeout_ms, bool reuseport)
> + socklen_t addrlen, int timeout_ms,
> + int *(*setsockopt)(int fd, int val), int val)
There is no need for setsockopt to return pointer to int,
the code could be simplified e.g. as in the patch below (on top of this series):
---
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index f6810bb54edc..30ac03322c61 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -80,7 +80,7 @@ int settimeo(int fd, int timeout_ms)
static int __start_server(int type, int protocol, const struct sockaddr *addr,
socklen_t addrlen, int timeout_ms,
- int *(*setsockopt)(int fd, int val), int val)
+ int (*setsockopt)(int fd, int val), int val)
{
int fd;
@@ -167,7 +167,7 @@ int *start_reuseport_server(int family, int type, const char *addr_str,
return NULL;
fds[0] = __start_server(type, 0, (struct sockaddr *)&addr, addrlen,
- timeout_ms, (void *)setsockopt_reuse, 1);
+ timeout_ms, setsockopt_reuse, 1);
if (fds[0] == -1)
goto close_fds;
nr_fds = 1;
@@ -177,7 +177,7 @@ int *start_reuseport_server(int family, int type, const char *addr_str,
for (; nr_fds < nr_listens; nr_fds++) {
fds[nr_fds] = __start_server(type, 0, (struct sockaddr *)&addr,
- addrlen, timeout_ms, (void *)setsockopt_reuse, 1);
+ addrlen, timeout_ms, setsockopt_reuse, 1);
if (fds[nr_fds] == -1)
goto close_fds;
}
@@ -195,7 +195,7 @@ int start_server_addr(const struct sockaddr *addr, socklen_t addrlen, int type)
}
int start_server_setsockopt(const struct sockaddr *addr, socklen_t addrlen, int type,
- int *(*setsockopt)(int fd, int val), int val)
+ int (*setsockopt)(int fd, int val), int val)
{
return __start_server(type, 0, addr, addrlen, 0, setsockopt, val);
}
diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
index 543295230062..0f0f1803e0c8 100644
--- a/tools/testing/selftests/bpf/network_helpers.h
+++ b/tools/testing/selftests/bpf/network_helpers.h
@@ -56,7 +56,7 @@ int *start_reuseport_server(int family, int type, const char *addr_str,
unsigned int nr_listens);
int start_server_addr(const struct sockaddr *addr, socklen_t addrlen, int type);
int start_server_setsockopt(const struct sockaddr *addr, socklen_t addrlen, int type,
- int *(*setsockopt)(int fd, int val), int val);
+ int (*setsockopt)(int fd, int val), int val);
void free_fds(int *fds, unsigned int nr_close_fds);
int connect_to_addr(const struct sockaddr_storage *addr, socklen_t len, int type);
int connect_to_fd(int server_fd, int timeout_ms);
diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
index 4fee4b343255..0c39170d543a 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
@@ -97,7 +97,7 @@ static int start_server_lo(void)
if (make_sockaddr(AF_INET, "127.0.0.1", 0, &addr, &addrlen))
return -1;
return start_server_setsockopt((struct sockaddr *)&addr, addrlen,
- SOCK_STREAM, (void *)setsockopt_loop, 0);
+ SOCK_STREAM, setsockopt_loop, 0);
}
static void run_test(int cgroup_fd)
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH bpf-next v2 10/14] selftests/bpf: Add start_server_setsockopt helper
2024-04-11 1:03 [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Geliang Tang
` (8 preceding siblings ...)
2024-04-11 1:03 ` [PATCH bpf-next v2 09/14] selftests/bpf: Add function pointer for __start_server Geliang Tang
@ 2024-04-11 1:03 ` Geliang Tang
2024-04-11 1:03 ` [PATCH bpf-next v2 11/14] selftests/bpf: Use start_server_setsockopt in sockopt_inherit Geliang Tang
` (4 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Geliang Tang @ 2024-04-11 1:03 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 adds a new helper start_server_setsockopt(), which is a wrapper
of __start_server(), and accepts a function pointer setsockopt as a
parameter.
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
tools/testing/selftests/bpf/network_helpers.c | 6 ++++++
tools/testing/selftests/bpf/network_helpers.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index 2a389e8c2503..227ed132b84a 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -194,6 +194,12 @@ int start_server_addr(const struct sockaddr *addr, socklen_t addrlen, int type)
return __start_server(type, 0, addr, addrlen, 0, NULL, 0);
}
+int start_server_setsockopt(const struct sockaddr *addr, socklen_t addrlen, int type,
+ int *(*setsockopt)(int fd, int val), int val)
+{
+ return __start_server(type, 0, addr, addrlen, 0, setsockopt, val);
+}
+
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 738b1764f562..543295230062 100644
--- a/tools/testing/selftests/bpf/network_helpers.h
+++ b/tools/testing/selftests/bpf/network_helpers.h
@@ -55,6 +55,8 @@ 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(const struct sockaddr *addr, socklen_t addrlen, int type);
+int start_server_setsockopt(const struct sockaddr *addr, socklen_t addrlen, int type,
+ int *(*setsockopt)(int fd, int val), int val);
void free_fds(int *fds, unsigned int nr_close_fds);
int connect_to_addr(const struct sockaddr_storage *addr, socklen_t len, int type);
int connect_to_fd(int server_fd, int timeout_ms);
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH bpf-next v2 11/14] selftests/bpf: Use start_server_setsockopt in sockopt_inherit
2024-04-11 1:03 [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Geliang Tang
` (9 preceding siblings ...)
2024-04-11 1:03 ` [PATCH bpf-next v2 10/14] selftests/bpf: Add start_server_setsockopt helper Geliang Tang
@ 2024-04-11 1:03 ` Geliang Tang
2024-04-11 22:10 ` Martin KaFai Lau
2024-04-11 1:03 ` [PATCH bpf-next v2 12/14] selftests/bpf: Use connect_to_fd " Geliang Tang
` (3 subsequent siblings)
14 siblings, 1 reply; 21+ messages in thread
From: Geliang Tang @ 2024-04-11 1:03 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>
Include network_helpers.h in prog_tests/sockopt_inherit.c, use public
helpers make_sockaddr() and start_server_setsockopt() instead of the
local defined function start_server(). This can avoid duplicate code.
Add a helper setsockopt_loop() to set SOL_CUSTOM sockopt looply, and
pass it to start_server_setsockopt().
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
.../bpf/prog_tests/sockopt_inherit.c | 33 ++++++++-----------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
index 917f486db826..f000d9229765 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <test_progs.h>
#include "cgroup_helpers.h"
+#include "network_helpers.h"
#include "sockopt_inherit.skel.h"
@@ -98,23 +99,12 @@ static void *server_thread(void *arg)
return (void *)(long)err;
}
-static int start_server(void)
+static int setsockopt_loop(int fd, int val)
{
- struct sockaddr_in addr = {
- .sin_family = AF_INET,
- .sin_addr.s_addr = htonl(INADDR_LOOPBACK),
- };
char buf;
int err;
- int fd;
int i;
- fd = socket(AF_INET, SOCK_STREAM, 0);
- if (fd < 0) {
- log_err("Failed to create server socket");
- return -1;
- }
-
for (i = CUSTOM_INHERIT1; i <= CUSTOM_LISTENER; i++) {
buf = 0x01;
err = setsockopt(fd, SOL_CUSTOM, i, &buf, 1);
@@ -125,13 +115,18 @@ static int start_server(void)
}
}
- if (bind(fd, (const struct sockaddr *)&addr, sizeof(addr)) < 0) {
- log_err("Failed to bind socket");
- close(fd);
- return -1;
- }
+ return 0;
+}
- return fd;
+static int start_server_lo(void)
+{
+ struct sockaddr_storage addr;
+ socklen_t addrlen;
+
+ if (make_sockaddr(AF_INET, "127.0.0.1", 0, &addr, &addrlen))
+ return -1;
+ return start_server_setsockopt((struct sockaddr *)&addr, addrlen,
+ SOCK_STREAM, (void *)setsockopt_loop, 0);
}
static void run_test(int cgroup_fd)
@@ -160,7 +155,7 @@ static void run_test(int cgroup_fd)
if (!ASSERT_OK_PTR(link_setsockopt, "cg-attach-setsockopt"))
goto close_bpf_object;
- server_fd = start_server();
+ server_fd = start_server_lo();
if (!ASSERT_GE(server_fd, 0, "start_server"))
goto close_bpf_object;
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next v2 11/14] selftests/bpf: Use start_server_setsockopt in sockopt_inherit
2024-04-11 1:03 ` [PATCH bpf-next v2 11/14] selftests/bpf: Use start_server_setsockopt in sockopt_inherit Geliang Tang
@ 2024-04-11 22:10 ` Martin KaFai Lau
2024-04-12 17:35 ` Eduard Zingerman
0 siblings, 1 reply; 21+ messages in thread
From: Martin KaFai Lau @ 2024-04-11 22:10 UTC (permalink / raw)
To: Geliang Tang
Cc: Geliang Tang, Andrii Nakryiko, Eduard Zingerman, Mykola Lysenko,
Alexei Starovoitov, Daniel Borkmann, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Shuah Khan, bpf, linux-kselftest
On 4/10/24 6:03 PM, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Include network_helpers.h in prog_tests/sockopt_inherit.c, use public
> helpers make_sockaddr() and start_server_setsockopt() instead of the
> local defined function start_server(). This can avoid duplicate code.
>
> Add a helper setsockopt_loop() to set SOL_CUSTOM sockopt looply, and
> pass it to start_server_setsockopt().
>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> .../bpf/prog_tests/sockopt_inherit.c | 33 ++++++++-----------
> 1 file changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
> index 917f486db826..f000d9229765 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> #include <test_progs.h>
> #include "cgroup_helpers.h"
> +#include "network_helpers.h"
>
> #include "sockopt_inherit.skel.h"
>
> @@ -98,23 +99,12 @@ static void *server_thread(void *arg)
> return (void *)(long)err;
> }
>
> -static int start_server(void)
> +static int setsockopt_loop(int fd, int val)
> {
> - struct sockaddr_in addr = {
> - .sin_family = AF_INET,
> - .sin_addr.s_addr = htonl(INADDR_LOOPBACK),
> - };
> char buf;
> int err;
> - int fd;
> int i;
>
> - fd = socket(AF_INET, SOCK_STREAM, 0);
> - if (fd < 0) {
> - log_err("Failed to create server socket");
> - return -1;
> - }
> -
> for (i = CUSTOM_INHERIT1; i <= CUSTOM_LISTENER; i++) {
> buf = 0x01;
> err = setsockopt(fd, SOL_CUSTOM, i, &buf, 1);
> @@ -125,13 +115,18 @@ static int start_server(void)
> }
> }
>
> - if (bind(fd, (const struct sockaddr *)&addr, sizeof(addr)) < 0) {
> - log_err("Failed to bind socket");
> - close(fd);
> - return -1;
> - }
> + return 0;
> +}
>
> - return fd;
> +static int start_server_lo(void)
> +{
> + struct sockaddr_storage addr;
> + socklen_t addrlen;
> +
> + if (make_sockaddr(AF_INET, "127.0.0.1", 0, &addr, &addrlen))
> + return -1;
> + return start_server_setsockopt((struct sockaddr *)&addr, addrlen,
> + SOCK_STREAM, (void *)setsockopt_loop, 0);
The start_server_setsockopt is only limited to integer socket option. It is not
very flexible. It seems this is the only test that will be useful. Lets drop
start_server_setsockopt addition from this set for now and discuss it separately.
> }
>
> static void run_test(int cgroup_fd)
> @@ -160,7 +155,7 @@ static void run_test(int cgroup_fd)
> if (!ASSERT_OK_PTR(link_setsockopt, "cg-attach-setsockopt"))
> goto close_bpf_object;
>
> - server_fd = start_server();
> + server_fd = start_server_lo();
> if (!ASSERT_GE(server_fd, 0, "start_server"))
> goto close_bpf_object;
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next v2 11/14] selftests/bpf: Use start_server_setsockopt in sockopt_inherit
2024-04-11 22:10 ` Martin KaFai Lau
@ 2024-04-12 17:35 ` Eduard Zingerman
0 siblings, 0 replies; 21+ messages in thread
From: Eduard Zingerman @ 2024-04-12 17:35 UTC (permalink / raw)
To: Martin KaFai Lau, Geliang Tang
Cc: Geliang Tang, Andrii Nakryiko, Mykola Lysenko, Alexei Starovoitov,
Daniel Borkmann, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan, bpf,
linux-kselftest
On Thu, 2024-04-11 at 15:10 -0700, Martin KaFai Lau wrote:
[...]
> The start_server_setsockopt is only limited to integer socket option. It is not
> very flexible. It seems this is the only test that will be useful. Lets drop
> start_server_setsockopt addition from this set for now and discuss it separately.
I agree with this suggestion from Martin.
[...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH bpf-next v2 12/14] selftests/bpf: Use connect_to_fd in sockopt_inherit
2024-04-11 1:03 [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Geliang Tang
` (10 preceding siblings ...)
2024-04-11 1:03 ` [PATCH bpf-next v2 11/14] selftests/bpf: Use start_server_setsockopt in sockopt_inherit Geliang Tang
@ 2024-04-11 1:03 ` Geliang Tang
2024-04-11 1:03 ` [PATCH bpf-next v2 13/14] selftests/bpf: Use start_server_* in test_tcp_check_syncookie Geliang Tang
` (2 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Geliang Tang @ 2024-04-11 1:03 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_to_fd() exported in network_helpers.h
instead of the local defined function connect_to_server() in
prog_tests/sockopt_inherit.c. This can avoid duplicate code.
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
.../bpf/prog_tests/sockopt_inherit.c | 31 +------------------
1 file changed, 1 insertion(+), 30 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
index f000d9229765..4fee4b343255 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt_inherit.c
@@ -10,35 +10,6 @@
#define CUSTOM_INHERIT2 1
#define CUSTOM_LISTENER 2
-static int connect_to_server(int server_fd)
-{
- struct sockaddr_storage addr;
- socklen_t len = sizeof(addr);
- int fd;
-
- fd = socket(AF_INET, SOCK_STREAM, 0);
- if (fd < 0) {
- log_err("Failed to create client socket");
- return -1;
- }
-
- if (getsockname(server_fd, (struct sockaddr *)&addr, &len)) {
- log_err("Failed to get server addr");
- goto out;
- }
-
- if (connect(fd, (const struct sockaddr *)&addr, len) < 0) {
- log_err("Fail to connect to server");
- goto out;
- }
-
- return fd;
-
-out:
- close(fd);
- return -1;
-}
-
static int verify_sockopt(int fd, int optname, const char *msg, char expected)
{
socklen_t optlen = 1;
@@ -168,7 +139,7 @@ static void run_test(int cgroup_fd)
pthread_cond_wait(&server_started, &server_started_mtx);
pthread_mutex_unlock(&server_started_mtx);
- client_fd = connect_to_server(server_fd);
+ client_fd = connect_to_fd(server_fd, 0);
if (!ASSERT_GE(client_fd, 0, "connect_to_server"))
goto close_server_fd;
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH bpf-next v2 13/14] selftests/bpf: Use start_server_* in test_tcp_check_syncookie
2024-04-11 1:03 [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Geliang Tang
` (11 preceding siblings ...)
2024-04-11 1:03 ` [PATCH bpf-next v2 12/14] selftests/bpf: Use connect_to_fd " Geliang Tang
@ 2024-04-11 1:03 ` Geliang Tang
2024-04-11 1:03 ` [PATCH bpf-next v2 14/14] selftests/bpf: Use connect_to_addr " Geliang Tang
2024-04-12 18:52 ` [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Eduard Zingerman
14 siblings, 0 replies; 21+ messages in thread
From: Geliang Tang @ 2024-04-11 1:03 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>
Include network_helpers.h in test_tcp_check_syncookie_user.c, use
public helpers start_server_addr() and start_server_setsockopt() in it
instead of the local defined function start_server(). This can avoid
duplicate code.
Add a helper setsockopt_mode() to set IPV6_V6ONLY sockopt, and pass it
to start_server_setsockopt().
In order to use functions defined in network_helpers.c, Makefile needs
to be updated to avoid compilation errors.
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
tools/testing/selftests/bpf/Makefile | 1 +
.../bpf/test_tcp_check_syncookie_user.c | 50 ++++---------------
2 files changed, 12 insertions(+), 39 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index c8049d1d13b5..1199a02a5ae4 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -309,6 +309,7 @@ $(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)
$(OUTPUT)/test_maps: $(TESTING_HELPERS)
$(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS) $(UNPRIV_HELPERS)
$(OUTPUT)/xsk.o: $(BPFOBJ)
+$(OUTPUT)/test_tcp_check_syncookie_user: $(NETWORK_HELPERS)
BPFTOOL ?= $(DEFAULT_BPFTOOL)
$(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
diff --git a/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c b/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
index 32df93747095..e6ad4895d2b3 100644
--- a/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
+++ b/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
@@ -16,43 +16,13 @@
#include <bpf/libbpf.h>
#include "cgroup_helpers.h"
+#include "network_helpers.h"
-static int start_server(const struct sockaddr *addr, socklen_t len, bool dual)
+static int setsockopt_mode(int fd, int dual)
{
int mode = !dual;
- int fd;
- fd = socket(addr->sa_family, SOCK_STREAM, 0);
- if (fd == -1) {
- log_err("Failed to create server socket");
- goto out;
- }
-
- if (addr->sa_family == AF_INET6) {
- if (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (char *)&mode,
- sizeof(mode)) == -1) {
- log_err("Failed to set the dual-stack mode");
- goto close_out;
- }
- }
-
- if (bind(fd, addr, len) == -1) {
- log_err("Failed to bind server socket");
- goto close_out;
- }
-
- if (listen(fd, 128) == -1) {
- log_err("Failed to listen on server socket");
- goto close_out;
- }
-
- goto out;
-
-close_out:
- close(fd);
- fd = -1;
-out:
- return fd;
+ return setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (char *)&mode, sizeof(mode));
}
static int connect_to_server(const struct sockaddr *addr, socklen_t len)
@@ -259,18 +229,20 @@ int main(int argc, char **argv)
addr6dual.sin6_addr = in6addr_any;
addr6dual.sin6_port = 0;
- server = start_server((const struct sockaddr *)&addr4, sizeof(addr4),
- false);
+ server = start_server_addr((const struct sockaddr *)&addr4,
+ sizeof(addr4), SOCK_STREAM);
if (server == -1 || !get_port(server, &addr4.sin_port))
goto err;
- server_v6 = start_server((const struct sockaddr *)&addr6,
- sizeof(addr6), false);
+ server_v6 = start_server_setsockopt((const struct sockaddr *)&addr6,
+ sizeof(addr6), SOCK_STREAM,
+ (void *)setsockopt_mode, false);
if (server_v6 == -1 || !get_port(server_v6, &addr6.sin6_port))
goto err;
- server_dual = start_server((const struct sockaddr *)&addr6dual,
- sizeof(addr6dual), true);
+ server_dual = start_server_setsockopt((const struct sockaddr *)&addr6dual,
+ sizeof(addr6dual), SOCK_STREAM,
+ (void *)setsockopt_mode, true);
if (server_dual == -1 || !get_port(server_dual, &addr4dual.sin_port))
goto err;
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH bpf-next v2 14/14] selftests/bpf: Use connect_to_addr in test_tcp_check_syncookie
2024-04-11 1:03 [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Geliang Tang
` (12 preceding siblings ...)
2024-04-11 1:03 ` [PATCH bpf-next v2 13/14] selftests/bpf: Use start_server_* in test_tcp_check_syncookie Geliang Tang
@ 2024-04-11 1:03 ` Geliang Tang
2024-04-12 18:52 ` [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Eduard Zingerman
14 siblings, 0 replies; 21+ messages in thread
From: Geliang Tang @ 2024-04-11 1:03 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_to_addr() exported in
network_helpers.h instead of the local defined function connect_to_server()
in test_tcp_check_syncookie_user.c. This can avoid duplicate code.
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
.../bpf/test_tcp_check_syncookie_user.c | 26 +------------------
1 file changed, 1 insertion(+), 25 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c b/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
index e6ad4895d2b3..e678228b915f 100644
--- a/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
+++ b/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
@@ -25,30 +25,6 @@ static int setsockopt_mode(int fd, int dual)
return setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (char *)&mode, sizeof(mode));
}
-static int connect_to_server(const struct sockaddr *addr, socklen_t len)
-{
- int fd = -1;
-
- fd = socket(addr->sa_family, SOCK_STREAM, 0);
- if (fd == -1) {
- log_err("Failed to create client socket");
- goto out;
- }
-
- if (connect(fd, (const struct sockaddr *)addr, len) == -1) {
- log_err("Fail to connect to server");
- goto close_out;
- }
-
- goto out;
-
-close_out:
- close(fd);
- fd = -1;
-out:
- return fd;
-}
-
static int get_map_fd_by_prog_id(int prog_id, bool *xdp)
{
struct bpf_prog_info info = {};
@@ -114,7 +90,7 @@ static int run_test(int server_fd, int results_fd, bool xdp,
goto err;
}
- client = connect_to_server(addr, len);
+ client = connect_to_addr((struct sockaddr_storage *)addr, len, SOCK_STREAM);
if (client == -1)
goto err;
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH bpf-next v2 00/14] use start_server and connect_to helpers
2024-04-11 1:03 [PATCH bpf-next v2 00/14] use start_server and connect_to helpers Geliang Tang
` (13 preceding siblings ...)
2024-04-11 1:03 ` [PATCH bpf-next v2 14/14] selftests/bpf: Use connect_to_addr " Geliang Tang
@ 2024-04-12 18:52 ` Eduard Zingerman
14 siblings, 0 replies; 21+ messages in thread
From: Eduard Zingerman @ 2024-04-12 18:52 UTC (permalink / raw)
To: Geliang Tang, Andrii Nakryiko, 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
On Thu, 2024-04-11 at 09:03 +0800, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> 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.
Modulo feedback from Martin and connect_to_addr() not setting default
timeout this series looks good, thank you.
^ permalink raw reply [flat|nested] 21+ messages in thread