* [PATCH net 0/3] devlink: region snapshot locking fix and selftest adjustments
@ 2022-12-15 2:00 Jakub Kicinski
2022-12-15 2:01 ` [PATCH net 1/3] devlink: hold region lock when flushing snapshots Jakub Kicinski
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Jakub Kicinski @ 2022-12-15 2:00 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, jiri, jacob.e.keller, Jakub Kicinski
Minor fix for region snapshot locking and adjustments to selftests.
Jakub Kicinski (3):
devlink: hold region lock when flushing snapshots
selftests: devlink: fix the fd redirect in dummy_reporter_test
selftests: devlink: add a warning for interfaces coming up
net/core/devlink.c | 2 ++
.../selftests/drivers/net/netdevsim/devlink.sh | 4 ++--
.../selftests/drivers/net/netdevsim/devlink_trap.sh | 13 +++++++++++++
3 files changed, 17 insertions(+), 2 deletions(-)
--
2.38.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net 1/3] devlink: hold region lock when flushing snapshots
2022-12-15 2:00 [PATCH net 0/3] devlink: region snapshot locking fix and selftest adjustments Jakub Kicinski
@ 2022-12-15 2:01 ` Jakub Kicinski
2022-12-15 8:38 ` Jiri Pirko
2022-12-15 2:01 ` [PATCH net 2/3] selftests: devlink: fix the fd redirect in dummy_reporter_test Jakub Kicinski
` (3 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2022-12-15 2:01 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, jiri, jacob.e.keller, Jakub Kicinski, jiri
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:
<TASK>
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 <kuba@kernel.org>
---
CC: jiri@nvidia.com
---
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);
--
2.38.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net 2/3] selftests: devlink: fix the fd redirect in dummy_reporter_test
2022-12-15 2:00 [PATCH net 0/3] devlink: region snapshot locking fix and selftest adjustments Jakub Kicinski
2022-12-15 2:01 ` [PATCH net 1/3] devlink: hold region lock when flushing snapshots Jakub Kicinski
@ 2022-12-15 2:01 ` Jakub Kicinski
2022-12-15 2:01 ` [PATCH net 3/3] selftests: devlink: add a warning for interfaces coming up Jakub Kicinski
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2022-12-15 2:01 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, jiri, jacob.e.keller, Jakub Kicinski
$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 <kuba@kernel.org>
---
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"
--
2.38.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net 3/3] selftests: devlink: add a warning for interfaces coming up
2022-12-15 2:00 [PATCH net 0/3] devlink: region snapshot locking fix and selftest adjustments Jakub Kicinski
2022-12-15 2:01 ` [PATCH net 1/3] devlink: hold region lock when flushing snapshots Jakub Kicinski
2022-12-15 2:01 ` [PATCH net 2/3] selftests: devlink: fix the fd redirect in dummy_reporter_test Jakub Kicinski
@ 2022-12-15 2:01 ` Jakub Kicinski
2022-12-15 18:42 ` [PATCH net 0/3] devlink: region snapshot locking fix and selftest adjustments Jacob Keller
2022-12-16 10:30 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2022-12-15 2:01 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, jiri, jacob.e.keller, Jakub Kicinski
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 <kuba@kernel.org>
---
.../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"
--
2.38.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net 1/3] devlink: hold region lock when flushing snapshots
2022-12-15 2:01 ` [PATCH net 1/3] devlink: hold region lock when flushing snapshots Jakub Kicinski
@ 2022-12-15 8:38 ` Jiri Pirko
0 siblings, 0 replies; 7+ messages in thread
From: Jiri Pirko @ 2022-12-15 8:38 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, netdev, edumazet, pabeni, jacob.e.keller, jiri
Thu, Dec 15, 2022 at 03:01:00AM CET, kuba@kernel.org wrote:
>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:
> <TASK>
> 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 <kuba@kernel.org>
Ooup.
Thanks for the fix.
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net 0/3] devlink: region snapshot locking fix and selftest adjustments
2022-12-15 2:00 [PATCH net 0/3] devlink: region snapshot locking fix and selftest adjustments Jakub Kicinski
` (2 preceding siblings ...)
2022-12-15 2:01 ` [PATCH net 3/3] selftests: devlink: add a warning for interfaces coming up Jakub Kicinski
@ 2022-12-15 18:42 ` Jacob Keller
2022-12-16 10:30 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 7+ messages in thread
From: Jacob Keller @ 2022-12-15 18:42 UTC (permalink / raw)
To: Jakub Kicinski, davem; +Cc: netdev, edumazet, pabeni, jiri
On 12/14/2022 6:00 PM, Jakub Kicinski wrote:
> Minor fix for region snapshot locking and adjustments to selftests.
>
> Jakub Kicinski (3):
> devlink: hold region lock when flushing snapshots
> selftests: devlink: fix the fd redirect in dummy_reporter_test
> selftests: devlink: add a warning for interfaces coming up
>
> net/core/devlink.c | 2 ++
> .../selftests/drivers/net/netdevsim/devlink.sh | 4 ++--
> .../selftests/drivers/net/netdevsim/devlink_trap.sh | 13 +++++++++++++
> 3 files changed, 17 insertions(+), 2 deletions(-)
>
Whole series makes sense.
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net 0/3] devlink: region snapshot locking fix and selftest adjustments
2022-12-15 2:00 [PATCH net 0/3] devlink: region snapshot locking fix and selftest adjustments Jakub Kicinski
` (3 preceding siblings ...)
2022-12-15 18:42 ` [PATCH net 0/3] devlink: region snapshot locking fix and selftest adjustments Jacob Keller
@ 2022-12-16 10:30 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-12-16 10:30 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, netdev, edumazet, pabeni, jiri, jacob.e.keller
Hello:
This series was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:
On Wed, 14 Dec 2022 18:00:59 -0800 you wrote:
> Minor fix for region snapshot locking and adjustments to selftests.
>
> Jakub Kicinski (3):
> devlink: hold region lock when flushing snapshots
> selftests: devlink: fix the fd redirect in dummy_reporter_test
> selftests: devlink: add a warning for interfaces coming up
>
> [...]
Here is the summary with links:
- [net,1/3] devlink: hold region lock when flushing snapshots
https://git.kernel.org/netdev/net/c/b4cafb3d2c74
- [net,2/3] selftests: devlink: fix the fd redirect in dummy_reporter_test
https://git.kernel.org/netdev/net/c/2fc60e2ff972
- [net,3/3] selftests: devlink: add a warning for interfaces coming up
https://git.kernel.org/netdev/net/c/d1c4a3469e73
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-12-16 10:30 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-15 2:00 [PATCH net 0/3] devlink: region snapshot locking fix and selftest adjustments Jakub Kicinski
2022-12-15 2:01 ` [PATCH net 1/3] devlink: hold region lock when flushing snapshots Jakub Kicinski
2022-12-15 8:38 ` Jiri Pirko
2022-12-15 2:01 ` [PATCH net 2/3] selftests: devlink: fix the fd redirect in dummy_reporter_test Jakub Kicinski
2022-12-15 2:01 ` [PATCH net 3/3] selftests: devlink: add a warning for interfaces coming up Jakub Kicinski
2022-12-15 18:42 ` [PATCH net 0/3] devlink: region snapshot locking fix and selftest adjustments Jacob Keller
2022-12-16 10:30 ` patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.