Re: [RFC,PATCH] loopback: calls netif_receive_skb() instead of netif_rx()

Previous thread: alpha build errors with gcc 4.3 by Adrian Bunk on Monday, March 31, 2008 - 2:25 am. (1 message)

Next thread: [PATCH] Pass two of removing "__KERNEL__" tests from unexported headers. by Robert P. J. Day on Monday, March 31, 2008 - 3:25 am. (1 message)
From: Ingo Molnar
Date: Monday, March 31, 2008 - 2:48 am

that's just wrong - 4K stacks on x86 are 4K-sizeof(thread_info) - the 
task struct is allocated elsewhere. The patch below runs just fine on 
4K-stack x86.

	Ingo

------------->
Subject: net: loopback speedup
From: Ingo Molnar <mingo@elte.hu>
Date: Mon Mar 31 11:23:21 CEST 2008

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/net/loopback.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/net/loopback.c
===================================================================
--- linux.orig/drivers/net/loopback.c
+++ linux/drivers/net/loopback.c
@@ -158,7 +158,7 @@ static int loopback_xmit(struct sk_buff 
 	lb_stats->bytes += skb->len;
 	lb_stats->packets++;
 
-	netif_rx(skb);
+	netif_receive_skb(skb);
 
 	return 0;
 }
--

From: David Miller
Date: Monday, March 31, 2008 - 3:08 am

From: Ingo Molnar <mingo@elte.hu>

I don't think it's safe.

Every packet you receive can result in a sent packet, which
in turn can result in a full packet receive path being
taken, and yet again another sent packet.

And so on and so forth.

Some cases like this would be stack bugs, but wouldn't
you like that bug to be a very busy cpu instead of a
crash from overrunning the current stack?
--

From: Ingo Molnar
Date: Monday, March 31, 2008 - 3:44 am

sure.

But the core problem remains: our loopback networking scalability is 
poor. For plain localhost<->localhost connected sockets we hit the 
loopback device lock for every packet, and this very much shows up on 
real workloads on a quad already: the lock instruction in netif_rx is 
the most expensive instruction in a sysbench DB workload.

and it's not just about scalability, the plain algorithmic overhead is 
way too high as well:

 $ taskset 1 ./bw_tcp -s
 $ taskset 1 ./bw_tcp localhost
 Socket bandwidth using localhost: 2607.09 MB/sec
 $ taskset 1 ./bw_pipe
 Pipe bandwidth: 3680.44 MB/sec

i dont think this is acceptable. Either we should fix loopback TCP 
performance or we should transparently switch to VFS pipes as a 
transport method when an app establishes a plain loopback connection (as 
long as there are no frills like content-modifying component in the 
delivery path of packets after a connection has been established - which 
covers 99.9% of the real-life loopback cases).

I'm not suggesting we shouldnt use TCP for connection establishing - but 
if the TCP loopback packet transport is too slow we should use the VFS 
transport which is both more scalable, less cache-intense and has lower 
straight overhead as well.

	Ingo
--

From: David Miller
Date: Monday, March 31, 2008 - 4:02 am

From: Ingo Molnar <mingo@elte.hu>

Set your loopback MTU to some larger value if this result and
the locking overhead upsets you.

Also, woe be to the application that wants fast local interprocess
communication and doesn't use IPC_SHM, MAP_SHARED, pipes, or AF_UNIX
sockets.  (there's not just one better facility, there are _four_!)

From this perspective, people way-overemphasize loopback performance,
and 999 times out of 1000 they prove their points using synthetic
benchmarks.

And don't give me this garbage about the application wanting to be
generic and therefore use IP sockets for everything.  Either they want
to be generic, or they want the absolute best performance.  Trying
to get an "or" and have both at the same time will result in
ludicrious hacks ending up in the kernel.
--


yes, of course it "upsets me" - it shows up in macrobenchmarks as well 
(not just lmbench) - wouldnt (and shouldnt) that upset you?

And even with a ridiculously high MTU of 1048576 there's only a 13% 
improvement:

   # ifconfig lo mtu 1048576
   # taskset 1 ./bw_tcp -s
   # taskset 1 ./bw_tcp localhost
   Socket bandwidth using localhost: 2951.51 MB/sec

pipes are still another ~25% faster:

   # taskset 1 ./bw_pipe

i talked about the localhost data transport only (in the portion you 
dropped from your quotes), not about the connection API or the overall 
management of such sockets. There's absolutely no good technical reason 
i can see why plain loopback sockets should be forced to go over a 
global lock, or why apps should be forced to change to another API when 
the real problem is that kernel developers are lazy or incompetent to 
fix their code.

And i'm still trying to establish whether we have common ground for 
discussion: do you accept my numbers that TCP loopback transport 
performs badly when compared to pipes (i think you accepted that 
implicitly, but i dont want to put anything into your mouth).

Having agreed on that, do you share my view that it should be and could 
be fixed? Or do you claim that it cannot be fixed and wont ever be 
fixed?

	Ingo
--


Chiming in late here, but 1048576 can't possibly work with IP
which uses a 16-bit quantity as the length header.  In fact a
quick test seems to indicate that an 1048576 mtu doesn't generate
anything bigger than the default 16K mtu.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--

From: David Miller
Date: Sunday, April 20, 2008 - 8:38 pm

From: Herbert Xu <herbert@gondor.apana.org.au>

Right.

To move things forward, we should look into doing something
similar to what Al Viro suggested, which would be to return
an SKB pointer from the transmit path and call back into
netif_receive_skb() using that.
--

From: Ingo Molnar
Date: Monday, April 21, 2008 - 1:11 am

yep, basically the sk_peer trick that AF_UNIX is already using.

it just seems rather more tricky in the 'real skb' localhost case 
because there's no real established trust path we can pass this coupling 
of the two sockets over. Netfilter might affect it and deny a localhost 
connection. Lifetime rules seem rather tricky as well: either end of the 
localhost connection can go away independently so a refcount to the 
socket has to be kept. skb->sk might be something to use, but it looks 
like a dangerous complication and it would burden the fastpath with an 
extra sk reference inc/dec.

... so i'm not implying that any of this is an easy topic to solve (to 
me at least :). But fact is that database connections over localhost are 
very common on web apps and it is very convenient as well. I use it 
myself - AF_UNIX transport is often non-existing in apps and libraries 
or is often just an afterthought with limitations - apps tend to 
gravitate towards a single API. So i dont think "use AF_UNIX" is an 
acceptable answer in this case. I believe we should try to make 
localhost transport comparably fast to AF_UNIX.

	Ingo
--

From: David Miller
Date: Monday, April 21, 2008 - 1:16 am

From: Ingo Molnar <mingo@elte.hu>

Please read again, that isn't the suggestion being discussed.

What's being discussed is having the top of the transmit call path
getting a socket "buffer" pointer, that it can feed back into the
packet input path directly.  Loopback would return buffer pointers
from ->hard_start_xmit() instead of passing them netif_rx().  The top
of the transmit call path, upon getting a non-NULL buffer returned,
would pass it to netif_receive_skb().

We're not talking about sockets, although that is another idea (which
I'm working on a patch for, and I have a mechanism for what you refer
to as "path validation").
--

From: Herbert Xu
Date: Monday, April 21, 2008 - 3:19 am

Yes this will definitely reduce the per-packet cost.  The other
low-hanging fruit is to raise the loopback MTU to just below 64K.
I belive the current value is a legacy from the days when we didn't
support skb page frags so everything had to be physically contiguous.

Longer term we could look at generating packets > 64K on lo, for
IPv6 anyway.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--

From: David Miller
Date: Monday, April 21, 2008 - 3:22 am

From: Herbert Xu <herbert@gondor.apana.org.au>

It's legacy from when my top-of-the-line UltraSPARC-I 130Mhz cpus
got the best loopback results using that value :-)
--

From: Eric Dumazet
Date: Monday, March 31, 2008 - 3:01 am

Yes, this error was corrected by Andi already :)

Thank you Ingo but this patch was already suggested by me previously ( 
http://marc.info/?l=linux-netdev&m=120361996713007&w=2 ) and was 
rejected, since we can very easily consume all stack space, especially 
with 4K stacks.
(try with NFS mounts and XFS for example)



Only safe way is to check available free stack space, since we can nest  
loopback_xmit() several time.
In case of protocol errors (like in TCP, if we answer to an ACK by 
another ACK, or ICMP loops), we would exhaust stack instead of delaying 
packets for next softirq run.

Problem is to check available space :

It depends on stack growing UP or DOWN, and depends on caller running on 




--

From: Ingo Molnar
Date: Monday, March 31, 2008 - 3:12 am

ok - i wish such threads were on lkml so that everyone not just the 
netdev kabal can read it. It's quite ugly, but if we want to check stack 
free space i'd suggest for you to put a stack_can_recurse() call into 
arch/x86/kernel/process.c and offer a default __weak implementation in 
kernel/fork.c that always returns 0.

the rule on x86 should be something like this: on 4K stacks and 64-bit 
[which have irqstacks] free stack space can go as low as 25%. On 8K 
stacks [which doesnt have irqstacks but nests irqs] it should not go 
below 50% before falling back to the explicitly queued packet branch.

this way other pieces of kernel code code can choose between on-stack 
fast recursion and explicit iterators. Although i'm not sure i like the 
whole concept to begin with ...

	Ingo
--

From: Eric Dumazet
Date: Tuesday, April 1, 2008 - 2:19 am

Hi Ingo

I took the time to prepare a patch to implement =20
arch_stack_can_recurse() as you suggested.

Thank you

[PATCH] x86 : arch_stack_can_recurse() introduction

Some paths in kernel would like to chose between on-stack fast recursion =

and explicit iterators.

One identified spot is in net loopback driver, where we can avoid=20
netif_rx() and its slowdown if
sufficient stack space is available.

We introduce a generic arch_stack_can_recurse() which default to a weak=20
function returning 0.

 On x86 arch, we implement following logic :

   32 bits and 4K stacks (separate irq stacks) : can use up to 25% of sta=
ck
   64 bits, 8K stacks (separate irq stacks)    : can use up to 25% of sta=
ck
   32 bits and 8K stacks (no irq stacks)       : can use up to 50% of sta=
ck

Example of use in drivers/net/loopback.c, function  loopback_xmit()

if (arch_stack_can_recurse())
    netif_receive_skb(skb); /* immediate delivery to stack */
else
    netif_rx(skb); /* defer to softirq handling */

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>


From: Pavel Machek
Date: Thursday, April 3, 2008 - 7:06 am

From: Eric Dumazet
Date: Thursday, April 3, 2008 - 9:19 am

Patch is OK, my english might be a litle bit unsual :)




--

Previous thread: alpha build errors with gcc 4.3 by Adrian Bunk on Monday, March 31, 2008 - 2:25 am. (1 message)

Next thread: [PATCH] Pass two of removing "__KERNEL__" tests from unexported headers. by Robert P. J. Day on Monday, March 31, 2008 - 3:25 am. (1 message)