aboutsummaryrefslogtreecommitdiff
path: root/datapath/dp_dev.h
diff options
context:
space:
mode:
authorBen Pfaff <blp@nicira.com>2009-07-08 12:23:32 -0700
committerBen Pfaff <blp@nicira.com>2009-07-08 14:13:15 -0700
commit72ca14c15430b1e06b06fa2042dab347c1c7d7df (patch)
treec5a79176da08f9cae4061e1b2e9153e8094d6002 /datapath/dp_dev.h
parent6d867ef335eab5cee83e353a0c7e8d143a2d3419 (diff)
datapath: Fix race against workqueue in dp_dev and simplify code.
The dp_dev_destroy() function failed to cancel the xmit_queue work, which allowed it to run after the device had been destroyed, accessing freed memory. However, simply canceling the work with cancel_work_sync() would be insufficient, since other packets could get queued while the work function was running. Stopping the queue with netif_tx_disable() doesn't help, because the final action in dp_dev_do_xmit() is to re-enable the TX queue. This issue led me to re-examine why the dp_dev needs to use a work_struct at all. This was implemented in commit 71f13ed0b "Send of0 packets from workqueue, to avoid recursive locking of ofN device" due to a complaint from lockdep about recursive locking. However, there's no actual reason that we need any locking around dp_dev_xmit(). Until now, it has accepted the standard locking provided by the network stack. But looking at the other software devices (veth, loopback), those use NETIF_F_LLTX, which disables this locking, and presumably do so for this very reason. In fact, the lwn article at http://lwn.net/Articles/121566/ hints that NETIF_F_LLTX, which is otherwise discouraged in the kernel, is acceptable for "certain types of software device." So this commit switches to using NETIF_F_LLTX for dp_dev and gets rid of the work_struct. In the process, I noticed that veth and loopback also take advantage of a network device destruction "hook" using the net_device "destructor" member. Using this we can automatically get called on network device destruction at the point where rtnl_unlock() is called. This allows us to stop stringing the dp_devs that are being destroyed onto a list so that we can free them, and thus simplifies the code along all the paths that call dp_dev_destroy(). This commit gets rid of a call to synchronize_rcu() (disguised as a call to synchronize_net(), which is a macro that expands to synchronize_rcu()), so it probably speeds up deleting ports, too.
Diffstat (limited to 'datapath/dp_dev.h')
-rw-r--r--datapath/dp_dev.h6
1 files changed, 2 insertions, 4 deletions
diff --git a/datapath/dp_dev.h b/datapath/dp_dev.h
index 1b17d8f9..015c71ce 100644
--- a/datapath/dp_dev.h
+++ b/datapath/dp_dev.h
@@ -9,16 +9,14 @@
#ifndef DP_DEV_H
#define DP_DEV_H 1
+#include <linux/percpu.h>
+
struct dp_dev {
struct datapath *dp;
int port_no;
struct net_device *dev;
struct net_device_stats stats;
- struct sk_buff_head xmit_queue;
- struct work_struct xmit_work;
-
- struct list_head list;
};
static inline struct dp_dev *dp_dev_priv(struct net_device *netdev)