* [PATCH bpf-next v4 0/3] export send_recv_data
@ 2024-04-10 6:13 Geliang Tang
2024-04-10 6:13 ` [PATCH bpf-next v4 1/3] selftests/bpf: Add struct send_recv_arg Geliang Tang
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Geliang Tang @ 2024-04-10 6:13 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, mptcp, linux-kselftest
From: Geliang Tang <tanggeliang@kylinos.cn>
v4:
- fix a bug in v3, it should be 'if (err)', not 'if (!err)'.
- move "selftests/bpf: Use log_err in network_helpers" out of this
series.
v3:
- add two more patches.
- use log_err instead of ASSERT in v3.
- let send_recv_data return int as Martin suggested.
v2:
Address Martin's comments for v1 (thanks.)
- drop patch 1, "export send_byte helper".
- drop "WRITE_ONCE(arg.stop, 0)".
- rebased.
send_recv_data will be re-used in MPTCP bpf tests, but not included
in this set because it depends on other patches that have not been
in the bpf-next yet. It will be sent as another set soon.
Geliang Tang (3):
selftests/bpf: Add struct send_recv_arg
selftests/bpf: Export send_recv_data helper
selftests/bpf: Support nonblock for send_recv_data
tools/testing/selftests/bpf/network_helpers.c | 103 ++++++++++++++++++
tools/testing/selftests/bpf/network_helpers.h | 1 +
.../selftests/bpf/prog_tests/bpf_tcp_ca.c | 71 +-----------
3 files changed, 105 insertions(+), 70 deletions(-)
--
2.40.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf-next v4 1/3] selftests/bpf: Add struct send_recv_arg
2024-04-10 6:13 [PATCH bpf-next v4 0/3] export send_recv_data Geliang Tang
@ 2024-04-10 6:13 ` Geliang Tang
2024-04-10 6:13 ` [PATCH bpf-next v4 2/3] selftests/bpf: Export send_recv_data helper Geliang Tang
2024-04-10 6:13 ` [PATCH bpf-next v4 3/3] selftests/bpf: Support nonblock for send_recv_data Geliang Tang
2 siblings, 0 replies; 7+ messages in thread
From: Geliang Tang @ 2024-04-10 6:13 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, mptcp, linux-kselftest
From: Geliang Tang <tanggeliang@kylinos.cn>
Avoid setting total_bytes and stop as global variables, this patch adds
a new struct named send_recv_arg to pass arguments between threads. Put
these two variables together with fd into this struct and pass it to
server thread, so that server thread can access these two variables without
setting them as global ones.
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
.../selftests/bpf/prog_tests/bpf_tcp_ca.c | 34 ++++++++++++-------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c b/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
index 077b107130f6..64f172f02a9a 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
@@ -21,7 +21,6 @@
static const unsigned int total_bytes = 10 * 1024 * 1024;
static int expected_stg = 0xeB9F;
-static int stop;
static int settcpca(int fd, const char *tcp_ca)
{
@@ -34,13 +33,20 @@ static int settcpca(int fd, const char *tcp_ca)
return 0;
}
+struct send_recv_arg {
+ int fd;
+ uint32_t bytes;
+ int stop;
+};
+
static void *server(void *arg)
{
- int lfd = (int)(long)arg, err = 0, fd;
+ struct send_recv_arg *a = (struct send_recv_arg *)arg;
ssize_t nr_sent = 0, bytes = 0;
char batch[1500];
+ int err = 0, fd;
- fd = accept(lfd, NULL, NULL);
+ fd = accept(a->fd, NULL, NULL);
while (fd == -1) {
if (errno == EINTR)
continue;
@@ -53,9 +59,9 @@ static void *server(void *arg)
goto done;
}
- while (bytes < total_bytes && !READ_ONCE(stop)) {
+ while (bytes < a->bytes && !READ_ONCE(a->stop)) {
nr_sent = send(fd, &batch,
- MIN(total_bytes - bytes, sizeof(batch)), 0);
+ MIN(a->bytes - bytes, sizeof(batch)), 0);
if (nr_sent == -1 && errno == EINTR)
continue;
if (nr_sent == -1) {
@@ -65,13 +71,13 @@ static void *server(void *arg)
bytes += nr_sent;
}
- ASSERT_EQ(bytes, total_bytes, "send");
+ ASSERT_EQ(bytes, a->bytes, "send");
done:
if (fd >= 0)
close(fd);
if (err) {
- WRITE_ONCE(stop, 1);
+ WRITE_ONCE(a->stop, 1);
return ERR_PTR(err);
}
return NULL;
@@ -80,18 +86,22 @@ static void *server(void *arg)
static void do_test(const char *tcp_ca, const struct bpf_map *sk_stg_map)
{
ssize_t nr_recv = 0, bytes = 0;
+ struct send_recv_arg arg = {
+ .bytes = total_bytes,
+ .stop = 0,
+ };
int lfd = -1, fd = -1;
pthread_t srv_thread;
void *thread_ret;
char batch[1500];
int err;
- WRITE_ONCE(stop, 0);
-
lfd = start_server(AF_INET6, SOCK_STREAM, NULL, 0, 0);
if (!ASSERT_NEQ(lfd, -1, "socket"))
return;
+ arg.fd = lfd;
+
fd = socket(AF_INET6, SOCK_STREAM, 0);
if (!ASSERT_NEQ(fd, -1, "socket")) {
close(lfd);
@@ -123,12 +133,12 @@ static void do_test(const char *tcp_ca, const struct bpf_map *sk_stg_map)
goto done;
}
- err = pthread_create(&srv_thread, NULL, server, (void *)(long)lfd);
+ err = pthread_create(&srv_thread, NULL, server, (void *)&arg);
if (!ASSERT_OK(err, "pthread_create"))
goto done;
/* recv total_bytes */
- while (bytes < total_bytes && !READ_ONCE(stop)) {
+ while (bytes < total_bytes && !READ_ONCE(arg.stop)) {
nr_recv = recv(fd, &batch,
MIN(total_bytes - bytes, sizeof(batch)), 0);
if (nr_recv == -1 && errno == EINTR)
@@ -140,7 +150,7 @@ static void do_test(const char *tcp_ca, const struct bpf_map *sk_stg_map)
ASSERT_EQ(bytes, total_bytes, "recv");
- WRITE_ONCE(stop, 1);
+ WRITE_ONCE(arg.stop, 1);
pthread_join(srv_thread, &thread_ret);
ASSERT_OK(IS_ERR(thread_ret), "thread_ret");
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf-next v4 2/3] selftests/bpf: Export send_recv_data helper
2024-04-10 6:13 [PATCH bpf-next v4 0/3] export send_recv_data Geliang Tang
2024-04-10 6:13 ` [PATCH bpf-next v4 1/3] selftests/bpf: Add struct send_recv_arg Geliang Tang
@ 2024-04-10 6:13 ` Geliang Tang
2024-04-10 21:20 ` Martin KaFai Lau
2024-04-10 6:13 ` [PATCH bpf-next v4 3/3] selftests/bpf: Support nonblock for send_recv_data Geliang Tang
2 siblings, 1 reply; 7+ messages in thread
From: Geliang Tang @ 2024-04-10 6:13 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, mptcp, linux-kselftest
From: Geliang Tang <tanggeliang@kylinos.cn>
This patch extracts the code to send and receive data into a new
helper named send_recv_data() in network_helpers.c and export it
in network_helpers.h.
This helper will be used for MPTCP BPF selftests.
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
tools/testing/selftests/bpf/network_helpers.c | 96 +++++++++++++++++++
tools/testing/selftests/bpf/network_helpers.h | 1 +
.../selftests/bpf/prog_tests/bpf_tcp_ca.c | 81 +---------------
3 files changed, 98 insertions(+), 80 deletions(-)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index 04175e16195a..137cd18ef3f2 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -545,3 +545,99 @@ int set_hw_ring_size(char *ifname, struct ethtool_ringparam *ring_param)
close(sockfd);
return 0;
}
+
+struct send_recv_arg {
+ int fd;
+ uint32_t bytes;
+ int stop;
+};
+
+static void *send_recv_server(void *arg)
+{
+ struct send_recv_arg *a = (struct send_recv_arg *)arg;
+ ssize_t nr_sent = 0, bytes = 0;
+ char batch[1500];
+ int err = 0, fd;
+
+ fd = accept(a->fd, NULL, NULL);
+ while (fd == -1) {
+ if (errno == EINTR)
+ continue;
+ err = -errno;
+ goto done;
+ }
+
+ if (settimeo(fd, 0)) {
+ err = -errno;
+ goto done;
+ }
+
+ while (bytes < a->bytes && !READ_ONCE(a->stop)) {
+ nr_sent = send(fd, &batch,
+ MIN(a->bytes - bytes, sizeof(batch)), 0);
+ if (nr_sent == -1 && errno == EINTR)
+ continue;
+ if (nr_sent == -1) {
+ err = -errno;
+ break;
+ }
+ bytes += nr_sent;
+ }
+
+ if (bytes != a->bytes)
+ log_err("send");
+
+done:
+ if (fd >= 0)
+ close(fd);
+ if (err) {
+ WRITE_ONCE(a->stop, 1);
+ return ERR_PTR(err);
+ }
+ return NULL;
+}
+
+int send_recv_data(int lfd, int fd, uint32_t total_bytes)
+{
+ ssize_t nr_recv = 0, bytes = 0;
+ struct send_recv_arg arg = {
+ .fd = lfd,
+ .bytes = total_bytes,
+ .stop = 0,
+ };
+ pthread_t srv_thread;
+ void *thread_ret;
+ char batch[1500];
+ int err;
+
+ err = pthread_create(&srv_thread, NULL, send_recv_server, (void *)&arg);
+ if (err) {
+ log_err("pthread_create");
+ return err;
+ }
+
+ /* recv total_bytes */
+ while (bytes < total_bytes && !READ_ONCE(arg.stop)) {
+ nr_recv = recv(fd, &batch,
+ MIN(total_bytes - bytes, sizeof(batch)), 0);
+ if (nr_recv == -1 && errno == EINTR)
+ continue;
+ if (nr_recv == -1)
+ break;
+ bytes += nr_recv;
+ }
+
+ if (bytes != total_bytes) {
+ log_err("recv");
+ return -1;
+ }
+
+ WRITE_ONCE(arg.stop, 1);
+ pthread_join(srv_thread, &thread_ret);
+ if (IS_ERR(thread_ret)) {
+ log_err("thread_ret");
+ return -1;
+ }
+
+ return 0;
+}
diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
index 6457445cc6e2..70f4e4c92733 100644
--- a/tools/testing/selftests/bpf/network_helpers.h
+++ b/tools/testing/selftests/bpf/network_helpers.h
@@ -76,6 +76,7 @@ struct nstoken;
*/
struct nstoken *open_netns(const char *name);
void close_netns(struct nstoken *token);
+int send_recv_data(int lfd, int fd, uint32_t total_bytes);
static __u16 csum_fold(__u32 csum)
{
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c b/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
index 64f172f02a9a..907bac46c774 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
@@ -33,75 +33,15 @@ static int settcpca(int fd, const char *tcp_ca)
return 0;
}
-struct send_recv_arg {
- int fd;
- uint32_t bytes;
- int stop;
-};
-
-static void *server(void *arg)
-{
- struct send_recv_arg *a = (struct send_recv_arg *)arg;
- ssize_t nr_sent = 0, bytes = 0;
- char batch[1500];
- int err = 0, fd;
-
- fd = accept(a->fd, NULL, NULL);
- while (fd == -1) {
- if (errno == EINTR)
- continue;
- err = -errno;
- goto done;
- }
-
- if (settimeo(fd, 0)) {
- err = -errno;
- goto done;
- }
-
- while (bytes < a->bytes && !READ_ONCE(a->stop)) {
- nr_sent = send(fd, &batch,
- MIN(a->bytes - bytes, sizeof(batch)), 0);
- if (nr_sent == -1 && errno == EINTR)
- continue;
- if (nr_sent == -1) {
- err = -errno;
- break;
- }
- bytes += nr_sent;
- }
-
- ASSERT_EQ(bytes, a->bytes, "send");
-
-done:
- if (fd >= 0)
- close(fd);
- if (err) {
- WRITE_ONCE(a->stop, 1);
- return ERR_PTR(err);
- }
- return NULL;
-}
-
static void do_test(const char *tcp_ca, const struct bpf_map *sk_stg_map)
{
- ssize_t nr_recv = 0, bytes = 0;
- struct send_recv_arg arg = {
- .bytes = total_bytes,
- .stop = 0,
- };
int lfd = -1, fd = -1;
- pthread_t srv_thread;
- void *thread_ret;
- char batch[1500];
int err;
lfd = start_server(AF_INET6, SOCK_STREAM, NULL, 0, 0);
if (!ASSERT_NEQ(lfd, -1, "socket"))
return;
- arg.fd = lfd;
-
fd = socket(AF_INET6, SOCK_STREAM, 0);
if (!ASSERT_NEQ(fd, -1, "socket")) {
close(lfd);
@@ -133,26 +73,7 @@ static void do_test(const char *tcp_ca, const struct bpf_map *sk_stg_map)
goto done;
}
- err = pthread_create(&srv_thread, NULL, server, (void *)&arg);
- if (!ASSERT_OK(err, "pthread_create"))
- goto done;
-
- /* recv total_bytes */
- while (bytes < total_bytes && !READ_ONCE(arg.stop)) {
- nr_recv = recv(fd, &batch,
- MIN(total_bytes - bytes, sizeof(batch)), 0);
- if (nr_recv == -1 && errno == EINTR)
- continue;
- if (nr_recv == -1)
- break;
- bytes += nr_recv;
- }
-
- ASSERT_EQ(bytes, total_bytes, "recv");
-
- WRITE_ONCE(arg.stop, 1);
- pthread_join(srv_thread, &thread_ret);
- ASSERT_OK(IS_ERR(thread_ret), "thread_ret");
+ ASSERT_OK(send_recv_data(lfd, fd, total_bytes), "send_recv_data");
done:
close(lfd);
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf-next v4 3/3] selftests/bpf: Support nonblock for send_recv_data
2024-04-10 6:13 [PATCH bpf-next v4 0/3] export send_recv_data Geliang Tang
2024-04-10 6:13 ` [PATCH bpf-next v4 1/3] selftests/bpf: Add struct send_recv_arg Geliang Tang
2024-04-10 6:13 ` [PATCH bpf-next v4 2/3] selftests/bpf: Export send_recv_data helper Geliang Tang
@ 2024-04-10 6:13 ` Geliang Tang
2024-04-10 21:34 ` Martin KaFai Lau
2 siblings, 1 reply; 7+ messages in thread
From: Geliang Tang @ 2024-04-10 6:13 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, mptcp, linux-kselftest
From: Geliang Tang <tanggeliang@kylinos.cn>
Some tests, such as the MPTCP bpf tests, require send_recv_data helper
to run in nonblock mode.
This patch adds nonblock support for send_recv_data(). Check if it is
currently in nonblock mode, and if so, ignore EWOULDBLOCK to continue
sending and receiving.
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
tools/testing/selftests/bpf/network_helpers.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index 137cd18ef3f2..ca16ef2b648e 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -555,6 +555,7 @@ struct send_recv_arg {
static void *send_recv_server(void *arg)
{
struct send_recv_arg *a = (struct send_recv_arg *)arg;
+ int flags = fcntl(a->fd, F_GETFL);
ssize_t nr_sent = 0, bytes = 0;
char batch[1500];
int err = 0, fd;
@@ -578,6 +579,8 @@ static void *send_recv_server(void *arg)
if (nr_sent == -1 && errno == EINTR)
continue;
if (nr_sent == -1) {
+ if (flags & O_NONBLOCK && errno == EWOULDBLOCK)
+ continue;
err = -errno;
break;
}
@@ -599,6 +602,7 @@ static void *send_recv_server(void *arg)
int send_recv_data(int lfd, int fd, uint32_t total_bytes)
{
+ int flags = fcntl(lfd, F_GETFL);
ssize_t nr_recv = 0, bytes = 0;
struct send_recv_arg arg = {
.fd = lfd,
@@ -622,8 +626,11 @@ int send_recv_data(int lfd, int fd, uint32_t total_bytes)
MIN(total_bytes - bytes, sizeof(batch)), 0);
if (nr_recv == -1 && errno == EINTR)
continue;
- if (nr_recv == -1)
+ if (nr_recv == -1) {
+ if (flags & O_NONBLOCK && errno == EWOULDBLOCK)
+ continue;
break;
+ }
bytes += nr_recv;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v4 2/3] selftests/bpf: Export send_recv_data helper
2024-04-10 6:13 ` [PATCH bpf-next v4 2/3] selftests/bpf: Export send_recv_data helper Geliang Tang
@ 2024-04-10 21:20 ` Martin KaFai Lau
0 siblings, 0 replies; 7+ messages in thread
From: Martin KaFai Lau @ 2024-04-10 21:20 UTC (permalink / raw
To: Geliang Tang
Cc: Geliang Tang, bpf, 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, mptcp, linux-kselftest
On 4/9/24 11:13 PM, Geliang Tang wrote:
> +int send_recv_data(int lfd, int fd, uint32_t total_bytes)
> +{
> + ssize_t nr_recv = 0, bytes = 0;
> + struct send_recv_arg arg = {
> + .fd = lfd,
> + .bytes = total_bytes,
> + .stop = 0,
> + };
> + pthread_t srv_thread;
> + void *thread_ret;
> + char batch[1500];
> + int err;
> +
> + err = pthread_create(&srv_thread, NULL, send_recv_server, (void *)&arg);
> + if (err) {
> + log_err("pthread_create");
> + return err;
> + }
> +
> + /* recv total_bytes */
> + while (bytes < total_bytes && !READ_ONCE(arg.stop)) {
> + nr_recv = recv(fd, &batch,
> + MIN(total_bytes - bytes, sizeof(batch)), 0);
> + if (nr_recv == -1 && errno == EINTR)
> + continue;
> + if (nr_recv == -1)
> + break;
> + bytes += nr_recv;
> + }
> +
> + if (bytes != total_bytes) {
> + log_err("recv");
> + return -1;
This is still not right. It needs to write arg.stop and do pthread_join().
pw-bot: cr
> + }
> +
> + WRITE_ONCE(arg.stop, 1);
> + pthread_join(srv_thread, &thread_ret);
> + if (IS_ERR(thread_ret)) {
> + log_err("thread_ret");
> + return -1;
> + }
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v4 3/3] selftests/bpf: Support nonblock for send_recv_data
2024-04-10 6:13 ` [PATCH bpf-next v4 3/3] selftests/bpf: Support nonblock for send_recv_data Geliang Tang
@ 2024-04-10 21:34 ` Martin KaFai Lau
2024-05-07 4:04 ` Geliang Tang
0 siblings, 1 reply; 7+ messages in thread
From: Martin KaFai Lau @ 2024-04-10 21:34 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, mptcp, linux-kselftest
On 4/9/24 11:13 PM, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Some tests, such as the MPTCP bpf tests, require send_recv_data helper
> to run in nonblock mode.
>
> This patch adds nonblock support for send_recv_data(). Check if it is
> currently in nonblock mode, and if so, ignore EWOULDBLOCK to continue
> sending and receiving.
>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> tools/testing/selftests/bpf/network_helpers.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> index 137cd18ef3f2..ca16ef2b648e 100644
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/tools/testing/selftests/bpf/network_helpers.c
> @@ -555,6 +555,7 @@ struct send_recv_arg {
> static void *send_recv_server(void *arg)
> {
> struct send_recv_arg *a = (struct send_recv_arg *)arg;
> + int flags = fcntl(a->fd, F_GETFL);
> ssize_t nr_sent = 0, bytes = 0;
> char batch[1500];
> int err = 0, fd;
> @@ -578,6 +579,8 @@ static void *send_recv_server(void *arg)
> if (nr_sent == -1 && errno == EINTR)
> continue;
> if (nr_sent == -1) {
> + if (flags & O_NONBLOCK && errno == EWOULDBLOCK)
I still don't see why it needs to be a non blocking IO. mptcp should work
with blocking IO also, no? Does it really need non blocking IO to make
mptcp test work? I would rather stay with blocking IO in selftest as much as
possible for simplicity reason.
I am afraid the root cause of the EAGAIN thread has not been figured out yet:
https://lore.kernel.org/all/b3943f9a8bf595212b00e96ba850bf32893312cc.camel@kernel.org/
Lets drop patch 3 until it is understood why mptcp needs EAGAIN or non-blocking IO.
It feels like there is some flakiness and it should be understood and avoided.
Other than the comment in patch 2, the first two patches lgtm. Please respin with
the first two patches.
> + continue;
> err = -errno;
> break;
> }
> @@ -599,6 +602,7 @@ static void *send_recv_server(void *arg)
>
> int send_recv_data(int lfd, int fd, uint32_t total_bytes)
> {
> + int flags = fcntl(lfd, F_GETFL);
> ssize_t nr_recv = 0, bytes = 0;
> struct send_recv_arg arg = {
> .fd = lfd,
> @@ -622,8 +626,11 @@ int send_recv_data(int lfd, int fd, uint32_t total_bytes)
> MIN(total_bytes - bytes, sizeof(batch)), 0);
> if (nr_recv == -1 && errno == EINTR)
> continue;
> - if (nr_recv == -1)
> + if (nr_recv == -1) {
> + if (flags & O_NONBLOCK && errno == EWOULDBLOCK)
> + continue;
> break;
> + }
> bytes += nr_recv;
> }
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v4 3/3] selftests/bpf: Support nonblock for send_recv_data
2024-04-10 21:34 ` Martin KaFai Lau
@ 2024-05-07 4:04 ` Geliang Tang
0 siblings, 0 replies; 7+ messages in thread
From: Geliang Tang @ 2024-05-07 4:04 UTC (permalink / raw
To: Martin KaFai Lau
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, mptcp, linux-kselftest
On Wed, 2024-04-10 at 14:34 -0700, Martin KaFai Lau wrote:
> On 4/9/24 11:13 PM, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> >
> > Some tests, such as the MPTCP bpf tests, require send_recv_data
> > helper
> > to run in nonblock mode.
> >
> > This patch adds nonblock support for send_recv_data(). Check if it
> > is
> > currently in nonblock mode, and if so, ignore EWOULDBLOCK to
> > continue
> > sending and receiving.
> >
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> > tools/testing/selftests/bpf/network_helpers.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/bpf/network_helpers.c
> > b/tools/testing/selftests/bpf/network_helpers.c
> > index 137cd18ef3f2..ca16ef2b648e 100644
> > --- a/tools/testing/selftests/bpf/network_helpers.c
> > +++ b/tools/testing/selftests/bpf/network_helpers.c
> > @@ -555,6 +555,7 @@ struct send_recv_arg {
> > static void *send_recv_server(void *arg)
> > {
> > struct send_recv_arg *a = (struct send_recv_arg *)arg;
> > + int flags = fcntl(a->fd, F_GETFL);
> > ssize_t nr_sent = 0, bytes = 0;
> > char batch[1500];
> > int err = 0, fd;
> > @@ -578,6 +579,8 @@ static void *send_recv_server(void *arg)
> > if (nr_sent == -1 && errno == EINTR)
> > continue;
> > if (nr_sent == -1) {
> > + if (flags & O_NONBLOCK && errno ==
> > EWOULDBLOCK)
>
> I still don't see why it needs to be a non blocking IO. mptcp should
> work
> with blocking IO also, no? Does it really need non blocking IO to
> make
> mptcp test work? I would rather stay with blocking IO in selftest as
> much as
> possible for simplicity reason.
>
> I am afraid the root cause of the EAGAIN thread has not been figured
> out yet:
> https://lore.kernel.org/all/b3943f9a8bf595212b00e96ba850bf32893312cc.camel@kernel.org/
>
> Lets drop patch 3 until it is understood why mptcp needs EAGAIN or
> non-blocking IO.
> It feels like there is some flakiness and it should be understood and
> avoided.
Hi Martin,
I finally found the root cause of this issue. It is indeed an MPTCP
bug. It took me a long time to debug, and the fix is here:
https://patchwork.kernel.org/project/mptcp/patch/0ccc1c26d27d6ee7be22806a97983d37c6ca548c.1715053270.git.tanggeliang@kylinos.cn/
Thank you for insisting on not accepting these work around patches from
me in the user space, almost hiding a kernel bug.
-Geliang
>
> Other than the comment in patch 2, the first two patches lgtm. Please
> respin with
> the first two patches.
>
> > + continue;
> > err = -errno;
> > break;
> > }
> > @@ -599,6 +602,7 @@ static void *send_recv_server(void *arg)
> >
> > int send_recv_data(int lfd, int fd, uint32_t total_bytes)
> > {
> > + int flags = fcntl(lfd, F_GETFL);
> > ssize_t nr_recv = 0, bytes = 0;
> > struct send_recv_arg arg = {
> > .fd = lfd,
> > @@ -622,8 +626,11 @@ int send_recv_data(int lfd, int fd, uint32_t
> > total_bytes)
> > MIN(total_bytes - bytes,
> > sizeof(batch)), 0);
> > if (nr_recv == -1 && errno == EINTR)
> > continue;
> > - if (nr_recv == -1)
> > + if (nr_recv == -1) {
> > + if (flags & O_NONBLOCK && errno ==
> > EWOULDBLOCK)
> > + continue;
> > break;
> > + }
> > bytes += nr_recv;
> > }
> >
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-05-07 4:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-10 6:13 [PATCH bpf-next v4 0/3] export send_recv_data Geliang Tang
2024-04-10 6:13 ` [PATCH bpf-next v4 1/3] selftests/bpf: Add struct send_recv_arg Geliang Tang
2024-04-10 6:13 ` [PATCH bpf-next v4 2/3] selftests/bpf: Export send_recv_data helper Geliang Tang
2024-04-10 21:20 ` Martin KaFai Lau
2024-04-10 6:13 ` [PATCH bpf-next v4 3/3] selftests/bpf: Support nonblock for send_recv_data Geliang Tang
2024-04-10 21:34 ` Martin KaFai Lau
2024-05-07 4:04 ` Geliang Tang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).