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

[PATCH net v2 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 a network namespace. Built and booted UML kernels before and after
the fix; the accompanying test fails in the four mixed up/down cases
before the fix and passes all six cases after it.

Fixes: 4752eeb3d891 ("veth: implement support for set_channel ethtool op")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Xia Yan <yana06070@163.com>
---
 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 643b97d..4e75830 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -1372,9 +1372,10 @@ static int veth_set_channels(struct net_device *dev,
 		 * 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.43.0

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

[Xia Yan]

Add a regression test for veth_set_channels() incorrectly reporting
LOWER_UP on a running interface whose peer is administratively down.

Test the up/up and both mixed up/down combinations with GRO enabled and
disabled. Resize the RX and TX channel counts in both directions on
each end and check LOWER_UP on each running interface. Also verify that
rejecting an out-of-range channel count leaves that state unchanged.

IFF_LOWER_UP is masked on administratively down interfaces, so do not
use it to assert their raw carrier state. Omit the down/down combination,
where neither endpoint can report LOWER_UP.

On an unpatched UML kernel the four mixed up/down cases fail. All six
cases pass with the carrier restoration fix.

Assisted-by: LLM
Signed-off-by: Xia Yan <yana06070@163.com>
---

Notes:
    Changes in v2:
    - Limit carrier assertions to running endpoints and omit down/down, since
      IFF_LOWER_UP does not expose a down endpoint's raw carrier state.
    - Use log_test_skip() on veth creation failure to avoid a stale retmsg.
    - Include the discovery and reproduction details in the driver commit.
    - Driver code is unchanged.
    
    v1: https://lore.kernel.org/netdev/20261015085821.12725-1-yana06070@163.com/
    Review: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/8081
    
    Validation for the revised selftest on the original net baseline:
    - bash -n passed.
    - Unpatched UML kernel: 2 pass, 4 fail (the mixed up/down cases).
    - Patched UML kernel: 6 pass.
    - Injected veth creation failure after an earlier failed case: SKIP no
      longer repeats the earlier error; the aggregate failure is preserved.
    
    Reused the existing UML kernels; no kernel rebuild was performed for
    this revision. Prior build results and their limits remain archived.

 tools/testing/selftests/net/Makefile          |  1 +
 .../selftests/net/veth_channels_carrier.sh    | 93 +++++++++++++++++++
 2 files changed, 94 insertions(+)
 create mode 100755 tools/testing/selftests/net/veth_channels_carrier.sh

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 590b33e..4004014 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -128,6 +128,7 @@ TEST_PROGS := \
 	udpgso_bench.sh \
 	unicast_extensions.sh \
 	veth.sh \
+	veth_channels_carrier.sh \
 	vlan_bridge_binding.sh \
 	vlan_hw_filter.sh \
 	vrf-xfrm-tests.sh \
diff --git a/tools/testing/selftests/net/veth_channels_carrier.sh b/tools/testing/selftests/net/veth_channels_carrier.sh
new file mode 100755
index 0000000..03c2995
--- /dev/null
+++ b/tools/testing/selftests/net/veth_channels_carrier.sh
@@ -0,0 +1,93 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+source "$(dirname "${BASH_SOURCE[0]}")/lib.sh"
+
+require_command ip
+require_command ethtool
+
+if [ "$EUID" -ne 0 ]; then
+	echo "SKIP: root privileges are required"
+	exit "$ksft_skip"
+fi
+
+trap cleanup_all_ns EXIT
+setup_ns ns || exit $?
+
+check_carrier()
+{
+	local dev=$1
+	local state=$2
+	local expected=$3
+	local carrier=0
+	local link
+
+	# IFF_LOWER_UP is reported only for running interfaces.
+	[ "$state" = up ] || return 0
+
+	link=$(ip -n "$ns" -o link show dev "$dev")
+	check_err $? "Failed to read $dev link state"
+	[[ $link == *LOWER_UP* ]] && carrier=1
+	[ "$carrier" -eq "$expected" ]
+	check_err $? "$dev carrier is $carrier, expected $expected"
+}
+
+check_state()
+{
+	local left=$1
+	local right=$2
+	local gro=$3
+	local expected=0
+	local count dev
+
+	RET=0
+	if ! ip -n "$ns" link add vca numtxqueues 2 numrxqueues 2 \
+		type veth peer name vcb numtxqueues 2 numrxqueues 2; then
+		log_test_skip "veth carrier: $left/$right, GRO $gro"
+		return
+	fi
+
+	ip -n "$ns" link set vca "$left"
+	check_err $? "Failed to configure vca"
+	ip -n "$ns" link set vcb "$right"
+	check_err $? "Failed to configure vcb"
+	ip netns exec "$ns" ethtool -K vca gro "$gro" >/dev/null
+	check_err $? "Failed to configure GRO"
+	ip netns exec "$ns" ethtool -K vcb gro "$gro" >/dev/null
+	check_err $? "Failed to configure peer GRO"
+
+	[ "$left" = up ] && [ "$right" = up ] && expected=1
+	check_carrier vca "$left" "$expected"
+	check_carrier vcb "$right" "$expected"
+
+	for dev in vca vcb; do
+		for count in 1 2; do
+			ip netns exec "$ns" ethtool -L "$dev" \
+				rx "$count" tx "$count"
+			check_err $? "Failed to resize $dev channels"
+			check_carrier vca "$left" "$expected"
+			check_carrier vcb "$right" "$expected"
+		done
+
+		ip netns exec "$ns" ethtool -L "$dev" rx 3 tx 3 \
+			>/dev/null 2>&1
+		check_fail $? "Out-of-range channel counts were accepted"
+		check_carrier vca "$left" "$expected"
+		check_carrier vcb "$right" "$expected"
+	done
+
+	ip -n "$ns" link del vca
+	check_err $? "Failed to delete the veth pair"
+	log_test "veth carrier: $left/$right, GRO $gro"
+}
+
+for gro in off on; do
+	for left in down up; do
+		for right in down up; do
+			[ "$left" = down ] && [ "$right" = down ] && continue
+			check_state "$left" "$right" "$gro"
+		done
+	done
+done
+
+exit "$EXIT_STATUS"
-- 
2.43.0

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

[Xia Yan]

The Sashiko board (automated patch review) suggested putting the
carrier cases in veth.sh [1]. I've done that for v3, reusing its
namespace setup and cleanup.

The six up/up and mixed up/down cases, with GRO on and off, are kept.
The driver change is unchanged.

[1] https://sashiko.dev/#/patchset/20261020135544.24441-1-yana06070%40163.com

Best regards,
Xia Yan