* [PATCH bpf-next v5 0/2] Add F_SETFL for fcntl in test_sockmap
@ 2024-04-23 10:26 Geliang Tang
2024-04-23 10:26 ` [PATCH bpf-next v5 1/2] selftests/bpf: Check recv lengths " Geliang Tang
2024-04-23 10:26 ` [PATCH bpf-next v5 2/2] selftests/bpf: Add F_SETFL for fcntl " Geliang Tang
0 siblings, 2 replies; 4+ messages in thread
From: Geliang Tang @ 2024-04-23 10:26 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, Jakub Sitnicki
Cc: Geliang Tang, bpf, linux-kselftest, Geliang Tang
From: Geliang Tang <tanggeliang@kylinos.cn>
v5:
- add a new patch "Check recv lengths in test_sockmap" instead of using
"continue" in msg_loop.
v4:
- address Martin's comments for v3. (thanks.)
- add Yonghong's "Acked-by" tags. (thanks.)
- update subject-prefix from "bpf-next" to "bpf".
Patch 1, v3 of "selftests/bpf: Add F_SETFL for fcntl":
- detect nonblock flag automaticly, then test_sockmap can run in both
block and nonblock modes.
- use continue instead of again in v2.
Patch 2, fix for umount cgroup2 error.
Geliang Tang (2):
selftests/bpf: Check recv lengths in test_sockmap
selftests/bpf: Add F_SETFL for fcntl in test_sockmap
tools/testing/selftests/bpf/test_sockmap.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
--
2.40.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH bpf-next v5 1/2] selftests/bpf: Check recv lengths in test_sockmap
2024-04-23 10:26 [PATCH bpf-next v5 0/2] Add F_SETFL for fcntl in test_sockmap Geliang Tang
@ 2024-04-23 10:26 ` Geliang Tang
2024-04-29 20:15 ` Jakub Sitnicki
2024-04-23 10:26 ` [PATCH bpf-next v5 2/2] selftests/bpf: Add F_SETFL for fcntl " Geliang Tang
1 sibling, 1 reply; 4+ messages in thread
From: Geliang Tang @ 2024-04-23 10:26 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, Jakub Sitnicki
Cc: Geliang Tang, bpf, linux-kselftest, Geliang Tang
From: Geliang Tang <tanggeliang@kylinos.cn>
The values of recv and recvp in msg_loop may be negative, so it's necessary
to check if they are positive before using them.
Fixes: 16962b2404ac ("bpf: sockmap, add selftests")
Fixes: 753fb2ee0934 ("bpf: sockmap, add msg_peek tests to test_sockmap")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
---
tools/testing/selftests/bpf/test_sockmap.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
index 43612de44fbf..24b55da9d4af 100644
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -680,7 +680,8 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
}
}
- s->bytes_recvd += recv;
+ if (recv > 0)
+ s->bytes_recvd += recv;
if (opt->check_recved_len && s->bytes_recvd > total_bytes) {
errno = EMSGSIZE;
@@ -694,12 +695,14 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
iov_length * cnt :
iov_length * iov_count;
- errno = msg_verify_data(&msg, recv, chunk_sz);
- if (errno) {
- perror("data verify msg failed");
- goto out_errno;
+ if (recv > 0) {
+ errno = msg_verify_data(&msg, recv, chunk_sz);
+ if (errno) {
+ perror("data verify msg failed");
+ goto out_errno;
+ }
}
- if (recvp) {
+ if (recvp > 0) {
errno = msg_verify_data(&msg_peek,
recvp,
chunk_sz);
--
2.40.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH bpf-next v5 2/2] selftests/bpf: Add F_SETFL for fcntl in test_sockmap
2024-04-23 10:26 [PATCH bpf-next v5 0/2] Add F_SETFL for fcntl in test_sockmap Geliang Tang
2024-04-23 10:26 ` [PATCH bpf-next v5 1/2] selftests/bpf: Check recv lengths " Geliang Tang
@ 2024-04-23 10:26 ` Geliang Tang
1 sibling, 0 replies; 4+ messages in thread
From: Geliang Tang @ 2024-04-23 10:26 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, Jakub Sitnicki
Cc: Geliang Tang, bpf, linux-kselftest, Geliang Tang
From: Geliang Tang <tanggeliang@kylinos.cn>
Incorrect arguments are passed to fcntl() in test_sockmap.c when invoking
it to set file status flags. If O_NONBLOCK is used as 2nd argument and
passed into fcntl, -EINVAL will be returned (See do_fcntl() in fs/fcntl.c).
The correct approach is to use F_SETFL as 2nd argument, and O_NONBLOCK as
3rd one.
Fixes: 16962b2404ac ("bpf: sockmap, add selftests")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
---
tools/testing/selftests/bpf/test_sockmap.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
index 24b55da9d4af..74a2c6e74fae 100644
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -603,7 +603,9 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
struct timeval timeout;
fd_set w;
- fcntl(fd, fd_flags);
+ if (fcntl(fd, F_SETFL, fd_flags))
+ goto out_errno;
+
/* Account for pop bytes noting each iteration of apply will
* call msg_pop_data helper so we need to account for this
* by calculating the number of apply iterations. Note user
--
2.40.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next v5 1/2] selftests/bpf: Check recv lengths in test_sockmap
2024-04-23 10:26 ` [PATCH bpf-next v5 1/2] selftests/bpf: Check recv lengths " Geliang Tang
@ 2024-04-29 20:15 ` Jakub Sitnicki
0 siblings, 0 replies; 4+ messages in thread
From: Jakub Sitnicki @ 2024-04-29 20:15 UTC (permalink / raw
To: Geliang Tang, John Fastabend
Cc: 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, Geliang Tang, bpf,
linux-kselftest
On Tue, Apr 23, 2024 at 06:26 PM +08, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> The values of recv and recvp in msg_loop may be negative, so it's necessary
> to check if they are positive before using them.
>
> Fixes: 16962b2404ac ("bpf: sockmap, add selftests")
> Fixes: 753fb2ee0934 ("bpf: sockmap, add msg_peek tests to test_sockmap")
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> Acked-by: Yonghong Song <yonghong.song@linux.dev>
> ---
> tools/testing/selftests/bpf/test_sockmap.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
> index 43612de44fbf..24b55da9d4af 100644
> --- a/tools/testing/selftests/bpf/test_sockmap.c
> +++ b/tools/testing/selftests/bpf/test_sockmap.c
> @@ -680,7 +680,8 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
> }
> }
>
> - s->bytes_recvd += recv;
> + if (recv > 0)
> + s->bytes_recvd += recv;
>
> if (opt->check_recved_len && s->bytes_recvd > total_bytes) {
> errno = EMSGSIZE;
I'm concerned why are we getting false-positives from select() here?
This is what leads to test failures once socket is non-blocking.
[pid 544] pselect6(29, [28], NULL, NULL, {tv_sec=3, tv_nsec=0}, NULL) = 1 (in [28], left {tv_sec=2, tv_nsec=999997014})
[pid 544] recvmsg(28, <unfinished ...>
[pid 545] +++ exited with 0 +++
[pid 544] <... recvmsg resumed>{msg_namelen=0}, MSG_NOSIGNAL) = -1 EAGAIN (Resource temporarily unavailable)
Is there an explanation? Or are we ignoring an issue in sockmap code by
"skipping" over EAGAIN errors from recvmsg() in the test?
Didn't have time to dig deeper yet.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-29 20:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-23 10:26 [PATCH bpf-next v5 0/2] Add F_SETFL for fcntl in test_sockmap Geliang Tang
2024-04-23 10:26 ` [PATCH bpf-next v5 1/2] selftests/bpf: Check recv lengths " Geliang Tang
2024-04-29 20:15 ` Jakub Sitnicki
2024-04-23 10:26 ` [PATCH bpf-next v5 2/2] selftests/bpf: Add F_SETFL for fcntl " 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).