From b4cafb3d2c740f8d1b1234b43ac4a60e5291c960 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 14 Dec 2022 18:01:00 -0800 Subject: [PATCH 1/3] devlink: hold region lock when flushing snapshots Netdevsim triggers a splat on reload, when it destroys regions with snapshots pending: WARNING: CPU: 1 PID: 787 at net/core/devlink.c:6291 devlink_region_snapshot_del+0x12e/0x140 CPU: 1 PID: 787 Comm: devlink Not tainted 6.1.0-07460-g7ae9888d6e1c #580 RIP: 0010:devlink_region_snapshot_del+0x12e/0x140 Call Trace: devl_region_destroy+0x70/0x140 nsim_dev_reload_down+0x2f/0x60 [netdevsim] devlink_reload+0x1f7/0x360 devlink_nl_cmd_reload+0x6ce/0x860 genl_family_rcv_msg_doit.isra.0+0x145/0x1c0 This is the locking assert in devlink_region_snapshot_del(), we're supposed to be holding the region->snapshot_lock here. Fixes: 2dec18ad826f ("net: devlink: remove region snapshots list dependency on devlink->lock") Signed-off-by: Jakub Kicinski Reviewed-by: Jiri Pirko Signed-off-by: David S. Miller --- net/core/devlink.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/core/devlink.c b/net/core/devlink.c index 6004bd0ccee4..d2df30829083 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -11925,8 +11925,10 @@ void devl_region_destroy(struct devlink_region *region) devl_assert_locked(devlink); /* Free all snapshots of region */ + mutex_lock(®ion->snapshot_lock); list_for_each_entry_safe(snapshot, ts, ®ion->snapshot_list, list) devlink_region_snapshot_del(region, snapshot); + mutex_unlock(®ion->snapshot_lock); list_del(®ion->list); mutex_destroy(®ion->snapshot_lock); From 2fc60e2ff972d3dca836bff0b08cbe503c4ca1ce Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 14 Dec 2022 18:01:01 -0800 Subject: [PATCH 2/3] selftests: devlink: fix the fd redirect in dummy_reporter_test $number + > bash means redirect FD $number, e.g. commonly used 2> redirects stderr (fd 2). The test uses 8192> to write the number 8192 to a file, this results in: ./devlink.sh: line 499: 8192: Bad file descriptor Oddly the test also papers over this issue by checking for failure (expecting an error rather than success) so it passes, anyway. Fixes: ff18176ad806 ("selftests: Add a test of large binary to devlink health test") Signed-off-by: Jakub Kicinski Signed-off-by: David S. Miller --- tools/testing/selftests/drivers/net/netdevsim/devlink.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/drivers/net/netdevsim/devlink.sh b/tools/testing/selftests/drivers/net/netdevsim/devlink.sh index 9de1d123f4f5..a08c02abde12 100755 --- a/tools/testing/selftests/drivers/net/netdevsim/devlink.sh +++ b/tools/testing/selftests/drivers/net/netdevsim/devlink.sh @@ -496,8 +496,8 @@ dummy_reporter_test() check_reporter_info dummy healthy 3 3 10 true - echo 8192> $DEBUGFS_DIR/health/binary_len - check_fail $? "Failed set dummy reporter binary len to 8192" + echo 8192 > $DEBUGFS_DIR/health/binary_len + check_err $? "Failed set dummy reporter binary len to 8192" local dump=$(devlink health dump show $DL_HANDLE reporter dummy -j) check_err $? "Failed show dump of dummy reporter" From d1c4a3469e73730f7cbbcec661c2a9081af1aa45 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 14 Dec 2022 18:01:02 -0800 Subject: [PATCH 3/3] selftests: devlink: add a warning for interfaces coming up NetworkManager (and other daemons) may bring the interface up and cause failures in quiescence checks. Print a helpful warning, and take the interface down again. I seem to forget about this every time I run these tests on a new VM. Signed-off-by: Jakub Kicinski Signed-off-by: David S. Miller --- .../selftests/drivers/net/netdevsim/devlink_trap.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh b/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh index 109900c817be..b64d98ca0df7 100755 --- a/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh +++ b/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh @@ -47,6 +47,17 @@ if [ -d "${NETDEVSIM_PATH}/devices/netdevsim${DEV_ADDR}" ]; then exit 1 fi +check_netdev_down() +{ + state=$(cat /sys/class/net/${NETDEV}/flags) + + if [ $((state & 1)) -ne 0 ]; then + echo "WARNING: unexpected interface UP, disable NetworkManager?" + + ip link set dev $NETDEV down + fi +} + init_test() { RET=0 @@ -151,6 +162,7 @@ trap_stats_test() RET=0 + check_netdev_down for trap_name in $(devlink_traps_get); do devlink_trap_stats_idle_test $trap_name check_err $? "Stats of trap $trap_name not idle when netdev down" @@ -254,6 +266,7 @@ trap_group_stats_test() RET=0 + check_netdev_down for group_name in $(devlink_trap_groups_get); do devlink_trap_group_stats_idle_test $group_name check_err $? "Stats of trap group $group_name not idle when netdev down"