* [PATCH RESEND net-next 0/2] net_rwsem fixes
@ 2018-03-30 16:38 Kirill Tkhai
2018-03-30 16:38 ` [PATCH RESEND net-next 1/2] net: Remove net_rwsem from {, un}register_netdevice_notifier() Kirill Tkhai
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Kirill Tkhai @ 2018-03-30 16:38 UTC (permalink / raw)
To: davem, zhangshengju, ktkhai, jakub.kicinski, dsahern, mschiffer,
daniel, netdev
(Forgot to CC netdev@ :)
Hi,
there is wext_netdev_notifier_call()->wireless_nlevent_flush()
netdevice notifier, which takes net_rwsem, so we can't take
net_rwsem in {,un}register_netdevice_notifier().
Since {,un}register_netdevice_notifier() is executed under
pernet_ops_rwsem, net_namespace_list can't change, while we
holding it, so there is no need net_rwsem in these functions [1/2].
The same is in [2/2]. We make callers of __rtnl_link_unregister()
take pernet_ops_rwsem, and close the race with setup_net()
and cleanup_net(), so __rtnl_link_unregister() does not need it.
This also fixes the problem of that __rtnl_link_unregister() does
not see initializing and exiting nets.
Thanks,
Kirill
---
Kirill Tkhai (2):
net: Remove net_rwsem from {,un}register_netdevice_notifier()
net: Do not take net_rwsem in __rtnl_link_unregister()
drivers/net/dummy.c | 2 ++
drivers/net/ifb.c | 2 ++
net/core/dev.c | 5 -----
net/core/net_namespace.c | 1 +
net/core/rtnetlink.c | 6 +++---
5 files changed, 8 insertions(+), 8 deletions(-)
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH RESEND net-next 1/2] net: Remove net_rwsem from {, un}register_netdevice_notifier()
2018-03-30 16:38 [PATCH RESEND net-next 0/2] net_rwsem fixes Kirill Tkhai
@ 2018-03-30 16:38 ` Kirill Tkhai
2018-03-30 16:38 ` [PATCH RESEND net-next 2/2] net: Do not take net_rwsem in __rtnl_link_unregister() Kirill Tkhai
2018-04-01 2:25 ` [PATCH RESEND net-next 0/2] net_rwsem fixes David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Kirill Tkhai @ 2018-03-30 16:38 UTC (permalink / raw)
To: davem, zhangshengju, ktkhai, jakub.kicinski, dsahern, mschiffer,
daniel, netdev
These functions take net_rwsem, while wireless_nlevent_flush()
also takes it. But down_read() can't be taken recursive,
because of rw_semaphore design, which prevents it to be occupied
by only readers forever.
Since we take pernet_ops_rwsem in {,un}register_netdevice_notifier(),
net list can't change, so these down_read()/up_read() can be removed.
Fixes: f0b07bb151b0 "net: Introduce net_rwsem to protect net_namespace_list"
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
net/core/dev.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 07da7add4845..8edb58829124 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1633,7 +1633,6 @@ int register_netdevice_notifier(struct notifier_block *nb)
goto unlock;
if (dev_boot_phase)
goto unlock;
- down_read(&net_rwsem);
for_each_net(net) {
for_each_netdev(net, dev) {
err = call_netdevice_notifier(nb, NETDEV_REGISTER, dev);
@@ -1647,7 +1646,6 @@ int register_netdevice_notifier(struct notifier_block *nb)
call_netdevice_notifier(nb, NETDEV_UP, dev);
}
}
- up_read(&net_rwsem);
unlock:
rtnl_unlock();
@@ -1671,7 +1669,6 @@ int register_netdevice_notifier(struct notifier_block *nb)
}
outroll:
- up_read(&net_rwsem);
raw_notifier_chain_unregister(&netdev_chain, nb);
goto unlock;
}
@@ -1704,7 +1701,6 @@ int unregister_netdevice_notifier(struct notifier_block *nb)
if (err)
goto unlock;
- down_read(&net_rwsem);
for_each_net(net) {
for_each_netdev(net, dev) {
if (dev->flags & IFF_UP) {
@@ -1715,7 +1711,6 @@ int unregister_netdevice_notifier(struct notifier_block *nb)
call_netdevice_notifier(nb, NETDEV_UNREGISTER, dev);
}
}
- up_read(&net_rwsem);
unlock:
rtnl_unlock();
up_write(&pernet_ops_rwsem);
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH RESEND net-next 2/2] net: Do not take net_rwsem in __rtnl_link_unregister()
2018-03-30 16:38 [PATCH RESEND net-next 0/2] net_rwsem fixes Kirill Tkhai
2018-03-30 16:38 ` [PATCH RESEND net-next 1/2] net: Remove net_rwsem from {, un}register_netdevice_notifier() Kirill Tkhai
@ 2018-03-30 16:38 ` Kirill Tkhai
2018-04-01 2:25 ` [PATCH RESEND net-next 0/2] net_rwsem fixes David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Kirill Tkhai @ 2018-03-30 16:38 UTC (permalink / raw)
To: davem, zhangshengju, ktkhai, jakub.kicinski, dsahern, mschiffer,
daniel, netdev
This function calls call_netdevice_notifier(), which also
may take net_rwsem. So, we can't use net_rwsem here.
This patch makes callers of this functions take pernet_ops_rwsem,
like register_netdevice_notifier() does. This will protect
the modifications of net_namespace_list, and allows notifiers
to take it (they won't have to care about context).
Since __rtnl_link_unregister() is used on module load
and unload (which are not frequent operations), this looks
for me better, than make all call_netdevice_notifier()
always executing in "protected net_namespace_list" context.
Also, this fixes the problem we had a deal in 328fbe747ad4
"Close race between {un, }register_netdevice_notifier and ...",
and guarantees __rtnl_link_unregister() does not skip
exitting net.
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
drivers/net/dummy.c | 2 ++
drivers/net/ifb.c | 2 ++
net/core/net_namespace.c | 1 +
net/core/rtnetlink.c | 6 +++---
4 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index 30b1c8512049..0d15a12a4560 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c
@@ -219,6 +219,7 @@ static int __init dummy_init_module(void)
{
int i, err = 0;
+ down_write(&pernet_ops_rwsem);
rtnl_lock();
err = __rtnl_link_register(&dummy_link_ops);
if (err < 0)
@@ -233,6 +234,7 @@ static int __init dummy_init_module(void)
out:
rtnl_unlock();
+ up_write(&pernet_ops_rwsem);
return err;
}
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index 0008da7e9d4c..5f2897ec0edc 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -330,6 +330,7 @@ static int __init ifb_init_module(void)
{
int i, err;
+ down_write(&pernet_ops_rwsem);
rtnl_lock();
err = __rtnl_link_register(&ifb_link_ops);
if (err < 0)
@@ -344,6 +345,7 @@ static int __init ifb_init_module(void)
out:
rtnl_unlock();
+ up_write(&pernet_ops_rwsem);
return err;
}
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 7fdf321d4997..a11e03f920d3 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -51,6 +51,7 @@ static bool init_net_initialized;
* outside.
*/
DECLARE_RWSEM(pernet_ops_rwsem);
+EXPORT_SYMBOL_GPL(pernet_ops_rwsem);
#define MIN_PERNET_OPS_ID \
((sizeof(struct net_generic) + sizeof(void *) - 1) / sizeof(void *))
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index e86b28482ca7..45936922d7e2 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -412,17 +412,17 @@ static void __rtnl_kill_links(struct net *net, struct rtnl_link_ops *ops)
* __rtnl_link_unregister - Unregister rtnl_link_ops from rtnetlink.
* @ops: struct rtnl_link_ops * to unregister
*
- * The caller must hold the rtnl_mutex.
+ * The caller must hold the rtnl_mutex and guarantee net_namespace_list
+ * integrity (hold pernet_ops_rwsem for writing to close the race
+ * with setup_net() and cleanup_net()).
*/
void __rtnl_link_unregister(struct rtnl_link_ops *ops)
{
struct net *net;
- down_read(&net_rwsem);
for_each_net(net) {
__rtnl_kill_links(net, ops);
}
- up_read(&net_rwsem);
list_del(&ops->list);
}
EXPORT_SYMBOL_GPL(__rtnl_link_unregister);
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND net-next 0/2] net_rwsem fixes
2018-03-30 16:38 [PATCH RESEND net-next 0/2] net_rwsem fixes Kirill Tkhai
2018-03-30 16:38 ` [PATCH RESEND net-next 1/2] net: Remove net_rwsem from {, un}register_netdevice_notifier() Kirill Tkhai
2018-03-30 16:38 ` [PATCH RESEND net-next 2/2] net: Do not take net_rwsem in __rtnl_link_unregister() Kirill Tkhai
@ 2018-04-01 2:25 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2018-04-01 2:25 UTC (permalink / raw)
To: ktkhai; +Cc: zhangshengju, jakub.kicinski, dsahern, mschiffer, daniel, netdev
From: Kirill Tkhai <ktkhai@virtuozzo.com>
Date: Fri, 30 Mar 2018 19:38:16 +0300
> there is wext_netdev_notifier_call()->wireless_nlevent_flush()
> netdevice notifier, which takes net_rwsem, so we can't take
> net_rwsem in {,un}register_netdevice_notifier().
>
> Since {,un}register_netdevice_notifier() is executed under
> pernet_ops_rwsem, net_namespace_list can't change, while we
> holding it, so there is no need net_rwsem in these functions [1/2].
>
> The same is in [2/2]. We make callers of __rtnl_link_unregister()
> take pernet_ops_rwsem, and close the race with setup_net()
> and cleanup_net(), so __rtnl_link_unregister() does not need it.
> This also fixes the problem of that __rtnl_link_unregister() does
> not see initializing and exiting nets.
Series applied.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-04-01 2:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-30 16:38 [PATCH RESEND net-next 0/2] net_rwsem fixes Kirill Tkhai
2018-03-30 16:38 ` [PATCH RESEND net-next 1/2] net: Remove net_rwsem from {, un}register_netdevice_notifier() Kirill Tkhai
2018-03-30 16:38 ` [PATCH RESEND net-next 2/2] net: Do not take net_rwsem in __rtnl_link_unregister() Kirill Tkhai
2018-04-01 2:25 ` [PATCH RESEND net-next 0/2] net_rwsem fixes 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).