[PATCH net v5 1/2] veth: keep carrier off when resizing channels with a down peer

[PATCH net v5 1/2] veth: keep carrier off when resizing channels with a down peer

[Xia Yan]

veth_set_channels() temporarily turns carrier off while reconfiguring a
running interface. It then turns carrier on for both ends without
checking whether the peer is administratively up. If only one end is up,
changing its channel counts therefore reports LOWER_UP even though the
peer remains down.

For example, after creating a pair with two RX and TX queues per end:

  ip link set vca up
  ethtool -L vca rx 1 tx 1

vca incorrectly reports LOWER_UP while vcb is still down.

Only restore carrier when the peer has IFF_UP, matching veth_open().
The local interface is already known to be running at this point.

Found during LLM-assisted source review and reproduced with a veth pair
in network namespaces, without kernel instrumentation or error injection.
The companion veth.sh test reproduces the two mixed up/down failures
before the fix and passes after it.

Fixes: 4752eeb3d891 ("veth: implement support for set_channel ethtool op")
Cc: stable@vger.kernel.org
Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>
Assisted-by: LLM
Signed-off-by: Xia Yan <yana06070@163.com>
---
Changes in v5:
- Driver code is unchanged.
- The companion test drops unnecessary quotes around literal up
  and carries Hangbin's review and test tags.

v4: https://lore.kernel.org/netdev/20261024093007.12981-1-yana06070@163.com/

Changes in v4:
- Driver code is unchanged. Add Hangbin's Reviewed-by.
- The companion test drops the extra GRO on/off runs, as discussed
  with Hangbin, and keeps up/up and both mixed up/down states.

v3: https://lore.kernel.org/netdev/20261022085842.2527-1-yana06070@163.com/

Runtime testing of v4 on 513444cc62e2, x86_64 QEMU/KVM with three vCPUs:
- Complete default veth.sh:
  before: 56 checks pass, two mixed up/down carrier checks fail;
  after: all 58 checks pass. The 55 existing checks pass on both.
- Script exit status: 1 before, 0 after. No checks were skipped.
- Both guests powered down normally.
- The runtime results above are from v4. The only v5 script change
  removes quotes around a literal argument.

 drivers/net/veth.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 643b97dc52456df80de10068ccac9af2ada70256..4e75830061ccd3def0476b19a2f92e818464fce9 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -1372,9 +1372,10 @@ out:
 		 * to identify the range we have to disable
 		 */
 		veth_disable_range_safe(dev, new_rx_count, old_rx_count);
-		netif_carrier_on(dev);
-		if (peer)
+		if (peer && (peer->flags & IFF_UP)) {
+			netif_carrier_on(dev);
 			netif_carrier_on(peer);
+		}
 	}
 
 	/* update XDP supported features */

base-commit: 513444cc62e231a11c9b15ea88de0890da9b2b4f

-- 
2.39.0.windows.2

[PATCH net v5 2/2] selftests: net: check veth carrier after changing channel counts

[Xia Yan]

Extend veth.sh to check that changing channel counts preserves carrier
state. When only one end is up, the running end must not report LOWER_UP.

Cover up/up and both mixed up/down combinations. Resize RX and TX queues
in both directions on each endpoint, and check carrier after a rejected
out-of-range request as well. Create the pair with two queues explicitly
so the carrier cases also run on single-CPU systems.

Reuse the existing namespace setup and cleanup. Run each carrier case
in a subshell to keep its variables separate from the other tests, and
accumulate failures in the caller. Only check LOWER_UP on running
interfaces, where the flag exposes carrier state.

Tested-by: Hangbin Liu <liuhangbin@kylinos.cn>
Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>
Assisted-by: LLM
Signed-off-by: Xia Yan <yana06070@163.com>
---
Changes in v5:
- Drop unnecessary quotes around literal up, as suggested by Hangbin.
- Clarify that the v4 quoting change concerned variable expansions.
- Add Hangbin's Tested-by and Reviewed-by tags.

v4: https://lore.kernel.org/netdev/20261024093007.12981-1-yana06070@163.com/

Changes in v4:
- Drop the extra GRO on/off runs, as discussed with Hangbin.
  Keep up/up and both mixed up/down states.
- Use subshells for the new helpers and accumulate failures in the
  caller. Quote variable expansions and wrap long lines.
- Driver code is unchanged.

v3: https://lore.kernel.org/netdev/20261022085842.2527-1-yana06070@163.com/

Runtime testing of v4 on 513444cc62e2, x86_64 QEMU/KVM with three vCPUs:
- Complete default veth.sh:
  before: 56 checks pass, two mixed up/down carrier checks fail;
  after: all 58 checks pass. The 55 existing checks pass on both.
- Script exit status: 1 before, 0 after. No checks were skipped.
- Both guests powered down normally.
- The runtime results above are from v4. The only v5 script change
  removes quotes around a literal argument.

The repeated-word warning on chk_channels_carrier is for the two
endpoint state arguments.

 tools/testing/selftests/net/veth.sh | 81 ++++++++++++++++++++++++++++-
 1 file changed, 80 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/veth.sh b/tools/testing/selftests/net/veth.sh
index 8b867e0675a7c1eea300468c26f2a9528ddc1194..5fb4b8c199b906785ab09cae35cf1e70a042bbce 100755
--- a/tools/testing/selftests/net/veth.sh
+++ b/tools/testing/selftests/net/veth.sh
@@ -41,13 +41,16 @@ create_ns() {
 		ip -n $ns link set dev lo up
 	done
 
-	ip link add name veth$SRC type veth peer name veth$DST
+	ip link add name "veth$SRC" "$@" type veth \
+		peer name "veth$DST" "$@" || return 1
 
 	for ns in $SRC $DST; do
 		ip link set dev veth$ns netns $BASE$ns up
 		ip -n $BASE$ns addr add dev veth$ns $BM_NET_V4$ns/24
 		ip -n $BASE$ns addr add dev veth$ns $BM_NET_V6$ns/64 nodad
 	done
+
+	return 0
 }
 
 __chk_flag() {
@@ -130,6 +133,78 @@ chk_channels() {
 	fi
 }
 
+chk_carrier() (
+	target=$1
+	state=$2
+	expected=$3
+	carrier=0
+
+	# IFF_LOWER_UP is reported only for running interfaces.
+	[ "$state" = up ] || return 0
+
+	if ! link=$(ip -n "$BASE$target" -o link show dev "veth$target"); then
+		echo " fail - cannot read veth$target link state"
+		return 1
+	fi
+	case "$link" in
+		*LOWER_UP*) carrier=1 ;;
+	esac
+	if [ "$carrier" -ne "$expected" ]; then
+		printf ' fail - veth%s carrier is %s, expected %s\n' \
+			"$target" "$carrier" "$expected"
+		return 1
+	fi
+)
+
+# Keep per-case variables separate from the other tests.
+chk_channels_carrier() (
+	src_state=$1
+	dst_state=$2
+	expected=0
+	err=0
+	msg="carrier after channel change ($src_state/$dst_state)"
+
+	if ! create_ns numtxqueues 2 numrxqueues 2; then
+		printf "%-60s" "$msg"
+		echo " skip - cannot create veth pair"
+		cleanup
+		return 0
+	fi
+
+	ip -n "$NS_SRC" link set dev "veth$SRC" "$src_state" || err=1
+	ip -n "$NS_DST" link set dev "veth$DST" "$dst_state" || err=1
+	[ "$src_state" = up ] && [ "$dst_state" = up ] && expected=1
+	chk_carrier "$SRC" "$src_state" "$expected" || err=1
+	chk_carrier "$DST" "$dst_state" "$expected" || err=1
+
+	for target in "$SRC" "$DST"; do
+		for count in 1 2; do
+			ip netns exec "$BASE$target" ethtool -L "veth$target" \
+				rx "$count" tx "$count" || err=1
+			chk_carrier "$SRC" "$src_state" "$expected" || err=1
+			chk_carrier "$DST" "$dst_state" "$expected" || err=1
+		done
+
+		if ip netns exec "$BASE$target" ethtool -L "veth$target" \
+			rx 3 tx 3 \
+			>/dev/null 2>&1; then
+			echo " fail - out-of-range channel counts were accepted"
+			err=1
+		fi
+		chk_carrier "$SRC" "$src_state" "$expected" || err=1
+		chk_carrier "$DST" "$dst_state" "$expected" || err=1
+	done
+
+	cleanup
+	printf "%-60s" "$msg"
+	if [ "$err" -eq 0 ]; then
+		echo " ok "
+	else
+		echo " fail "
+	fi
+	return "$err"
+)
+
 chk_gro() {
 	local msg="$1"
 	local expected=$2
@@ -257,6 +332,10 @@ fi
 [ $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"
 
+chk_channels_carrier up up || ret=1
+chk_channels_carrier up down || ret=1
+chk_channels_carrier down up || ret=1
+
 create_ns
 chk_gro_flag "default - gro flag" $SRC off
 chk_gro_flag "        - peer gro flag" $DST off

-- 
2.39.0.windows.2

Re: [PATCH net v5 1/2] veth: keep carrier off when resizing channels with a down peer

[Jiayuan Chen]

On 10/29/26 10:42 AM, Xia Yan wrote:
> veth_set_channels() temporarily turns carrier off while reconfiguring a
> running interface. It then turns carrier on for both ends without
> checking whether the peer is administratively up. If only one end is up,
> changing its channel counts therefore reports LOWER_UP even though the
> peer remains down.
>
> For example, after creating a pair with two RX and TX queues per end:
>
>    ip link set vca up
>    ethtool -L vca rx 1 tx 1
>
> vca incorrectly reports LOWER_UP while vcb is still down.
>
> Only restore carrier when the peer has IFF_UP, matching veth_open().
> The local interface is already known to be running at this point.
>
> Found during LLM-assisted source review and reproduced with a veth pair
> in network namespaces, without kernel instrumentation or error injection.
> The companion veth.sh test reproduces the two mixed up/down failures
> before the fix and passes after it.
>
> Fixes: 4752eeb3d891 ("veth: implement support for set_channel ethtool op")
> Cc: stable@vger.kernel.org
> Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>
> Assisted-by: LLM
> Signed-off-by: Xia Yan <yana06070@163.com>



Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>


Thanks, I clearly remember it has been reported somewhere...

> ---
> Changes in v5:
> - Driver code is unchanged.
> - The companion test drops unnecessary quotes around literal up
>    and carries Hangbin's review and test tags.
>
> v4: https://lore.kernel.org/netdev/20261024093007.12981-1-yana06070@163.com/
>
> Changes in v4:
> - Driver code is unchanged. Add Hangbin's Reviewed-by.
> - The companion test drops the extra GRO on/off runs, as discussed
>    with Hangbin, and keeps up/up and both mixed up/down states.
>
> v3: https://lore.kernel.org/netdev/20261022085842.2527-1-yana06070@163.com/
>
> Runtime testing of v4 on 513444cc62e2, x86_64 QEMU/KVM with three vCPUs:
> - Complete default veth.sh:
>    before: 56 checks pass, two mixed up/down carrier checks fail;
>    after: all 58 checks pass. The 55 existing checks pass on both.
> - Script exit status: 1 before, 0 after. No checks were skipped.
> - Both guests powered down normally.
> - The runtime results above are from v4. The only v5 script change
>    removes quotes around a literal argument.
>
>   drivers/net/veth.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 643b97dc52456df80de10068ccac9af2ada70256..4e75830061ccd3def0476b19a2f92e818464fce9 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -1372,9 +1372,10 @@ out:
>   		 * to identify the range we have to disable
>   		 */
>   		veth_disable_range_safe(dev, new_rx_count, old_rx_count);
> -		netif_carrier_on(dev);
> -		if (peer)
> +		if (peer && (peer->flags & IFF_UP)) {
> +			netif_carrier_on(dev);
>   			netif_carrier_on(peer);
> +		}
>   	}
>   
>   	/* update XDP supported features */
>
> base-commit: 513444cc62e231a11c9b15ea88de0890da9b2b4f
>