Re: [Devel] [PATCH vz10 v2 3/6] selftests: net: make the veth GRO checks independent of host tunnels

[Konstantin Khorenko <khorenko@virtuozzo.com>]

  The diagnosis is right, the fix is not.

  udpgso_bench_rx -G does setsockopt(UDP_GRO) on the receiver, and that
  setsockopt calls udp_tunnel_encap_enable(), which is a
  static_branch_inc() on udp_encap_needed_key. So the -G receiver is
  itself the UDP encapsulation socket the message wants to be independent
  of. After this patch the lookup in udp4_gro_receive() happens always,
  on every machine, and finds this receiver with GRO_ENABLED set.
  udp_gro_receive() then aggregates through the

        (sk && udp_test_bit(GRO_ENABLED, sk))

  branch and NETIF_F_GRO_UDP_FWD is never consulted.

  That changes what the test covers. The three checks that expect 1 after
  "ethtool -K ... rx-udp-gro-forwarding on" (lines 252, 296, 359) no
  longer test rx-udp-gro-forwarding at all; they pass with the feature
  off or broken, because the aggregation is now a property of the socket.
  The checks that expect 10 still get 10 only because GRO is off on the
  device in those cases and udp_gro_receive() is never reached.

  So "where no encapsulation socket exists the lookup still does not
  happen and the device flag still decides, so what the checks cover
  there does not change" is not true, and the comment in veth.sh repeats
  it.

  Not taking this one. Please send a v3 of this patch alone that:

    - leaves the udpgso_bench_rx invocation as upstream has it, without -G;

    - before the aggregation checks, detects whether UDP encapsulation is
      enabled on the machine and SKIPs those checks if it is. The static
      key itself is not visible from userspace, but the things that turn
      it on are: vxlan, geneve, bareudp, fou, l2tp devices in any
      namespace ("ip -all netns link"), and UDP sockets with
      encapsulation enabled ("ss -uapn" for a vxlan/ovs port such as
      4789/6081). The test then stays meaningful where it can run and
      honest where it cannot;

    - drops the comment; the reason for the SKIP goes in the commit
      message, one line at the SKIP site at most;

    - says in the message that the aggregation checks need a host with
      no UDP encapsulation and are skipped otherwise.

  The rest of the series is fine, I am taking 1, 2, 4, 5, and replacing
  6 with the upstream cherry-pick as i've already said.

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 9/22/26 14:02, Eva Kurchatova wrote:
> Three aggregation checks report 10 packets where they want 1 on a host
> that has a vxlan, and pass on a host that does not. The kernel is doing
> what it is written to do.
> 
> udp4_gro_receive() looks up the receiving socket only when
> udp_encap_needed_key is on, and that key is global: any UDP
> encapsulation socket on the machine, in any namespace, turns it on.
> udp_gro_receive() then aggregates for a packet that has no socket, or
> for a socket that asked for UDP GRO, and consults NETIF_F_GRO_UDP_FWD
> only in the first case. So with a vxlan somewhere on the box the lookup
> finds the test's own receiver, and rx-udp-gro-forwarding never gets a
> say.
> 
> Ask for UDP GRO on the receiving socket, which makes the aggregation
> happen either way. Where no encapsulation socket exists the lookup
> still does not happen and the device flag still decides, so what the
> checks cover there does not change.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-139651
> Feature: fix selftests
> Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
> ---
>  tools/testing/selftests/net/veth.sh | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/veth.sh b/tools/testing/selftests/net/veth.sh
> index 6bb7dfaa30b6..b95167f0b4c2 100755
> --- a/tools/testing/selftests/net/veth.sh
> +++ b/tools/testing/selftests/net/veth.sh
> @@ -105,7 +105,15 @@ chk_gro() {
>  	NSTAT_HISTORY=$STATS ip netns exec $NS_DST nstat -n
>  
>  	printf "%-60s" "$msg"
> -	ip netns exec $BASE$DST ./udpgso_bench_rx -C 1000 -R 10 &
> +	# -G makes the receiving socket ask for UDP GRO.  Without it the
> +	# aggregation checks depend on no UDP encapsulation socket existing
> +	# anywhere on the machine: udp4_gro_receive() only looks up the
> +	# socket when udp_encap_needed_key is on, and that key is global.
> +	# With a vxlan on the host the lookup finds this receiver, and
> +	# NETIF_F_GRO_UDP_FWD is then not consulted at all, so nothing is
> +	# aggregated.  On a machine without such a socket the lookup does
> +	# not happen and the device flag still decides, as before.
> +	ip netns exec $BASE$DST ./udpgso_bench_rx -G -C 1000 -R 10 &
>  	local spid=$!
>  	sleep 0.1
>  

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel