netpoll: recursion fixes, queueing, and cleanups
Subsystems (3)
cover netpoll: recursion fixes, queueing, and cleanups
This patch series against -rc4 fixes up some recursion deadlocks in netpoll and adds support for fallback to queueing. Various cleanups along the way.
Holds up under load testing via ipt_LOG on a dual Opteron with tg3.
PATCH 1/6 netpoll: shorten carrier detect timeout
Shorten carrier detect timeout to 4 seconds.
net/core/netpoll.c +2 -2
@@ -584,7 +584,7 @@rtnl_shunlock (); atleast = jiffies +HZ /10 ; - atmost = jiffies +10 *HZ ; + atmost = jiffies +4 *HZ ;while (!netif_carrier_ok (ndev)) {if (time_after (jiffies, atmost)) { printk(KERN_NOTICE
@@ -597,7 +597,7 @@if (time_before (jiffies, atleast)) {printk (KERN_NOTICE "%s: carrier detect appears flaky," -" waiting 10 seconds\n" , +" waiting 4 seconds\n" , np->name);while (time_before (jiffies, atmost))cond_resched ();
PATCH 2/6 netpoll: filter inlines
Add netpoll rx helpers
Move skb_free for rx into __netpoll_rx
net/core/netpoll.c +8 -2
@@ -368,7 +368,7 @@netpoll_send_skb (np, send_skb); } -int netpoll_rx (struct sk_buff *skb) +int __netpoll_rx (struct sk_buff *skb) {int proto, len, ulen;struct iphdr *iph;
@@ -440,12 +440,18 @@ (char *)(uh+1 ), ulen - sizeof(struct udphdr)); +kfree_skb (skb);return 1 ; }spin_unlock_irqrestore (&rx_list_lock, flags); out: -return atomic_read (&trapped); +if (atomic_read (&trapped)) { +kfree_skb (skb); +return 1 ; + } + +return 0 ; }int netpoll_parse_options (struct netpoll *np,char *opt)
include/linux/netpoll.h +9 -1
@@ -30,7 +30,15 @@int netpoll_trap (void );void netpoll_set_trap (int trap);void netpoll_cleanup (struct netpoll *np); -int netpoll_rx (struct sk_buff *skb); +int __netpoll_rx (struct sk_buff *skb); +#ifdef CONFIG_NETPOLL +static inline int netpoll_rx (struct sk_buff *skb) +{ +return skb->dev->netpoll_rx &&__netpoll_rx (skb); +} +#else +#define netpoll_rx (a) 0 +#endif #endif
net/core/dev.c +5 -11
@@ -1427,13 +1427,10 @@struct softnet_data *queue;unsigned long flags; -#ifdef CONFIG_NETPOLL -if (skb->dev->netpoll_rx &&netpoll_rx (skb)) { -kfree_skb (skb); +/* if netpoll wants it, pretend we never saw it */ +if (netpoll_rx (skb))return NET_RX_DROP ; - } -#endif - +if (!skb->stamp.tv_sec)net_timestamp (&skb->stamp);
@@ -1629,12 +1626,9 @@int ret =NET_RX_DROP ;unsigned short type; -#ifdef CONFIG_NETPOLL -if (skb->dev->netpoll_rx && skb->dev->poll &&netpoll_rx (skb)) { -kfree_skb (skb); +/* if we've gotten here through NAPI, check netpoll */ +if (skb->dev->poll &&netpoll_rx (skb))return NET_RX_DROP ; - } -#endif if (!skb->stamp.tv_sec)net_timestamp (&skb->stamp);
PATCH 3/6 netpoll: add netpoll point to net_device
Add struct netpoll pointer to struct netdevice Move netpoll rx flags to netpoll struct
Stop traversing rx_list and get np pointer from skb->dev->np Remove now unneeded rx_list
include/linux/netdevice.h +2 -2
@@ -41,7 +41,7 @@struct divert_blk ;struct vlan_group ;struct ethtool_ops ; - +struct netpoll ;/* source back-compat hooks */ #define SET_ETHTOOL_OPS (netdev,ops) \ ( (netdev)->ethtool_ops = (ops) )
@@ -471,7 +471,7 @@int (*neigh_setup)(struct net_device *dev,struct neigh_parms *);int (*accept_fastpath)(struct net_device *,struct dst_entry *);#ifdef CONFIG_NETPOLL -int netpoll_rx; +struct netpoll *np;#endif #ifdef CONFIG_NET_POLL_CONTROLLER void (*poll_controller)(struct net_device *dev);
net/core/netpoll.c +27 -62
149 lines not on this page · raw message
include/linux/netpoll.h +2 -2
@@ -16,11 +16,11 @@struct netpoll {struct net_device *dev;char dev_name[16 ], *name; +int rx_flags;void (*rx_hook)(struct netpoll *,int ,char *,int );u32 local_ip, remote_ip;u16 local_port, remote_port;unsigned char local_mac[6 ], remote_mac[6 ]; -struct list_head rx_list; };void netpoll_poll (struct netpoll *np);
@@ -35,7 +35,7 @@#ifdef CONFIG_NETPOLL static inline int netpoll_rx (struct sk_buff *skb) { -return skb->dev->netpoll_rx &&__netpoll_rx (skb); +return skb->dev->np && skb->dev->np->rx_flags &&__netpoll_rx (skb); }#else #define netpoll_rx (a) 0
PATCH 4/6 netpoll: fix ->poll() locking
Introduce a per-client poll lock and flag. The lock assures we never have more than one caller in dev->poll(). The flag provides recursion avoidance on UP where the lock disappears.
net/core/netpoll.c +21 -9
@@ -36,7 +36,6 @@static struct sk_buff *skbs;static atomic_t trapped; -static DEFINE_SPINLOCK (netpoll_poll_lock);#define NETPOLL_RX_ENABLED 1#define NETPOLL_RX_DROP 2
@@ -63,8 +62,15 @@ }/* -* Check whether delayed processing was scheduled for our current CPU, -* and then manually invoke NAPI polling to pump data off the card. +* Check whether delayed processing was scheduled for our NIC. If so, +* we attempt to grab the poll lock and use ->poll() to pump the card. +* If this fails, either we've recursed in ->poll() or it's already +* running on another CPU. +* +* Note: we don't mask interrupts with this lock because we're using +* trylock here and interrupts are already disabled in the softirq +* case. Further, we test the poll_owner to avoid recursion on UP +* systems where the lock doesn't exist. * * In cases where there is bi-directional communications, reading only * one message at a time can lead to packets being dropped by the
@@ -74,13 +80,10 @@static void poll_napi (struct netpoll *np) {int budget =16 ; -unsigned long flags; -struct softnet_data *queue; -spin_lock_irqsave (&netpoll_poll_lock, flags); - queue = &__get_cpu_var (softnet_data);if (test_bit (__LINK_STATE_RX_SCHED, &np->dev->state) && - !list_empty (&queue->poll_list)) { + np->poll_owner !=__smp_processor_id () && +spin_trylock (&np->poll_lock)) { np->rx_flags |=NETPOLL_RX_DROP ;atomic_inc (&trapped);
@@ -88,8 +91,8 @@atomic_dec (&trapped); np->rx_flags &= ~NETPOLL_RX_DROP ; +spin_unlock (&np->poll_lock); } -spin_unlock_irqrestore (&netpoll_poll_lock, flags); }void netpoll_poll (struct netpoll *np)
@@ -194,6 +197,12 @@return ; } +/* avoid ->poll recursion */ +if (np->poll_owner ==__smp_processor_id ()) { +__kfree_skb (skb); +return ; + } +spin_lock (&np->dev->xmit_lock); np->dev->xmit_lock_owner =smp_processor_id ();
@@ -542,6 +551,9 @@struct net_device *ndev =NULL ;struct in_device *in_dev; + np->poll_lock =SPIN_LOCK_UNLOCKED ; + np->poll_owner =-1 ; +if (np->dev_name) ndev =dev_get_by_name (np->dev_name);if (!ndev) {
include/linux/netpoll.h +21 -0
@@ -21,6 +21,8 @@u32 local_ip, remote_ip;u16 local_port, remote_port;unsigned char local_mac[6 ], remote_mac[6 ]; +spinlock_t poll_lock; +int poll_owner; };void netpoll_poll (struct netpoll *np);
@@ -37,8 +39,27 @@ {return skb->dev->np && skb->dev->np->rx_flags &&__netpoll_rx (skb); } + +static inline void netpoll_poll_lock (struct net_device *dev) +{ +if (dev->np) { +spin_lock (&dev->np->poll_lock); + dev->np->poll_owner =__smp_processor_id (); + } +} + +static inline void netpoll_poll_unlock (struct net_device *dev) +{ +if (dev->np) { +spin_unlock (&dev->np->poll_lock); + dev->np->poll_owner =-1 ; + } +} +#else #define netpoll_rx (a) 0 +#define netpoll_poll_lock (a) +#define netpoll_poll_unlock (a)#endif #endif
net/core/dev.c +3 -0
@@ -1775,8 +1775,10 @@ dev =list_entry (queue->poll_list.next, struct net_device, poll_list); +netpoll_poll_lock (dev);if (dev->quota <=0 || dev->poll (dev, &budget)) { +netpoll_poll_unlock (dev);local_irq_disable ();list_del (&dev->poll_list);list_add_tail (&dev->poll_list, &queue->poll_list);
@@ -1785,6 +1787,7 @@else dev->quota = dev->weight; }else { +netpoll_poll_unlock (dev);dev_put (dev);local_irq_disable (); }
PATCH 5/6 netpoll: add optional dropping and queueing support
This adds a callback for packets we can't deliver immediately and a helper function for clients to queue such packets to the device post-interrupt.
Netconsole is modified to use the queueing function for best-effort delivery.
drivers/net/netconsole.c +1 -0
@@ -60,6 +60,7 @@ .local_port = 6665, .remote_port = 6666, .remote_mac = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + .drop = netpoll_queue, };static int configured =0 ;
net/core/netpoll.c +57 -2
95 lines not on this page · raw message
include/linux/netpoll.h +2 -0
@@ -18,6 +18,7 @@char dev_name[16 ], *name;int rx_flags;void (*rx_hook)(struct netpoll *,int ,char *,int ); +void (*drop)(struct sk_buff *skb);u32 local_ip, remote_ip;u16 local_port, remote_port;unsigned char local_mac[6 ], remote_mac[6 ];
@@ -33,6 +34,7 @@void netpoll_set_trap (int trap);void netpoll_cleanup (struct netpoll *np);int __netpoll_rx (struct sk_buff *skb); +void netpoll_queue (struct sk_buff *skb);#ifdef CONFIG_NETPOLL static inline int netpoll_rx (struct sk_buff *skb)
PATCH 6/6 netpoll: handle xmit_lock recursion similarly
Handle possible recursion on xmit_lock while we're at it.
net/core/netpoll.c +3 -2
@@ -247,8 +247,9 @@return ; } -/* avoid ->poll recursion */ -if (np->poll_owner ==__smp_processor_id ()) { +/* avoid recursion */ +if (np->poll_owner ==__smp_processor_id () || + np->dev->xmit_lock_owner ==__smp_processor_id ()) {if (np->drop) np->drop (skb);else
David S. Miller
maintainerQuoted 5 lines · On Thu, 17 Feb 2005 23:25:18 -0600 Matt Mackall <[email protected]> wrote:
This patch series against -rc4 fixes up some recursion deadlocks in netpoll and adds support for fallback to queueing. Various cleanups along the way.
Holds up under load testing via
ipt_LOGon a dual Opteron with tg3.
I've applied all 6 patches to my tree. Sorry for taking so long Matt.