* [patch net-next v2 0/3] net: devlink: move netdev notifier block to dest namespace during reload
@ 2022-11-08 13:22 Jiri Pirko
2022-11-08 13:22 ` [patch net-next v2 1/3] net: introduce a helper to move notifier block to different namespace Jiri Pirko
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Jiri Pirko @ 2022-11-08 13:22 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, idosch, bigeasy, imagedong, kuniyu, petrm
From: Jiri Pirko <jiri@nvidia.com>
Patch #1 is just a dependency of patch #2, which is the actual fix.
Patch #3 adds a small check that would uncover the original bug
instantly.
Jiri Pirko (3):
net: introduce a helper to move notifier block to different namespace
net: devlink: move netdev notifier block to dest namespace during
reload
net: devlink: add WARN_ON to check return value of
unregister_netdevice_notifier_net() call
include/linux/netdevice.h | 2 ++
net/core/dev.c | 22 ++++++++++++++++++----
net/core/devlink.c | 9 ++++++---
3 files changed, 26 insertions(+), 7 deletions(-)
--
2.37.3
^ permalink raw reply [flat|nested] 14+ messages in thread
* [patch net-next v2 1/3] net: introduce a helper to move notifier block to different namespace
2022-11-08 13:22 [patch net-next v2 0/3] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
@ 2022-11-08 13:22 ` Jiri Pirko
2022-11-09 11:56 ` Ido Schimmel
2022-11-08 13:22 ` [patch net-next v2 2/3] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Jiri Pirko @ 2022-11-08 13:22 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, idosch, bigeasy, imagedong, kuniyu, petrm
From: Jiri Pirko <jiri@nvidia.com>
Currently, net_dev() netdev notifier variant follows the netdev with
per-net notifier from namespace to namespace. This is implemented
by move_netdevice_notifiers_dev_net() helper.
For devlink it is needed to re-register per-net notifier during
devlink reload. Introduce a new helper called
move_netdevice_notifier_net() and share the unregister/register code
with existing move_netdevice_notifiers_dev_net() helper.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v1->v2:
- made __move_netdevice_notifier_net() static
- remove unnecessary move_netdevice_notifier_net() export
---
include/linux/netdevice.h | 2 ++
net/core/dev.c | 22 ++++++++++++++++++----
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d45713a06568..6be93b59cfea 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2828,6 +2828,8 @@ int unregister_netdevice_notifier(struct notifier_block *nb);
int register_netdevice_notifier_net(struct net *net, struct notifier_block *nb);
int unregister_netdevice_notifier_net(struct net *net,
struct notifier_block *nb);
+void move_netdevice_notifier_net(struct net *src_net, struct net *dst_net,
+ struct notifier_block *nb);
int register_netdevice_notifier_dev_net(struct net_device *dev,
struct notifier_block *nb,
struct netdev_net_notifier *nn);
diff --git a/net/core/dev.c b/net/core/dev.c
index 3bacee3bee78..0078a2734a4c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1876,6 +1876,22 @@ int unregister_netdevice_notifier_net(struct net *net,
}
EXPORT_SYMBOL(unregister_netdevice_notifier_net);
+static void __move_netdevice_notifier_net(struct net *src_net,
+ struct net *dst_net,
+ struct notifier_block *nb)
+{
+ __unregister_netdevice_notifier_net(src_net, nb);
+ __register_netdevice_notifier_net(dst_net, nb, true);
+}
+
+void move_netdevice_notifier_net(struct net *src_net, struct net *dst_net,
+ struct notifier_block *nb)
+{
+ rtnl_lock();
+ __move_netdevice_notifier_net(src_net, dst_net, nb);
+ rtnl_unlock();
+}
+
int register_netdevice_notifier_dev_net(struct net_device *dev,
struct notifier_block *nb,
struct netdev_net_notifier *nn)
@@ -1912,10 +1928,8 @@ static void move_netdevice_notifiers_dev_net(struct net_device *dev,
{
struct netdev_net_notifier *nn;
- list_for_each_entry(nn, &dev->net_notifier_list, list) {
- __unregister_netdevice_notifier_net(dev_net(dev), nn->nb);
- __register_netdevice_notifier_net(net, nn->nb, true);
- }
+ list_for_each_entry(nn, &dev->net_notifier_list, list)
+ __move_netdevice_notifier_net(dev_net(dev), net, nn->nb);
}
/**
--
2.37.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [patch net-next v2 2/3] net: devlink: move netdev notifier block to dest namespace during reload
2022-11-08 13:22 [patch net-next v2 0/3] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
2022-11-08 13:22 ` [patch net-next v2 1/3] net: introduce a helper to move notifier block to different namespace Jiri Pirko
@ 2022-11-08 13:22 ` Jiri Pirko
2022-11-08 13:22 ` [patch net-next v2 3/3] net: devlink: add WARN_ON to check return value of unregister_netdevice_notifier_net() call Jiri Pirko
2022-11-10 0:20 ` [patch net-next v2 0/3] net: devlink: move netdev notifier block to dest namespace during reload patchwork-bot+netdevbpf
3 siblings, 0 replies; 14+ messages in thread
From: Jiri Pirko @ 2022-11-08 13:22 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, idosch, bigeasy, imagedong, kuniyu, petrm
From: Jiri Pirko <jiri@nvidia.com>
The notifier block tracking netdev changes in devlink is registered
during devlink_alloc() per-net, it is then unregistered
in devlink_free(). When devlink moves from net namespace to another one,
the notifier block needs to move along.
Fix this by adding forgotten call to move the block.
Reported-by: Ido Schimmel <idosch@idosch.org>
Fixes: 02a68a47eade ("net: devlink: track netdev with devlink_port assigned")
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
net/core/devlink.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 40fcdded57e6..ea0b319385fc 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -4502,8 +4502,11 @@ static int devlink_reload(struct devlink *devlink, struct net *dest_net,
if (err)
return err;
- if (dest_net && !net_eq(dest_net, curr_net))
+ if (dest_net && !net_eq(dest_net, curr_net)) {
+ move_netdevice_notifier_net(curr_net, dest_net,
+ &devlink->netdevice_nb);
write_pnet(&devlink->_net, dest_net);
+ }
err = devlink->ops->reload_up(devlink, action, limit, actions_performed, extack);
devlink_reload_failed_set(devlink, !!err);
--
2.37.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [patch net-next v2 3/3] net: devlink: add WARN_ON to check return value of unregister_netdevice_notifier_net() call
2022-11-08 13:22 [patch net-next v2 0/3] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
2022-11-08 13:22 ` [patch net-next v2 1/3] net: introduce a helper to move notifier block to different namespace Jiri Pirko
2022-11-08 13:22 ` [patch net-next v2 2/3] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
@ 2022-11-08 13:22 ` Jiri Pirko
2022-11-09 11:49 ` Ido Schimmel
2022-11-10 0:20 ` [patch net-next v2 0/3] net: devlink: move netdev notifier block to dest namespace during reload patchwork-bot+netdevbpf
3 siblings, 1 reply; 14+ messages in thread
From: Jiri Pirko @ 2022-11-08 13:22 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, idosch, bigeasy, imagedong, kuniyu, petrm
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>
---
net/core/devlink.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index ea0b319385fc..6096baf74b00 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -9843,8 +9843,8 @@ void devlink_free(struct devlink *devlink)
xa_destroy(&devlink->snapshot_ids);
- unregister_netdevice_notifier_net(devlink_net(devlink),
- &devlink->netdevice_nb);
+ WARN_ON(unregister_netdevice_notifier_net(devlink_net(devlink),
+ &devlink->netdevice_nb));
xa_erase(&devlinks, devlink->index);
--
2.37.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [patch net-next v2 3/3] net: devlink: add WARN_ON to check return value of unregister_netdevice_notifier_net() call
2022-11-08 13:22 ` [patch net-next v2 3/3] net: devlink: add WARN_ON to check return value of unregister_netdevice_notifier_net() call Jiri Pirko
@ 2022-11-09 11:49 ` Ido Schimmel
2022-11-09 16:26 ` Eric Dumazet
0 siblings, 1 reply; 14+ messages in thread
From: Ido Schimmel @ 2022-11-09 11:49 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, kuba, pabeni, edumazet, bigeasy, imagedong, kuniyu, petrm
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>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch net-next v2 1/3] net: introduce a helper to move notifier block to different namespace
2022-11-08 13:22 ` [patch net-next v2 1/3] net: introduce a helper to move notifier block to different namespace Jiri Pirko
@ 2022-11-09 11:56 ` Ido Schimmel
0 siblings, 0 replies; 14+ messages in thread
From: Ido Schimmel @ 2022-11-09 11:56 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, kuba, pabeni, edumazet, bigeasy, imagedong, kuniyu, petrm
On Tue, Nov 08, 2022 at 02:22:06PM +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> Currently, net_dev() netdev notifier variant follows the netdev with
> per-net notifier from namespace to namespace. This is implemented
> by move_netdevice_notifiers_dev_net() helper.
>
> For devlink it is needed to re-register per-net notifier during
> devlink reload. Introduce a new helper called
> move_netdevice_notifier_net() and share the unregister/register code
> with existing move_netdevice_notifiers_dev_net() helper.
>
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
I'm not sure what is net_dev(), but the code looks fine:
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch net-next v2 3/3] net: devlink: add WARN_ON to check return value of unregister_netdevice_notifier_net() call
2022-11-09 11:49 ` Ido Schimmel
@ 2022-11-09 16:26 ` Eric Dumazet
2022-11-09 21:45 ` Jakub Kicinski
2022-11-10 7:53 ` Jiri Pirko
0 siblings, 2 replies; 14+ messages in thread
From: Eric Dumazet @ 2022-11-09 16:26 UTC (permalink / raw)
To: Ido Schimmel
Cc: Jiri Pirko, netdev, davem, kuba, pabeni, bigeasy, imagedong,
kuniyu, petrm
On Wed, Nov 9, 2022 at 3:49 AM Ido Schimmel <idosch@idosch.org> 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()
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch net-next v2 3/3] net: devlink: add WARN_ON to check return value of unregister_netdevice_notifier_net() call
2022-11-09 16:26 ` Eric Dumazet
@ 2022-11-09 21:45 ` Jakub Kicinski
2022-11-10 7:54 ` Jiri Pirko
2022-11-10 7:53 ` Jiri Pirko
1 sibling, 1 reply; 14+ messages in thread
From: Jakub Kicinski @ 2022-11-09 21:45 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ido Schimmel, Jiri Pirko, netdev, davem, pabeni, bigeasy,
imagedong, kuniyu, petrm
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?
Let me take the first 2 in, to lower the syzbot volume.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch net-next v2 0/3] net: devlink: move netdev notifier block to dest namespace during reload
2022-11-08 13:22 [patch net-next v2 0/3] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
` (2 preceding siblings ...)
2022-11-08 13:22 ` [patch net-next v2 3/3] net: devlink: add WARN_ON to check return value of unregister_netdevice_notifier_net() call Jiri Pirko
@ 2022-11-10 0:20 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 14+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-11-10 0:20 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, kuba, pabeni, edumazet, idosch, bigeasy,
imagedong, kuniyu, petrm
Hello:
This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:
On Tue, 8 Nov 2022 14:22:05 +0100 you wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> Patch #1 is just a dependency of patch #2, which is the actual fix.
> Patch #3 adds a small check that would uncover the original bug
> instantly.
>
> Jiri Pirko (3):
> net: introduce a helper to move notifier block to different namespace
> net: devlink: move netdev notifier block to dest namespace during
> reload
> net: devlink: add WARN_ON to check return value of
> unregister_netdevice_notifier_net() call
>
> [...]
Here is the summary with links:
- [net-next,v2,1/3] net: introduce a helper to move notifier block to different namespace
https://git.kernel.org/netdev/net-next/c/3e52fba03a20
- [net-next,v2,2/3] net: devlink: move netdev notifier block to dest namespace during reload
https://git.kernel.org/netdev/net-next/c/15feb56e30ef
- [net-next,v2,3/3] net: devlink: add WARN_ON to check return value of unregister_netdevice_notifier_net() call
(no matching commit)
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] 14+ messages in thread
* Re: [patch net-next v2 3/3] net: devlink: add WARN_ON to check return value of unregister_netdevice_notifier_net() call
2022-11-09 16:26 ` Eric Dumazet
2022-11-09 21:45 ` Jakub Kicinski
@ 2022-11-10 7:53 ` Jiri Pirko
1 sibling, 0 replies; 14+ messages in thread
From: Jiri Pirko @ 2022-11-10 7:53 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ido Schimmel, netdev, davem, kuba, pabeni, bigeasy, imagedong,
kuniyu, petrm
Wed, Nov 09, 2022 at 05:26:10PM CET, edumazet@google.com wrote:
>On Wed, Nov 9, 2022 at 3:49 AM Ido Schimmel <idosch@idosch.org> 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()
Well, in this case, I think that plain WARN_ON is fine as this happens
only during driver cleanup which is not expected to happen very often
(or not at all) in real world scenarios.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch net-next v2 3/3] net: devlink: add WARN_ON to check return value of unregister_netdevice_notifier_net() call
2022-11-09 21:45 ` Jakub Kicinski
@ 2022-11-10 7:54 ` Jiri Pirko
2022-11-10 17:21 ` Eric Dumazet
0 siblings, 1 reply; 14+ messages in thread
From: Jiri Pirko @ 2022-11-10 7:54 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Eric Dumazet, Ido Schimmel, netdev, davem, pabeni, bigeasy,
imagedong, kuniyu, petrm
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch net-next v2 3/3] net: devlink: add WARN_ON to check return value of unregister_netdevice_notifier_net() call
2022-11-10 7:54 ` Jiri Pirko
@ 2022-11-10 17:21 ` Eric Dumazet
2022-11-10 18:04 ` Ido Schimmel
0 siblings, 1 reply; 14+ messages in thread
From: Eric Dumazet @ 2022-11-10 17:21 UTC (permalink / raw)
To: Jiri Pirko
Cc: Jakub Kicinski, Ido Schimmel, netdev, davem, pabeni, bigeasy,
imagedong, kuniyu, petrm
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
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>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch net-next v2 3/3] net: devlink: add WARN_ON to check return value of unregister_netdevice_notifier_net() call
2022-11-10 17:21 ` Eric Dumazet
@ 2022-11-10 18:04 ` Ido Schimmel
2022-11-10 18:07 ` Eric Dumazet
0 siblings, 1 reply; 14+ messages in thread
From: Ido Schimmel @ 2022-11-10 18:04 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jiri Pirko, Jakub Kicinski, netdev, davem, pabeni, bigeasy,
imagedong, kuniyu, petrm
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>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch net-next v2 3/3] net: devlink: add WARN_ON to check return value of unregister_netdevice_notifier_net() call
2022-11-10 18:04 ` Ido Schimmel
@ 2022-11-10 18:07 ` Eric Dumazet
0 siblings, 0 replies; 14+ messages in thread
From: Eric Dumazet @ 2022-11-10 18:07 UTC (permalink / raw)
To: Ido Schimmel
Cc: Jiri Pirko, Jakub Kicinski, netdev, davem, pabeni, bigeasy,
imagedong, kuniyu, petrm
On Thu, Nov 10, 2022 at 10:04 AM Ido Schimmel <idosch@idosch.org> wrote:
>
> 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/
OK, I will dup the new syzbot report, thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-11-10 18:07 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-08 13:22 [patch net-next v2 0/3] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
2022-11-08 13:22 ` [patch net-next v2 1/3] net: introduce a helper to move notifier block to different namespace Jiri Pirko
2022-11-09 11:56 ` Ido Schimmel
2022-11-08 13:22 ` [patch net-next v2 2/3] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
2022-11-08 13:22 ` [patch net-next v2 3/3] net: devlink: add WARN_ON to check return value of unregister_netdevice_notifier_net() call Jiri Pirko
2022-11-09 11:49 ` Ido Schimmel
2022-11-09 16:26 ` Eric Dumazet
2022-11-09 21:45 ` Jakub Kicinski
2022-11-10 7:54 ` Jiri Pirko
2022-11-10 17:21 ` Eric Dumazet
2022-11-10 18:04 ` Ido Schimmel
2022-11-10 18:07 ` Eric Dumazet
2022-11-10 7:53 ` Jiri Pirko
2022-11-10 0:20 ` [patch net-next v2 0/3] net: devlink: move netdev notifier block to dest namespace during reload 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.