linux-kernel - Re: Revert "gro: Fix legacy path napi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 26 Mar 2009 09:18:08 +0000
From:	Jarek Poplawski <jarkao2@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	herbert@...dor.apana.org.au, mingo@...e.hu,
	r.schwebel@...gutronix.de, torvalds@...ux-foundation.org,
	blaschka@...ux.vnet.ibm.com, tglx@...utronix.de,
	a.p.zijlstra@...llo.nl, linux-kernel@...r.kernel.org,
	kernel@...gutronix.de, Adam Richter <adam_richter2004@...oo.com>,
	Sascha Hauer <s.hauer@...gutronix.de>
Subject: Re: Revert "gro: Fix legacy path napi_complete crash",

On Wed, Mar 25, 2009 at 08:20:50PM -0700, David Miller wrote:
...
> Adam Richter has successfully tested Jarek's variant, and if Ingo's
> tests show that it makes his problem go away too then I'm definitely
> going with Jarek's patch.

I hope this patch version on top of Herbert's revert could be useful
then.

Thanks,
Jarek P.
-------------------->
GRO: Re-enable GRO on legacy netif_rx path.

Replacing __napi_complete() with napi_complete() in process_backlog()
caused various breaks for non-napi NICs. It looks like the following
scenario can happen:

process_backlog()			netif_rx()

if (!skb)
local_irq_enable()
					if (queue.qlen) //NO
					napi_schedule() //NOTHING
					__skb_queue_tail() //qlen > 0
napi_complete()
...					...
					Every next netif_rx() sees
					qlen > 0, so napi is never
					scheduled again.

This patch fixes it by open-coding napi_complete() with additional
check for empty queue before __napi_complete().

With help of debugging by Ingo Molnar, Sascha Hauer and Herbert Xu.

Reported-by: Ingo Molnar <mingo@...e.hu>
Tested-by: Adam Richter <adam_richter2004@...oo.com>
Signed-off-by: Jarek Poplawski <jarkao2@...il.com>
---
(vs. 2.6.29 with Herbert's "GRO: Disable GRO on legacy netif_rx path")

diff -Nurp a/net/core/dev.c b/net/core/dev.c
--- a/net/core/dev.c	2009-03-26 08:32:51.000000000 +0000
+++ b/net/core/dev.c	2009-03-26 08:37:58.000000000 +0000
@@ -2588,15 +2588,22 @@ static int process_backlog(struct napi_s
 		local_irq_disable();
 		skb = __skb_dequeue(&queue->input_pkt_queue);
 		if (!skb) {
-			__napi_complete(napi);
 			local_irq_enable();
-			break;
+			napi_gro_flush(napi);
+			local_irq_disable();
+			if (skb_queue_empty(&queue->input_pkt_queue))
+				__napi_complete(napi);
+			local_irq_enable();
+			goto out;
 		}
 		local_irq_enable();
 
-		netif_receive_skb(skb);
+		napi_gro_receive(napi, skb);
 	} while (++work < quota && jiffies == start_time);
 
+	napi_gro_flush(napi);
+
+out:
 	return work;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/