ACK/Cmnt: [SRU][J][PATCH 1/1] selftests: udpgro: no need to load xdp for gro

[Andrei Gherzan]

On 26/09/04 12:09PM, Aaron Ma via kernel-team wrote:
> From: Hangbin Liu <liuhangbin at gmail.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/2129861
> 
> 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 at redhat.com>
> Closes: https://issues.redhat.com/browse/RHEL-53858
> Reviewed-by: Toke Høiland-Jørgensen <toke at redhat.com>
> Acked-by: Paolo Abeni <pabeni at redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin at gmail.com>
> Signed-off-by: David S. Miller <davem at davemloft.net>
> (backported from commit d7818402b1d80347c764001583f6d63fa68c2e1a)
> [aaron: adjusted context due to missing commit:
>  6b88ce902f0b ("selftests: net: name bpf objects consistently and simplify Makefile")]
> Signed-off-by: Aaron Ma <aaron.ma at canonical.com>

This looks good to me. Furthermore, it might be a decent candidate for upstreaming given that:

- Fixes a real, demonstrated bug (selftest failure), not a "trivial" fix —
  (test output before/after included)
- Obviously correct, small, self-contained (9-line diff, single logical change)
- Has a Fixes: tag pointing to the commit that introduced the regression
- Reviewed/Acked by upstream maintainers
- No Cc: stable at vger.kernel.org tag in the original commit message, but that
  shouldn't be a deal-breaker

> ---
>  tools/testing/selftests/net/udpgro.sh | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/udpgro.sh b/tools/testing/selftests/net/udpgro.sh
> index b38efda51b636..799dbc2b4b01c 100755
> --- a/tools/testing/selftests/net/udpgro.sh
> +++ b/tools/testing/selftests/net/udpgro.sh
> @@ -36,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/xdp_dummy.o section xdp
> +	ip netns exec "${PEER_NS}" ethtool -K veth1 gro on
>  }
>  
>  run_one() {
> @@ -204,11 +204,6 @@ run_all() {
>  	return $ret
>  }
>  
> -if [ ! -f ../bpf/xdp_dummy.o ]; then
> -	echo "Missing xdp_dummy helper. Build bpf selftest first"
> -	exit -1
> -fi
> -
>  if [[ $# -eq 0 ]]; then
>  	run_all
>  elif [[ $1 == "__subprocess" ]]; then

Acked-by: Andrei Gherzan <andrei.gherzan at canonical.com>

-- 
Andrei Gherzan
gpg: rsa4096/D4D94F67AD0E9640
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20260904/51f93c2b/attachment.sig>