Re: [patch net-next v2 3/3] net: devlink: add WARN_ON to check return value of unregister_netdevice_notifier_net() call

[Ido Schimmel <idosch@idosch.org>]

On Thu, Nov 10, 2022 at 09:21:23AM -0800, Eric Dumazet wrote:
> On Wed, Nov 9, 2022 at 11:54 PM Jiri Pirko <jiri@resnulli.us> wrote:
> >
> > Wed, Nov 09, 2022 at 10:45:36PM CET, kuba@kernel.org wrote:
> > >On Wed, 9 Nov 2022 08:26:10 -0800 Eric Dumazet wrote:
> > >> > On Tue, Nov 08, 2022 at 02:22:08PM +0100, Jiri Pirko wrote:
> > >> > > From: Jiri Pirko <jiri@nvidia.com>
> > >> > >
> > >> > > As the return value is not 0 only in case there is no such notifier
> > >> > > block registered, add a WARN_ON() to yell about it.
> > >> > >
> > >> > > Suggested-by: Ido Schimmel <idosch@idosch.org>
> > >> > > Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> > >> >
> > >> > Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> > >>
> > >> Please consider WARN_ON_ONCE(), or DEBUG_NET_WARN_ON_ONCE()
> > >
> > >Do you have any general guidance on when to pick WARN() vs WARN_ONCE()?
> > >Or should we always prefer _ONCE() going forward?
> >
> > Good question. If so, it should be documented or spotted by checkpatch.
> >
> > >
> > >Let me take the first 2 in, to lower the syzbot volume.
> 
> Well, I am not sure what you call 'lower syzbot volume'
> 
> netdevsim netdevsim0 netdevsim3 (unregistering): unset [1, 0] type 2
> family 0 port 6081 - 0
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 41 at net/core/devlink.c:10001
> devl_port_unregister+0x2f6/0x390 net/core/devlink.c:10001

Hi Eric,

That's a different bug than the one fixed by this patchset. Should be
fixed by this patch:

https://patchwork.kernel.org/project/netdevbpf/patch/20221110085150.520800-1-idosch@nvidia.com/

> Modules linked in:
> CPU: 0 PID: 41 Comm: kworker/u4:2 Not tainted
> 6.1.0-rc3-syzkaller-00887-g0c9ef08a4d0f #0
> Hardware name: Google Google Compute Engine/Google Compute Engine,
> BIOS Google 10/26/2022
> Workqueue: netns cleanup_net
> RIP: 0010:devl_port_unregister+0x2f6/0x390 net/core/devlink.c:10001
> Code: e8 3f 37 0b fa 85 ed 0f 85 7a fd ff ff e8 62 3a 0b fa 0f 0b e9
> 6e fd ff ff e8 56 3a 0b fa 0f 0b e9 53 ff ff ff e8 4a 3a 0b fa <0f> 0b
> e9 94 fd ff ff e8 ae ac 57 fa e9 78 ff ff ff e8 74 ac 57 fa
> RSP: 0018:ffffc90000b27a08 EFLAGS: 00010293
> RAX: 0000000000000000 RBX: ffff88806ee3f810 RCX: 0000000000000000
> RDX: ffff8880175e1d40 RSI: ffffffff877177d6 RDI: 0000000000000005
> RBP: 0000000000000002 R08: 0000000000000005 R09: 0000000000000000
> R10: 0000000000000002 R11: 0000000000000000 R12: ffff88806ee3f810
> R13: ffff88806ee3f808 R14: ffff88806ee3e800 R15: ffff88806ee3f800
> FS: 0000000000000000(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 000000c00023dee0 CR3: 0000000074faf000 CR4: 00000000003506f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> <TASK>
> __nsim_dev_port_del+0x1bb/0x240 drivers/net/netdevsim/dev.c:1433
> nsim_dev_port_del_all drivers/net/netdevsim/dev.c:1443 [inline]
> nsim_dev_reload_destroy+0x171/0x510 drivers/net/netdevsim/dev.c:1660
> nsim_dev_reload_down+0x6b/0xd0 drivers/net/netdevsim/dev.c:968
> devlink_reload+0x1c4/0x6e0 net/core/devlink.c:4501
> devlink_pernet_pre_exit+0x104/0x1c0 net/core/devlink.c:12615
> ops_pre_exit_list net/core/net_namespace.c:159 [inline]
> cleanup_net+0x451/0xb10 net/core/net_namespace.c:594
> process_one_work+0x9bf/0x1710 kernel/workqueue.c:2289
> worker_thread+0x665/0x1080 kernel/workqueue.c:2436
> kthread+0x2e4/0x3a0 kernel/kthread.c:376
> ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:306
> </TASK>