netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/5] net: netdevsim: fix inconsistent carrier state after link/unlink
@ 2025-12-29 18:32 yk
  2025-12-29 18:32 ` [PATCH net 1/5] " yk
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: yk @ 2025-12-29 18:32 UTC (permalink / raw)
  To: Jakub Kicinski, netdev
  Cc: Yohei Kojima, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Shuah Khan, Simon Horman, Breno Leitao, linux-kernel,
	linux-kselftest

From: Yohei Kojima <yk@y-koj.net>

This series fixes netdevsim's inconsistent behavior between carrier
and link/unlink state.

More specifically, this fixes a bug that the carrier goes DOWN although
two netdevsim were peered, depending on the order of peering and ifup.
Especially in a NetworkManager-enabled environment, netdevsim test fails
because of this.

The first patch fixes the bug itself in netdevsim/bus.c by adding
netif_carrier_on() into a proper function. The second and third patches
clean up netdevsim test and add a regression test for this bug.

The fourth and fifth patches improve TCP Fast Open (TFO) test, which
depends on netdevsim. In a NetworkManager-enabled environment, although
TFO test times out because of this bug, the test exits with 0 without
reporting any error.  This behavior implies that nothing would be
reported even if TFO got broken at some point.

The fourth and fifth patches are intentionally placed after the first
patch, because fixing TFO test without fixing netdevsim results in
a spurious test failure in a NetworkManager-enabled environment.

Yohei Kojima (5):
  net: netdevsim: fix inconsistent carrier state after link/unlink
  selftests: netdevsim: test that linking already-connected devices
    fails
  selftests: netdevsim: add carrier state consistency test
  selftests: net: improve error handling in TFO test
  selftests: net: report SKIP if TFO test processes timed out

 drivers/net/netdevsim/bus.c                   |  6 ++
 .../selftests/drivers/net/netdevsim/peer.sh   | 79 ++++++++++++++++++-
 tools/testing/selftests/net/tfo.c             | 10 ++-
 tools/testing/selftests/net/tfo_passive.sh    | 15 +++-
 4 files changed, 101 insertions(+), 9 deletions(-)

-- 
2.51.2


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

* [PATCH net 1/5] net: netdevsim: fix inconsistent carrier state after link/unlink
  2025-12-29 18:32 [PATCH net 0/5] net: netdevsim: fix inconsistent carrier state after link/unlink yk
@ 2025-12-29 18:32 ` yk
  2025-12-29 18:39   ` Andrew Lunn
  2025-12-29 18:32 ` [PATCH net 2/5] selftests: netdevsim: test that linking already-connected devices fails yk
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: yk @ 2025-12-29 18:32 UTC (permalink / raw)
  To: Jakub Kicinski, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Breno Leitao
  Cc: Yohei Kojima, Andrew Lunn, netdev, linux-kernel

From: Yohei Kojima <yk@y-koj.net>

This patch fixes the edge case behavior on ifup/ifdown and
linking/unlinking two netdevsim interfaces:

1. unlink two interfaces netdevsim1 and netdevsim2
2. ifdown netdevsim1
3. ifup netdevsim1
4. link two interfaces netdevsim1 and netdevsim2
5. (Now two interfaces are linked in terms of netdevsim peer, but
    carrier state of the two interfaces remains DOWN.)

This inconsistent behavior is caused by the current implementation,
which only cares about the "link, then ifup" order, not "ifup, then
link" order. This patch fixes the inconsistency by calling
netif_carrier_on() when two netdevsim interfaces are linked.

This patch solves buggy behavior on NetworkManager-based systems which
causes the netdevsim test to fail with the following error:

  # timeout set to 600
  # selftests: drivers/net/netdevsim: peer.sh
  # 2025/12/25 00:54:03 socat[9115] W address is opened in read-write mode but only supports read-only
  # 2025/12/25 00:56:17 socat[9115] W connect(7, AF=2 192.168.1.1:1234, 16): Connection timed out
  # 2025/12/25 00:56:17 socat[9115] E TCP:192.168.1.1:1234: Connection timed out
  # expected 3 bytes, got 0
  # 2025/12/25 00:56:17 socat[9109] W exiting on signal 15
  not ok 13 selftests: drivers/net/netdevsim: peer.sh # exit=1

