| Message ID | 20230417122423.193237-1-yang.yang29@zte.com.cn (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests: net: udpgso_bench_rx: Fix verifty, gsosize, and packet number exceptions | expand |
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
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;