[linux-next,1/3] selftests: net: udpgso_bench_rx: Fix verifty exceptions
Commit Message
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
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
Why are you running two senders concurrently? The test is not intended
to handle that case.
> 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>
> Why are you running two senders concurrently? The test is not intended
> to handle that case.
Sorry for the inaccuracy of the description here, these two commands,
i.e. with or without GSO, cause the problem. The same goes for patch 2/3.
The problem is easily reproducible in the latest kernel, QEMU environment, E1000.
bash# udpgso_bench_tx -l 4 -4 -D "$DST"
udpgso_bench_tx: write: Connection refused
bash# udpgso_bench_rx -4 -G -S 1472 -v
udpgso_bench_rx: data[1472]: len 17664, a(97) != q(113)
bash# udpgso_bench_tx -l 4 -4 -D "$DST" -S 0
udpgso_bench_tx: sendmsg: Connection refused
bash# udpgso_bench_rx -4 -G -S 1472 -v
udpgso_bench_rx: data[61824]: len 64768, a(97) != w(119)
In one test, the verification data is printed as follows:
abcd...xyz
...
abcd...xyz
abcd...opabcd...xyz
This is because the sender intercepts from the buf at a certain length,
which is not aligned according to 26 bytes, and multiple packets are
merged. The verification of the receiving end needs to be performed
after splitting.
Yang Yang wrote:
> > Why are you running two senders concurrently? The test is not intended
> > to handle that case.
>
> Sorry for the inaccuracy of the description here, these two commands,
> i.e. with or without GSO, cause the problem. The same goes for patch 2/3.
> The problem is easily reproducible in the latest kernel, QEMU environment, E1000.
>
> bash# udpgso_bench_tx -l 4 -4 -D "$DST"
> udpgso_bench_tx: write: Connection refused
> bash# udpgso_bench_rx -4 -G -S 1472 -v
> udpgso_bench_rx: data[1472]: len 17664, a(97) != q(113)
>
> bash# udpgso_bench_tx -l 4 -4 -D "$DST" -S 0
> udpgso_bench_tx: sendmsg: Connection refused
> bash# udpgso_bench_rx -4 -G -S 1472 -v
> udpgso_bench_rx: data[61824]: len 64768, a(97) != w(119)
I still don't follow: are you running the tx and rx commands
sequentially or in parallel? On different (virtual) hosts?
> In one test, the verification data is printed as follows:
> abcd...xyz
> ...
> abcd...xyz
> abcd...opabcd...xyz
>
> This is because the sender intercepts from the buf at a certain length,
> which is not aligned according to 26 bytes, and multiple packets are
> merged. The verification of the receiving end needs to be performed
> after splitting.
The 26 bytes is a reference to the pattern printed by the test, which
iterates over A-Z.
Is your point that each individual segment starts at A, so that a test
for pattern {ABC..Z}+ only matches if the segments size is a multiple
of 26, else a the pattern will have discontinuities?
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
@@ -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;