This patch also fixes timeout on TCP Fast Open (TFO) test because the
test also depends on netdevsim.

Fixes: 1a8fed52f7be ("netdevsim: set the carrier when the device goes up")
Signed-off-by: Yohei Kojima <yk@y-koj.net>
---
 drivers/net/netdevsim/bus.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index 70e8c38ddad6..fa94c680c92a 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -332,6 +332,9 @@ static ssize_t link_device_store(const struct bus_type *bus, const char *buf, si
 	rcu_assign_pointer(nsim_a->peer, nsim_b);
 	rcu_assign_pointer(nsim_b->peer, nsim_a);
 
+	netif_carrier_on(dev_a);
+	netif_carrier_on(dev_b);
+
 out_err:
 	put_net(ns_b);
 	put_net(ns_a);
@@ -381,6 +384,9 @@ static ssize_t unlink_device_store(const struct bus_type *bus, const char *buf,
 	if (!peer)
 		goto out_put_netns;
 
+	netif_carrier_off(dev);
+	netif_carrier_off(peer->netdev);
+
 	err = 0;
 	RCU_INIT_POINTER(nsim->peer, NULL);
 	RCU_INIT_POINTER(peer->peer, NULL);
-- 
2.51.2


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

* [PATCH net 2/5] selftests: netdevsim: test that linking already-connected devices fails
  2025-12-29 18:32 [PATCH net 0/5] net: netdevsim: fix inconsistent carrier state after link/unlink yk
  2025-12-29 18:32 ` [PATCH net 1/5] " yk
@ 2025-12-29 18:32 ` yk
  2025-12-29 18:32 ` [PATCH net 3/5] selftests: netdevsim: add carrier state consistency test yk
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: yk @ 2025-12-29 18:32 UTC (permalink / raw)
  To: Jakub Kicinski, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Shuah Khan
  Cc: Yohei Kojima, netdev, linux-kselftest, linux-kernel

From: Yohei Kojima <yk@y-koj.net>

This patch adds a testcase to check if linking already-connected
netdevsim interfaces fails.

This patch also moves the testcase on invalid ifidx before linking
two netdevsims so that the test would fail if argument validation code
got broken: after linking two netdevsims, the test might not fail
because it attempts to link an already-connected netdevsim with
non-existing netdevsim.

Additionally, this patch adds comments for readability and details the
error message.

Signed-off-by: Yohei Kojima <yk@y-koj.net>
---
 .../selftests/drivers/net/netdevsim/peer.sh      | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/netdevsim/peer.sh b/tools/testing/selftests/drivers/net/netdevsim/peer.sh
index 7f32b5600925..338c844fe632 100755
--- a/tools/testing/selftests/drivers/net/netdevsim/peer.sh
+++ b/tools/testing/selftests/drivers/net/netdevsim/peer.sh
@@ -76,6 +76,7 @@ NSIM_DEV_2_FD=$((256 + RANDOM % 256))
 exec {NSIM_DEV_2_FD}</var/run/netns/nscl
 NSIM_DEV_2_IFIDX=$(ip netns exec nscl cat /sys/class/net/$NSIM_DEV_2_NAME/ifindex)
 
+# argument error checking
 echo "$NSIM_DEV_1_FD:$NSIM_DEV_1_IFIDX $NSIM_DEV_2_FD:2000" > $NSIM_DEV_SYS_LINK 2>/dev/null
 if [ $? -eq 0 ]; then
 	echo "linking with non-existent netdevsim should fail"
@@ -97,6 +98,14 @@ if [ $? -eq 0 ]; then
 	exit 1
 fi
 
+echo "$NSIM_DEV_1_FD:$NSIM_DEV_1_IFIDX $NSIM_DEV_2_FD:a" > $NSIM_DEV_SYS_LINK 2>/dev/null
+if [ $? -eq 0 ]; then
+	echo "linking with invalid ifidx should fail"
+	cleanup_ns
+	exit 1
+fi
+
+# link two netdevsim interfaces
 echo "$NSIM_DEV_1_FD:$NSIM_DEV_1_IFIDX $NSIM_DEV_2_FD:$NSIM_DEV_2_IFIDX" > $NSIM_DEV_SYS_LINK
 if [ $? -ne 0 ]; then
 	echo "linking netdevsim1 with netdevsim2 should succeed"
@@ -104,11 +113,10 @@ if [ $? -ne 0 ]; then
 	exit 1
 fi
 
-# argument error checking
-
-echo "$NSIM_DEV_1_FD:$NSIM_DEV_1_IFIDX $NSIM_DEV_2_FD:a" > $NSIM_DEV_SYS_LINK 2>/dev/null
+# semantic error checking
+echo "$NSIM_DEV_1_FD:$NSIM_DEV_1_IFIDX $NSIM_DEV_2_FD:$NSIM_DEV_2_IFIDX" > $NSIM_DEV_SYS_LINK 2>/dev/null
 if [ $? -eq 0 ]; then
-	echo "invalid arg should fail"
+	echo "linking already-connected netdevsim should fail"
 	cleanup_ns
 	exit 1
 fi
-- 
2.51.2


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

* [PATCH net 3/5] selftests: netdevsim: add carrier state consistency test
  2025-12-29 18:32 [PATCH net 0/5] net: netdevsim: fix inconsistent carrier state after link/unlink yk
  2025-12-29 18:32 ` [PATCH net 1/5] " yk
  2025-12-29 18:32 ` [PATCH net 2/5] selftests: netdevsim: test that linking already-connected devices fails yk
@ 2025-12-29 18:32 ` yk
  2025-12-29 18:32 ` [PATCH net 4/5] selftests: net: improve error handling in TFO test yk
  2025-12-29 18:32 ` [PATCH net 5/5] selftests: net: report SKIP if TFO test processes timed out yk
  4 siblings, 0 replies; 12+ messages in thread
From: yk @ 2025-12-29 18:32 UTC (permalink / raw)
  To: Jakub Kicinski, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Shuah Khan
  Cc: Yohei Kojima, netdev, linux-kselftest, linux-kernel

From: Yohei Kojima <yk@y-koj.net>

This commit adds a test case for netdevsim carrier state consistency.
Specifically, the added test verifies the carrier state during the
following operations:

1. Unlink two netdevsims
2. ifdown one netdevsim, then ifup again
3. Link the netdevsims again
4. ifdown one netdevsim, then ifup again

These steps verifies that the carrier is UP iff two netdevsims are
linked and ifuped.

Signed-off-by: Yohei Kojima <yk@y-koj.net>
---
 .../selftests/drivers/net/netdevsim/peer.sh   | 63 +++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/netdevsim/peer.sh b/tools/testing/selftests/drivers/net/netdevsim/peer.sh
index 338c844fe632..d15218e4bf5c 100755
--- a/tools/testing/selftests/drivers/net/netdevsim/peer.sh
+++ b/tools/testing/selftests/drivers/net/netdevsim/peer.sh
@@ -52,6 +52,43 @@ cleanup_ns()
 	ip netns del nssv
 }
 
+is_carrier_up()
+{
+	local netns="$1"
+	local nsim_dev="$2"
+
+	# 0: DOWN
+	# 1: UP
+	local is_up=$(ip netns exec "$netns"	\
+		cat /sys/class/net/"$nsim_dev"/carrier 2>/dev/null)
+
+	test "$is_up" -eq 1
+}
+
+assert_carrier_up()
+{
+	local netns="$1"
+	local nsim_dev="$2"
+
+	if ! is_carrier_up "$netns" "$nsim_dev"; then
+		echo "$nsim_dev's carrier should be UP, but it isn't"
+		cleanup_ns
+		exit 1
+	fi
+}
+
+assert_carrier_down()
+{
+	local netns="$1"
+	local nsim_dev="$2"
+
+	if is_carrier_up "$netns" "$nsim_dev"; then
+		echo "$nsim_dev's carrier should be DOWN, but it isn't"
+		cleanup_ns
+		exit 1
+	fi
+}
+
 ###
 ### Code start
 ###
@@ -121,6 +158,32 @@ if [ $? -eq 0 ]; then
 	exit 1
 fi
 
+# netdevsim carrier state consistency checking
+assert_carrier_up nssv "$NSIM_DEV_1_NAME"
+assert_carrier_up nscl "$NSIM_DEV_2_NAME"
+
+echo "$NSIM_DEV_1_FD:$NSIM_DEV_1_IFIDX" > $NSIM_DEV_SYS_UNLINK
+
+assert_carrier_down nssv "$NSIM_DEV_1_NAME"
+assert_carrier_down nscl "$NSIM_DEV_2_NAME"
+
+ip netns exec nssv ip link set dev "$NSIM_DEV_1_NAME" down
+ip netns exec nssv ip link set dev "$NSIM_DEV_1_NAME" up
+
+assert_carrier_down nssv "$NSIM_DEV_1_NAME"
+assert_carrier_down nscl "$NSIM_DEV_2_NAME"
+
+echo "$NSIM_DEV_1_FD:$NSIM_DEV_1_IFIDX $NSIM_DEV_2_FD:$NSIM_DEV_2_IFIDX" > $NSIM_DEV_SYS_LINK
+
+assert_carrier_up nssv "$NSIM_DEV_1_NAME"
+assert_carrier_up nscl "$NSIM_DEV_2_NAME"
+
+ip netns exec nssv ip link set dev "$NSIM_DEV_1_NAME" down
+ip netns exec nssv ip link set dev "$NSIM_DEV_1_NAME" up
+
+assert_carrier_up nssv "$NSIM_DEV_1_NAME"
+assert_carrier_up nscl "$NSIM_DEV_2_NAME"
+
 # send/recv packets
 
 tmp_file=$(mktemp)
-- 
2.51.2


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

* [PATCH net 4/5] selftests: net: improve error handling in TFO test
  2025-12-29 18:32 [PATCH net 0/5] net: netdevsim: fix inconsistent carrier state after link/unlink yk
                   ` (2 preceding siblings ...)
  2025-12-29 18:32 ` [PATCH net 3/5] selftests: netdevsim: add carrier state consistency test yk
@ 2025-12-29 18:32 ` yk
  2025-12-29 18:32 ` [PATCH net 5/5] selftests: net: report SKIP if TFO test processes timed out yk
  4 siblings, 0 replies; 12+ messages in thread
From: yk @ 2025-12-29 18:32 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Shuah Khan
  Cc: Yohei Kojima, netdev, linux-kselftest, linux-kernel

From: Yohei Kojima <yk@y-koj.net>

This commit improves the error handling in TCP Fast Open (TFO) test by
(1) adding the sendto() return value check and (2) changing read() error
handling code to exit with 1.

Signed-off-by: Yohei Kojima <yk@y-koj.net>
---
 tools/testing/selftests/net/tfo.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/tfo.c b/tools/testing/selftests/net/tfo.c
index 8d82140f0f76..4572eb9b8968 100644
--- a/tools/testing/selftests/net/tfo.c
+++ b/tools/testing/selftests/net/tfo.c
@@ -82,7 +82,8 @@ static void run_server(void)
 		error(1, errno, "getsockopt(SO_INCOMING_NAPI_ID)");
 
 	if (read(connfd, buf, 64) < 0)
-		perror("read()");
+		error(1, errno, "read()");
+
 	fprintf(outfile, "%d\n", opt);
 
 	fclose(outfile);
@@ -92,14 +93,17 @@ static void run_server(void)
 
 static void run_client(void)
 {
-	int fd;
+	int fd, ret;
 	char *msg = "Hello, world!";
 
 	fd = socket(AF_INET6, SOCK_STREAM, 0);
 	if (fd == -1)
 		error(1, errno, "socket()");
 
-	sendto(fd, msg, strlen(msg), MSG_FASTOPEN, (struct sockaddr *)&cfg_addr, sizeof(cfg_addr));
+	ret = sendto(fd, msg, strlen(msg), MSG_FASTOPEN,
+		     (struct sockaddr *)&cfg_addr, sizeof(cfg_addr));
+	if (ret < 0)
+		error(1, errno, "sendto()");
 
 	close(fd);
 }
-- 
2.51.2


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

* [PATCH net 5/5] selftests: net: report SKIP if TFO test processes timed out
  2025-12-29 18:32 [PATCH net 0/5] net: netdevsim: fix inconsistent carrier state after link/unlink yk
                   ` (3 preceding siblings ...)
  2025-12-29 18:32 ` [PATCH net 4/5] selftests: net: improve error handling in TFO test yk
@ 2025-12-29 18:32 ` yk
  4 siblings, 0 replies; 12+ messages in thread
From: yk @ 2025-12-29 18:32 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Shuah Khan
  Cc: Yohei Kojima, netdev, linux-kselftest, linux-kernel

From: Yohei Kojima <yk@y-koj.net>

This patch improves the TCP Fast Open (TFO) test to report the timeout
events and client/server error events by introducing better process
management.

Previously, TFO test didn't provide any information about the test
client/server processes' exit status, and just reported "ok". This
behavior is sometimes misleading in case TFO is unsupported by the
kernel, or there was a bug in the backing network devices (netdevsim).

Signed-off-by: Yohei Kojima <yk@y-koj.net>
---
 tools/testing/selftests/net/tfo_passive.sh | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/tfo_passive.sh b/tools/testing/selftests/net/tfo_passive.sh
index a4550511830a..1e89f1006c42 100755
--- a/tools/testing/selftests/net/tfo_passive.sh
+++ b/tools/testing/selftests/net/tfo_passive.sh
@@ -76,7 +76,7 @@ echo "$NSIM_SV_FD:$NSIM_SV_IFIDX $NSIM_CL_FD:$NSIM_CL_IFIDX" > \
 if [ $? -ne 0 ]; then
 	echo "linking netdevsim1 with netdevsim2 should succeed"
 	cleanup_ns
-	exit 1
+	exit "$ksft_fail"
 fi
 
 out_file=$(mktemp)
@@ -85,12 +85,15 @@ timeout -k 1s 30s ip netns exec nssv ./tfo        \
 				-s                \
 				-p ${SERVER_PORT} \
 				-o ${out_file}&
+server_pid="$!"
 
 wait_local_port_listen nssv ${SERVER_PORT} tcp
 
 ip netns exec nscl ./tfo -c -h ${SERVER_IP} -p ${SERVER_PORT}
+client_exit_status="$?"
 
-wait
+wait "$server_pid"
+server_exit_status="$?"
 
 res=$(cat $out_file)
 rm $out_file
@@ -101,6 +104,14 @@ if [ "$res" = "0" ]; then
 	exit 1
 fi
 
+if [ "$client_exit_status" -ne 0 ] || [ "$server_exit_status" -ne 0 ]; then
+	# Note: timeout(1) exits with 124 if it timed out
+	echo "client exited with ${client_exit_status}"
+	echo "server exited with ${server_exit_status}"
+	cleanup_ns
+	exit "$ksft_skip"
+fi
+
 echo "$NSIM_SV_FD:$NSIM_SV_IFIDX" > $NSIM_DEV_SYS_UNLINK
 
 echo $NSIM_CL_ID > $NSIM_DEV_SYS_DEL
-- 
2.51.2


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

* Re: [PATCH net 1/5] net: netdevsim: fix inconsistent carrier state after link/unlink
  2025-12-29 18:32 ` [PATCH net 1/5] " yk
@ 2025-12-29 18:39   ` Andrew Lunn
  2025-12-29 19:56     ` Yohei Kojima
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Lunn @ 2025-12-29 18:39 UTC (permalink / raw)
  To: yk
  Cc: Jakub Kicinski, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Breno Leitao, netdev, linux-kernel

On Tue, Dec 30, 2025 at 03:32:34AM +0900, yk@y-koj.net wrote:
> From: Yohei Kojima <yk@y-koj.net>
> 
> This patch fixes the edge case behavior on ifup/ifdown and
> linking/unlinking two netdevsim interfaces:
> 
> 1. unlink two interfaces netdevsim1 and netdevsim2
> 2. ifdown netdevsim1
> 3. ifup netdevsim1
> 4. link two interfaces netdevsim1 and netdevsim2
> 5. (Now two interfaces are linked in terms of netdevsim peer, but
>     carrier state of the two interfaces remains DOWN.)
> 
> This inconsistent behavior is caused by the current implementation,
> which only cares about the "link, then ifup" order, not "ifup, then
> link" order. This patch fixes the inconsistency by calling
> netif_carrier_on() when two netdevsim interfaces are linked.
> 
> This patch solves buggy behavior on NetworkManager-based systems which
> causes the netdevsim test to fail with the following error:
> 
>   # timeout set to 600
>   # selftests: drivers/net/netdevsim: peer.sh
>   # 2025/12/25 00:54:03 socat[9115] W address is opened in read-write mode but only supports read-only
>   # 2025/12/25 00:56:17 socat[9115] W connect(7, AF=2 192.168.1.1:1234, 16): Connection timed out
>   # 2025/12/25 00:56:17 socat[9115] E TCP:192.168.1.1:1234: Connection timed out
>   # expected 3 bytes, got 0
>   # 2025/12/25 00:56:17 socat[9109] W exiting on signal 15
>   not ok 13 selftests: drivers/net/netdevsim: peer.sh # exit=1
> 
> This patch also fixes timeout on TCP Fast Open (TFO) test because the
> test also depends on netdevsim.
> 
> Fixes: 1a8fed52f7be ("netdevsim: set the carrier when the device goes up")

Stable rules say:

   It must either fix a real bug that bothers people or just add a device ID.

netdevsim is not a real device. Do its bugs actually bother people?
Should this patch have a Fixes: tag?

       Andrew

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

* Re: [PATCH net 1/5] net: netdevsim: fix inconsistent carrier state after link/unlink
  2025-12-29 18:39   ` Andrew Lunn
@ 2025-12-29 19:56     ` Yohei Kojima
  2025-12-30 11:02       ` Andrew Lunn
  0 siblings, 1 reply; 12+ messages in thread
From: Yohei Kojima @ 2025-12-29 19:56 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jakub Kicinski, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Breno Leitao, netdev, linux-kernel

On Mon, Dec 29, 2025 at 07:39:29PM +0100, Andrew Lunn wrote:
> On Tue, Dec 30, 2025 at 03:32:34AM +0900, yk@y-koj.net wrote:
> > From: Yohei Kojima <yk@y-koj.net>
> > 
> > This patch fixes the edge case behavior on ifup/ifdown and
> > linking/unlinking two netdevsim interfaces:
> > 
> > 1. unlink two interfaces netdevsim1 and netdevsim2
> > 2. ifdown netdevsim1
> > 3. ifup netdevsim1
> > 4. link two interfaces netdevsim1 and netdevsim2
> > 5. (Now two interfaces are linked in terms of netdevsim peer, but
> >     carrier state of the two interfaces remains DOWN.)
> > 
> > This inconsistent behavior is caused by the current implementation,
> > which only cares about the "link, then ifup" order, not "ifup, then
> > link" order. This patch fixes the inconsistency by calling
> > netif_carrier_on() when two netdevsim interfaces are linked.
> > 
> > This patch solves buggy behavior on NetworkManager-based systems which
> > causes the netdevsim test to fail with the following error:
> > 
> >   # timeout set to 600
> >   # selftests: drivers/net/netdevsim: peer.sh
> >   # 2025/12/25 00:54:03 socat[9115] W address is opened in read-write mode but only supports read-only
> >   # 2025/12/25 00:56:17 socat[9115] W connect(7, AF=2 192.168.1.1:1234, 16): Connection timed out
> >   # 2025/12/25 00:56:17 socat[9115] E TCP:192.168.1.1:1234: Connection timed out
> >   # expected 3 bytes, got 0
> >   # 2025/12/25 00:56:17 socat[9109] W exiting on signal 15
> >   not ok 13 selftests: drivers/net/netdevsim: peer.sh # exit=1
> > 
> > This patch also fixes timeout on TCP Fast Open (TFO) test because the
> > test also depends on netdevsim.
> > 
> > Fixes: 1a8fed52f7be ("netdevsim: set the carrier when the device goes up")
> 
> Stable rules say:
> 
>    It must either fix a real bug that bothers people or just add a device ID.

Thank you for the quick reply. I don't intend for this patch to be
backported to the stable tree. My understanding was that bugfix patches
to the net tree should have Fixes: tag for historical tracking.

> 
> netdevsim is not a real device. Do its bugs actually bother people?

This patch fixes a real bug that is seen when a developer tries to test
TFO or netdevsim tests on NetworkManager-enabled systems: it causes
false positives in kselftests on such systems.

> Should this patch have a Fixes: tag?

The patch 1a8fed52f7be ("netdevsim: set the carrier when the device goes
up"), which does a similar change, has Fixes: tag. Since this patch fixes
the corner-case behavior which was missed in the previous fix, this
patch should have Fixes: tag for consistency.

> 
>        Andrew

Thank you,
Yohei Kojima

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

* Re: [PATCH net 1/5] net: netdevsim: fix inconsistent carrier state after link/unlink
  2025-12-29 19:56     ` Yohei Kojima
@ 2025-12-30 11:02       ` Andrew Lunn
  2025-12-30 16:20         ` Yohei Kojima
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Lunn @ 2025-12-30 11:02 UTC (permalink / raw)
  To: Yohei Kojima
  Cc: Jakub Kicinski, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Breno Leitao, netdev, linux-kernel

> Thank you for the quick reply. I don't intend for this patch to be
> backported to the stable tree. My understanding was that bugfix patches
> to the net tree should have Fixes: tag for historical tracking.
> 
> > 
> > netdevsim is not a real device. Do its bugs actually bother people?
> 
> This patch fixes a real bug that is seen when a developer tries to test
> TFO or netdevsim tests on NetworkManager-enabled systems: it causes
> false positives in kselftests on such systems.

O.K, then keep the Fixes tag and submit it for net. However, the tests
should be considered development work, and submitted to net-next, if
they are not fixes. Please split this into two series.

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

    Andrew

---
pw-bot: cr

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

* Re: [PATCH net 1/5] net: netdevsim: fix inconsistent carrier state after link/unlink
  2025-12-30 11:02       ` Andrew Lunn
@ 2025-12-30 16:20         ` Yohei Kojima
  2025-12-30 18:38           ` Andrew Lunn
  0 siblings, 1 reply; 12+ messages in thread
From: Yohei Kojima @ 2025-12-30 16:20 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jakub Kicinski, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Breno Leitao, netdev, linux-kernel

On Tue, Dec 30, 2025 at 12:02:22PM +0100, Andrew Lunn wrote:
> > Thank you for the quick reply. I don't intend for this patch to be
> > backported to the stable tree. My understanding was that bugfix patches
> > to the net tree should have Fixes: tag for historical tracking.
> > 
> > > 
> > > netdevsim is not a real device. Do its bugs actually bother people?
> > 
> > This patch fixes a real bug that is seen when a developer tries to test
> > TFO or netdevsim tests on NetworkManager-enabled systems: it causes
> > false positives in kselftests on such systems.
> 
> O.K, then keep the Fixes tag and submit it for net. However, the tests
> should be considered development work, and submitted to net-next, if
> they are not fixes. Please split this into two series.

Sure, I've submitted the v2 patch here.

https://lore.kernel.org/netdev/cover.1767108538.git.yk@y-koj.net/

Following your suggestion, I've removed the unrelated TFO tests and
the netdevsim test improvement. I will post the removed patches as a
separate series once net-next reopens.

However, I kept the regression test for this patch in the v2 series, as
the "1.5.10. Co-posting selftests" section in the maintainer-netdev
document says:

  Selftests should be part of the same series as the code changes.
  Specifically for fixes both code change and related test should
  go into the same tree (the tests may lack a Fixes tag, which is
  expected). Mixing code changes and test changes in a single commit
  is discouraged.

> 
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> 
>     Andrew

Thank you,
Yohei Kojima

> 
> ---
> pw-bot: cr

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

* Re: [PATCH net 1/5] net: netdevsim: fix inconsistent carrier state after link/unlink
  2025-12-30 16:20         ` Yohei Kojima
@ 2025-12-30 18:38           ` Andrew Lunn
  2025-12-30 18:44             ` Yohei Kojima
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Lunn @ 2025-12-30 18:38 UTC (permalink / raw)
  To: Yohei Kojima
  Cc: Jakub Kicinski, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Breno Leitao, netdev, linux-kernel

> Sure, I've submitted the v2 patch here.
> 
> https://lore.kernel.org/netdev/cover.1767108538.git.yk@y-koj.net/
> 
> Following your suggestion, I've removed the unrelated TFO tests and
> the netdevsim test improvement. I will post the removed patches as a
> separate series once net-next reopens.
> 
> However, I kept the regression test for this patch in the v2 series, as
> the "1.5.10. Co-posting selftests" section in the maintainer-netdev
> document says:

Thanks for doing this. I don't know enough about netdevsim to be able
to do a proper review, so i will let somebody else do that.

   Andrew

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

* Re: [PATCH net 1/5] net: netdevsim: fix inconsistent carrier state after link/unlink
  2025-12-30 18:38           ` Andrew Lunn
@ 2025-12-30 18:44             ` Yohei Kojima
  0 siblings, 0 replies; 12+ messages in thread
From: Yohei Kojima @ 2025-12-30 18:44 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jakub Kicinski, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Breno Leitao, netdev, linux-kernel

On Tue, Dec 30, 2025 at 07:38:01PM +0100, Andrew Lunn wrote:
> > Sure, I've submitted the v2 patch here.
> > 
> > https://lore.kernel.org/netdev/cover.1767108538.git.yk@y-koj.net/
> > 
> > Following your suggestion, I've removed the unrelated TFO tests and
> > the netdevsim test improvement. I will post the removed patches as a
> > separate series once net-next reopens.
> > 
> > However, I kept the regression test for this patch in the v2 series, as
> > the "1.5.10. Co-posting selftests" section in the maintainer-netdev
> > document says:
> 
> Thanks for doing this. I don't know enough about netdevsim to be able
> to do a proper review, so i will let somebody else do that.

Okay, thank you for the early review!

> 
>    Andrew

Best regards,
Yohei Kojima

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

end of thread, other threads:[~2025-12-30 18:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-29 18:32 [PATCH net 0/5] net: netdevsim: fix inconsistent carrier state after link/unlink yk
2025-12-29 18:32 ` [PATCH net 1/5] " yk
2025-12-29 18:39   ` Andrew Lunn
2025-12-29 19:56     ` Yohei Kojima
2025-12-30 11:02       ` Andrew Lunn
2025-12-30 16:20         ` Yohei Kojima
2025-12-30 18:38           ` Andrew Lunn
2025-12-30 18:44             ` Yohei Kojima
2025-12-29 18:32 ` [PATCH net 2/5] selftests: netdevsim: test that linking already-connected devices fails yk
2025-12-29 18:32 ` [PATCH net 3/5] selftests: netdevsim: add carrier state consistency test yk
2025-12-29 18:32 ` [PATCH net 4/5] selftests: net: improve error handling in TFO test yk
2025-12-29 18:32 ` [PATCH net 5/5] selftests: net: report SKIP if TFO test processes timed out yk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).