From: Yohei Kojima 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 --- 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 From: Yohei Kojima 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 --- .../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} $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 From: Yohei Kojima 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 --- .../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 From: Yohei Kojima 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 --- 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 From: Yohei Kojima 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 --- 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