Commit 50624c93 authored by Eric W. Biederman's avatar Eric W. Biederman Committed by David S. Miller

net: Delay default_device_exit_batch until no devices are unregistering v2

There is currently serialization network namespaces exiting and
network devices exiting as the final part of netdev_run_todo does not
happen under the rtnl_lock.  This is compounded by the fact that the
only list of devices unregistering in netdev_run_todo is local to the
netdev_run_todo.

This lack of serialization in extreme cases results in network devices
unregistering in netdev_run_todo after the loopback device of their
network namespace has been freed (making dst_ifdown unsafe), and after
the their network namespace has exited (making the NETDEV_UNREGISTER,
and NETDEV_UNREGISTER_FINAL callbacks unsafe).

Add the missing serialization by a per network namespace count of how
many network devices are unregistering and having a wait queue that is
woken up whenever the count is decreased.  The count and wait queue
allow default_device_exit_batch to wait until all of the unregistration
activity for a network namespace has finished before proceeding to
unregister the loopback device and then allowing the network namespace
to exit.

Only a single global wait queue is used because there is a single global
lock, and there is a single waiter, per network namespace wait queues
would be a waste of resources.

The per network namespace count of unregistering devices gives a
progress guarantee because the number of network devices unregistering
in an exiting network namespace must ultimately drop to zero (assuming
network device unregistration completes).

The basic logic remains the same as in v1.  This patch is now half
comment and half rtnl_lock_unregistering an expanded version of
wait_event performs no extra work in the common case where no network
devices are unregistering when we get to default_device_exit_batch.
Reported-by: default avatarFrancesco Ruggeri <fruggeri@aristanetworks.com>
Signed-off-by: default avatar"Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 7df37ff3
...@@ -74,6 +74,7 @@ struct net { ...@@ -74,6 +74,7 @@ struct net {
struct hlist_head *dev_index_head; struct hlist_head *dev_index_head;
unsigned int dev_base_seq; /* protected by rtnl_mutex */ unsigned int dev_base_seq; /* protected by rtnl_mutex */
int ifindex; int ifindex;
unsigned int dev_unreg_count;
/* core fib_rules */ /* core fib_rules */
struct list_head rules_ops; struct list_head rules_ops;
......
...@@ -5247,10 +5247,12 @@ static int dev_new_index(struct net *net) ...@@ -5247,10 +5247,12 @@ static int dev_new_index(struct net *net)
/* Delayed registration/unregisteration */ /* Delayed registration/unregisteration */
static LIST_HEAD(net_todo_list); static LIST_HEAD(net_todo_list);
static DECLARE_WAIT_QUEUE_HEAD(netdev_unregistering_wq);
static void net_set_todo(struct net_device *dev) static void net_set_todo(struct net_device *dev)
{ {
list_add_tail(&dev->todo_list, &net_todo_list); list_add_tail(&dev->todo_list, &net_todo_list);
dev_net(dev)->dev_unreg_count++;
} }
static void rollback_registered_many(struct list_head *head) static void rollback_registered_many(struct list_head *head)
...@@ -5918,6 +5920,12 @@ void netdev_run_todo(void) ...@@ -5918,6 +5920,12 @@ void netdev_run_todo(void)
if (dev->destructor) if (dev->destructor)
dev->destructor(dev); dev->destructor(dev);
/* Report a network device has been unregistered */
rtnl_lock();
dev_net(dev)->dev_unreg_count--;
__rtnl_unlock();
wake_up(&netdev_unregistering_wq);
/* Free network device */ /* Free network device */
kobject_put(&dev->dev.kobj); kobject_put(&dev->dev.kobj);
} }
...@@ -6603,6 +6611,34 @@ static void __net_exit default_device_exit(struct net *net) ...@@ -6603,6 +6611,34 @@ static void __net_exit default_device_exit(struct net *net)
rtnl_unlock(); rtnl_unlock();
} }
static void __net_exit rtnl_lock_unregistering(struct list_head *net_list)
{
/* Return with the rtnl_lock held when there are no network
* devices unregistering in any network namespace in net_list.
*/
struct net *net;
bool unregistering;
DEFINE_WAIT(wait);
for (;;) {
prepare_to_wait(&netdev_unregistering_wq, &wait,
TASK_UNINTERRUPTIBLE);
unregistering = false;
rtnl_lock();
list_for_each_entry(net, net_list, exit_list) {
if (net->dev_unreg_count > 0) {
unregistering = true;
break;
}
}
if (!unregistering)
break;
__rtnl_unlock();
schedule();
}
finish_wait(&netdev_unregistering_wq, &wait);
}
static void __net_exit default_device_exit_batch(struct list_head *net_list) static void __net_exit default_device_exit_batch(struct list_head *net_list)
{ {
/* At exit all network devices most be removed from a network /* At exit all network devices most be removed from a network
...@@ -6614,7 +6650,18 @@ static void __net_exit default_device_exit_batch(struct list_head *net_list) ...@@ -6614,7 +6650,18 @@ static void __net_exit default_device_exit_batch(struct list_head *net_list)
struct net *net; struct net *net;
LIST_HEAD(dev_kill_list); LIST_HEAD(dev_kill_list);
rtnl_lock(); /* To prevent network device cleanup code from dereferencing
* loopback devices or network devices that have been freed
* wait here for all pending unregistrations to complete,
* before unregistring the loopback device and allowing the
* network namespace be freed.
*
* The netdev todo list containing all network devices
* unregistrations that happen in default_device_exit_batch
* will run in the rtnl_unlock() at the end of
* default_device_exit_batch.
*/
rtnl_lock_unregistering(net_list);
list_for_each_entry(net, net_list, exit_list) { list_for_each_entry(net, net_list, exit_list) {
for_each_netdev_reverse(net, dev) { for_each_netdev_reverse(net, dev) {
if (dev->rtnl_link_ops) if (dev->rtnl_link_ops)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment