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

[PATCH net v4 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 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/

Testing 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.
- bash -n and git diff --check pass.

 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 v4 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.

Assisted-by: LLM
Signed-off-by: Xia Yan <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 single-value arguments and wrap long lines.
- Driver code is unchanged.

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

Testing 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.
- bash -n and git diff --check pass.

 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..e438e9ee7b33b151099d8ce779a9c9b47b0cc0af 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 v4 2/2] selftests: net: check veth carrier after changing channel counts

[Hangbin Liu]

Hi Yan Xia,
On Sat, Oct 24, 2026 at 05:30:07PM +0800, Xia Yan wrote:
> 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.
> 
> Assisted-by: LLM
> Signed-off-by: Xia Yan <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 single-value arguments and wrap long lines.

Which single-value arguments do you mean?

> - Driver code is unchanged.
> 
> v3: https://lore.kernel.org/netdev/20261022085842.2527-1-yana06070@163.com/
> 
> Testing 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.
> - bash -n and git diff --check pass.
> 
>  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..e438e9ee7b33b151099d8ce779a9c9b47b0cc0af 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

Nit: there is an extra "" quote.

> +chk_channels_carrier up down || ret=1
> +chk_channels_carrier down up || ret=1

Note: When set the src state as down first. The ethtool -L will has no
effect as it link is down. So the errors would look like

# ./veth.sh
carrier after channel change (up/up)                         ok
 fail - veth2 carrier is 1, expected 0
 fail - veth2 carrier is 1, expected 0
 fail - veth2 carrier is 1, expected 0
 fail - veth2 carrier is 1, expected 0
 fail - veth2 carrier is 1, expected 0
 fail - veth2 carrier is 1, expected 0
carrier after channel change (up/down)                       fail
 fail - veth1 carrier is 1, expected 0
 fail - veth1 carrier is 1, expected 0
 fail - veth1 carrier is 1, expected 0
carrier after channel change (down/up)                       fail


This may leading the result a bit weird as the failure count is not sync.
But the test passed on patched kernel. So no need to change from my side.

> +
>  create_ns
>  chk_gro_flag "default - gro flag" $SRC off
>  chk_gro_flag "        - peer gro flag" $DST off
> 

Tested-by: Hangbin Liu <liuhangbin@kylinos.cn>
Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>

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

[Xia Yan]

Hi Hangbin,

Thanks for testing and reviewing this.

> Which single-value arguments do you mean?

I meant variable expansions used as command arguments, such as
"$BASE$target" and "veth$target". "Quote variable expansions" would have
been clearer wording.

> Nit: there is an extra "" quote.

You're right, the literal up does not need quotes. I'll drop those
quotes and clarify the changelog if another revision is needed.

Thanks,
Xia