Netdev List
 help / color / mirror / Atom feed
* [PATCH net 0/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled while down
@ 2026-10-06 17:32 Tianyi Gao
  2026-10-06 17:32 ` [PATCH net 1/2] " Tianyi Gao
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Tianyi Gao @ 2026-10-06 17:32 UTC (permalink / raw)
  To: netdev
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, toke, lorenzo,
	horms, bpf, shuah, hawk, kernel-team, ast, daniel, john.fastabend,
	sdf, linux-kselftest, Tianyi Gao

Toggling GRO on a veth device while it is down leaves its peer's
NETDEV_XDP_ACT_NDO_XMIT flag stale once the device comes up. XDP
redirects into the peer are then rejected with -EOPNOTSUPP, or, if GRO
was turned off, accepted and then dropped with -ENXIO. Patch 1 refreshes
the peer's flags in veth_open().

Patch 2 extends veth.sh to check the peer's flag after GRO is toggled
while the device is down, in both directions.

Tianyi Gao (2):
  veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled while down
  selftests: net: veth: test peer ndo-xmit after GRO toggle while down

 drivers/net/veth.c                  |  1 +
 tools/testing/selftests/net/veth.sh | 46 +++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)


base-commit: d5a007b9b457c915ab1a53227e8939e4018aa97a
-- 
2.55.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net 1/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled while down
  2026-10-06 17:32 [PATCH net 0/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled while down Tianyi Gao
@ 2026-10-06 17:32 ` Tianyi Gao
  2026-10-07 10:20   ` Toke Høiland-Jørgensen
                     ` (2 more replies)
  2026-10-06 17:32 ` [PATCH net 2/2] selftests: net: veth: test peer ndo-xmit after GRO toggle " Tianyi Gao
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 11+ messages in thread
From: Tianyi Gao @ 2026-10-06 17:32 UTC (permalink / raw)
  To: netdev
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, toke, lorenzo,
	horms, bpf, shuah, hawk, kernel-team, ast, daniel, john.fastabend,
	sdf, linux-kselftest, Tianyi Gao

A veth device advertises NETDEV_XDP_ACT_NDO_XMIT only if its peer has an
XDP program attached or GRO enabled, that is, only if the peer will have
NAPI to receive the frames.

veth_set_features() updates the peer's flag when GRO is toggled, but
returns early if the device is down, and veth_open() only refreshes the
flags of the device being opened. Toggling GRO while the device is down
therefore leaves the peer's flag stale after the device comes up.

If GRO was enabled while down, the device comes up with NAPI but the
peer does not advertise NDO_XMIT, and devmap rejects redirects to the
peer with -EOPNOTSUPP. If GRO was disabled while down, the device comes
up without NAPI but the peer still advertises NDO_XMIT, so redirects are
accepted and then dropped in veth_xdp_xmit() with -ENXIO.

Commit 7a6102aa6df0 ("veth: Update XDP feature set when bringing up
device") made veth_open() refresh the device's own flags. Refresh the
peer's flags there too. The peer's flag depends on this device's XDP
program and GRO setting, not on whether the peer is up, so it is
correct to set it even if the peer is down.

Fixes: 8267fc71abb2 ("veth: take into account peer device for NETDEV_XDP_ACT_NDO_XMIT xdp_features flag")
Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
 drivers/net/veth.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 71227d0389aa..643b97dc5245 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -1415,6 +1415,7 @@ static int veth_open(struct net_device *dev)
 	}
 
 	veth_set_xdp_features(dev);
+	veth_set_xdp_features(peer);
 
 	return 0;
 }
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net 2/2] selftests: net: veth: test peer ndo-xmit after GRO toggle while down
  2026-10-06 17:32 [PATCH net 0/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled while down Tianyi Gao
  2026-10-06 17:32 ` [PATCH net 1/2] " Tianyi Gao
@ 2026-10-06 17:32 ` Tianyi Gao
  2026-10-10 17:49   ` netdev-bot+sashiko
  2026-10-06 17:39 ` [PATCH net 0/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled " netdev-bot+sinfo
  2026-10-08 17:50 ` patchwork-bot+netdevbpf
  3 siblings, 1 reply; 11+ messages in thread
From: Tianyi Gao @ 2026-10-06 17:32 UTC (permalink / raw)
  To: netdev
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, toke, lorenzo,
	horms, bpf, shuah, hawk, kernel-team, ast, daniel, john.fastabend,
	sdf, linux-kselftest, Tianyi Gao

Test that toggling GRO on a veth device while it is down updates the
peer's ndo-xmit XDP feature once the device comes up, for both GRO on
and GRO off.

xdp-features is only visible through netlink, so read it with the ynl
CLI, as double_udp_encap.sh does. Skip the checks if the CLI is not
found, as in an installed kselftest tree.

Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
---
This commit only covers the peer's ndo-xmit flag. Paolo asked for
selftests on the device's own XDP features when commit 7a6102aa6df0
("veth: Update XDP feature set when bringing up device") was applied
[1], which refreshes them in veth_open(); those are out of scope here.

[1] https://lore.kernel.org/all/155aabf8b873bb8cdcafbd6139c42b08513e5fe6.camel@redhat.com/
---
 tools/testing/selftests/net/veth.sh | 46 +++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/tools/testing/selftests/net/veth.sh b/tools/testing/selftests/net/veth.sh
index 9709dd067c72..8b867e0675a7 100755
--- a/tools/testing/selftests/net/veth.sh
+++ b/tools/testing/selftests/net/veth.sh
@@ -9,6 +9,8 @@ readonly DST=1
 readonly DST_NAT=100
 readonly NS_SRC=$BASE$SRC
 readonly NS_DST=$BASE$DST
+# shellcheck disable=SC2155 # prefer RO variable over return value from cmd
+readonly YNL="$(dirname "$(readlink -f "$0")")/../../../net/ynl/pyynl/cli.py"
 
 # "baremetal" network used for raw UDP traffic
 readonly BM_NET_V4=192.168.1.
@@ -74,6 +76,37 @@ chk_tso_flag() {
 	__chk_flag "$1" $2 $3 tcp-segmentation-offload
 }
 
+chk_xdp_feature() {
+	local msg="$1"
+	local target=$2
+	local expected=$3
+	local feature=$4
+	local ifindex
+	local out
+	local st
+	local flag
+
+	ifindex=`ip netns exec $BASE$target cat /sys/class/net/veth$target/ifindex`
+	out=`ip netns exec $BASE$target "$YNL" --family netdev --do dev-get \
+		--output-json --json "{\"ifindex\": $ifindex}" 2>&1`
+	st=$?
+
+	printf "%-60s" "$msg"
+	if [ $st -ne 0 ]; then
+		echo " fail - ynl cli error: $out"
+		ret=1
+		return
+	fi
+
+	flag=`echo "$out" | grep -c "\"$feature\""`
+	if [ "$flag" = "$expected" ]; then
+		echo " ok "
+	else
+		echo " fail - expected $expected found $flag"
+		ret=1
+	fi
+}
+
 chk_channels() {
 	local msg="$1"
 	local target=$2
@@ -222,6 +255,7 @@ fi
 
 [ $CPUS -lt 2 ] && echo "Only one CPU available, some tests will be skipped"
 [ $STRESS -gt 0 -a $CPUS -lt 3 ] && echo " stress test will be skipped, too"
+[ ! -f "$YNL" ] && echo "ynl cli not found, ndo-xmit tests will be skipped"
 
 create_ns
 chk_gro_flag "default - gro flag" $SRC off
@@ -283,6 +317,18 @@ chk_gro_flag "        - peer gro flag" $SRC off
 chk_tso_flag "        - tso flag" $SRC on
 chk_tso_flag "        - peer tso flag" $DST on
 ip -n $NS_DST link set dev veth$DST up
+if [ -f "$YNL" ]; then
+	chk_xdp_feature "        - peer ndo-xmit" $SRC 1 ndo-xmit
+	# make sure the peer flag is set before disabling gro while down
+	ip netns exec $NS_DST ethtool -K veth$DST gro off
+	ip netns exec $NS_DST ethtool -K veth$DST gro on
+	chk_xdp_feature "        - peer ndo-xmit re-armed" $SRC 1 ndo-xmit
+	ip -n $NS_DST link set dev veth$DST down
+	ip netns exec $NS_DST ethtool -K veth$DST gro off
+	ip -n $NS_DST link set dev veth$DST up
+	chk_xdp_feature "        - peer ndo-xmit cleared" $SRC 0 ndo-xmit
+	ip netns exec $NS_DST ethtool -K veth$DST gro on
+fi
 ip netns exec $NS_SRC ethtool -K veth$SRC tx-udp-segmentation off
 ip netns exec $NS_DST ethtool -K veth$DST rx-udp-gro-forwarding on
 chk_gro "        - aggregation with TSO off" 1
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH net 0/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled while down
  2026-10-06 17:32 [PATCH net 0/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled while down Tianyi Gao
  2026-10-06 17:32 ` [PATCH net 1/2] " Tianyi Gao
  2026-10-06 17:32 ` [PATCH net 2/2] selftests: net: veth: test peer ndo-xmit after GRO toggle " Tianyi Gao
@ 2026-10-06 17:39 ` netdev-bot+sinfo
  2026-10-06 17:48   ` Tianyi Gao
  2026-10-08 17:50 ` patchwork-bot+netdevbpf
  3 siblings, 1 reply; 11+ messages in thread
From: netdev-bot+sinfo @ 2026-10-06 17:39 UTC (permalink / raw)
  To: Tianyi Gao
  Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni, toke,
	lorenzo, horms, bpf, shuah, hawk, kernel-team, ast, daniel,
	john.fastabend, sdf, linux-kselftest

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net 0/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled while down
  2026-10-06 17:39 ` [PATCH net 0/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled " netdev-bot+sinfo
@ 2026-10-06 17:48   ` Tianyi Gao
  0 siblings, 0 replies; 11+ messages in thread
From: Tianyi Gao @ 2026-10-06 17:48 UTC (permalink / raw)
  To: netdev-bot+sinfo
  Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni, toke,
	lorenzo, horms, bpf, shuah, hawk, kernel-team, ast, daniel,
	john.fastabend, sdf, linux-kselftest

Hi,

I was testing XDP redirect from a physical NIC into a
veth whose peer is in another network namespace, and found that the
redirect worked or not depending on the order of enabling GRO on the
peer and bringing the peer up.

Cheers,
Tianyi

On Tue, Oct 6, 2026 at 6:39 PM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net 1/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled while down
  2026-10-06 17:32 ` [PATCH net 1/2] " Tianyi Gao
@ 2026-10-07 10:20   ` Toke Høiland-Jørgensen
  2026-10-07 10:24   ` Jesper Dangaard Brouer
  2026-10-07 12:45   ` Lorenzo Bianconi
  2 siblings, 0 replies; 11+ messages in thread
From: Toke Høiland-Jørgensen @ 2026-10-07 10:20 UTC (permalink / raw)
  To: Tianyi Gao, netdev
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, lorenzo, horms, bpf,
	shuah, hawk, kernel-team, ast, daniel, john.fastabend, sdf,
	linux-kselftest, Tianyi Gao

Tianyi Gao <tianyi@cloudflare.com> writes:

> A veth device advertises NETDEV_XDP_ACT_NDO_XMIT only if its peer has an
> XDP program attached or GRO enabled, that is, only if the peer will have
> NAPI to receive the frames.
>
> veth_set_features() updates the peer's flag when GRO is toggled, but
> returns early if the device is down, and veth_open() only refreshes the
> flags of the device being opened. Toggling GRO while the device is down
> therefore leaves the peer's flag stale after the device comes up.
>
> If GRO was enabled while down, the device comes up with NAPI but the
> peer does not advertise NDO_XMIT, and devmap rejects redirects to the
> peer with -EOPNOTSUPP. If GRO was disabled while down, the device comes
> up without NAPI but the peer still advertises NDO_XMIT, so redirects are
> accepted and then dropped in veth_xdp_xmit() with -ENXIO.
>
> Commit 7a6102aa6df0 ("veth: Update XDP feature set when bringing up
> device") made veth_open() refresh the device's own flags. Refresh the
> peer's flags there too. The peer's flag depends on this device's XDP
> program and GRO setting, not on whether the peer is up, so it is
> correct to set it even if the peer is down.
>
> Fixes: 8267fc71abb2 ("veth: take into account peer device for NETDEV_XDP_ACT_NDO_XMIT xdp_features flag")
> Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>

Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net 1/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled while down
  2026-10-06 17:32 ` [PATCH net 1/2] " Tianyi Gao
  2026-10-07 10:20   ` Toke Høiland-Jørgensen
@ 2026-10-07 10:24   ` Jesper Dangaard Brouer
  2026-10-07 12:45   ` Lorenzo Bianconi
  2 siblings, 0 replies; 11+ messages in thread
From: Jesper Dangaard Brouer @ 2026-10-07 10:24 UTC (permalink / raw)
  To: Tianyi Gao, netdev
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, toke, lorenzo,
	horms, bpf, shuah, kernel-team, ast, daniel, john.fastabend, sdf,
	linux-kselftest



On 10/6/26 19:32, Tianyi Gao wrote:
> A veth device advertises NETDEV_XDP_ACT_NDO_XMIT only if its peer has an
> XDP program attached or GRO enabled, that is, only if the peer will have
> NAPI to receive the frames.
> 
> veth_set_features() updates the peer's flag when GRO is toggled, but
> returns early if the device is down, and veth_open() only refreshes the
> flags of the device being opened. Toggling GRO while the device is down
> therefore leaves the peer's flag stale after the device comes up.
> 
> If GRO was enabled while down, the device comes up with NAPI but the
> peer does not advertise NDO_XMIT, and devmap rejects redirects to the
> peer with -EOPNOTSUPP. If GRO was disabled while down, the device comes
> up without NAPI but the peer still advertises NDO_XMIT, so redirects are
> accepted and then dropped in veth_xdp_xmit() with -ENXIO.
> 
> Commit 7a6102aa6df0 ("veth: Update XDP feature set when bringing up
> device") made veth_open() refresh the device's own flags. Refresh the
> peer's flags there too. The peer's flag depends on this device's XDP
> program and GRO setting, not on whether the peer is up, so it is
> correct to set it even if the peer is down.
> 
> Fixes: 8267fc71abb2 ("veth: take into account peer device for NETDEV_XDP_ACT_NDO_XMIT xdp_features flag")
> Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>
> ---

Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>

>   drivers/net/veth.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 71227d0389aa..643b97dc5245 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -1415,6 +1415,7 @@ static int veth_open(struct net_device *dev)
>   	}
>   
>   	veth_set_xdp_features(dev);
> +	veth_set_xdp_features(peer);
>   
>   	return 0;
>   }


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net 1/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled while down
  2026-10-06 17:32 ` [PATCH net 1/2] " Tianyi Gao
  2026-10-07 10:20   ` Toke Høiland-Jørgensen
  2026-10-07 10:24   ` Jesper Dangaard Brouer
@ 2026-10-07 12:45   ` Lorenzo Bianconi
  2 siblings, 0 replies; 11+ messages in thread
From: Lorenzo Bianconi @ 2026-10-07 12:45 UTC (permalink / raw)
  To: Tianyi Gao
  Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni, toke,
	lorenzo, horms, bpf, shuah, hawk, kernel-team, ast, daniel,
	john.fastabend, sdf, linux-kselftest

[-- Attachment #1: Type: text/plain, Size: 1834 bytes --]

> A veth device advertises NETDEV_XDP_ACT_NDO_XMIT only if its peer has an
> XDP program attached or GRO enabled, that is, only if the peer will have
> NAPI to receive the frames.
> 
> veth_set_features() updates the peer's flag when GRO is toggled, but
> returns early if the device is down, and veth_open() only refreshes the
> flags of the device being opened. Toggling GRO while the device is down
> therefore leaves the peer's flag stale after the device comes up.
> 
> If GRO was enabled while down, the device comes up with NAPI but the
> peer does not advertise NDO_XMIT, and devmap rejects redirects to the
> peer with -EOPNOTSUPP. If GRO was disabled while down, the device comes
> up without NAPI but the peer still advertises NDO_XMIT, so redirects are
> accepted and then dropped in veth_xdp_xmit() with -ENXIO.
> 
> Commit 7a6102aa6df0 ("veth: Update XDP feature set when bringing up
> device") made veth_open() refresh the device's own flags. Refresh the
> peer's flags there too. The peer's flag depends on this device's XDP
> program and GRO setting, not on whether the peer is up, so it is
> correct to set it even if the peer is down.
> 
> Fixes: 8267fc71abb2 ("veth: take into account peer device for NETDEV_XDP_ACT_NDO_XMIT xdp_features flag")
> Signed-off-by: Tianyi Gao <tianyi@cloudflare.com>

Acked-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

> ---
>  drivers/net/veth.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 71227d0389aa..643b97dc5245 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -1415,6 +1415,7 @@ static int veth_open(struct net_device *dev)
>  	}
>  
>  	veth_set_xdp_features(dev);
> +	veth_set_xdp_features(peer);
>  
>  	return 0;
>  }
> -- 
> 2.55.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net 0/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled while down
  2026-10-06 17:32 [PATCH net 0/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled while down Tianyi Gao
                   ` (2 preceding siblings ...)
  2026-10-06 17:39 ` [PATCH net 0/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled " netdev-bot+sinfo
@ 2026-10-08 17:50 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-10-08 17:50 UTC (permalink / raw)
  To: Tianyi Gao
  Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni, toke,
	lorenzo, horms, bpf, shuah, hawk, kernel-team, ast, daniel,
	john.fastabend, sdf, linux-kselftest

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue,  6 Oct 2026 18:32:35 +0100 you wrote:
> Toggling GRO on a veth device while it is down leaves its peer's
> NETDEV_XDP_ACT_NDO_XMIT flag stale once the device comes up. XDP
> redirects into the peer are then rejected with -EOPNOTSUPP, or, if GRO
> was turned off, accepted and then dropped with -ENXIO. Patch 1 refreshes
> the peer's flags in veth_open().
> 
> Patch 2 extends veth.sh to check the peer's flag after GRO is toggled
> while the device is down, in both directions.
> 
> [...]

Here is the summary with links:
  - [net,1/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled while down
    https://git.kernel.org/netdev/net/c/3f090039995f
  - [net,2/2] selftests: net: veth: test peer ndo-xmit after GRO toggle while down
    https://git.kernel.org/netdev/net/c/c017800af5c7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net 2/2] selftests: net: veth: test peer ndo-xmit after GRO toggle while down
  2026-10-06 17:32 ` [PATCH net 2/2] selftests: net: veth: test peer ndo-xmit after GRO toggle " Tianyi Gao
@ 2026-10-10 17:49   ` netdev-bot+sashiko
  2026-10-10 18:10     ` Tianyi Gao
  0 siblings, 1 reply; 11+ messages in thread
From: netdev-bot+sashiko @ 2026-10-10 17:49 UTC (permalink / raw)
  To: tianyi
  Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni, toke,
	lorenzo, horms, bpf, shuah, hawk, kernel-team, ast, daniel,
	john.fastabend, sdf, linux-kselftest

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new ndo-xmit checks in tools/testing/selftests/net/veth.sh
  are only skipped when cli.py is missing.
- [Medium] The new `if [ -f "$YNL" ]` block (veth.sh lines 320-331) is
  inserted between the 'with gro enabled on link down' setup and the…

--

selftests: net: veth: test peer ndo-xmit after GRO toggle while down

This adds chk_xdp_feature() to veth.sh. It reads xdp-features through the
ynl CLI and checks that toggling GRO on a veth device while it is down
updates the peer's ndo-xmit XDP feature once the device comes up. The new
checks are skipped when cli.py is not found.

> diff --git a/tools/testing/selftests/net/veth.sh b/tools/testing/selftests/net/veth.sh
> index 9709dd067c728..8b867e0675a7c 100755
> --- a/tools/testing/selftests/net/veth.sh
> +++ b/tools/testing/selftests/net/veth.sh

[ ... ]

> @@ -74,6 +76,37 @@ chk_tso_flag() {
>  	__chk_flag "$1" $2 $3 tcp-segmentation-offload
>  }
>  
> +chk_xdp_feature() {

[ ... ]

> +	ifindex=`ip netns exec $BASE$target cat /sys/class/net/veth$target/ifindex`
> +	out=`ip netns exec $BASE$target "$YNL" --family netdev --do dev-get \
> +		--output-json --json "{\"ifindex\": $ifindex}" 2>&1`
> +	st=$?
> +
> +	printf "%-60s" "$msg"
> +	if [ $st -ne 0 ]; then
> +		echo " fail - ynl cli error: $out"
> +		ret=1
> +		return
> +	fi

[Severity: Medium]
If cli.py is present but can't run, should that be a skip instead of a
failure?

The only checks on the environment just test whether the file exists:

    [ ! -f "$YNL" ] && echo "ynl cli not found, ndo-xmit tests will be skipped"
    ...
    if [ -f "$YNL" ]; then

The CLI's library always imports PyYAML, in
tools/net/ynl/pyynl/lib/nlspec.py:

    import yaml as pyyaml

The CLI also has to find the netdev spec through specdir.find_spec(). That
raises FileNotFoundError if the spec directory is missing.

Suppose veth.sh runs from a kernel source tree on a host that has no
python3-yaml, no python3, or no spec directory. Each chk_xdp_feature() call
then exits non-zero, prints "fail - ynl cli error" and sets ret=1. Before
this patch, veth.sh needed only iproute2 and ethtool and passed on such a
host.

The commit message says the checks are skipped "if the CLI is not found".
The code does that. Could the script also do a trial run of the CLI before
it enables the ndo-xmit checks, and skip them if that run fails?

[ ... ]

> @@ -283,6 +317,18 @@ chk_gro_flag "        - peer gro flag" $SRC off
>  chk_tso_flag "        - tso flag" $SRC on
>  chk_tso_flag "        - peer tso flag" $DST on
>  ip -n $NS_DST link set dev veth$DST up
> +if [ -f "$YNL" ]; then
> +	chk_xdp_feature "        - peer ndo-xmit" $SRC 1 ndo-xmit
> +	# make sure the peer flag is set before disabling gro while down
> +	ip netns exec $NS_DST ethtool -K veth$DST gro off
> +	ip netns exec $NS_DST ethtool -K veth$DST gro on
> +	chk_xdp_feature "        - peer ndo-xmit re-armed" $SRC 1 ndo-xmit
> +	ip -n $NS_DST link set dev veth$DST down
> +	ip netns exec $NS_DST ethtool -K veth$DST gro off
> +	ip -n $NS_DST link set dev veth$DST up
> +	chk_xdp_feature "        - peer ndo-xmit cleared" $SRC 0 ndo-xmit
> +	ip netns exec $NS_DST ethtool -K veth$DST gro on
> +fi
>  ip netns exec $NS_SRC ethtool -K veth$SRC tx-udp-segmentation off
>  ip netns exec $NS_DST ethtool -K veth$DST rx-udp-gro-forwarding on
>  chk_gro "        - aggregation with TSO off" 1

[Severity: Medium]
Does this block change which code path the chk_gro "aggregation with TSO
off" check below it tests?

Before this patch, the "with gro enabled on link down" section ran link
down, gro on, link up, and then chk_gro. veth_set_features() returns early
when the device is down:

drivers/net/veth.c:veth_set_features() {
    ...
	if (!(changed & NETIF_F_GRO) || !(dev->flags & IFF_UP) || priv->_xdp_prog)
		return 0;
    ...
}

So GRO NAPI was enabled only by veth_open():

drivers/net/veth.c:veth_open() {
    ...
	} else if (veth_gro_requested(dev)) {
		err = veth_napi_enable(dev);
    ...
}

When cli.py exists, the new block ends with link down, gro off, link up.
veth_open() enables no NAPI there. The block then runs ethtool -K veth$DST
gro on while the device is up, so NAPI is enabled in veth_set_features()
instead:

drivers/net/veth.c:veth_set_features() {
    ...
	if (features & NETIF_F_GRO) {
		err = veth_napi_enable(dev);
    ...
}

The final chk_gro now tests the "GRO enabled while up" path, which the
earlier "with gro on" section already covers. The GRO NAPI path in
veth_open() is no longer covered by any aggregation check. Which path is
tested also depends on whether cli.py exists.

The ndo-xmit checks don't fill this gap. veth_set_xdp_features() looks at
veth_gro_requested(), not at whether NAPI actually started.

Could the block end by restoring the down-state setup (link down, gro on,
link up) instead of turning GRO on while the device is up?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006173241.65945-1-tianyi%40cloudflare.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net 2/2] selftests: net: veth: test peer ndo-xmit after GRO toggle while down
  2026-10-10 17:49   ` netdev-bot+sashiko
@ 2026-10-10 18:10     ` Tianyi Gao
  0 siblings, 0 replies; 11+ messages in thread
From: Tianyi Gao @ 2026-10-10 18:10 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: andrew+netdev, ast, bpf, daniel, davem, edumazet, hawk, horms,
	john.fastabend, kernel-team, kuba, linux-kselftest, lorenzo,
	netdev, pabeni, sdf, shuah, tianyi, toke

Hi,

On Sat, 10 Oct 2026 17:49:52 +0000, netdev-bot+sashiko@kernel.org wrote:
> [Severity: Medium]
> If cli.py is present but can't run, should that be a skip instead of a
> failure?

I don't think so. Running net selftests from a source tree already
needs the ynl CLI and its dependencies: double_udp_encap.sh calls it
without any check, and the Python tests import the same library. A CLI
that is present but fails points to a broken setup, so failing is
correct. The file check only covers the installed layout, where cli.py
is not shipped.

> [Severity: Medium]
> Does this block change which code path the chk_gro "aggregation with TSO
> off" check below it tests?

Yes. A kprobe on __veth_napi_enable_range confirms that the NAPI is now
enabled from veth_set_features() instead of veth_open(). The series is
already applied, so I will fix it in a follow-up for net-next, together
with the selftest requested in [1]. It will end the block with GRO
enabled while the device is down.

[1] https://lore.kernel.org/all/155aabf8b873bb8cdcafbd6139c42b08513e5fe6.camel@redhat.com/

Cheers,
Tianyi

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-10-10 18:10 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-06 17:32 [PATCH net 0/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled while down Tianyi Gao
2026-10-06 17:32 ` [PATCH net 1/2] " Tianyi Gao
2026-10-07 10:20   ` Toke Høiland-Jørgensen
2026-10-07 10:24   ` Jesper Dangaard Brouer
2026-10-07 12:45   ` Lorenzo Bianconi
2026-10-06 17:32 ` [PATCH net 2/2] selftests: net: veth: test peer ndo-xmit after GRO toggle " Tianyi Gao
2026-10-10 17:49   ` netdev-bot+sashiko
2026-10-10 18:10     ` Tianyi Gao
2026-10-06 17:39 ` [PATCH net 0/2] veth: fix peer NETDEV_XDP_ACT_NDO_XMIT after GRO is toggled " netdev-bot+sinfo
2026-10-06 17:48   ` Tianyi Gao
2026-10-08 17:50 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox