[PATCH 0/8] netpoll: A Halloween horror mystery

[PATCH 0/8] netpoll: A Halloween horror mystery

[Stephen Hemminger]

It was dull and cloudy day in Portland when Steve first
went hunting for suspend bugs in the sky2 driver. The hunt
was motivated by a certain mini owned by penguin, but he
could never get the license number. Anyway, he stumbled
down some blind alleys and met the:

	NETPOLL BEAST

It wasn't that beast was ugly, like some of the other
things he had seen. It was just an untidy mess, the kind
of thing you didn't want to bring home to mother, instead
he would have rather booted over the fence to the Viro
shredder.  But since he was in the neighborhood, he
got out his keyboard and went to work.

--

[PATCH 1/8] netpoll: skb private pool management

[Stephen Hemminger]

[-- Attachment #1: netpoll-skb-pool.patch --]
[-- Type: text/plain, Size: 2733 bytes --]

It was a dark and stormy night when Steve first saw the
netpoll beast. The beast was odd, and misshapen but not
extremely ugly.

"Let me take off one of your warts" he said. This wart
is where you tried to make an skb list yourself. If the
beast had ever run out of memory, he would have stupefied
himself unnecessarily.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
---
 net/core/netpoll.c |   52 +++++++++++++++++++---------------------------------
 1 file changed, 19 insertions(+), 33 deletions(-)

--- linux-2.6.orig/net/core/netpoll.c
+++ linux-2.6/net/core/netpoll.c
@@ -36,9 +36,7 @@
 #define MAX_QUEUE_DEPTH (MAX_SKBS / 2)
 #define MAX_RETRIES 20000
 
-static DEFINE_SPINLOCK(skb_list_lock);
-static int nr_skbs;
-static struct sk_buff *skbs;
+static struct sk_buff_head skb_pool;
 
 static DEFINE_SPINLOCK(queue_lock);
 static int queue_depth;
@@ -188,19 +186,14 @@ void netpoll_poll(struct netpoll *np)
 static void refill_skbs(void)
 {
 	struct sk_buff *skb;
-	unsigned long flags;
 
-	spin_lock_irqsave(&skb_list_lock, flags);
-	while (nr_skbs < MAX_SKBS) {
+	while (skb_queue_len(&skb_pool) < MAX_SKBS) {
 		skb = alloc_skb(MAX_SKB_SIZE, GFP_ATOMIC);
 		if (!skb)
 			break;
 
-		skb->next = skbs;
-		skbs = skb;
-		nr_skbs++;
+		skb_queue_tail(&skb_pool, skb);
 	}
-	spin_unlock_irqrestore(&skb_list_lock, flags);
 }
 
 static void zap_completion_queue(void)
@@ -229,38 +222,25 @@ static void zap_completion_queue(void)
 	put_cpu_var(softnet_data);
 }
 
-static struct sk_buff * find_skb(struct netpoll *np, int len, int reserve)
+static struct sk_buff *find_skb(struct netpoll *np, int len, int reserve)
 {
-	int once = 1, count = 0;
-	unsigned long flags;
-	struct sk_buff *skb = NULL;
+	int count = 0;
+	struct sk_buff *skb;
 
 	zap_completion_queue();
+	refill_skbs();
 repeat:
-	if (nr_skbs < MAX_SKBS)
-		refill_skbs();
 
 	skb = alloc_skb(len, GFP_ATOMIC);
-
-	if (!skb) {
-		spin_lock_irqsave(&skb_list_lock, flags);
-		skb = skbs;
-		if (skb) {
-			skbs = skb->next;
-			skb->next = NULL;
-			nr_skbs--;
-		}
-		spin_unlock_irqrestore(&skb_list_lock, flags);
-	}
+	if (!skb)
+		skb = skb_dequeue(&skb_pool);
 
 	if(!skb) {
-		count++;
-		if (once && (count == 1000000)) {
-			printk("out of netpoll skbs!\n");
-			once = 0;
+		if (++count < 10) {
+			netpoll_poll(np);
+			goto repeat;
 		}
-		netpoll_poll(np);
-		goto repeat;
+		return NULL;
 	}
 
 	atomic_set(&skb->users, 1);
@@ -764,6 +744,12 @@ int netpoll_setup(struct netpoll *np)
 	return -1;
 }
 
+static int __init netpoll_init(void) {
+	skb_queue_head_init(&skb_pool);
+	return 0;
+}
+core_initcall(netpoll_init);
+
 void netpoll_cleanup(struct netpoll *np)
 {
 	struct netpoll_info *npinfo;

--
Stephen Hemminger <shemminger@osdl.org>

Re: [PATCH 1/8] netpoll: skb private pool management

[David Miller]

From: Stephen Hemminger <shemminger@osdl.org>
Date: Thu, 26 Oct 2006 15:46:49 -0700

> @@ -188,19 +186,14 @@ void netpoll_poll(struct netpoll *np)
>  static void refill_skbs(void)
>  {
>  	struct sk_buff *skb;
> -	unsigned long flags;
>  
> -	spin_lock_irqsave(&skb_list_lock, flags);
> -	while (nr_skbs < MAX_SKBS) {
> +	while (skb_queue_len(&skb_pool) < MAX_SKBS) {

Previously, the lock actually protected nr_skbs from going over
MAX_SKBS properly, but the new code does not.  skb_queue_len()
is lockless.

Stephen, I really appreciate your efforts to clean up netpoll,
but on every iteration I am finding simple errors on the first
patch every time.

Re: [PATCH 1/8] netpoll: skb private pool management

[Stephen Hemminger]

On Thu, 26 Oct 2006 17:12:47 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <shemminger@osdl.org>
> Date: Thu, 26 Oct 2006 15:46:49 -0700
> 
> > @@ -188,19 +186,14 @@ void netpoll_poll(struct netpoll *np)
> >  static void refill_skbs(void)
> >  {
> >  	struct sk_buff *skb;
> > -	unsigned long flags;
> >  
> > -	spin_lock_irqsave(&skb_list_lock, flags);
> > -	while (nr_skbs < MAX_SKBS) {
> > +	while (skb_queue_len(&skb_pool) < MAX_SKBS) {
> 
> Previously, the lock actually protected nr_skbs from going over
> MAX_SKBS properly, but the new code does not.  skb_queue_len()
> is lockless.
> 
> Stephen, I really appreciate your efforts to clean up netpoll,
> but on every iteration I am finding simple errors on the first
> patch every time.

racing over by one is not a big issue.

Re: [PATCH 1/8] netpoll: skb private pool management

[David Miller]

From: Stephen Hemminger <shemminger@osdl.org>
Date: Thu, 26 Oct 2006 18:04:02 -0700

> On Thu, 26 Oct 2006 17:12:47 -0700 (PDT)
> David Miller <davem@davemloft.net> wrote:
> 
> > From: Stephen Hemminger <shemminger@osdl.org>
> > Date: Thu, 26 Oct 2006 15:46:49 -0700
> > 
> > > @@ -188,19 +186,14 @@ void netpoll_poll(struct netpoll *np)
> > >  static void refill_skbs(void)
> > >  {
> > >  	struct sk_buff *skb;
> > > -	unsigned long flags;
> > >  
> > > -	spin_lock_irqsave(&skb_list_lock, flags);
> > > -	while (nr_skbs < MAX_SKBS) {
> > > +	while (skb_queue_len(&skb_pool) < MAX_SKBS) {
> > 
> > Previously, the lock actually protected nr_skbs from going over
> > MAX_SKBS properly, but the new code does not.  skb_queue_len()
> > is lockless.
> > 
> > Stephen, I really appreciate your efforts to clean up netpoll,
> > but on every iteration I am finding simple errors on the first
> > patch every time.
> 
> racing over by one is not a big issue.

It's potentially racing by more than that, depending upon whether any
cpus take interrupts and are stalled for significiant time after
making the decision to add.  The upper bound is something like
(2 * NCPUS) - 1.

It's a bug Stephen.

Re: [PATCH 1/8] netpoll: skb private pool management

[Stephen Hemminger]

It was a dark and stormy night when Steve first saw the
netpoll beast. The beast was odd, and misshapen but not
extremely ugly.

"Let me take off one of your warts" he said. This wart
is where you tried to make an skb list yourself. If the
beast had ever run out of memory, he would have stupefied
himself unnecessarily.

The first try was painful, so he tried again till the bleeding
stopped.


Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
---
 net/core/netpoll.c |   53 +++++++++++++++++++++--------------------------------
 1 file changed, 21 insertions(+), 32 deletions(-)

--- netpoll.orig/net/core/netpoll.c	2006-10-26 19:12:36.000000000 -0700
+++ netpoll/net/core/netpoll.c	2006-10-26 19:16:05.000000000 -0700
@@ -36,9 +36,7 @@
 #define MAX_QUEUE_DEPTH (MAX_SKBS / 2)
 #define MAX_RETRIES 20000
 
-static DEFINE_SPINLOCK(skb_list_lock);
-static int nr_skbs;
-static struct sk_buff *skbs;
+static struct sk_buff_head skb_pool;
 
 static DEFINE_SPINLOCK(queue_lock);
 static int queue_depth;
@@ -190,17 +188,15 @@
 	struct sk_buff *skb;
 	unsigned long flags;
 
-	spin_lock_irqsave(&skb_list_lock, flags);
-	while (nr_skbs < MAX_SKBS) {
+	spin_lock_irqsave(&skb_pool->lock, flags);
+	while (skb_pool.qlen < MAX_SKBS) {
 		skb = alloc_skb(MAX_SKB_SIZE, GFP_ATOMIC);
 		if (!skb)
 			break;
 
-		skb->next = skbs;
-		skbs = skb;
-		nr_skbs++;
+		__skb_queue_tail(&skb_pool, skb);
 	}
-	spin_unlock_irqrestore(&skb_list_lock, flags);
+	spin_unlock_irqrestore(&skb_pool->lock, flags);
 }
 
 static void zap_completion_queue(void)
@@ -229,38 +225,25 @@
 	put_cpu_var(softnet_data);
 }
 
-static struct sk_buff * find_skb(struct netpoll *np, int len, int reserve)
+static struct sk_buff *find_skb(struct netpoll *np, int len, int reserve)
 {
-	int once = 1, count = 0;
-	unsigned long flags;
-	struct sk_buff *skb = NULL;
+	int count = 0;
+	struct sk_buff *skb;
 
 	zap_completion_queue();
+	refill_skbs();
 repeat:
-	if (nr_skbs < MAX_SKBS)
-		refill_skbs();
 
 	skb = alloc_skb(len, GFP_ATOMIC);
-
-	if (!skb) {
-		spin_lock_irqsave(&skb_list_lock, flags);
-		skb = skbs;
-		if (skb) {
-			skbs = skb->next;
-			skb->next = NULL;
-			nr_skbs--;
-		}
-		spin_unlock_irqrestore(&skb_list_lock, flags);
-	}
+	if (!skb)
+		skb = skb_dequeue(&skb_pool);
 
 	if(!skb) {
-		count++;
-		if (once && (count == 1000000)) {
-			printk("out of netpoll skbs!\n");
-			once = 0;
+		if (++count < 10) {
+			netpoll_poll(np);
+			goto repeat;
 		}
-		netpoll_poll(np);
-		goto repeat;
+		return NULL;
 	}
 
 	atomic_set(&skb->users, 1);
@@ -764,6 +747,12 @@
 	return -1;
 }
 
+static int __init netpoll_init(void) {
+	skb_queue_head_init(&skb_pool);
+	return 0;
+}
+core_initcall(netpoll_init);
+
 void netpoll_cleanup(struct netpoll *np)
 {
 	struct netpoll_info *npinfo;

Re: [PATCH 1/8] netpoll: skb private pool management

[David Miller]

From: Stephen Hemminger <shemminger@osdl.org>
Date: Thu, 26 Oct 2006 19:29:34 -0700

> It was a dark and stormy night when Steve first saw the
> netpoll beast. The beast was odd, and misshapen but not
> extremely ugly.
> 
> "Let me take off one of your warts" he said. This wart
> is where you tried to make an skb list yourself. If the
> beast had ever run out of memory, he would have stupefied
> himself unnecessarily.
> 
> The first try was painful, so he tried again till the bleeding
> stopped.

Maybe spend some time with compile tests instead of the cute
story line? :-)

> -static DEFINE_SPINLOCK(skb_list_lock);
> -static int nr_skbs;
> -static struct sk_buff *skbs;
> +static struct sk_buff_head skb_pool;

skb_pool is a scaler.

> +	spin_lock_irqsave(&skb_pool->lock, flags);

Yet is being accessed as a pointer.

net/core/netpoll.c:191: error: invalid type argument of ^[$,1rx^[(B->^[$,1ry^[(B

netpoll patches.

[Stephen Hemminger]

The following changes since commit 5d1be92267c04a86232969710e32b6f99bb4ec12:
  Peter Zijlstra:
        [SCTP]: Cleanup of the sctp state table code.

are found in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/shemminger/netpoll-2.6.20

Stephen Hemminger:
      netpoll: private skb pool (rev3)
      netpoll info leak
      netpoll per device txq
      netpoll setup error handling
      netpoll deferred transmit path
      netpoll retry cleanup
      netpoll queue cleanup
      netpoll header cleanup

 drivers/net/netconsole.c |    8 +-
 include/linux/netpoll.h  |   15 ++-
 net/core/netpoll.c       |  233 +++++++++++++++++++++-------------------------
 3 files changed, 121 insertions(+), 135 deletions(-)

Re: netpoll patches.

[David Miller]

From: Stephen Hemminger <shemminger@osdl.org>
Date: Tue, 14 Nov 2006 13:55:00 -0800

> The following changes since commit 5d1be92267c04a86232969710e32b6f99bb4ec12:
>   Peter Zijlstra:
>         [SCTP]: Cleanup of the sctp state table code.
> 
> are found in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/shemminger/netpoll-2.6.20
> 
> Stephen Hemminger:
>       netpoll: private skb pool (rev3)
>       netpoll info leak
>       netpoll per device txq
>       netpoll setup error handling
>       netpoll deferred transmit path
>       netpoll retry cleanup
>       netpoll queue cleanup
>       netpoll header cleanup

Pulled, thanks a lot Stephen.

I made a small follow-on patch to tidy up some coding style
things I noticed while re-reviewing this work.

Thanks again for taming the beast :)

[PATCH 2/8] netpoll info leak

[Stephen Hemminger]

[-- Attachment #1: netpoll-info-leak.patch --]
[-- Type: text/plain, Size: 1986 bytes --]

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 <shemminger@osdl.org>
---
 include/linux/netpoll.h |    1 +
 net/core/netpoll.c      |   25 +++++++++++++++++++------
 2 files changed, 20 insertions(+), 6 deletions(-)

--- linux-2.6.orig/include/linux/netpoll.h
+++ linux-2.6/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;
--- linux-2.6.orig/net/core/netpoll.c
+++ linux-2.6/net/core/netpoll.c
@@ -649,8 +649,11 @@ int netpoll_setup(struct netpoll *np)
 		npinfo->tries = MAX_RETRIES;
 		spin_lock_init(&npinfo->rx_lock);
 		skb_queue_head_init(&npinfo->arp_tx);
-	} else
+		atomic_set(&npinfo->refcnt, 1);
+	} else {
 		npinfo = ndev->npinfo;
+		atomic_inc(&npinfo->refcnt);
+	}
 
 	if (!ndev->poll_controller) {
 		printk(KERN_ERR "%s: %s doesn't support polling, aborting.\n",
@@ -757,12 +760,22 @@ void netpoll_cleanup(struct netpoll *np)
 
 	if (np->dev) {
 		npinfo = np->dev->npinfo;
-		if (npinfo && npinfo->rx_np == np) {
-			spin_lock_irqsave(&npinfo->rx_lock, flags);
-			npinfo->rx_np = NULL;
-			npinfo->rx_flags &= ~NETPOLL_RX_ENABLED;
-			spin_unlock_irqrestore(&npinfo->rx_lock, flags);
+		if (npinfo) {
+			if (npinfo->rx_np == np) {
+				spin_lock_irqsave(&npinfo->rx_lock, flags);
+				npinfo->rx_np = NULL;
+				npinfo->rx_flags &= ~NETPOLL_RX_ENABLED;
+				spin_unlock_irqrestore(&npinfo->rx_lock, flags);
+			}
+
+			np->dev->npinfo = NULL;
+			if (atomic_dec_and_test(&npinfo->refcnt)) {
+				skb_queue_purge(&npinfo->arp_tx);
+
+				kfree(npinfo);
+			}
 		}
+
 		dev_put(np->dev);
 	}
 

--
Stephen Hemminger <shemminger@osdl.org>

[PATCH 3/8] netpoll per device txq

[Stephen Hemminger]

[-- Attachment #1: netpoll-per-dev.patch --]
[-- Type: text/plain, Size: 2990 bytes --]

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 <shemminger@osdl.org>
---
 include/linux/netpoll.h |    2 +
 net/core/netpoll.c      |   50 ++++++++++++++----------------------------------
 2 files changed, 17 insertions(+), 35 deletions(-)

--- linux-2.6.orig/include/linux/netpoll.h
+++ linux-2.6/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);
--- linux-2.6.orig/net/core/netpoll.c
+++ linux-2.6/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 *sk
 
 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,
@@ -649,6 +624,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;
@@ -771,6 +749,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);
 			}

--
Stephen Hemminger <shemminger@osdl.org>

[PATCH 4/8] netpoll setup error handling

[Stephen Hemminger]

[-- Attachment #1: netpoll-setup-errors.patch --]
[-- Type: text/plain, Size: 2733 bytes --]

The beast was not always healthy. When it was sick,
it tended to be laconic and not tell anyone the real problem.
A few small changes had it telling the world about its
problems, if they really wanted to hear.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
---
 drivers/net/netconsole.c |    7 +++++--
 net/core/netpoll.c       |   20 +++++++++++++-------
 2 files changed, 18 insertions(+), 9 deletions(-)

--- linux-2.6.orig/drivers/net/netconsole.c
+++ linux-2.6/drivers/net/netconsole.c
@@ -102,6 +102,8 @@ __setup("netconsole=", option_setup);
 
 static int init_netconsole(void)
 {
+	int err;
+
 	if(strlen(config))
 		option_setup(config);
 
@@ -110,8 +112,9 @@ static int init_netconsole(void)
 		return 0;
 	}
 
-	if(netpoll_setup(&np))
-		return -EINVAL;
+	err = netpoll_setup(&np);
+	if (err)
+		return err;
 
 	register_console(&netconsole);
 	printk(KERN_INFO "netconsole: network logging started\n");
--- linux-2.6.orig/net/core/netpoll.c
+++ linux-2.6/net/core/netpoll.c
@@ -602,20 +602,23 @@ int netpoll_setup(struct netpoll *np)
 	struct in_device *in_dev;
 	struct netpoll_info *npinfo;
 	unsigned long flags;
+	int err;
 
 	if (np->dev_name)
 		ndev = dev_get_by_name(np->dev_name);
 	if (!ndev) {
 		printk(KERN_ERR "%s: %s doesn't exist, aborting.\n",
 		       np->name, np->dev_name);
-		return -1;
+		return -ENODEV;
 	}
 
 	np->dev = ndev;
 	if (!ndev->npinfo) {
 		npinfo = kmalloc(sizeof(*npinfo), GFP_KERNEL);
-		if (!npinfo)
+		if (!npinfo) {
+			err = -ENOMEM;
 			goto release;
+		}
 
 		npinfo->rx_flags = 0;
 		npinfo->rx_np = NULL;
@@ -636,6 +639,7 @@ int netpoll_setup(struct netpoll *np)
 	if (!ndev->poll_controller) {
 		printk(KERN_ERR "%s: %s doesn't support polling, aborting.\n",
 		       np->name, np->dev_name);
+		err = -ENOTSUPP;
 		goto release;
 	}
 
@@ -646,13 +650,14 @@ int netpoll_setup(struct netpoll *np)
 		       np->name, np->dev_name);
 
 		rtnl_lock();
-		if (dev_change_flags(ndev, ndev->flags | IFF_UP) < 0) {
+		err = dev_open(ndev);
+		rtnl_unlock();
+
+		if (err) {
 			printk(KERN_ERR "%s: failed to open %s\n",
-			       np->name, np->dev_name);
-			rtnl_unlock();
+			       np->name, ndev->name);
 			goto release;
 		}
-		rtnl_unlock();
 
 		atleast = jiffies + HZ/10;
  		atmost = jiffies + 4*HZ;
@@ -690,6 +695,7 @@ int netpoll_setup(struct netpoll *np)
 			rcu_read_unlock();
 			printk(KERN_ERR "%s: no IP address for %s, aborting\n",
 			       np->name, np->dev_name);
+			err = -EDESTADDRREQ;
 			goto release;
 		}
 
@@ -722,7 +728,7 @@ int netpoll_setup(struct netpoll *np)
 		kfree(npinfo);
 	np->dev = NULL;
 	dev_put(ndev);
-	return -1;
+	return err;
 }
 
 static int __init netpoll_init(void) {

--
Stephen Hemminger <shemminger@osdl.org>

[PATCH 5/8] netpoll deferred transmit path

[Stephen Hemminger]

[-- Attachment #1: netpoll-defer-tx.patch --]
[-- Type: text/plain, Size: 1618 bytes --]

When the netpoll beast got busy, he tended to babble.
Instead of talking out of his large mouth as normal,
he tended to try to snort out other orifices. This lead
to words (skbs) ending up in odd places (like NIT) that
he did not intend.

The normal way of talking wouldn't work, but he could
at least change to using the same tone all the time.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
---
 net/core/netpoll.c |   21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

--- linux-2.6.orig/net/core/netpoll.c
+++ linux-2.6/net/core/netpoll.c
@@ -55,9 +55,25 @@ static void queue_process(void *p)
 	struct netpoll_info *npinfo = p;
 	struct sk_buff *skb;
 
-	while ((skb = skb_dequeue(&npinfo->txq)))
-		dev_queue_xmit(skb);
+	while ((skb = skb_dequeue(&npinfo->txq))) {
+		struct net_device *dev = skb->dev;
 
+		if (!netif_device_present(dev) || !netif_running(dev)) {
+			__kfree_skb(skb);
+			continue;
+		}
+
+		netif_tx_lock_bh(dev);
+		if (netif_queue_stopped(dev) ||
+		    dev->hard_start_xmit(skb, dev) != NETDEV_TX_OK) {
+			skb_queue_head(&npinfo->txq, skb);
+			netif_tx_unlock_bh(dev);
+
+			schedule_delayed_work(&npinfo->tx_work, HZ/10);
+			return;
+		}
+		netif_tx_unlock_bh(dev);
+	}
 }
 
 void netpoll_queue(struct sk_buff *skb)
@@ -756,6 +772,7 @@ void netpoll_cleanup(struct netpoll *np)
 			if (atomic_dec_and_test(&npinfo->refcnt)) {
 				skb_queue_purge(&npinfo->arp_tx);
  				skb_queue_purge(&npinfo->txq);
+				cancel_rearming_delayed_work(&npinfo->tx_work);
  				flush_scheduled_work();
 
 				kfree(npinfo);

--
Stephen Hemminger <shemminger@osdl.org>

[PATCH 6/8] netpoll retry cleanup

[Stephen Hemminger]

[-- Attachment #1: netpoll-retry-cleanup.patch --]
[-- Type: text/plain, Size: 3605 bytes --]

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 <shemminger@osdl.org>
---
 include/linux/netpoll.h |    1 
 net/core/netpoll.c      |   71 ++++++++++++++++++++++--------------------------
 2 files changed, 33 insertions(+), 39 deletions(-)

--- linux-2.6.orig/net/core/netpoll.c
+++ linux-2.6/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);
 	}
 }
@@ -241,50 +242,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 (!np || !np->dev || !netif_running(np->dev)) {
-		__kfree_skb(skb);
-		return;
-	}
+			if (status == NETDEV_TX_OK)
+				break;
+
+			/* 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)
@@ -640,7 +635,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);
--- linux-2.6.orig/include/linux/netpoll.h
+++ linux-2.6/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 */

--
Stephen Hemminger <shemminger@osdl.org>

[PATCH 7/8] netpoll queue cleanup

[Stephen Hemminger]

[-- Attachment #1: netpoll-queue-clean.patch --]
[-- Type: text/plain, Size: 2817 bytes --]

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 <shemminger@osdl.org>
---
 drivers/net/netconsole.c |    1 -
 include/linux/netpoll.h  |    4 ++--
 net/core/netpoll.c       |   23 +++--------------------
 3 files changed, 5 insertions(+), 23 deletions(-)

--- linux-2.6.orig/drivers/net/netconsole.c
+++ linux-2.6/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;
 
--- linux-2.6.orig/include/linux/netpoll.h
+++ linux-2.6/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)
--- linux-2.6.orig/net/core/netpoll.c
+++ linux-2.6/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)
 {
@@ -253,7 +240,7 @@ static void netpoll_send_skb(struct netp
  	}
 
 	/* 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)) {
 
@@ -274,11 +261,8 @@ static void netpoll_send_skb(struct netp
 	}
 
 	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);
 	}
 }
 
@@ -800,4 +784,3 @@ EXPORT_SYMBOL(netpoll_setup);
 EXPORT_SYMBOL(netpoll_cleanup);
 EXPORT_SYMBOL(netpoll_send_udp);
 EXPORT_SYMBOL(netpoll_poll);
-EXPORT_SYMBOL(netpoll_queue);

--
Stephen Hemminger <shemminger@osdl.org>

[PATCH 8/8] netpoll header cleanup

[Stephen Hemminger]

[-- Attachment #1: netpoll-header.patch --]
[-- Type: text/plain, Size: 893 bytes --]

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 <shemminger@osdl.org>
---
 include/linux/netpoll.h |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

--- linux-2.6.orig/include/linux/netpoll.h
+++ linux-2.6/include/linux/netpoll.h
@@ -12,16 +12,15 @@
 #include <linux/rcupdate.h>
 #include <linux/list.h>
 
-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 {

--
Stephen Hemminger <shemminger@osdl.org>