Browse Source

[NETPOLL]: fix initialization/NAPI race

This fixes a race during initialization with the NAPI softirq
processing by using an RCU approach.

This race was discovered when refill_skbs() was added to
the setup code.

Signed-off-by: Matt Mackall <mpm@selenic.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Matt Mackall 20 years ago
parent
commit
53fb95d3c1
3 changed files with 21 additions and 10 deletions
  1. 13 6
      include/linux/netpoll.h
  2. 5 4
      net/core/dev.c
  3. 3 0
      net/core/netpoll.c

+ 13 - 6
include/linux/netpoll.h

@@ -9,6 +9,7 @@
 
 
 #include <linux/netdevice.h>
 #include <linux/netdevice.h>
 #include <linux/interrupt.h>
 #include <linux/interrupt.h>
+#include <linux/rcupdate.h>
 #include <linux/list.h>
 #include <linux/list.h>
 
 
 struct netpoll;
 struct netpoll;
@@ -61,25 +62,31 @@ static inline int netpoll_rx(struct sk_buff *skb)
 	return ret;
 	return ret;
 }
 }
 
 
-static inline void netpoll_poll_lock(struct net_device *dev)
+static inline void *netpoll_poll_lock(struct net_device *dev)
 {
 {
+	rcu_read_lock(); /* deal with race on ->npinfo */
 	if (dev->npinfo) {
 	if (dev->npinfo) {
 		spin_lock(&dev->npinfo->poll_lock);
 		spin_lock(&dev->npinfo->poll_lock);
 		dev->npinfo->poll_owner = smp_processor_id();
 		dev->npinfo->poll_owner = smp_processor_id();
+		return dev->npinfo;
 	}
 	}
+	return NULL;
 }
 }
 
 
-static inline void netpoll_poll_unlock(struct net_device *dev)
+static inline void netpoll_poll_unlock(void *have)
 {
 {
-	if (dev->npinfo) {
-		dev->npinfo->poll_owner = -1;
-		spin_unlock(&dev->npinfo->poll_lock);
+	struct netpoll_info *npi = have;
+
+	if (npi) {
+		npi->poll_owner = -1;
+		spin_unlock(&npi->poll_lock);
 	}
 	}
+	rcu_read_unlock();
 }
 }
 
 
 #else
 #else
 #define netpoll_rx(a) 0
 #define netpoll_rx(a) 0
-#define netpoll_poll_lock(a)
+#define netpoll_poll_lock(a) 0
 #define netpoll_poll_unlock(a)
 #define netpoll_poll_unlock(a)
 #endif
 #endif
 
 

+ 5 - 4
net/core/dev.c

@@ -1696,7 +1696,8 @@ static void net_rx_action(struct softirq_action *h)
 	struct softnet_data *queue = &__get_cpu_var(softnet_data);
 	struct softnet_data *queue = &__get_cpu_var(softnet_data);
 	unsigned long start_time = jiffies;
 	unsigned long start_time = jiffies;
 	int budget = netdev_budget;
 	int budget = netdev_budget;
-	
+	void *have;
+
 	local_irq_disable();
 	local_irq_disable();
 
 
 	while (!list_empty(&queue->poll_list)) {
 	while (!list_empty(&queue->poll_list)) {
@@ -1709,10 +1710,10 @@ static void net_rx_action(struct softirq_action *h)
 
 
 		dev = list_entry(queue->poll_list.next,
 		dev = list_entry(queue->poll_list.next,
 				 struct net_device, poll_list);
 				 struct net_device, poll_list);
-		netpoll_poll_lock(dev);
+		have = netpoll_poll_lock(dev);
 
 
 		if (dev->quota <= 0 || dev->poll(dev, &budget)) {
 		if (dev->quota <= 0 || dev->poll(dev, &budget)) {
-			netpoll_poll_unlock(dev);
+			netpoll_poll_unlock(have);
 			local_irq_disable();
 			local_irq_disable();
 			list_del(&dev->poll_list);
 			list_del(&dev->poll_list);
 			list_add_tail(&dev->poll_list, &queue->poll_list);
 			list_add_tail(&dev->poll_list, &queue->poll_list);
@@ -1721,7 +1722,7 @@ static void net_rx_action(struct softirq_action *h)
 			else
 			else
 				dev->quota = dev->weight;
 				dev->quota = dev->weight;
 		} else {
 		} else {
-			netpoll_poll_unlock(dev);
+			netpoll_poll_unlock(have);
 			dev_put(dev);
 			dev_put(dev);
 			local_irq_disable();
 			local_irq_disable();
 		}
 		}

+ 3 - 0
net/core/netpoll.c

@@ -732,6 +732,9 @@ int netpoll_setup(struct netpoll *np)
 	/* last thing to do is link it to the net device structure */
 	/* last thing to do is link it to the net device structure */
 	ndev->npinfo = npinfo;
 	ndev->npinfo = npinfo;
 
 
+	/* avoid racing with NAPI reading npinfo */
+	synchronize_rcu();
+
 	return 0;
 	return 0;
 
 
  release:
  release: