* [patch net] devlink: change port event netdev notifier from per-net to global
@ 2023-02-06 9:41 Jiri Pirko
2023-02-06 17:39 ` Jacob Keller
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Jiri Pirko @ 2023-02-06 9:41 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, pabeni, edumazet, jacob.e.keller
From: Jiri Pirko <jiri@nvidia.com>
Currently only the network namespace of devlink instance is monitored
for port events. If netdev is moved to a different namespace and then
unregistered, NETDEV_PRE_UNINIT is missed which leads to trigger
following WARN_ON in devl_port_unregister().
WARN_ON(devlink_port->type != DEVLINK_PORT_TYPE_NOTSET);
Fix this by changing the netdev notifier from per-net to global so no
event is missed.
Fixes: 02a68a47eade ("net: devlink: track netdev with devlink_port assigned")
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
net/core/devlink.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 032d6d0a5ce6..909a10e4b0dd 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -9979,7 +9979,7 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
goto err_xa_alloc;
devlink->netdevice_nb.notifier_call = devlink_netdevice_event;
- ret = register_netdevice_notifier_net(net, &devlink->netdevice_nb);
+ ret = register_netdevice_notifier(&devlink->netdevice_nb);
if (ret)
goto err_register_netdevice_notifier;
@@ -10171,8 +10171,7 @@ void devlink_free(struct devlink *devlink)
xa_destroy(&devlink->snapshot_ids);
xa_destroy(&devlink->ports);
- WARN_ON_ONCE(unregister_netdevice_notifier_net(devlink_net(devlink),
- &devlink->netdevice_nb));
+ WARN_ON_ONCE(unregister_netdevice_notifier(&devlink->netdevice_nb));
xa_erase(&devlinks, devlink->index);
@@ -10503,6 +10502,8 @@ static int devlink_netdevice_event(struct notifier_block *nb,
break;
case NETDEV_REGISTER:
case NETDEV_CHANGENAME:
+ if (devlink_net(devlink) != dev_net(netdev))
+ return NOTIFY_OK;
/* Set the netdev on top of previously set type. Note this
* event happens also during net namespace change so here
* we take into account netdev pointer appearing in this
@@ -10512,6 +10513,8 @@ static int devlink_netdevice_event(struct notifier_block *nb,
netdev);
break;
case NETDEV_UNREGISTER:
+ if (devlink_net(devlink) != dev_net(netdev))
+ return NOTIFY_OK;
/* Clear netdev pointer, but not the type. This event happens
* also during net namespace change so we need to clear
* pointer to netdev that is going to another net namespace.
--
2.39.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [patch net] devlink: change port event netdev notifier from per-net to global
2023-02-06 9:41 [patch net] devlink: change port event netdev notifier from per-net to global Jiri Pirko
@ 2023-02-06 17:39 ` Jacob Keller
2023-02-07 14:40 ` patchwork-bot+netdevbpf
2023-02-14 8:41 ` Ido Schimmel
2 siblings, 0 replies; 5+ messages in thread
From: Jacob Keller @ 2023-02-06 17:39 UTC (permalink / raw)
To: Jiri Pirko, netdev; +Cc: davem, kuba, pabeni, edumazet
On 2/6/2023 1:41 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> Currently only the network namespace of devlink instance is monitored
> for port events. If netdev is moved to a different namespace and then
> unregistered, NETDEV_PRE_UNINIT is missed which leads to trigger
> following WARN_ON in devl_port_unregister().
> WARN_ON(devlink_port->type != DEVLINK_PORT_TYPE_NOTSET);
>
> Fix this by changing the netdev notifier from per-net to global so no
> event is missed.
>
> Fixes: 02a68a47eade ("net: devlink: track netdev with devlink_port assigned")
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Seems reasonable.
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch net] devlink: change port event netdev notifier from per-net to global
2023-02-06 9:41 [patch net] devlink: change port event netdev notifier from per-net to global Jiri Pirko
2023-02-06 17:39 ` Jacob Keller
@ 2023-02-07 14:40 ` patchwork-bot+netdevbpf
2023-02-14 8:41 ` Ido Schimmel
2 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-02-07 14:40 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, kuba, pabeni, edumazet, jacob.e.keller
Hello:
This patch was applied to netdev/net.git (master)
by Paolo Abeni <pabeni@redhat.com>:
On Mon, 6 Feb 2023 10:41:51 +0100 you wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> Currently only the network namespace of devlink instance is monitored
> for port events. If netdev is moved to a different namespace and then
> unregistered, NETDEV_PRE_UNINIT is missed which leads to trigger
> following WARN_ON in devl_port_unregister().
> WARN_ON(devlink_port->type != DEVLINK_PORT_TYPE_NOTSET);
>
> [...]
Here is the summary with links:
- [net] devlink: change port event netdev notifier from per-net to global
https://git.kernel.org/netdev/net/c/565b4824c39f
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] 5+ messages in thread
* Re: [patch net] devlink: change port event netdev notifier from per-net to global
2023-02-06 9:41 [patch net] devlink: change port event netdev notifier from per-net to global Jiri Pirko
2023-02-06 17:39 ` Jacob Keller
2023-02-07 14:40 ` patchwork-bot+netdevbpf
@ 2023-02-14 8:41 ` Ido Schimmel
2023-02-14 10:47 ` Jiri Pirko
2 siblings, 1 reply; 5+ messages in thread
From: Ido Schimmel @ 2023-02-14 8:41 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, kuba, pabeni, edumazet, jacob.e.keller
On Mon, Feb 06, 2023 at 10:41:51AM +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> Currently only the network namespace of devlink instance is monitored
> for port events. If netdev is moved to a different namespace and then
> unregistered, NETDEV_PRE_UNINIT is missed which leads to trigger
> following WARN_ON in devl_port_unregister().
> WARN_ON(devlink_port->type != DEVLINK_PORT_TYPE_NOTSET);
>
> Fix this by changing the netdev notifier from per-net to global so no
> event is missed.
>
> Fixes: 02a68a47eade ("net: devlink: track netdev with devlink_port assigned")
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
> net/core/devlink.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index 032d6d0a5ce6..909a10e4b0dd 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -9979,7 +9979,7 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
> goto err_xa_alloc;
>
> devlink->netdevice_nb.notifier_call = devlink_netdevice_event;
> - ret = register_netdevice_notifier_net(net, &devlink->netdevice_nb);
> + ret = register_netdevice_notifier(&devlink->netdevice_nb);
> if (ret)
> goto err_register_netdevice_notifier;
>
> @@ -10171,8 +10171,7 @@ void devlink_free(struct devlink *devlink)
> xa_destroy(&devlink->snapshot_ids);
> xa_destroy(&devlink->ports);
>
> - WARN_ON_ONCE(unregister_netdevice_notifier_net(devlink_net(devlink),
> - &devlink->netdevice_nb));
> + WARN_ON_ONCE(unregister_netdevice_notifier(&devlink->netdevice_nb));
>
> xa_erase(&devlinks, devlink->index);
>
> @@ -10503,6 +10502,8 @@ static int devlink_netdevice_event(struct notifier_block *nb,
> break;
> case NETDEV_REGISTER:
> case NETDEV_CHANGENAME:
> + if (devlink_net(devlink) != dev_net(netdev))
> + return NOTIFY_OK;
> /* Set the netdev on top of previously set type. Note this
> * event happens also during net namespace change so here
> * we take into account netdev pointer appearing in this
> @@ -10512,6 +10513,8 @@ static int devlink_netdevice_event(struct notifier_block *nb,
> netdev);
> break;
> case NETDEV_UNREGISTER:
> + if (devlink_net(devlink) != dev_net(netdev))
> + return NOTIFY_OK;
> /* Clear netdev pointer, but not the type. This event happens
> * also during net namespace change so we need to clear
> * pointer to netdev that is going to another net namespace.
Since the notifier block is no longer registered per-netns it shouldn't
be moved to a different netns on reload. I'm testing the following diff
[1] against net-next (although it should be eventually submitted to
net).
[1]
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d9cdbc047b49..efbee940bb03 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2858,8 +2858,6 @@ 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 7307a0c15c9f..709b1a02820b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1870,14 +1870,6 @@ static void __move_netdevice_notifier_net(struct net *src_net,
__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)
diff --git a/net/devlink/dev.c b/net/devlink/dev.c
index ab4e0f3c4e3d..c879c3c78e18 100644
--- a/net/devlink/dev.c
+++ b/net/devlink/dev.c
@@ -343,8 +343,6 @@ static void devlink_reload_netns_change(struct devlink *devlink,
* reload process so the notifications are generated separatelly.
*/
devlink_notify_unregister(devlink);
- move_netdevice_notifier_net(curr_net, dest_net,
- &devlink->netdevice_nb);
write_pnet(&devlink->_net, dest_net);
devlink_notify_register(devlink);
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [patch net] devlink: change port event netdev notifier from per-net to global
2023-02-14 8:41 ` Ido Schimmel
@ 2023-02-14 10:47 ` Jiri Pirko
0 siblings, 0 replies; 5+ messages in thread
From: Jiri Pirko @ 2023-02-14 10:47 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, davem, kuba, pabeni, edumazet, jacob.e.keller
Tue, Feb 14, 2023 at 09:41:36AM CET, idosch@idosch.org wrote:
>On Mon, Feb 06, 2023 at 10:41:51AM +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>>
>> Currently only the network namespace of devlink instance is monitored
>> for port events. If netdev is moved to a different namespace and then
>> unregistered, NETDEV_PRE_UNINIT is missed which leads to trigger
>> following WARN_ON in devl_port_unregister().
>> WARN_ON(devlink_port->type != DEVLINK_PORT_TYPE_NOTSET);
>>
>> Fix this by changing the netdev notifier from per-net to global so no
>> event is missed.
>>
>> Fixes: 02a68a47eade ("net: devlink: track netdev with devlink_port assigned")
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>> ---
>> net/core/devlink.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>> index 032d6d0a5ce6..909a10e4b0dd 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> @@ -9979,7 +9979,7 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
>> goto err_xa_alloc;
>>
>> devlink->netdevice_nb.notifier_call = devlink_netdevice_event;
>> - ret = register_netdevice_notifier_net(net, &devlink->netdevice_nb);
>> + ret = register_netdevice_notifier(&devlink->netdevice_nb);
>> if (ret)
>> goto err_register_netdevice_notifier;
>>
>> @@ -10171,8 +10171,7 @@ void devlink_free(struct devlink *devlink)
>> xa_destroy(&devlink->snapshot_ids);
>> xa_destroy(&devlink->ports);
>>
>> - WARN_ON_ONCE(unregister_netdevice_notifier_net(devlink_net(devlink),
>> - &devlink->netdevice_nb));
>> + WARN_ON_ONCE(unregister_netdevice_notifier(&devlink->netdevice_nb));
>>
>> xa_erase(&devlinks, devlink->index);
>>
>> @@ -10503,6 +10502,8 @@ static int devlink_netdevice_event(struct notifier_block *nb,
>> break;
>> case NETDEV_REGISTER:
>> case NETDEV_CHANGENAME:
>> + if (devlink_net(devlink) != dev_net(netdev))
>> + return NOTIFY_OK;
>> /* Set the netdev on top of previously set type. Note this
>> * event happens also during net namespace change so here
>> * we take into account netdev pointer appearing in this
>> @@ -10512,6 +10513,8 @@ static int devlink_netdevice_event(struct notifier_block *nb,
>> netdev);
>> break;
>> case NETDEV_UNREGISTER:
>> + if (devlink_net(devlink) != dev_net(netdev))
>> + return NOTIFY_OK;
>> /* Clear netdev pointer, but not the type. This event happens
>> * also during net namespace change so we need to clear
>> * pointer to netdev that is going to another net namespace.
>
>Since the notifier block is no longer registered per-netns it shouldn't
>be moved to a different netns on reload. I'm testing the following diff
>[1] against net-next (although it should be eventually submitted to
>net).
Oh yeah. That is needed. Thanks!
>
>[1]
>diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>index d9cdbc047b49..efbee940bb03 100644
>--- a/include/linux/netdevice.h
>+++ b/include/linux/netdevice.h
>@@ -2858,8 +2858,6 @@ 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 7307a0c15c9f..709b1a02820b 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -1870,14 +1870,6 @@ static void __move_netdevice_notifier_net(struct net *src_net,
> __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)
>diff --git a/net/devlink/dev.c b/net/devlink/dev.c
>index ab4e0f3c4e3d..c879c3c78e18 100644
>--- a/net/devlink/dev.c
>+++ b/net/devlink/dev.c
>@@ -343,8 +343,6 @@ static void devlink_reload_netns_change(struct devlink *devlink,
> * reload process so the notifications are generated separatelly.
> */
> devlink_notify_unregister(devlink);
>- move_netdevice_notifier_net(curr_net, dest_net,
>- &devlink->netdevice_nb);
> write_pnet(&devlink->_net, dest_net);
> devlink_notify_register(devlink);
> }
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-02-14 10:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-06 9:41 [patch net] devlink: change port event netdev notifier from per-net to global Jiri Pirko
2023-02-06 17:39 ` Jacob Keller
2023-02-07 14:40 ` patchwork-bot+netdevbpf
2023-02-14 8:41 ` Ido Schimmel
2023-02-14 10:47 ` Jiri Pirko
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.