* [PATCH bpf 0/2] Two fixes for test_sockmap
@ 2024-10-12 20:37 zijianzhang
2024-10-12 20:37 ` [PATCH bpf 1/2] selftests/bpf: Fix msg_verify_data in test_sockmap zijianzhang
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: zijianzhang @ 2024-10-12 20:37 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, mykolal, shuah,
bhole_prashant_q7, jakub, xiyou.wangcong, zijianzhang
From: Zijian Zhang <zijianzhang@bytedance.com>
Function msg_verify_data should have context of bytes_cnt and k instead of
assuming they are zero. Otherwise, test_sockmap with data integrity test
will report some errors. I also fix the logic related to size and index j
1/ 6 sockmap::txmsg test passthrough:FAIL
2/ 6 sockmap::txmsg test redirect:FAIL
7/12 sockmap::txmsg test apply:FAIL
10/11 sockmap::txmsg test push_data:FAIL
11/17 sockmap::txmsg test pull-data:FAIL
12/ 9 sockmap::txmsg test pop-data:FAIL
13/ 1 sockmap::txmsg test push/pop data:FAIL
...
Pass: 24 Fail: 52
After fixing msg_verify_data, some of the errors are solved, but for push
pull and pop, we may need more fixes to msg_verify_data, added a TODO
10/11 sockmap::txmsg test push_data:FAIL
11/17 sockmap::txmsg test pull-data:FAIL
12/ 9 sockmap::txmsg test pop-data:FAIL
...
Pass: 37 Fail: 15
Besides, added a custom errno EDATAINTEGRITY for msg_verify_data, we
shall not ignore the error in txmsg_cork case, and fixed the txmsg_redir
in test_txmsg_pull "Test pull + redirect" case.
Zijian Zhang (2):
selftests/bpf: Fix msg_verify_data in test_sockmap
selftests/bpf: Fix txmsg_redir of test_txmsg_pull in test_sockmap
tools/testing/selftests/bpf/test_sockmap.c | 32 ++++++++++++++--------
1 file changed, 21 insertions(+), 11 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH bpf 1/2] selftests/bpf: Fix msg_verify_data in test_sockmap 2024-10-12 20:37 [PATCH bpf 0/2] Two fixes for test_sockmap zijianzhang @ 2024-10-12 20:37 ` zijianzhang 2024-10-12 20:37 ` [PATCH bpf 2/2] selftests/bpf: Fix txmsg_redir of test_txmsg_pull " zijianzhang ` (2 subsequent siblings) 3 siblings, 0 replies; 6+ messages in thread From: zijianzhang @ 2024-10-12 20:37 UTC (permalink / raw) To: bpf Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, mykolal, shuah, bhole_prashant_q7, jakub, xiyou.wangcong, zijianzhang From: Zijian Zhang <zijianzhang@bytedance.com> Function msg_verify_data should have context of bytes_cnt and k instead of assuming they are zero. Otherwise, test_sockmap with data integrity test will report some errors. I also fix the logic related to size and index j 1/ 6 sockmap::txmsg test passthrough:FAIL 2/ 6 sockmap::txmsg test redirect:FAIL 7/12 sockmap::txmsg test apply:FAIL 10/11 sockmap::txmsg test push_data:FAIL 11/17 sockmap::txmsg test pull-data:FAIL 12/ 9 sockmap::txmsg test pop-data:FAIL 13/ 1 sockmap::txmsg test push/pop data:FAIL ... Pass: 24 Fail: 52 After applying this patch, some of the errors are solved, but for push, pull and pop, we may need more fixes to msg_verify_data, added a TODO 10/11 sockmap::txmsg test push_data:FAIL 11/17 sockmap::txmsg test pull-data:FAIL 12/ 9 sockmap::txmsg test pop-data:FAIL ... Pass: 37 Fail: 15 Besides, added a custom errno EDATAINTEGRITY for msg_verify_data, we shall not ignore the error in txmsg_cork case. Fixes: 753fb2ee0934 ("bpf: sockmap, add msg_peek tests to test_sockmap") Fixes: 16edddfe3c5d ("selftests/bpf: test_sockmap, check test failure") Signed-off-by: Zijian Zhang <zijianzhang@bytedance.com> --- tools/testing/selftests/bpf/test_sockmap.c | 30 ++++++++++++++-------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c index 3e02d7267de8..8249f3c1fbd6 100644 --- a/tools/testing/selftests/bpf/test_sockmap.c +++ b/tools/testing/selftests/bpf/test_sockmap.c @@ -56,6 +56,8 @@ static void running_handler(int a); #define BPF_SOCKHASH_FILENAME "test_sockhash_kern.bpf.o" #define CG_PATH "/sockmap" +#define EDATAINTEGRITY 2001 + /* global sockets */ int s1, s2, c1, c2, p1, p2; int test_cnt; @@ -510,23 +512,25 @@ static int msg_alloc_iov(struct msghdr *msg, return -ENOMEM; } -static int msg_verify_data(struct msghdr *msg, int size, int chunk_sz) +/* TODO: Add verification logic for push, pull and pop data */ +static int msg_verify_data(struct msghdr *msg, int size, int chunk_sz, + unsigned char *k_p, int *bytes_cnt_p) { - int i, j = 0, bytes_cnt = 0; - unsigned char k = 0; + int i, j, bytes_cnt = *bytes_cnt_p; + unsigned char k = *k_p; - for (i = 0; i < msg->msg_iovlen; i++) { + for (i = 0, j = 0; i < msg->msg_iovlen && size; i++, j = 0) { unsigned char *d = msg->msg_iov[i].iov_base; /* Special case test for skb ingress + ktls */ if (i == 0 && txmsg_ktls_skb) { if (msg->msg_iov[i].iov_len < 4) - return -EIO; + return -EDATAINTEGRITY; if (memcmp(d, "PASS", 4) != 0) { fprintf(stderr, "detected skb data error with skb ingress update @iov[%i]:%i \"%02x %02x %02x %02x\" != \"PASS\"\n", i, 0, d[0], d[1], d[2], d[3]); - return -EIO; + return -EDATAINTEGRITY; } j = 4; /* advance index past PASS header */ } @@ -536,7 +540,7 @@ static int msg_verify_data(struct msghdr *msg, int size, int chunk_sz) fprintf(stderr, "detected data corruption @iov[%i]:%i %02x != %02x, %02x ?= %02x\n", i, j, d[j], k - 1, d[j+1], k); - return -EIO; + return -EDATAINTEGRITY; } bytes_cnt++; if (bytes_cnt == chunk_sz) { @@ -546,6 +550,8 @@ static int msg_verify_data(struct msghdr *msg, int size, int chunk_sz) size--; } } + *k_p = k; + *bytes_cnt_p = bytes_cnt; return 0; } @@ -602,6 +608,8 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt, float total_bytes, txmsg_pop_total; int fd_flags = O_NONBLOCK; struct timeval timeout; + unsigned char k = 0; + int bytes_cnt = 0; fd_set w; fcntl(fd, fd_flags); @@ -696,7 +704,7 @@ 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); + errno = msg_verify_data(&msg, recv, chunk_sz, &k, &bytes_cnt); if (errno) { perror("data verify msg failed"); goto out_errno; @@ -704,7 +712,9 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt, if (recvp) { errno = msg_verify_data(&msg_peek, recvp, - chunk_sz); + chunk_sz, + &k, + &bytes_cnt); if (errno) { perror("data verify msg_peek failed"); goto out_errno; @@ -812,7 +822,7 @@ static int sendmsg_test(struct sockmap_options *opt) s.bytes_sent, sent_Bps, sent_Bps/giga, s.bytes_recvd, recvd_Bps, recvd_Bps/giga, peek_flag ? "(peek_msg)" : ""); - if (err && txmsg_cork) + if (err && err != -EDATAINTEGRITY && txmsg_cork) err = 0; exit(err ? 1 : 0); } else if (rxpid == -1) { -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf 2/2] selftests/bpf: Fix txmsg_redir of test_txmsg_pull in test_sockmap 2024-10-12 20:37 [PATCH bpf 0/2] Two fixes for test_sockmap zijianzhang 2024-10-12 20:37 ` [PATCH bpf 1/2] selftests/bpf: Fix msg_verify_data in test_sockmap zijianzhang @ 2024-10-12 20:37 ` zijianzhang 2024-10-16 18:10 ` [PATCH bpf 0/2] Two fixes for test_sockmap John Fastabend 2024-10-16 20:50 ` patchwork-bot+netdevbpf 3 siblings, 0 replies; 6+ messages in thread From: zijianzhang @ 2024-10-12 20:37 UTC (permalink / raw) To: bpf Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, mykolal, shuah, bhole_prashant_q7, jakub, xiyou.wangcong, zijianzhang From: Zijian Zhang <zijianzhang@bytedance.com> txmsg_redir in "Test pull + redirect" case of test_txmsg_pull should be 1 instead of 0. Fixes: 328aa08a081b ("bpf: Selftests, break down test_sockmap into subtests") Signed-off-by: Zijian Zhang <zijianzhang@bytedance.com> --- tools/testing/selftests/bpf/test_sockmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c index 8249f3c1fbd6..075c93ed143e 100644 --- a/tools/testing/selftests/bpf/test_sockmap.c +++ b/tools/testing/selftests/bpf/test_sockmap.c @@ -1606,7 +1606,7 @@ static void test_txmsg_pull(int cgrp, struct sockmap_options *opt) test_send_large(opt, cgrp); /* Test pull + redirect */ - txmsg_redir = 0; + txmsg_redir = 1; txmsg_start = 1; txmsg_end = 2; test_send(opt, cgrp); -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH bpf 0/2] Two fixes for test_sockmap 2024-10-12 20:37 [PATCH bpf 0/2] Two fixes for test_sockmap zijianzhang 2024-10-12 20:37 ` [PATCH bpf 1/2] selftests/bpf: Fix msg_verify_data in test_sockmap zijianzhang 2024-10-12 20:37 ` [PATCH bpf 2/2] selftests/bpf: Fix txmsg_redir of test_txmsg_pull " zijianzhang @ 2024-10-16 18:10 ` John Fastabend 2024-10-16 19:24 ` Zijian Zhang 2024-10-16 20:50 ` patchwork-bot+netdevbpf 3 siblings, 1 reply; 6+ messages in thread From: John Fastabend @ 2024-10-16 18:10 UTC (permalink / raw) To: zijianzhang, bpf Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, mykolal, shuah, bhole_prashant_q7, jakub, xiyou.wangcong, zijianzhang zijianzhang@ wrote: > From: Zijian Zhang <zijianzhang@bytedance.com> > > Function msg_verify_data should have context of bytes_cnt and k instead of > assuming they are zero. Otherwise, test_sockmap with data integrity test > will report some errors. I also fix the logic related to size and index j > > 1/ 6 sockmap::txmsg test passthrough:FAIL > 2/ 6 sockmap::txmsg test redirect:FAIL > 7/12 sockmap::txmsg test apply:FAIL > 10/11 sockmap::txmsg test push_data:FAIL > 11/17 sockmap::txmsg test pull-data:FAIL > 12/ 9 sockmap::txmsg test pop-data:FAIL > 13/ 1 sockmap::txmsg test push/pop data:FAIL > ... > Pass: 24 Fail: 52 > > After fixing msg_verify_data, some of the errors are solved, but for push > pull and pop, we may need more fixes to msg_verify_data, added a TODO > > 10/11 sockmap::txmsg test push_data:FAIL > 11/17 sockmap::txmsg test pull-data:FAIL > 12/ 9 sockmap::txmsg test pop-data:FAIL > ... > Pass: 37 Fail: 15 Thanks. Did you plan on fixing the rest next? Otherwise I'll add it to my list. > > Besides, added a custom errno EDATAINTEGRITY for msg_verify_data, we > shall not ignore the error in txmsg_cork case, and fixed the txmsg_redir > in test_txmsg_pull "Test pull + redirect" case. > > > Zijian Zhang (2): > selftests/bpf: Fix msg_verify_data in test_sockmap > selftests/bpf: Fix txmsg_redir of test_txmsg_pull in test_sockmap > > tools/testing/selftests/bpf/test_sockmap.c | 32 ++++++++++++++-------- > 1 file changed, 21 insertions(+), 11 deletions(-) > > -- > 2.20.1 > For the series, Acked-by: John Fastabend <john.fastabend@gmail.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH bpf 0/2] Two fixes for test_sockmap 2024-10-16 18:10 ` [PATCH bpf 0/2] Two fixes for test_sockmap John Fastabend @ 2024-10-16 19:24 ` Zijian Zhang 0 siblings, 0 replies; 6+ messages in thread From: Zijian Zhang @ 2024-10-16 19:24 UTC (permalink / raw) To: John Fastabend, bpf Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, kpsingh, sdf, haoluo, jolsa, mykolal, shuah, bhole_prashant_q7, jakub, xiyou.wangcong On 10/16/24 11:10 AM, John Fastabend wrote: > zijianzhang@ wrote: >> From: Zijian Zhang <zijianzhang@bytedance.com> >> >> Function msg_verify_data should have context of bytes_cnt and k instead of >> assuming they are zero. Otherwise, test_sockmap with data integrity test >> will report some errors. I also fix the logic related to size and index j >> >> 1/ 6 sockmap::txmsg test passthrough:FAIL >> 2/ 6 sockmap::txmsg test redirect:FAIL >> 7/12 sockmap::txmsg test apply:FAIL >> 10/11 sockmap::txmsg test push_data:FAIL >> 11/17 sockmap::txmsg test pull-data:FAIL >> 12/ 9 sockmap::txmsg test pop-data:FAIL >> 13/ 1 sockmap::txmsg test push/pop data:FAIL >> ... >> Pass: 24 Fail: 52 >> >> After fixing msg_verify_data, some of the errors are solved, but for push >> pull and pop, we may need more fixes to msg_verify_data, added a TODO >> >> 10/11 sockmap::txmsg test push_data:FAIL >> 11/17 sockmap::txmsg test pull-data:FAIL >> 12/ 9 sockmap::txmsg test pop-data:FAIL >> ... >> Pass: 37 Fail: 15 > > Thanks. Did you plan on fixing the rest next? Otherwise I'll add it to > my list. > No problem, I will fix it next. >> >> Besides, added a custom errno EDATAINTEGRITY for msg_verify_data, we >> shall not ignore the error in txmsg_cork case, and fixed the txmsg_redir >> in test_txmsg_pull "Test pull + redirect" case. >> >> >> Zijian Zhang (2): >> selftests/bpf: Fix msg_verify_data in test_sockmap >> selftests/bpf: Fix txmsg_redir of test_txmsg_pull in test_sockmap >> >> tools/testing/selftests/bpf/test_sockmap.c | 32 ++++++++++++++-------- >> 1 file changed, 21 insertions(+), 11 deletions(-) >> >> -- >> 2.20.1 >> > > > For the series, > > Acked-by: John Fastabend <john.fastabend@gmail.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf 0/2] Two fixes for test_sockmap 2024-10-12 20:37 [PATCH bpf 0/2] Two fixes for test_sockmap zijianzhang ` (2 preceding siblings ...) 2024-10-16 18:10 ` [PATCH bpf 0/2] Two fixes for test_sockmap John Fastabend @ 2024-10-16 20:50 ` patchwork-bot+netdevbpf 3 siblings, 0 replies; 6+ messages in thread From: patchwork-bot+netdevbpf @ 2024-10-16 20:50 UTC (permalink / raw) To: Zijian Zhang Cc: bpf, ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, mykolal, shuah, bhole_prashant_q7, jakub, xiyou.wangcong Hello: This series was applied to bpf/bpf-next.git (net) by Martin KaFai Lau <martin.lau@kernel.org>: On Sat, 12 Oct 2024 20:37:29 +0000 you wrote: > From: Zijian Zhang <zijianzhang@bytedance.com> > > Function msg_verify_data should have context of bytes_cnt and k instead of > assuming they are zero. Otherwise, test_sockmap with data integrity test > will report some errors. I also fix the logic related to size and index j > > 1/ 6 sockmap::txmsg test passthrough:FAIL > 2/ 6 sockmap::txmsg test redirect:FAIL > 7/12 sockmap::txmsg test apply:FAIL > 10/11 sockmap::txmsg test push_data:FAIL > 11/17 sockmap::txmsg test pull-data:FAIL > 12/ 9 sockmap::txmsg test pop-data:FAIL > 13/ 1 sockmap::txmsg test push/pop data:FAIL > ... > Pass: 24 Fail: 52 > > [...] Here is the summary with links: - [bpf,1/2] selftests/bpf: Fix msg_verify_data in test_sockmap https://git.kernel.org/bpf/bpf-next/c/ee9b352ce465 - [bpf,2/2] selftests/bpf: Fix txmsg_redir of test_txmsg_pull in test_sockmap https://git.kernel.org/bpf/bpf-next/c/b29e231d6630 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] 6+ messages in thread
end of thread, other threads:[~2024-10-16 20:50 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-12 20:37 [PATCH bpf 0/2] Two fixes for test_sockmap zijianzhang 2024-10-12 20:37 ` [PATCH bpf 1/2] selftests/bpf: Fix msg_verify_data in test_sockmap zijianzhang 2024-10-12 20:37 ` [PATCH bpf 2/2] selftests/bpf: Fix txmsg_redir of test_txmsg_pull " zijianzhang 2024-10-16 18:10 ` [PATCH bpf 0/2] Two fixes for test_sockmap John Fastabend 2024-10-16 19:24 ` Zijian Zhang 2024-10-16 20:50 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox