| Message ID | 202304241355464262541@zte.com.cn (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [linux-next,v3] selftests: net: udpgso_bench_rx: Fix verifty exceptions | expand |
yang.yang29@ 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. The problem is easily > reproducible in the latest kernel. > > Environment A, the sender: > bash# udpgso_bench_tx -l 4 -4 -D "$IP_B" > udpgso_bench_tx: write: Connection refused This error is not relevant to the bug that is being fixed > Environment B, the receiver: > bash# udpgso_bench_rx -4 -G -S 1472 -v > udpgso_bench_rx: data[1472]: len 17664, a(97) != q(113) > > If the packet is captured, you will see: > Environment A, the sender: > bash# tcpdump -i eth0 host "$IP_B" & > IP $IP_A.41025 > $IP_B.8000: UDP, length 1472 > IP $IP_A.41025 > $IP_B.8000: UDP, length 1472 > IP $IP_B > $IP_A: ICMP $IP_B udp port 8000 unreachable, length 556 Same here > Environment B, the receiver: > bash# tcpdump -i eth0 host "$IP_B" & > IP $IP_A.41025 > $IP_B.8000: UDP, length 7360 > IP $IP_A.41025 > $IP_B.8000: UDP, length 14720 > IP $IP_B > $IP_A: ICMP $IP_B udp port 8000 unreachable, length 556 And here > In one test, the verification data is printed as follows: > abcd...xyz | 1... > .. | > abcd...xyz | > abcd...opabcd...xyz | ...1472... Not xyzabcd, messages are merged > .. | > > The issue is that the test on receive for expected payload pattern > {AB..Z}+ fail for GRO packets if segment payload does not end on a Z. This is really the only pertinent explanation needed for the fix. > The issue still exists when using the GRO with -G, but not using the -S > to obtain gsosize. Therefore, a print has been added to remind users. So the issue is that -G/cfg_gro_segment enables UDP_GRO, but -S/cfg_expected_gso_size enables recvmsg cmsg UDP_GRO. We need gso_size to know whether discontinuities will appear, so cannot verify payload for -G without -S. There really is no reason to ever run the test in that configuration, should perhaps fail. Btw title should start with PATCH net as this is a fix, instead of PATCH linux-next. And it is verify not verifty. Also needs a Fixes tag: Fixes: 0a9ac2e954091 ("selftests: add GRO support to udp bench rx program") > Changes in v3: > - Simplify description and adjust judgment order. > > Changes in v2: > - Fix confusing descriptions. > > 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 | 34 +++++++++++++++++++++++---- > 1 file changed, 29 insertions(+), 5 deletions(-) > > diff --git a/tools/testing/selftests/net/udpgso_bench_rx.c b/tools/testing/selftests/net/udpgso_bench_rx.c > index f35a924d4a30..3ad18cbc570d 100644 > --- a/tools/testing/selftests/net/udpgso_bench_rx.c > +++ b/tools/testing/selftests/net/udpgso_bench_rx.c > @@ -189,26 +189,45 @@ 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 > cur++; > > - if (data[i] != cur) > + if (data[i] != cur) { > + if (cfg_gro_segment && !cfg_expected_gso_size) > + error(0, 0, "Use -S to obtain gsosize to guide " > + "splitting and verification."); > + This is not the place to add a gso_size test. Drop. > error(1, 0, "data[%d]: len %d, %c(%hhu) != %c(%hhu)\n", > i, len, > sanitized_char(data[i]), data[i], > sanitized_char(cur), cur); > + } > + } > +} > + > +static void do_verify_udp_gro(const char *data, int len, int segment_size) > +{ > + int start = 0; > + > + while (len - start > 0) { > + if (len - start > segment_size) > + do_verify_udp(data, start, segment_size); > + else > + do_verify_udp(data, start, len - start); Instead of adding start argument, just pass data + start as first argument. > + > + start += segment_size; > } > } > > @@ -268,7 +287,12 @@ static void do_flush_udp(int fd) > 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); This only test a fraction of the payload. The test should always test the entire payload. It should just be aware of discontinuity at gso_size. > } > if (cfg_expected_gso_size && cfg_expected_gso_size != gso_size) > error(1, 0, "recv: bad gso size, got %d, expected %d " > -- > 2.15.2
diff --git a/tools/testing/selftests/net/udpgso_bench_rx.c b/tools/testing/selftests/net/udpgso_bench_rx.c index f35a924d4a30..3ad18cbc570d 100644 --- a/tools/testing/selftests/net/udpgso_bench_rx.c +++ b/tools/testing/selftests/net/udpgso_bench_rx.c @@ -189,26 +189,45 @@ 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 cur++; - if (data[i] != cur) + if (data[i] != cur) { + if (cfg_gro_segment && !cfg_expected_gso_size) + error(0, 0, "Use -S to obtain gsosize to guide " + "splitting and verification."); + error(1, 0, "data[%d]: len %d, %c(%hhu) != %c(%hhu)\n", i, len, sanitized_char(data[i]), data[i], sanitized_char(cur), cur); + } + } +} + +static void do_verify_udp_gro(const char *data, int len, int segment_size) +{ + int start = 0; + + while (len - start > 0) { + if (len - start > segment_size) + do_verify_udp(data, start, segment_size); + else + do_verify_udp(data, start, len - start); + + start += segment_size; } } @@ -268,7 +287,12 @@ static void do_flush_udp(int fd) 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 "