* [patch net-next 0/2] net: devlink: move netdev notifier block to dest namespace during reload
@ 2022-11-07 14:52 Jiri Pirko
2022-11-07 14:52 ` [patch net-next 1/2] net: introduce a helper to move notifier block to different namespace Jiri Pirko
2022-11-07 14:52 ` [patch net-next 2/2] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
0 siblings, 2 replies; 10+ messages in thread
From: Jiri Pirko @ 2022-11-07 14:52 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.
Jiri Pirko (2):
net: introduce a helper to move notifier block to different namespace
net: devlink: move netdev notifier block to dest namespace during
reload
include/linux/netdevice.h | 2 ++
net/core/dev.c | 22 ++++++++++++++++++----
net/core/devlink.c | 5 ++++-
3 files changed, 24 insertions(+), 5 deletions(-)
--
2.37.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [patch net-next 1/2] net: introduce a helper to move notifier block to different namespace
2022-11-07 14:52 [patch net-next 0/2] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
@ 2022-11-07 14:52 ` Jiri Pirko
2022-11-08 4:29 ` Jakub Kicinski
2022-11-07 14:52 ` [patch net-next 2/2] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
1 sibling, 1 reply; 10+ messages in thread
From: Jiri Pirko @ 2022-11-07 14:52 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>
---
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..762d7255065d 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);
+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();
+}
+EXPORT_SYMBOL(move_netdevice_notifier_net);
+
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] 10+ messages in thread
* [patch net-next 2/2] net: devlink: move netdev notifier block to dest namespace during reload
2022-11-07 14:52 [patch net-next 0/2] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
2022-11-07 14:52 ` [patch net-next 1/2] net: introduce a helper to move notifier block to different namespace Jiri Pirko
@ 2022-11-07 14:52 ` Jiri Pirko
2022-11-07 16:52 ` Ido Schimmel
2022-11-08 8:11 ` Ido Schimmel
1 sibling, 2 replies; 10+ messages in thread
From: Jiri Pirko @ 2022-11-07 14:52 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")
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] 10+ messages in thread
* Re: [patch net-next 2/2] net: devlink: move netdev notifier block to dest namespace during reload
2022-11-07 14:52 ` [patch net-next 2/2] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
@ 2022-11-07 16:52 ` Ido Schimmel
2022-11-07 17:24 ` Jiri Pirko
2022-11-08 8:11 ` Ido Schimmel
1 sibling, 1 reply; 10+ messages in thread
From: Ido Schimmel @ 2022-11-07 16:52 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, kuba, pabeni, edumazet, bigeasy, imagedong, kuniyu, petrm
On Mon, Nov 07, 2022 at 03:52:13PM +0100, Jiri Pirko wrote:
> 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")
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Does not trigger with my reproducer. Will test the fix tonight in
regression and report tomorrow morning.
> ---
> 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);
> + }
I suggest adding this:
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 83fd10aeddd5..3b5aedc93335 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);
This tells about the failure right away. Instead, we saw random memory
corruptions in later tests.
>
> 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] 10+ messages in thread
* Re: [patch net-next 2/2] net: devlink: move netdev notifier block to dest namespace during reload
2022-11-07 16:52 ` Ido Schimmel
@ 2022-11-07 17:24 ` Jiri Pirko
0 siblings, 0 replies; 10+ messages in thread
From: Jiri Pirko @ 2022-11-07 17:24 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev, davem, kuba, pabeni, edumazet, bigeasy, imagedong, kuniyu, petrm
Mon, Nov 07, 2022 at 05:52:08PM CET, idosch@idosch.org wrote:
>On Mon, Nov 07, 2022 at 03:52:13PM +0100, Jiri Pirko wrote:
>> 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")
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>
>Does not trigger with my reproducer. Will test the fix tonight in
>regression and report tomorrow morning.
Ok!
>
>> ---
>> 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);
>> + }
>
>I suggest adding this:
>
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index 83fd10aeddd5..3b5aedc93335 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);
>
>This tells about the failure right away. Instead, we saw random memory
>corruptions in later tests.
Should be a separate patch then.
>
>>
>> err = devlink->ops->reload_up(devlink, action, limit, actions_performed, extack);
>> devlink_reload_failed_set(devlink, !!err);
>> --
>> 2.37.3
>>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch net-next 1/2] net: introduce a helper to move notifier block to different namespace
2022-11-07 14:52 ` [patch net-next 1/2] net: introduce a helper to move notifier block to different namespace Jiri Pirko
@ 2022-11-08 4:29 ` Jakub Kicinski
2022-11-08 6:53 ` Jiri Pirko
0 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2022-11-08 4:29 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, pabeni, edumazet, idosch, bigeasy, imagedong,
kuniyu, petrm
On Mon, 7 Nov 2022 15:52:12 +0100 Jiri Pirko wrote:
> +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);
> +}
'static' missing
> +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();
> +}
> +EXPORT_SYMBOL(move_netdevice_notifier_net);
Do we need to export this? Maybe let's wait for a module user?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch net-next 1/2] net: introduce a helper to move notifier block to different namespace
2022-11-08 4:29 ` Jakub Kicinski
@ 2022-11-08 6:53 ` Jiri Pirko
0 siblings, 0 replies; 10+ messages in thread
From: Jiri Pirko @ 2022-11-08 6:53 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, davem, pabeni, edumazet, idosch, bigeasy, imagedong,
kuniyu, petrm
Tue, Nov 08, 2022 at 05:29:37AM CET, kuba@kernel.org wrote:
>On Mon, 7 Nov 2022 15:52:12 +0100 Jiri Pirko wrote:
>> +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);
>> +}
>
>'static' missing
Yep.
>
>> +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();
>> +}
>> +EXPORT_SYMBOL(move_netdevice_notifier_net);
>
>Do we need to export this? Maybe let's wait for a module user?
Ah, right.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch net-next 2/2] net: devlink: move netdev notifier block to dest namespace during reload
2022-11-07 14:52 ` [patch net-next 2/2] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
2022-11-07 16:52 ` Ido Schimmel
@ 2022-11-08 8:11 ` Ido Schimmel
2022-11-08 12:59 ` Jiri Pirko
1 sibling, 1 reply; 10+ messages in thread
From: Ido Schimmel @ 2022-11-08 8:11 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, kuba, pabeni, edumazet, bigeasy, imagedong, kuniyu, petrm
On Mon, Nov 07, 2022 at 03:52:13PM +0100, Jiri Pirko wrote:
> 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")
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Ido Schimmel <idosch@nvidia.com>
Thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch net-next 2/2] net: devlink: move netdev notifier block to dest namespace during reload
2022-11-08 8:11 ` Ido Schimmel
@ 2022-11-08 12:59 ` Jiri Pirko
2022-11-08 13:07 ` Jiri Pirko
0 siblings, 1 reply; 10+ messages in thread
From: Jiri Pirko @ 2022-11-08 12:59 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev, davem, kuba, pabeni, edumazet, bigeasy, imagedong, kuniyu, petrm
Tue, Nov 08, 2022 at 09:11:49AM CET, idosch@idosch.org wrote:
>On Mon, Nov 07, 2022 at 03:52:13PM +0100, Jiri Pirko wrote:
>> 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")
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>
>Reviewed-by: Ido Schimmel <idosch@nvidia.com>
>Tested-by: Ido Schimmel <idosch@nvidia.com>
Sending v2 with cosmetical changes. Please put your tags there again.
Thanks!
>
>Thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch net-next 2/2] net: devlink: move netdev notifier block to dest namespace during reload
2022-11-08 12:59 ` Jiri Pirko
@ 2022-11-08 13:07 ` Jiri Pirko
0 siblings, 0 replies; 10+ messages in thread
From: Jiri Pirko @ 2022-11-08 13:07 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev, davem, kuba, pabeni, edumazet, bigeasy, imagedong, kuniyu, petrm
Tue, Nov 08, 2022 at 01:59:50PM CET, jiri@resnulli.us wrote:
>Tue, Nov 08, 2022 at 09:11:49AM CET, idosch@idosch.org wrote:
>>On Mon, Nov 07, 2022 at 03:52:13PM +0100, Jiri Pirko wrote:
>>> 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")
>>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>>
>>Reviewed-by: Ido Schimmel <idosch@nvidia.com>
>>Tested-by: Ido Schimmel <idosch@nvidia.com>
>
>Sending v2 with cosmetical changes. Please put your tags there again.
Actually, this patch stays untouched. So I'll add it.
>Thanks!
>
>>
>>Thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-11-08 13:08 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-07 14:52 [patch net-next 0/2] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
2022-11-07 14:52 ` [patch net-next 1/2] net: introduce a helper to move notifier block to different namespace Jiri Pirko
2022-11-08 4:29 ` Jakub Kicinski
2022-11-08 6:53 ` Jiri Pirko
2022-11-07 14:52 ` [patch net-next 2/2] net: devlink: move netdev notifier block to dest namespace during reload Jiri Pirko
2022-11-07 16:52 ` Ido Schimmel
2022-11-07 17:24 ` Jiri Pirko
2022-11-08 8:11 ` Ido Schimmel
2022-11-08 12:59 ` Jiri Pirko
2022-11-08 13:07 ` Jiri Pirko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).