[patch 12/19] fix irq problem with NAPI + NETPOLL

[patch 12/19] fix irq problem with NAPI + NETPOLL

[akpm]

From: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

It seems netif_receive_skb() was designed not to call from irq context, but
NAPI + NETPOLL break this rule.  If netif_receive_skb() was called from irq
context, redirect to netif_rx() instead of processing the skb in that
context.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Cc: Jeff Garzik <jeff@garzik.org>
Cc: Francois Romieu <romieu@fr.zoreil.com>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 net/core/dev.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff -puN net/core/dev.c~8139too-fix-irq-problem-with-napi-netpoll net/core/dev.c
--- a/net/core/dev.c~8139too-fix-irq-problem-with-napi-netpoll
+++ a/net/core/dev.c
@@ -1769,8 +1769,15 @@ int netif_receive_skb(struct sk_buff *sk
 	__be16 type;
 
 	/* if we've gotten here through NAPI, check netpoll */
-	if (skb->dev->poll && netpoll_rx(skb))
-		return NET_RX_DROP;
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	if (skb->dev->poll && skb->dev->poll_controller) {
+		/* NAPI poll might be called in irq context on NETPOLL */
+		if (in_irq() || irqs_disabled())
+			return netif_rx(skb);
+		if (netpoll_rx(skb))
+			return NET_RX_DROP;
+	}
+#endif
 
 	if (!skb->tstamp.off_sec)
 		net_timestamp(skb);
_

Re: [patch 12/19] fix irq problem with NAPI + NETPOLL

[Jeff Garzik]

akpm@linux-foundation.org wrote:
> From: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> 
> It seems netif_receive_skb() was designed not to call from irq context, but
> NAPI + NETPOLL break this rule.  If netif_receive_skb() was called from irq
> context, redirect to netif_rx() instead of processing the skb in that
> context.
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> Cc: Jeff Garzik <jeff@garzik.org>
> Cc: Francois Romieu <romieu@fr.zoreil.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  net/core/dev.c |   11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)

net/* stuff, I'll let DaveM make the call and apply it...

Re: [patch 12/19] fix irq problem with NAPI + NETPOLL

[Andrew Morton]

On Tue, 06 Mar 2007 06:06:11 -0500 Jeff Garzik <jeff@garzik.org> wrote:

> akpm@linux-foundation.org wrote:
> > From: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> > 
> > It seems netif_receive_skb() was designed not to call from irq context, but
> > NAPI + NETPOLL break this rule.  If netif_receive_skb() was called from irq
> > context, redirect to netif_rx() instead of processing the skb in that
> > context.
> > 
> > Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> > Cc: Jeff Garzik <jeff@garzik.org>
> > Cc: Francois Romieu <romieu@fr.zoreil.com>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > ---
> > 
> >  net/core/dev.c |   11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> net/* stuff, I'll let DaveM make the call and apply it...

argh, sorry, this patch (which is mysteriously called
8139too-fix-irq-problem-with-napi-netpoll.patch) was supposed to go to
davem and you were supposed to receive
8139too-force-media-setting-fix.patch (which you were cc'ed on).

Re: [patch 12/19] fix irq problem with NAPI + NETPOLL

[David Miller]

From: akpm@linux-foundation.org
Date: Tue, 06 Mar 2007 02:41:56 -0800

> From: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> 
> It seems netif_receive_skb() was designed not to call from irq context, but
> NAPI + NETPOLL break this rule.  If netif_receive_skb() was called from irq
> context, redirect to netif_rx() instead of processing the skb in that
> context.
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> Cc: Jeff Garzik <jeff@garzik.org>
> Cc: Francois Romieu <romieu@fr.zoreil.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

This patch can't be applied, sorry.

netpoll_rx() should be invokable from hardware interrupt context.
What is the crash you are seeing?

It looks like perhaps the kfree_skb() calls need to be modified
in __netpoll_rx().

Stephen?  Maybe this is a result of that set of cleanups you did
to the netpoll code?  It wouldn't surprise me :)

Re: [patch 12/19] fix irq problem with NAPI + NETPOLL

[Atsushi Nemoto]

On Wed, 07 Mar 2007 15:18:44 -0800 (PST), David Miller <davem@davemloft.net> wrote:
> netpoll_rx() should be invokable from hardware interrupt context.
> What is the crash you are seeing?

The problem is not netpoll_rx().  It should be called from irq context.
The problem is, netif_receive_skb() is called from irq context though
it seems not designed to do so.

> It looks like perhaps the kfree_skb() calls need to be modified
> in __netpoll_rx().

Well, it seems an another netpoll bug.
---
Atsushi Nemoto

Re: [patch 12/19] fix irq problem with NAPI + NETPOLL

[Atsushi Nemoto]

On Thu, 08 Mar 2007 10:35:13 +0900 (JST), Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
> > netpoll_rx() should be invokable from hardware interrupt context.
> > What is the crash you are seeing?
> 
> The problem is not netpoll_rx().  It should be called from irq context.
> The problem is, netif_receive_skb() is called from irq context though
> it seems not designed to do so.

Unfortunately I could not reproduce the crash, but IIRC the crash was
happened at upper protocol layer on hardware interrupt context.

Anyway, I think main path of netif_receive_skb() should not be
executed in hardware interrupt context.  Is it wrong?

> > It looks like perhaps the kfree_skb() calls need to be modified
> > in __netpoll_rx().
> 
> Well, it seems an another netpoll bug.

I suppose these kfree_skb() in __netpoll_rx() should be
dev_kfree_skb_any().  And I found an another abuse which is irrelevant
to netpoll.  The netif_rx() calls kfree_skb() at its bottom.  The
netif_rx() should be callable from hardware interrupt context, so it
should be changed to dev_kfree_skb_any().  Is it right?

---
Atsushi Nemoto