diff mbox series

[linux-next,1/3] selftests: net: udpgso_bench_rx: Fix verifty exceptions

Message ID 20230417122423.193237-1-yang.yang29@zte.com.cn
State New
Headers show
Series selftests: net: udpgso_bench_rx: Fix verifty, gsosize, and packet number exceptions | expand

Commit Message

Yang Yang April 17, 2023, 12:24 p.m. UTC
From: Zhang Yunkai (CGEL ZTE) <zhang.yunkai@zte.com.cn>

The verification function of this test case is likely to encounter the
following error, which may confuse users.

Executing the following command fails:
bash# udpgso_bench_tx -l 4 -4 -D "$DST"
bash# udpgso_bench_tx -l 4 -4 -D "$DST" -S 0
bash# udpgso_bench_rx -4 -G -S 1472 -v
udpgso_bench_rx: data[1472]: len 2944, a(97) != q(113)

This is because the sending buffers are not aligned by 26 bytes, and the
GRO is not merged sequentially, and the receiver does not judge this
situation. We do the validation after the data is split at the receiving
end, just as the application actually uses this feature.

Signed-off-by: Zhang Yunkai (CGEL ZTE) <zhang.yunkai@zte.com.cn>
Reviewed-by: xu xin (CGEL ZTE) <xu.xin16@zte.com.cn>
Reviewed-by: Yang Yang (CGEL ZTE) <yang.yang29@zte.com.cn>
Cc: Xuexin Jiang (CGEL ZTE) <jiang.xuexin@zte.com.cn>
---
 tools/testing/selftests/net/udpgso_bench_rx.c | 38 +++++++++++++++++++++------
 1 file changed, 30 insertions(+), 8 deletions(-)

Comments

Paolo Abeni April 18, 2023, 2:20 p.m. UTC | #1
On Mon, 2023-04-17 at 20:24 +0800, Yang Yang wrote:
> From: Zhang Yunkai (CGEL ZTE) <zhang.yunkai@zte.com.cn>
> 
> The verification function of this test case is likely to encounter the
> following error, which may confuse users.
> 
> Executing the following command fails:
> bash# udpgso_bench_tx -l 4 -4 -D "$DST"
> bash# udpgso_bench_tx -l 4 -4 -D "$DST" -S 0
> bash# udpgso_bench_rx -4 -G -S 1472 -v
> udpgso_bench_rx: data[1472]: len 2944, a(97) != q(113)


As noted by Willem, both the commit message and the above command
sequence is quite confusing. Please reorder the commands in the exact
sequence you run them, presumably:

udpgso_bench_rx -4 -G -S 1472 -v &
udpgso_bench_tx -l 4 -4 -D "$DST" -S 0


> This is because the sending buffers are not aligned by 26 bytes, and the
> GRO is not merged sequentially, and the receiver does not judge this
> situation. We do the validation after the data is split at the receiving
> end, just as the application actually uses this feature.

The wording from Willem response is much more clear. If applicable,
please use such text.

BTW I could not reproduce the issue with any permutation of the
suggested commands I could think of, so possibly that section need some
extra clarification.

Thanks,

Paolo
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/udpgso_bench_rx.c b/tools/testing/selftests/net/udpgso_bench_rx.c
index f35a924d4a30..a5b7f30659a5 100644
--- a/tools/testing/selftests/net/udpgso_bench_rx.c
+++ b/tools/testing/selftests/net/udpgso_bench_rx.c
@@ -189,16 +189,16 @@  static char sanitized_char(char val)
 	return (val >= 'a' && val <= 'z') ? val : '.';
 }
 
-static void do_verify_udp(const char *data, int len)
+static void do_verify_udp(const char *data, int start, int len)
 {
-	char cur = data[0];
+	char cur = data[start];
 	int i;
 
 	/* verify contents */
 	if (cur < 'a' || cur > 'z')
 		error(1, 0, "data initial byte out of range");
 
-	for (i = 1; i < len; i++) {
+	for (i = start + 1; i < start + len; i++) {
 		if (cur == 'z')
 			cur = 'a';
 		else
@@ -212,6 +212,24 @@  static void do_verify_udp(const char *data, int len)
 	}
 }
 
+static void do_verify_udp_gro(const char *data, int len, int gso_size)
+{
+	int remaining = len;
+	int start = 0;
+
+	while (remaining) {
+		if (remaining < 0)
+			break;
+
+		if (remaining > gso_size)
+			do_verify_udp(data, start, gso_size);
+		else
+			do_verify_udp(data, start, remaining);
+		start += gso_size;
+		remaining -= gso_size;
+	}
+}
+
 static int recv_msg(int fd, char *buf, int len, int *gso_size)
 {
 	char control[CMSG_SPACE(sizeof(int))] = {0};
@@ -264,16 +282,20 @@  static void do_flush_udp(int fd)
 		if (cfg_expected_pkt_len && ret != cfg_expected_pkt_len)
 			error(1, 0, "recv: bad packet len, got %d,"
 			      " expected %d\n", ret, cfg_expected_pkt_len);
+		if (cfg_expected_gso_size && cfg_expected_gso_size != gso_size)
+			error(1, 0, "recv: bad gso size, got %d, expected %d %s",
+				gso_size, cfg_expected_gso_size, "(-1 == no gso cmsg))\n");
 		if (len && cfg_verify) {
 			if (ret == 0)
 				error(1, errno, "recv: 0 byte datagram\n");
 
-			do_verify_udp(rbuf, ret);
+			if (!cfg_gro_segment)
+				do_verify_udp(rbuf, 0, ret);
+			else if (gso_size > 0)
+				do_verify_udp_gro(rbuf, ret, gso_size);
+			else
+				do_verify_udp_gro(rbuf, ret, ret);
 		}
-		if (cfg_expected_gso_size && cfg_expected_gso_size != gso_size)
-			error(1, 0, "recv: bad gso size, got %d, expected %d "
-			      "(-1 == no gso cmsg))\n", gso_size,
-			      cfg_expected_gso_size);
 
 		packets++;
 		bytes += ret;