* [patch net-next 0/3] net: introduce per-netns netdevice notifiers and use them in mlxsw @ 2019-09-30 8:15 Jiri Pirko 2019-09-30 8:15 ` [patch net-next 1/3] net: push loops and nb calls into helper functions Jiri Pirko ` (3 more replies) 0 siblings, 4 replies; 10+ messages in thread From: Jiri Pirko @ 2019-09-30 8:15 UTC (permalink / raw) To: netdev Cc: davem, idosch, pabeni, edumazet, petrm, sd, f.fainelli, stephen, mlxsw From: Jiri Pirko <jiri@mellanox.com> Some drivers, like mlxsw, are not interested in notifications coming in for netdevices from other network namespaces. So introduce per-netns notifiers and allow to reduce overhead by listening only for notifications from the same netns. This is also a preparation for upcoming patchset "devlink: allow devlink instances to change network namespace". This resolves deadlock during reload mlxsw into initial netns made possible by 328fbe747ad4 ("net: Close race between {un, }register_netdevice_notifier() and setup_net()/cleanup_net()"). Jiri Pirko (3): net: push loops and nb calls into helper functions net: introduce per-netns netdevice notifiers mlxsw: spectrum: Use per-netns netdevice notifier registration .../net/ethernet/mellanox/mlxsw/spectrum.c | 9 +- include/linux/netdevice.h | 6 + include/net/net_namespace.h | 6 +- net/core/dev.c | 176 +++++++++++++++--- 4 files changed, 165 insertions(+), 32 deletions(-) -- 2.21.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [patch net-next 1/3] net: push loops and nb calls into helper functions 2019-09-30 8:15 [patch net-next 0/3] net: introduce per-netns netdevice notifiers and use them in mlxsw Jiri Pirko @ 2019-09-30 8:15 ` Jiri Pirko 2019-09-30 8:15 ` [patch net-next 2/3] net: introduce per-netns netdevice notifiers Jiri Pirko ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Jiri Pirko @ 2019-09-30 8:15 UTC (permalink / raw) To: netdev Cc: davem, idosch, pabeni, edumazet, petrm, sd, f.fainelli, stephen, mlxsw From: Jiri Pirko <jiri@mellanox.com> Push iterations over net namespaces and netdevices from register_netdevice_notifier() and unregister_netdevice_notifier() into helper functions. Along with that introduce continue_reverse macros to make the code a bit nicer allowing to get rid of "last" marks. Signed-off-by: Jiri Pirko <jiri@mellanox.com> --- include/linux/netdevice.h | 3 ++ include/net/net_namespace.h | 3 +- net/core/dev.c | 89 +++++++++++++++++++++++++------------ 3 files changed, 66 insertions(+), 29 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 9eda1c31d1f7..4f390eec106b 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2564,6 +2564,9 @@ extern rwlock_t dev_base_lock; /* Device list lock */ list_for_each_entry_safe(d, n, &(net)->dev_base_head, dev_list) #define for_each_netdev_continue(net, d) \ list_for_each_entry_continue(d, &(net)->dev_base_head, dev_list) +#define for_each_netdev_continue_reverse(net, d) \ + list_for_each_entry_continue_reverse(d, &(net)->dev_base_head, \ + dev_list) #define for_each_netdev_continue_rcu(net, d) \ list_for_each_entry_continue_rcu(d, &(net)->dev_base_head, dev_list) #define for_each_netdev_in_bond_rcu(bond, slave) \ diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h index f8712bbeb2e0..c5a98e03591d 100644 --- a/include/net/net_namespace.h +++ b/include/net/net_namespace.h @@ -317,7 +317,8 @@ static inline struct net *read_pnet(const possible_net_t *pnet) /* Protected by net_rwsem */ #define for_each_net(VAR) \ list_for_each_entry(VAR, &net_namespace_list, list) - +#define for_each_net_continue_reverse(VAR) \ + list_for_each_entry_continue_reverse(VAR, &net_namespace_list, list) #define for_each_net_rcu(VAR) \ list_for_each_entry_rcu(VAR, &net_namespace_list, list) diff --git a/net/core/dev.c b/net/core/dev.c index bf3ed413abaf..6a87d0e71201 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1617,6 +1617,62 @@ static int call_netdevice_notifier(struct notifier_block *nb, unsigned long val, return nb->notifier_call(nb, val, &info); } +static int call_netdevice_register_notifiers(struct notifier_block *nb, + struct net_device *dev) +{ + int err; + + err = call_netdevice_notifier(nb, NETDEV_REGISTER, dev); + err = notifier_to_errno(err); + if (err) + return err; + + if (!(dev->flags & IFF_UP)) + return 0; + + call_netdevice_notifier(nb, NETDEV_UP, dev); + return 0; +} + +static void call_netdevice_unregister_notifiers(struct notifier_block *nb, + struct net_device *dev) +{ + if (dev->flags & IFF_UP) { + call_netdevice_notifier(nb, NETDEV_GOING_DOWN, + dev); + call_netdevice_notifier(nb, NETDEV_DOWN, dev); + } + call_netdevice_notifier(nb, NETDEV_UNREGISTER, dev); +} + +static int call_netdevice_register_net_notifiers(struct notifier_block *nb, + struct net *net) +{ + struct net_device *dev; + int err; + + for_each_netdev(net, dev) { + err = call_netdevice_register_notifiers(nb, dev); + if (err) + goto rollback; + } + return 0; + +rollback: + for_each_netdev_continue_reverse(net, dev) + call_netdevice_unregister_notifiers(nb, dev); + return err; +} + +static void call_netdevice_unregister_net_notifiers(struct notifier_block *nb, + struct net *net) +{ + struct net_device *dev; + + for_each_netdev(net, dev) + call_netdevice_unregister_notifiers(nb, dev); +} + static int dev_boot_phase = 1; /** @@ -1635,8 +1691,6 @@ static int dev_boot_phase = 1; int register_netdevice_notifier(struct notifier_block *nb) { - struct net_device *dev; - struct net_device *last; struct net *net; int err; @@ -1649,17 +1703,9 @@ int register_netdevice_notifier(struct notifier_block *nb) if (dev_boot_phase) goto unlock; for_each_net(net) { - for_each_netdev(net, dev) { - err = call_netdevice_notifier(nb, NETDEV_REGISTER, dev); - err = notifier_to_errno(err); - if (err) - goto rollback; - - if (!(dev->flags & IFF_UP)) - continue; - - call_netdevice_notifier(nb, NETDEV_UP, dev); - } + err = call_netdevice_register_net_notifiers(nb, net); + if (err) + goto rollback; } unlock: @@ -1668,22 +1714,9 @@ int register_netdevice_notifier(struct notifier_block *nb) return err; rollback: - last = dev; - for_each_net(net) { - for_each_netdev(net, dev) { - if (dev == last) - goto outroll; - - if (dev->flags & IFF_UP) { - call_netdevice_notifier(nb, NETDEV_GOING_DOWN, - dev); - call_netdevice_notifier(nb, NETDEV_DOWN, dev); - } - call_netdevice_notifier(nb, NETDEV_UNREGISTER, dev); - } - } + for_each_net_continue_reverse(net) + call_netdevice_unregister_net_notifiers(nb, net); -outroll: raw_notifier_chain_unregister(&netdev_chain, nb); goto unlock; } -- 2.21.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [patch net-next 2/3] net: introduce per-netns netdevice notifiers 2019-09-30 8:15 [patch net-next 0/3] net: introduce per-netns netdevice notifiers and use them in mlxsw Jiri Pirko 2019-09-30 8:15 ` [patch net-next 1/3] net: push loops and nb calls into helper functions Jiri Pirko @ 2019-09-30 8:15 ` Jiri Pirko 2019-09-30 13:38 ` Andrew Lunn 2019-10-02 15:47 ` David Miller 2019-09-30 8:15 ` [patch net-next 3/3] mlxsw: spectrum: Use per-netns netdevice notifier registration Jiri Pirko 2019-10-02 15:52 ` [patch net-next 0/3] net: introduce per-netns netdevice notifiers and use them in mlxsw David Miller 3 siblings, 2 replies; 10+ messages in thread From: Jiri Pirko @ 2019-09-30 8:15 UTC (permalink / raw) To: netdev Cc: davem, idosch, pabeni, edumazet, petrm, sd, f.fainelli, stephen, mlxsw From: Jiri Pirko <jiri@mellanox.com> Often the code for example in drivers is interested in getting notifier call only from certain network namespace. In addition to the existing global netdevice notifier chain introduce per-netns chains and allow users to register to that. Eventually this would eliminate unnecessary overhead in case there are many netdevices in many network namespaces. Signed-off-by: Jiri Pirko <jiri@mellanox.com> --- include/linux/netdevice.h | 3 ++ include/net/net_namespace.h | 3 ++ net/core/dev.c | 87 +++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 4f390eec106b..184f54f1b9e1 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2494,6 +2494,9 @@ const char *netdev_cmd_to_name(enum netdev_cmd cmd); int register_netdevice_notifier(struct notifier_block *nb); 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); struct netdev_notifier_info { struct net_device *dev; diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h index c5a98e03591d..5ac2bb16d4b3 100644 --- a/include/net/net_namespace.h +++ b/include/net/net_namespace.h @@ -36,6 +36,7 @@ #include <linux/ns_common.h> #include <linux/idr.h> #include <linux/skbuff.h> +#include <linux/notifier.h> struct user_namespace; struct proc_dir_entry; @@ -96,6 +97,8 @@ struct net { struct list_head dev_base_head; struct hlist_head *dev_name_head; struct hlist_head *dev_index_head; + struct raw_notifier_head netdev_chain; + unsigned int dev_base_seq; /* protected by rtnl_mutex */ int ifindex; unsigned int dev_unreg_count; diff --git a/net/core/dev.c b/net/core/dev.c index 6a87d0e71201..3302cefd3041 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1766,6 +1766,80 @@ int unregister_netdevice_notifier(struct notifier_block *nb) } EXPORT_SYMBOL(unregister_netdevice_notifier); +/** + * register_netdevice_notifier_net - register a per-netns network notifier block + * @net: network namespace + * @nb: notifier + * + * Register a notifier to be called when network device events occur. + * The notifier passed is linked into the kernel structures and must + * not be reused until it has been unregistered. A negative errno code + * is returned on a failure. + * + * When registered all registration and up events are replayed + * to the new notifier to allow device to have a race free + * view of the network device list. + */ + +int register_netdevice_notifier_net(struct net *net, struct notifier_block *nb) +{ + int err; + + rtnl_lock(); + err = raw_notifier_chain_register(&net->netdev_chain, nb); + if (err) + goto unlock; + if (dev_boot_phase) + goto unlock; + + err = call_netdevice_register_net_notifiers(nb, net); + if (err) + goto chain_unregister; + +unlock: + rtnl_unlock(); + return err; + +chain_unregister: + raw_notifier_chain_unregister(&netdev_chain, nb); + goto unlock; +} +EXPORT_SYMBOL(register_netdevice_notifier_net); + +/** + * unregister_netdevice_notifier_net - unregister a per-netns + * network notifier block + * @net: network namespace + * @nb: notifier + * + * Unregister a notifier previously registered by + * register_netdevice_notifier(). The notifier is unlinked into the + * kernel structures and may then be reused. A negative errno code + * is returned on a failure. + * + * After unregistering unregister and down device events are synthesized + * for all devices on the device list to the removed notifier to remove + * the need for special case cleanup code. + */ + +int unregister_netdevice_notifier_net(struct net *net, + struct notifier_block *nb) +{ + int err; + + rtnl_lock(); + err = raw_notifier_chain_unregister(&net->netdev_chain, nb); + if (err) + goto unlock; + + call_netdevice_unregister_net_notifiers(nb, net); + +unlock: + rtnl_unlock(); + return err; +} +EXPORT_SYMBOL(unregister_netdevice_notifier_net); + /** * call_netdevice_notifiers_info - call all network notifier blocks * @val: value passed unmodified to notifier function @@ -1778,7 +1852,18 @@ EXPORT_SYMBOL(unregister_netdevice_notifier); static int call_netdevice_notifiers_info(unsigned long val, struct netdev_notifier_info *info) { + struct net *net = dev_net(info->dev); + int ret; + ASSERT_RTNL(); + + /* Run per-netns notifier block chain first, then run the global one. + * Hopefully, one day, the global one is going to be removed after + * all notifier block registrators get converted to be per-netns. + */ + ret = raw_notifier_call_chain(&net->netdev_chain, val, info); + if (ret & NOTIFY_STOP_MASK) + return ret; return raw_notifier_call_chain(&netdev_chain, val, info); } @@ -9668,6 +9753,8 @@ static int __net_init netdev_init(struct net *net) if (net->dev_index_head == NULL) goto err_idx; + RAW_INIT_NOTIFIER_HEAD(&net->netdev_chain); + return 0; err_idx: -- 2.21.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [patch net-next 2/3] net: introduce per-netns netdevice notifiers 2019-09-30 8:15 ` [patch net-next 2/3] net: introduce per-netns netdevice notifiers Jiri Pirko @ 2019-09-30 13:38 ` Andrew Lunn 2019-09-30 14:23 ` Jiri Pirko 2019-10-02 15:47 ` David Miller 1 sibling, 1 reply; 10+ messages in thread From: Andrew Lunn @ 2019-09-30 13:38 UTC (permalink / raw) To: Jiri Pirko Cc: netdev, davem, idosch, pabeni, edumazet, petrm, sd, f.fainelli, stephen, mlxsw > static int call_netdevice_notifiers_info(unsigned long val, > struct netdev_notifier_info *info) > { > + struct net *net = dev_net(info->dev); > + int ret; > + > ASSERT_RTNL(); > + > + /* Run per-netns notifier block chain first, then run the global one. > + * Hopefully, one day, the global one is going to be removed after > + * all notifier block registrators get converted to be per-netns. > + */ Hi Jiri Is that really going to happen? register_netdevice_notifier() is used in 130 files. Do you plan to spend the time to make it happen? > + ret = raw_notifier_call_chain(&net->netdev_chain, val, info); > + if (ret & NOTIFY_STOP_MASK) > + return ret; > return raw_notifier_call_chain(&netdev_chain, val, info); > } Humm. I wonder about NOTIFY_STOP_MASK here. These are two separate chains. Should one chain be able to stop the other chain? Are there other examples where NOTIFY_STOP_MASK crosses a chain boundary? Andrew ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch net-next 2/3] net: introduce per-netns netdevice notifiers 2019-09-30 13:38 ` Andrew Lunn @ 2019-09-30 14:23 ` Jiri Pirko 2019-09-30 15:33 ` Andrew Lunn 0 siblings, 1 reply; 10+ messages in thread From: Jiri Pirko @ 2019-09-30 14:23 UTC (permalink / raw) To: Andrew Lunn Cc: netdev, davem, idosch, pabeni, edumazet, petrm, sd, f.fainelli, stephen, mlxsw Mon, Sep 30, 2019 at 03:38:24PM CEST, andrew@lunn.ch wrote: >> static int call_netdevice_notifiers_info(unsigned long val, >> struct netdev_notifier_info *info) >> { >> + struct net *net = dev_net(info->dev); >> + int ret; >> + >> ASSERT_RTNL(); >> + >> + /* Run per-netns notifier block chain first, then run the global one. >> + * Hopefully, one day, the global one is going to be removed after >> + * all notifier block registrators get converted to be per-netns. >> + */ > >Hi Jiri > >Is that really going to happen? register_netdevice_notifier() is used >in 130 files. Do you plan to spend the time to make it happen? That's why I prepended the sentency with "Hopefully, one day"... > >> + ret = raw_notifier_call_chain(&net->netdev_chain, val, info); >> + if (ret & NOTIFY_STOP_MASK) >> + return ret; >> return raw_notifier_call_chain(&netdev_chain, val, info); >> } > >Humm. I wonder about NOTIFY_STOP_MASK here. These are two separate >chains. Should one chain be able to stop the other chain? Are there Well if the failing item would be in the second chain, at the beginning of it, it would be stopped too. Does not matter where the stop happens, the point is that the whole processing stops. That is why I added the check here. >other examples where NOTIFY_STOP_MASK crosses a chain boundary? Not aware of it, no. Could you please describe what is wrong? > > Andrew ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch net-next 2/3] net: introduce per-netns netdevice notifiers 2019-09-30 14:23 ` Jiri Pirko @ 2019-09-30 15:33 ` Andrew Lunn 2019-09-30 18:01 ` Jiri Pirko 0 siblings, 1 reply; 10+ messages in thread From: Andrew Lunn @ 2019-09-30 15:33 UTC (permalink / raw) To: Jiri Pirko Cc: netdev, davem, idosch, pabeni, edumazet, petrm, sd, f.fainelli, stephen, mlxsw On Mon, Sep 30, 2019 at 04:23:49PM +0200, Jiri Pirko wrote: > Mon, Sep 30, 2019 at 03:38:24PM CEST, andrew@lunn.ch wrote: > >> static int call_netdevice_notifiers_info(unsigned long val, > >> struct netdev_notifier_info *info) > >> { > >> + struct net *net = dev_net(info->dev); > >> + int ret; > >> + > >> ASSERT_RTNL(); > >> + > >> + /* Run per-netns notifier block chain first, then run the global one. > >> + * Hopefully, one day, the global one is going to be removed after > >> + * all notifier block registrators get converted to be per-netns. > >> + */ > > > >Hi Jiri > > > >Is that really going to happen? register_netdevice_notifier() is used > >in 130 files. Do you plan to spend the time to make it happen? > > That's why I prepended the sentency with "Hopefully, one day"... > > > > > >> + ret = raw_notifier_call_chain(&net->netdev_chain, val, info); > >> + if (ret & NOTIFY_STOP_MASK) > >> + return ret; > >> return raw_notifier_call_chain(&netdev_chain, val, info); > >> } > > > >Humm. I wonder about NOTIFY_STOP_MASK here. These are two separate > >chains. Should one chain be able to stop the other chain? Are there > > Well if the failing item would be in the second chain, at the beginning > of it, it would be stopped too. Does not matter where the stop happens, > the point is that the whole processing stops. That is why I added the > check here. > > > >other examples where NOTIFY_STOP_MASK crosses a chain boundary? > > Not aware of it, no. Could you please describe what is wrong? You are expanding the meaning of NOTIFY_STOP_MASK. It now can stop some other chain. If this was one chain with a filter, i would not be asking. But this is two different chains, and one chain can stop another? At minimum, i think this needs to be reviewed by the core kernel people. But i'm also wondering if you are solving the problem at the wrong level. Are there other notifier chains which would benefit from respecting name space boundaries? Would a better solution be to extend struct notifier_block with some sort of filter? Do you have some performance numbers? Where are you getting your performance gains from? By the fact you are doing NOTIFY_STOP_MASK earlier, so preventing a long chain being walked? I notice notifer_block has a priority field. Did you try using that to put your notified earlier on the chain? Andrew ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch net-next 2/3] net: introduce per-netns netdevice notifiers 2019-09-30 15:33 ` Andrew Lunn @ 2019-09-30 18:01 ` Jiri Pirko 0 siblings, 0 replies; 10+ messages in thread From: Jiri Pirko @ 2019-09-30 18:01 UTC (permalink / raw) To: Andrew Lunn Cc: netdev, davem, idosch, pabeni, edumazet, petrm, sd, f.fainelli, stephen, mlxsw Mon, Sep 30, 2019 at 05:33:43PM CEST, andrew@lunn.ch wrote: >On Mon, Sep 30, 2019 at 04:23:49PM +0200, Jiri Pirko wrote: >> Mon, Sep 30, 2019 at 03:38:24PM CEST, andrew@lunn.ch wrote: >> >> static int call_netdevice_notifiers_info(unsigned long val, >> >> struct netdev_notifier_info *info) >> >> { >> >> + struct net *net = dev_net(info->dev); >> >> + int ret; >> >> + >> >> ASSERT_RTNL(); >> >> + >> >> + /* Run per-netns notifier block chain first, then run the global one. >> >> + * Hopefully, one day, the global one is going to be removed after >> >> + * all notifier block registrators get converted to be per-netns. >> >> + */ >> > >> >Hi Jiri >> > >> >Is that really going to happen? register_netdevice_notifier() is used >> >in 130 files. Do you plan to spend the time to make it happen? >> >> That's why I prepended the sentency with "Hopefully, one day"... >> >> >> > >> >> + ret = raw_notifier_call_chain(&net->netdev_chain, val, info); >> >> + if (ret & NOTIFY_STOP_MASK) >> >> + return ret; >> >> return raw_notifier_call_chain(&netdev_chain, val, info); >> >> } >> > >> >Humm. I wonder about NOTIFY_STOP_MASK here. These are two separate >> >chains. Should one chain be able to stop the other chain? Are there >> >> Well if the failing item would be in the second chain, at the beginning >> of it, it would be stopped too. Does not matter where the stop happens, >> the point is that the whole processing stops. That is why I added the >> check here. >> >> >> >other examples where NOTIFY_STOP_MASK crosses a chain boundary? >> >> Not aware of it, no. Could you please describe what is wrong? > >You are expanding the meaning of NOTIFY_STOP_MASK. It now can stop >some other chain. If this was one chain with a filter, i would not be Well, it was originally a single chain, so the semantics stays intact. Again, it is not some other independent chain. It's just netns one and general one, both serve the same purpose. >asking. But this is two different chains, and one chain can stop >another? At minimum, i think this needs to be reviewed by the core >kernel people. > >But i'm also wondering if you are solving the problem at the wrong >level. Are there other notifier chains which would benefit from >respecting name space boundaries? Would a better solution be to extend >struct notifier_block with some sort of filter? I mentioned my primary motivation in the cover letter. What I want to avoid is need of taking &pernet_ops_rwsem during registration of tne notifier and avoid deadlock in my usecase. Plus it seems very clear that if a notifier knows what netns is he interested in, he just registers in that particular netns chain. Having one fat generic chain with filters is basically what we have right now. > >Do you have some performance numbers? Where are you getting your >performance gains from? By the fact you are doing NOTIFY_STOP_MASK >earlier, so preventing a long chain being walked? I notice >notifer_block has a priority field. Did you try using that to put your >notified earlier on the chain? It is not about stopping the chain earlier, not at all. It is the fact that with many netdevices in many network namespaces you gat a lot of wasted calls to notifiers registators that does not care. > > Andrew ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch net-next 2/3] net: introduce per-netns netdevice notifiers 2019-09-30 8:15 ` [patch net-next 2/3] net: introduce per-netns netdevice notifiers Jiri Pirko 2019-09-30 13:38 ` Andrew Lunn @ 2019-10-02 15:47 ` David Miller 1 sibling, 0 replies; 10+ messages in thread From: David Miller @ 2019-10-02 15:47 UTC (permalink / raw) To: jiri Cc: netdev, idosch, pabeni, edumazet, petrm, sd, f.fainelli, stephen, mlxsw, andrew From: Jiri Pirko <jiri@resnulli.us> Date: Mon, 30 Sep 2019 10:15:10 +0200 > From: Jiri Pirko <jiri@mellanox.com> > > Often the code for example in drivers is interested in getting notifier > call only from certain network namespace. In addition to the existing > global netdevice notifier chain introduce per-netns chains and allow > users to register to that. Eventually this would eliminate unnecessary > overhead in case there are many netdevices in many network namespaces. > > Signed-off-by: Jiri Pirko <jiri@mellanox.com> Ok, so there was a discussion about stop semantics. Honestly, I think that's fine. Stop means the operation cannot be performed and whoever is firing off the notifier will have to fail and undo the config change being attempted. In that context, it doesn't matter who or where in the chain we trigger the stop. Given all of that I am pretty sure this change is fine and I will add it to net-next. We can fix any actual semantic problems this might introduce as a follow-on. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [patch net-next 3/3] mlxsw: spectrum: Use per-netns netdevice notifier registration 2019-09-30 8:15 [patch net-next 0/3] net: introduce per-netns netdevice notifiers and use them in mlxsw Jiri Pirko 2019-09-30 8:15 ` [patch net-next 1/3] net: push loops and nb calls into helper functions Jiri Pirko 2019-09-30 8:15 ` [patch net-next 2/3] net: introduce per-netns netdevice notifiers Jiri Pirko @ 2019-09-30 8:15 ` Jiri Pirko 2019-10-02 15:52 ` [patch net-next 0/3] net: introduce per-netns netdevice notifiers and use them in mlxsw David Miller 3 siblings, 0 replies; 10+ messages in thread From: Jiri Pirko @ 2019-09-30 8:15 UTC (permalink / raw) To: netdev Cc: davem, idosch, pabeni, edumazet, petrm, sd, f.fainelli, stephen, mlxsw From: Jiri Pirko <jiri@mellanox.com> The mlxsw_sp instance is not interested in events happening in other network namespaces. So use "_net" variants for netdevice notifier registration/unregistration and get only events which are happening in the net the instance is in. Signed-off-by: Jiri Pirko <jiri@mellanox.com> --- drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c index dcf9562bce8a..a54a0dc82ff2 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c @@ -4864,7 +4864,8 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core, * respin. */ mlxsw_sp->netdevice_nb.notifier_call = mlxsw_sp_netdevice_event; - err = register_netdevice_notifier(&mlxsw_sp->netdevice_nb); + err = register_netdevice_notifier_net(&init_net, + &mlxsw_sp->netdevice_nb); if (err) { dev_err(mlxsw_sp->bus_info->dev, "Failed to register netdev notifier\n"); goto err_netdev_notifier; @@ -4887,7 +4888,8 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core, err_ports_create: mlxsw_sp_dpipe_fini(mlxsw_sp); err_dpipe_init: - unregister_netdevice_notifier(&mlxsw_sp->netdevice_nb); + unregister_netdevice_notifier_net(&init_net, + &mlxsw_sp->netdevice_nb); err_netdev_notifier: if (mlxsw_sp->clock) mlxsw_sp->ptp_ops->fini(mlxsw_sp->ptp_state); @@ -4973,7 +4975,8 @@ static void mlxsw_sp_fini(struct mlxsw_core *mlxsw_core) mlxsw_sp_ports_remove(mlxsw_sp); mlxsw_sp_dpipe_fini(mlxsw_sp); - unregister_netdevice_notifier(&mlxsw_sp->netdevice_nb); + unregister_netdevice_notifier_net(&init_net, + &mlxsw_sp->netdevice_nb); if (mlxsw_sp->clock) { mlxsw_sp->ptp_ops->fini(mlxsw_sp->ptp_state); mlxsw_sp->ptp_ops->clock_fini(mlxsw_sp->clock); -- 2.21.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [patch net-next 0/3] net: introduce per-netns netdevice notifiers and use them in mlxsw 2019-09-30 8:15 [patch net-next 0/3] net: introduce per-netns netdevice notifiers and use them in mlxsw Jiri Pirko ` (2 preceding siblings ...) 2019-09-30 8:15 ` [patch net-next 3/3] mlxsw: spectrum: Use per-netns netdevice notifier registration Jiri Pirko @ 2019-10-02 15:52 ` David Miller 3 siblings, 0 replies; 10+ messages in thread From: David Miller @ 2019-10-02 15:52 UTC (permalink / raw) To: jiri Cc: netdev, idosch, pabeni, edumazet, petrm, sd, f.fainelli, stephen, mlxsw From: Jiri Pirko <jiri@resnulli.us> Date: Mon, 30 Sep 2019 10:15:08 +0200 > From: Jiri Pirko <jiri@mellanox.com> > > Some drivers, like mlxsw, are not interested in notifications coming in > for netdevices from other network namespaces. So introduce per-netns > notifiers and allow to reduce overhead by listening only for > notifications from the same netns. > > This is also a preparation for upcoming patchset "devlink: allow devlink > instances to change network namespace". This resolves deadlock during > reload mlxsw into initial netns made possible by > 328fbe747ad4 ("net: Close race between {un, }register_netdevice_notifier() and setup_net()/cleanup_net()"). Series applied. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-10-02 15:52 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-30 8:15 [patch net-next 0/3] net: introduce per-netns netdevice notifiers and use them in mlxsw Jiri Pirko 2019-09-30 8:15 ` [patch net-next 1/3] net: push loops and nb calls into helper functions Jiri Pirko 2019-09-30 8:15 ` [patch net-next 2/3] net: introduce per-netns netdevice notifiers Jiri Pirko 2019-09-30 13:38 ` Andrew Lunn 2019-09-30 14:23 ` Jiri Pirko 2019-09-30 15:33 ` Andrew Lunn 2019-09-30 18:01 ` Jiri Pirko 2019-10-02 15:47 ` David Miller 2019-09-30 8:15 ` [patch net-next 3/3] mlxsw: spectrum: Use per-netns netdevice notifier registration Jiri Pirko 2019-10-02 15:52 ` [patch net-next 0/3] net: introduce per-netns netdevice notifiers and use them in mlxsw David Miller
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).