There are 2 issues for the current udpgro test. The first one is the testing doesn't record all the failures, which may report pass but the test actually failed. e.g. https://netdev-3.bots.linux.dev/vmksft-net/results/725661/45-udpgro-sh/stdou...
The other one is after commit d7db7775ea2e ("net: veth: do not manipulate GRO when using XDP"), there is no need to load xdp program to enable GRO on veth device.
Hangbin Liu (2): selftests: udpgro: report error when receive failed selftests: udpgro: no need to load xdp for gro
tools/testing/selftests/net/udpgro.sh | 50 +++++++++++++-------------- 1 file changed, 25 insertions(+), 25 deletions(-)
Currently, we only check the latest senders's exit code. If the receiver report failed, it is not recoreded. Fix it by checking the exit code of all the involved processes.
Before: bad GRO lookup ok multiple GRO socks ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520
./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520
failed $ echo $? 0
After: bad GRO lookup ok multiple GRO socks ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520
./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520
failed $ echo $? 1
Fixes: 3327a9c46352 ("selftests: add functionals test for UDP GRO") Suggested-by: Paolo Abeni pabeni@redhat.com Signed-off-by: Hangbin Liu liuhangbin@gmail.com --- tools/testing/selftests/net/udpgro.sh | 41 ++++++++++++++++----------- 1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/net/udpgro.sh b/tools/testing/selftests/net/udpgro.sh index 11a1ebda564f..7e0164247b83 100755 --- a/tools/testing/selftests/net/udpgro.sh +++ b/tools/testing/selftests/net/udpgro.sh @@ -49,14 +49,15 @@ run_one() {
cfg_veth
- ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${rx_args} && \ - echo "ok" || \ - echo "failed" & + ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${rx_args} & + local PID1=$!
wait_local_port_listen ${PEER_NS} 8000 udp ./udpgso_bench_tx ${tx_args} - ret=$? - wait $(jobs -p) + check_err $? + wait ${PID1} + check_err $? + [ "$ret" -eq 0 ] && echo "ok" || echo "failed" return $ret }
@@ -93,16 +94,17 @@ run_one_nat() { # ... so that GRO will match the UDP_GRO enabled socket, but packets # will land on the 'plain' one ip netns exec "${PEER_NS}" ./udpgso_bench_rx -G ${family} -b ${addr1} -n 0 & - pid=$! - ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${family} -b ${addr2%/*} ${rx_args} && \ - echo "ok" || \ - echo "failed"& + local PID1=$! + ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${family} -b ${addr2%/*} ${rx_args} & + local PID2=$!
wait_local_port_listen "${PEER_NS}" 8000 udp ./udpgso_bench_tx ${tx_args} - ret=$? - kill -INT $pid - wait $(jobs -p) + check_err $? + kill -INT ${PID1} + wait ${PID2} + check_err $? + [ "$ret" -eq 0 ] && echo "ok" || echo "failed" return $ret }
@@ -115,16 +117,21 @@ run_one_2sock() { cfg_veth
ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${rx_args} -p 12345 & - ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 2000 -R 10 ${rx_args} && \ - echo "ok" || \ - echo "failed" & + local PID1=$! + ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 2000 -R 10 ${rx_args} & + local PID2=$!
wait_local_port_listen "${PEER_NS}" 12345 udp ./udpgso_bench_tx ${tx_args} -p 12345 + check_err $? wait_local_port_listen "${PEER_NS}" 8000 udp ./udpgso_bench_tx ${tx_args} - ret=$? - wait $(jobs -p) + check_err $? + wait ${PID1} + check_err $? + wait ${PID2} + check_err $? + [ "$ret" -eq 0 ] && echo "ok" || echo "failed" return $ret }
On 8/14/24 09:57, Hangbin Liu wrote:
Currently, we only check the latest senders's exit code. If the receiver report failed, it is not recoreded. Fix it by checking the exit code of all the involved processes.
Before: bad GRO lookup ok multiple GRO socks ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520
./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520
failed $ echo $? 0
After: bad GRO lookup ok multiple GRO socks ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520
./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520
failed $ echo $? 1
Fixes: 3327a9c46352 ("selftests: add functionals test for UDP GRO") Suggested-by: Paolo Abeni pabeni@redhat.com Signed-off-by: Hangbin Liu liuhangbin@gmail.com
tools/testing/selftests/net/udpgro.sh | 41 ++++++++++++++++----------- 1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/net/udpgro.sh b/tools/testing/selftests/net/udpgro.sh index 11a1ebda564f..7e0164247b83 100755 --- a/tools/testing/selftests/net/udpgro.sh +++ b/tools/testing/selftests/net/udpgro.sh @@ -49,14 +49,15 @@ run_one() { cfg_veth
- ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${rx_args} && \
echo "ok" || \echo "failed" &
- ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${rx_args} &
- local PID1=$!
wait_local_port_listen ${PEER_NS} 8000 udp ./udpgso_bench_tx ${tx_args}
- ret=$?
- wait $(jobs -p)
- check_err $?
- wait ${PID1}
- check_err $?
- [ "$ret" -eq 0 ] && echo "ok" || echo "failed"
I think that with the above, in case of a failure, every test after the failing one will should fail, regardless of the actual results, am I correct?
Thanks,
Paolo
On Wed, Aug 14, 2024 at 12:19:22PM +0200, Paolo Abeni wrote:
--- a/tools/testing/selftests/net/udpgro.sh +++ b/tools/testing/selftests/net/udpgro.sh @@ -49,14 +49,15 @@ run_one() { cfg_veth
- ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${rx_args} && \
echo "ok" || \echo "failed" &
- ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${rx_args} &
- local PID1=$! wait_local_port_listen ${PEER_NS} 8000 udp ./udpgso_bench_tx ${tx_args}
- ret=$?
- wait $(jobs -p)
- check_err $?
- wait ${PID1}
- check_err $?
- [ "$ret" -eq 0 ] && echo "ok" || echo "failed"
I think that with the above, in case of a failure, every test after the failing one will should fail, regardless of the actual results, am I correct?
No, only the failed test echo "failed". The passed tests still report "ok". The "check_err $?" in run_all function only record none 0 ret as return value.
Thanks Hangbin
On Wed, Aug 14, 2024 at 03:57:57PM +0800, Hangbin Liu wrote:
tools/testing/selftests/net/udpgro.sh | 41 ++++++++++++++++----------- 1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/net/udpgro.sh b/tools/testing/selftests/net/udpgro.sh index 11a1ebda564f..7e0164247b83 100755 --- a/tools/testing/selftests/net/udpgro.sh +++ b/tools/testing/selftests/net/udpgro.sh @@ -49,14 +49,15 @@ run_one() { cfg_veth
- ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${rx_args} && \
echo "ok" || \echo "failed" &
- ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${rx_args} &
- local PID1=$!
wait_local_port_listen ${PEER_NS} 8000 udp ./udpgso_bench_tx ${tx_args}
- ret=$?
- wait $(jobs -p)
- check_err $?
- wait ${PID1}
- check_err $?
- [ "$ret" -eq 0 ] && echo "ok" || echo "failed" return $ret
}
Self NACK. The ret need to define first in each function, or the check_err will failed... I forgot to the update the patch before post..
Thanks Hangbin
After commit d7db7775ea2e ("net: veth: do not manipulate GRO when using XDP"), there is no need to load XDP program to enable GRO. On the other hand, the current test is failed due to loading the XDP program. e.g.
# selftests: net: udpgro.sh # ipv4 # no GRO ok # no GRO chk cmsg ok # GRO ./udpgso_bench_rx: recv: bad packet len, got 1472, expected 14720 # # failed
[...]
# bad GRO lookup ok # multiple GRO socks ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520 # # ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520 # # failed ok 1 selftests: net: udpgro.sh
After fix, all the test passed.
# ./udpgro.sh ipv4 no GRO ok [...] multiple GRO socks ok
Fixes: d7db7775ea2e ("net: veth: do not manipulate GRO when using XDP") Reported-by: Yi Chen yiche@redhat.com Closes: https://issues.redhat.com/browse/RHEL-53858 Signed-off-by: Hangbin Liu liuhangbin@gmail.com --- tools/testing/selftests/net/udpgro.sh | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/tools/testing/selftests/net/udpgro.sh b/tools/testing/selftests/net/udpgro.sh index 7e0164247b83..180b337c8345 100755 --- a/tools/testing/selftests/net/udpgro.sh +++ b/tools/testing/selftests/net/udpgro.sh @@ -7,8 +7,6 @@ source net_helper.sh
readonly PEER_NS="ns-peer-$(mktemp -u XXXXXX)"
-BPF_FILE="xdp_dummy.bpf.o" - # set global exit status, but never reset nonzero one. check_err() { @@ -38,7 +36,7 @@ cfg_veth() { ip -netns "${PEER_NS}" addr add dev veth1 192.168.1.1/24 ip -netns "${PEER_NS}" addr add dev veth1 2001:db8::1/64 nodad ip -netns "${PEER_NS}" link set dev veth1 up - ip -n "${PEER_NS}" link set veth1 xdp object ${BPF_FILE} section xdp + ip netns exec "${PEER_NS}" ethtool -K veth1 gro on }
run_one() { @@ -203,11 +201,6 @@ run_all() { return $ret }
-if [ ! -f ${BPF_FILE} ]; then - echo "Missing ${BPF_FILE}. Run 'make' first" - exit -1 -fi - if [[ $# -eq 0 ]]; then run_all elif [[ $1 == "__subprocess" ]]; then
Hangbin Liu liuhangbin@gmail.com writes:
After commit d7db7775ea2e ("net: veth: do not manipulate GRO when using XDP"), there is no need to load XDP program to enable GRO. On the other hand, the current test is failed due to loading the XDP program. e.g.
# selftests: net: udpgro.sh # ipv4 # no GRO ok # no GRO chk cmsg ok # GRO ./udpgso_bench_rx: recv: bad packet len, got 1472, expected 14720 # # failed
[...]
# bad GRO lookup ok # multiple GRO socks ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520 # # ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520 # # failed ok 1 selftests: net: udpgro.sh
After fix, all the test passed.
# ./udpgro.sh ipv4 no GRO ok [...] multiple GRO socks ok
Fixes: d7db7775ea2e ("net: veth: do not manipulate GRO when using XDP") Reported-by: Yi Chen yiche@redhat.com Closes: https://issues.redhat.com/browse/RHEL-53858 Signed-off-by: Hangbin Liu liuhangbin@gmail.com
Reviewed-by: Toke Høiland-Jørgensen toke@redhat.com
On 8/14/24 09:57, Hangbin Liu wrote:
After commit d7db7775ea2e ("net: veth: do not manipulate GRO when using XDP"), there is no need to load XDP program to enable GRO. On the other hand, the current test is failed due to loading the XDP program. e.g.
# selftests: net: udpgro.sh # ipv4 # no GRO ok # no GRO chk cmsg ok # GRO ./udpgso_bench_rx: recv: bad packet len, got 1472, expected 14720 # # failed
[...]
# bad GRO lookup ok # multiple GRO socks ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520 # # ./udpgso_bench_rx: recv: bad packet len, got 1452, expected 14520 # # failed ok 1 selftests: net: udpgro.sh
After fix, all the test passed.
# ./udpgro.sh ipv4 no GRO ok [...] multiple GRO socks ok
Fixes: d7db7775ea2e ("net: veth: do not manipulate GRO when using XDP") Reported-by: Yi Chen yiche@redhat.com Closes: https://issues.redhat.com/browse/RHEL-53858 Signed-off-by: Hangbin Liu liuhangbin@gmail.com
LGTM,
Acked-by: Paolo Abeni pabeni@redhat.com
linux-kselftest-mirror@lists.linaro.org
-
Hangbin Liu -
Paolo Abeni -
Toke Høiland-Jørgensen