BPF List
 help / color / mirror / Atom feed
* [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