From 93ec2c723e3f8a216dde2899aeb85c648672bc6b Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Thu, 26 Oct 2006 15:46:50 -0700 Subject: netpoll info leak After looking harder, Steve noticed that the netpoll beast leaked a little every time it shutdown for a nap. Not a big leak, but a nuisance kind of thing. He took out his refcount duct tape and patched the leak. It was overkill since there was already other locking in that area, but it looked clean and wouldn't attract fleas. Signed-off-by: Stephen Hemminger --- include/linux/netpoll.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include/linux/netpoll.h') diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h index 1efe60c5c00c..39845fc975f9 100644 --- a/include/linux/netpoll.h +++ b/include/linux/netpoll.h @@ -25,6 +25,7 @@ struct netpoll { }; struct netpoll_info { + atomic_t refcnt; spinlock_t poll_lock; int poll_owner; int tries; -- cgit v1.3-8-gc7d7 From b6cd27ed33886a5ffaf0925a6d98e13e18e8a1af Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Thu, 26 Oct 2006 15:46:51 -0700 Subject: netpoll per device txq When the netpoll beast got really busy, it tended to clog things, so it stored them for later. But the beast was putting all it's skb's in one basket. This was bad because maybe some pipes were clogged and others were not. Signed-off-by: Stephen Hemminger --- include/linux/netpoll.h | 2 ++ net/core/netpoll.c | 50 +++++++++++++++---------------------------------- 2 files changed, 17 insertions(+), 35 deletions(-) (limited to 'include/linux/netpoll.h') diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h index 39845fc975f9..93a8b7664423 100644 --- a/include/linux/netpoll.h +++ b/include/linux/netpoll.h @@ -33,6 +33,8 @@ struct netpoll_info { spinlock_t rx_lock; struct netpoll *rx_np; /* netpoll that registered an rx_hook */ struct sk_buff_head arp_tx; /* list of arp requests to reply to */ + struct sk_buff_head txq; + struct work_struct tx_work; }; void netpoll_poll(struct netpoll *np); diff --git a/net/core/netpoll.c b/net/core/netpoll.c index c66df2f45d26..ac4e8b8f57d1 100644 --- a/net/core/netpoll.c +++ b/net/core/netpoll.c @@ -38,10 +38,6 @@ static struct sk_buff_head skb_pool; -static DEFINE_SPINLOCK(queue_lock); -static int queue_depth; -static struct sk_buff *queue_head, *queue_tail; - static atomic_t trapped; #define NETPOLL_RX_ENABLED 1 @@ -56,46 +52,25 @@ static void arp_reply(struct sk_buff *skb); static void queue_process(void *p) { - unsigned long flags; + struct netpoll_info *npinfo = p; struct sk_buff *skb; - while (queue_head) { - spin_lock_irqsave(&queue_lock, flags); - - skb = queue_head; - queue_head = skb->next; - if (skb == queue_tail) - queue_head = NULL; - - queue_depth--; - - spin_unlock_irqrestore(&queue_lock, flags); - + while ((skb = skb_dequeue(&npinfo->txq))) dev_queue_xmit(skb); - } -} -static DECLARE_WORK(send_queue, queue_process, NULL); +} void netpoll_queue(struct sk_buff *skb) { - unsigned long flags; + struct net_device *dev = skb->dev; + struct netpoll_info *npinfo = dev->npinfo; - if (queue_depth == MAX_QUEUE_DEPTH) { - __kfree_skb(skb); - return; + if (!npinfo) + kfree_skb(skb); + else { + skb_queue_tail(&npinfo->txq, skb); + schedule_work(&npinfo->tx_work); } - - spin_lock_irqsave(&queue_lock, flags); - if (!queue_head) - queue_head = skb; - else - queue_tail->next = skb; - queue_tail = skb; - queue_depth++; - spin_unlock_irqrestore(&queue_lock, flags); - - schedule_work(&send_queue); } static int checksum_udp(struct sk_buff *skb, struct udphdr *uh, @@ -658,6 +633,9 @@ int netpoll_setup(struct netpoll *np) npinfo->tries = MAX_RETRIES; spin_lock_init(&npinfo->rx_lock); skb_queue_head_init(&npinfo->arp_tx); + skb_queue_head_init(&npinfo->txq); + INIT_WORK(&npinfo->tx_work, queue_process, npinfo); + atomic_set(&npinfo->refcnt, 1); } else { npinfo = ndev->npinfo; @@ -780,6 +758,8 @@ void netpoll_cleanup(struct netpoll *np) np->dev->npinfo = NULL; if (atomic_dec_and_test(&npinfo->refcnt)) { skb_queue_purge(&npinfo->arp_tx); + skb_queue_purge(&npinfo->txq); + flush_scheduled_work(); kfree(npinfo); } -- cgit v1.3-8-gc7d7 From 2bdfe0baeca0e2750037b8fba71905c00ac3c515 Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Thu, 26 Oct 2006 15:46:54 -0700 Subject: netpoll retry cleanup The netpoll beast was still not happy. If the beast got clogged pipes, it tended to stare blankly off in space for a long time. The problem couldn't be completely fixed because the beast talked with irq's disabled. But it could be made less painful and shorter. Signed-off-by: Stephen Hemminger --- include/linux/netpoll.h | 1 - net/core/netpoll.c | 71 +++++++++++++++++++++++-------------------------- 2 files changed, 33 insertions(+), 39 deletions(-) (limited to 'include/linux/netpoll.h') diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h index 93a8b7664423..c65d12ec7bb0 100644 --- a/include/linux/netpoll.h +++ b/include/linux/netpoll.h @@ -28,7 +28,6 @@ struct netpoll_info { atomic_t refcnt; spinlock_t poll_lock; int poll_owner; - int tries; int rx_flags; spinlock_t rx_lock; struct netpoll *rx_np; /* netpoll that registered an rx_hook */ diff --git a/net/core/netpoll.c b/net/core/netpoll.c index 93cb828f3aaf..6b34c394672f 100644 --- a/net/core/netpoll.c +++ b/net/core/netpoll.c @@ -34,12 +34,12 @@ #define MAX_UDP_CHUNK 1460 #define MAX_SKBS 32 #define MAX_QUEUE_DEPTH (MAX_SKBS / 2) -#define MAX_RETRIES 20000 static struct sk_buff_head skb_pool; static atomic_t trapped; +#define USEC_PER_POLL 50 #define NETPOLL_RX_ENABLED 1 #define NETPOLL_RX_DROP 2 @@ -72,6 +72,7 @@ static void queue_process(void *p) schedule_delayed_work(&npinfo->tx_work, HZ/10); return; } + netif_tx_unlock_bh(dev); } } @@ -244,50 +245,44 @@ repeat: static void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb) { - int status; - struct netpoll_info *npinfo; + int status = NETDEV_TX_BUSY; + unsigned long tries; + struct net_device *dev = np->dev; + struct netpoll_info *npinfo = np->dev->npinfo; + + if (!npinfo || !netif_running(dev) || !netif_device_present(dev)) { + __kfree_skb(skb); + return; + } + + /* don't get messages out of order, and no recursion */ + if ( !(np->drop == netpoll_queue && skb_queue_len(&npinfo->txq)) + && npinfo->poll_owner != smp_processor_id() + && netif_tx_trylock(dev)) { + + /* try until next clock tick */ + for(tries = jiffies_to_usecs(1)/USEC_PER_POLL; tries > 0; --tries) { + if (!netif_queue_stopped(dev)) + status = dev->hard_start_xmit(skb, dev); + + if (status == NETDEV_TX_OK) + break; - if (!np || !np->dev || !netif_running(np->dev)) { - __kfree_skb(skb); - return; - } + /* tickle device maybe there is some cleanup */ + netpoll_poll(np); - npinfo = np->dev->npinfo; + udelay(USEC_PER_POLL); + } + netif_tx_unlock(dev); + } - /* avoid recursion */ - if (npinfo->poll_owner == smp_processor_id() || - np->dev->xmit_lock_owner == smp_processor_id()) { + if (status != NETDEV_TX_OK) { + /* requeue for later */ if (np->drop) np->drop(skb); else __kfree_skb(skb); - return; } - - do { - npinfo->tries--; - netif_tx_lock(np->dev); - - /* - * network drivers do not expect to be called if the queue is - * stopped. - */ - status = NETDEV_TX_BUSY; - if (!netif_queue_stopped(np->dev)) - status = np->dev->hard_start_xmit(skb, np->dev); - - netif_tx_unlock(np->dev); - - /* success */ - if(!status) { - npinfo->tries = MAX_RETRIES; /* reset */ - return; - } - - /* transmit busy */ - netpoll_poll(np); - udelay(50); - } while (npinfo->tries > 0); } void netpoll_send_udp(struct netpoll *np, const char *msg, int len) @@ -649,7 +644,7 @@ int netpoll_setup(struct netpoll *np) npinfo->rx_np = NULL; spin_lock_init(&npinfo->poll_lock); npinfo->poll_owner = -1; - npinfo->tries = MAX_RETRIES; + spin_lock_init(&npinfo->rx_lock); skb_queue_head_init(&npinfo->arp_tx); skb_queue_head_init(&npinfo->txq); -- cgit v1.3-8-gc7d7 From 5de4a473bda49554e4e9bd93b78f43c49a7ea69c Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Thu, 26 Oct 2006 15:46:55 -0700 Subject: netpoll queue cleanup The beast had a long and not very happy history. At one point, a friend (netdump) had asked that he open up a little. Well, the friend was long gone now, and the beast had this dangling piece hanging (netpoll_queue). It wasn't hard to stitch the netpoll_queue back in where it belonged and make everything tidy. Signed-off-by: Stephen Hemminger --- drivers/net/netconsole.c | 1 - include/linux/netpoll.h | 4 ++-- net/core/netpoll.c | 23 +++-------------------- 3 files changed, 5 insertions(+), 23 deletions(-) (limited to 'include/linux/netpoll.h') diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index e6e8a9797b6d..69233f6aa05c 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -60,7 +60,6 @@ static struct netpoll np = { .local_port = 6665, .remote_port = 6666, .remote_mac = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, - .drop = netpoll_queue, }; static int configured = 0; diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h index c65d12ec7bb0..b7eb008c43de 100644 --- a/include/linux/netpoll.h +++ b/include/linux/netpoll.h @@ -18,7 +18,7 @@ struct netpoll { struct net_device *dev; char dev_name[16], *name; 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]; @@ -44,7 +44,7 @@ int netpoll_trap(void); 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) diff --git a/net/core/netpoll.c b/net/core/netpoll.c index 6b34c394672f..0d1de3c47a01 100644 --- a/net/core/netpoll.c +++ b/net/core/netpoll.c @@ -77,19 +77,6 @@ static void queue_process(void *p) } } -void netpoll_queue(struct sk_buff *skb) -{ - struct net_device *dev = skb->dev; - struct netpoll_info *npinfo = dev->npinfo; - - if (!npinfo) - kfree_skb(skb); - else { - skb_queue_tail(&npinfo->txq, skb); - schedule_work(&npinfo->tx_work); - } -} - static int checksum_udp(struct sk_buff *skb, struct udphdr *uh, unsigned short ulen, u32 saddr, u32 daddr) { @@ -256,7 +243,7 @@ static void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb) } /* don't get messages out of order, and no recursion */ - if ( !(np->drop == netpoll_queue && skb_queue_len(&npinfo->txq)) + if ( skb_queue_len(&npinfo->txq) == 0 && npinfo->poll_owner != smp_processor_id() && netif_tx_trylock(dev)) { @@ -277,11 +264,8 @@ static void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb) } if (status != NETDEV_TX_OK) { - /* requeue for later */ - if (np->drop) - np->drop(skb); - else - __kfree_skb(skb); + skb_queue_tail(&npinfo->txq, skb); + schedule_work(&npinfo->tx_work); } } @@ -809,4 +793,3 @@ EXPORT_SYMBOL(netpoll_setup); EXPORT_SYMBOL(netpoll_cleanup); EXPORT_SYMBOL(netpoll_send_udp); EXPORT_SYMBOL(netpoll_poll); -EXPORT_SYMBOL(netpoll_queue); -- cgit v1.3-8-gc7d7 From bf6bce71eae386dbc37f93af7e5ad173450d9945 Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Thu, 26 Oct 2006 15:46:56 -0700 Subject: netpoll header cleanup As Steve left netpoll beast, hopefully not to return soon. He noticed that the header was messy. He straightened it up and polished it a little, then waved goodbye. Signed-off-by: Stephen Hemminger --- include/linux/netpoll.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'include/linux/netpoll.h') diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h index b7eb008c43de..2cc9867b1626 100644 --- a/include/linux/netpoll.h +++ b/include/linux/netpoll.h @@ -12,16 +12,15 @@ #include #include -struct netpoll; - struct netpoll { struct net_device *dev; - char dev_name[16], *name; + char dev_name[IFNAMSIZ]; + const char *name; 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]; + u8 local_mac[ETH_ALEN], remote_mac[ETH_ALEN]; }; struct netpoll_info { -- cgit v1.3-8-gc7d7