All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.