[PATCH] Increase snd/rcv buffers in pppoe

[PATCH] Increase snd/rcv buffers in pppoe

[Andi Kleen]

I noticed that a 64bit kernel only downloads half as fast over a PPPoE DSL
connection than a 32bit kernel on the same hardware. The reason seems
to be that PPPoE uses the default 64K snd/rcv buffers at socket creation.
The bigger sk_buff header size on the 64bit kernel pushed
the buffer into being too small and preventing good throughput.

This patch fixes it here. It simply doubles the buffers. There should
be only a few PPPoE sockets active so I don't think this is a problem.
The TX increase is probably not needed because upload speeds on DSL
are typically much slower than RX, but it also cannot hurt.

-Andi


diff -u linux-2.6.3-amd64/drivers/net/pppoe.c-o linux-2.6.3-amd64/drivers/net/pppoe.c
--- linux-2.6.3-amd64/drivers/net/pppoe.c-o	2004-02-19 23:28:07.000000000 +0100
+++ linux-2.6.3-amd64/drivers/net/pppoe.c	2004-02-24 22:02:17.000000000 +0100
@@ -506,6 +506,10 @@
 		goto out;
 
 	sock_init_data(sock, sk);
+
+	sk->sk_rcvbuf *= 2;
+	sk->sk_sndbuf *= 2;
+
 	sk_set_owner(sk, THIS_MODULE);
 	sock->state = SS_UNCONNECTED;
 	sock->ops   = &pppoe_ops;

Re: [PATCH] Increase snd/rcv buffers in pppoe

[David S. Miller]

On 23 Feb 2004 00:47:50 +0100
Andi Kleen <ak@muc.de> wrote:

> I noticed that a 64bit kernel only downloads half as fast over a PPPoE DSL
> connection than a 32bit kernel on the same hardware. The reason seems
> to be that PPPoE uses the default 64K snd/rcv buffers at socket creation.
> The bigger sk_buff header size on the 64bit kernel pushed
> the buffer into being too small and preventing good throughput.

I have no problem with this, but hey while we're at it why not do the same
for the core defaults instead?  I see no real harm in this at all as it's
not a pppoe specific issue.

What say you Andi?

Re: [PATCH] Increase snd/rcv buffers in pppoe

[Andi Kleen]

On Sun, Feb 22, 2004 at 11:26:01PM -0800, David S. Miller wrote:
> On 23 Feb 2004 00:47:50 +0100
> Andi Kleen <ak@muc.de> wrote:
> 
> > I noticed that a 64bit kernel only downloads half as fast over a PPPoE DSL
> > connection than a 32bit kernel on the same hardware. The reason seems
> > to be that PPPoE uses the default 64K snd/rcv buffers at socket creation.
> > The bigger sk_buff header size on the 64bit kernel pushed
> > the buffer into being too small and preventing good throughput.
> 
> I have no problem with this, but hey while we're at it why not do the same
> for the core defaults instead?  I see no real harm in this at all as it's
> not a pppoe specific issue.

The only issue I can think of is that it could be a problem for someone
with thousands of UDP (possible some other socket family) sockets. UDP doesn't
have the memory controls TCPs has to deal with that. Upping the defaults
to 128K could run him out of memory when many of the sockets are active
at the same time.

But increasing it to 128K is probably safe enough with the increased
memory sizes of today.  Here's a new patch.

-Andi


diff -u linux-2.6.3-averell32/include/linux/skbuff.h-o linux-2.6.3-averell32/include/linux/skbuff.h
--- linux-2.6.3-averell32/include/linux/skbuff.h-o	2004-02-19 23:28:07.000000000 +0100
+++ linux-2.6.3-averell32/include/linux/skbuff.h	2004-02-25 12:42:07.000000000 +0100
@@ -271,8 +271,8 @@
 				*end;
 };
 
-#define SK_WMEM_MAX	65535
-#define SK_RMEM_MAX	65535
+#define SK_WMEM_MAX    131072
+#define SK_RMEM_MAX	131072
 
 #ifdef __KERNEL__
 /*

Re: [PATCH] Increase snd/rcv buffers in pppoe

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20040223105359.GA91938@colin2.muc.de> (at 23 Feb 2004 11:53:59 +0100,Mon, 23 Feb 2004 11:53:59 +0100), Andi Kleen <ak@muc.de> says:

> -#define SK_WMEM_MAX	65535
> -#define SK_RMEM_MAX	65535
> +#define SK_WMEM_MAX    131072
> +#define SK_RMEM_MAX	131072

131071?

--yoshfuji

Re: [PATCH] Increase snd/rcv buffers in pppoe

[Andi Kleen]

On Mon, Feb 23, 2004 at 08:01:01PM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
> In article <20040223105359.GA91938@colin2.muc.de> (at 23 Feb 2004 11:53:59 +0100,Mon, 23 Feb 2004 11:53:59 +0100), Andi Kleen <ak@muc.de> says:
> 
> > -#define SK_WMEM_MAX	65535
> > -#define SK_RMEM_MAX	65535
> > +#define SK_WMEM_MAX    131072
> > +#define SK_RMEM_MAX	131072
> 
> 131071?

Probably, but it doesn't make any difference; see how the skbuff socket
accounting works. Really there isn't really a need to make it power 
of two (except for mystifying arbitary magic numbers for the users ;-) 
130000 or 200000 would do as well.

-Andi

Re: [PATCH] Increase snd/rcv buffers in pppoe

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20040223111659.GB10681@colin2.muc.de> (at 23 Feb 2004 12:16:59 +0100,Mon, 23 Feb 2004 12:16:59 +0100), Andi Kleen <ak@muc.de> says:

> On Mon, Feb 23, 2004 at 08:01:01PM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
> > In article <20040223105359.GA91938@colin2.muc.de> (at 23 Feb 2004 11:53:59 +0100,Mon, 23 Feb 2004 11:53:59 +0100), Andi Kleen <ak@muc.de> says:
> > 
> > > -#define SK_WMEM_MAX	65535
> > > -#define SK_RMEM_MAX	65535
> > > +#define SK_WMEM_MAX    131072
> > > +#define SK_RMEM_MAX	131072
> > 
> > 131071?
> 
> Probably, but it doesn't make any difference; see how the skbuff socket
> accounting works. Really there isn't really a need to make it power 
> of two (except for mystifying arbitary magic numbers for the users ;-) 
> 130000 or 200000 would do as well.

But, others use (2^n - 1) and
people will be confused if there are both 131071 and 131072.

--yoshfuji

Re: [PATCH] Increase snd/rcv buffers in pppoe

[David S. Miller]

Wait, I have an idea.

I was going to suggest ifdef'ing this change for 64-bit, but there is an even
nicer way to do this and it avoids the magic number argument we're having right
now too.

Let's compute this for _real_, as some kind of function on sizeof(struct sk_buff)
For example, the overhead of N 1-byte packets.

Andi has made a real observation in that various areas of performance are sensitive
to the snd/rcv buffer size, and the values we choose are inherently tied to the
size of struct sk_buff.  So whatever random value we choose today, could be useless
and cause bad performance the next time we change struct sk_buff in some way.

The proposal I make intends to avoid this endless tweaking.

Two more observations while grepping for SK_{R,W}MEM_MAX.

1) IPV4 icmp sents sk_sndbuf of it's sockets to "2 * SK_WMEM_MAX", that's not
   what it really wants.  What it really wants is enough space to hold
   ~2 full sized IPV4 packets, roughly 2 * 64K + struct sk_buff overhead
   and thus that is what it should be using there.

2) IPV6 icmp does the same as ipv4, except this value is even more wrong there
   especially considering jumbograms.  With current code, sending a jumbogram
   ipv6 icmp packet would simply fail, and I wonder if anyone has even tried
   this.

I'll cook something up to address all of this and it's going to go into 2.4.x as
well.

Re: [PATCH] Increase snd/rcv buffers in pppoe

[Andi Kleen]

On Mon, 23 Feb 2004 10:26:13 -0800
"David S. Miller" <davem@redhat.com> wrote:

> 
> The proposal I make intends to avoid this endless tweaking.

[...] Sounds good to me.
 
> Two more observations while grepping for SK_{R,W}MEM_MAX.
> 
> 1) IPV4 icmp sents sk_sndbuf of it's sockets to "2 * SK_WMEM_MAX", that's not
>    what it really wants.  What it really wants is enough space to hold
>    ~2 full sized IPV4 packets, roughly 2 * 64K + struct sk_buff overhead
>    and thus that is what it should be using there.

Just sk_buff overhead for what MTU? 576? (would be a bit extreme)
And in theory it could be one byte packets too.

> 2) IPV6 icmp does the same as ipv4, except this value is even more wrong there
>    especially considering jumbograms.  With current code, sending a jumbogram
>    ipv6 icmp packet would simply fail, and I wonder if anyone has even tried
>    this.

Isn't even ICMPv6 limited to the minimum guaranteed MTU (1000 something) like ICMPv4 is to
576 bytes?

-Andi

Re: [PATCH] Increase snd/rcv buffers in pppoe

[David S. Miller]

On Wed, 25 Feb 2004 21:15:26 +0100
Andi Kleen <ak@suse.de> wrote:

[ mostrows removed from CC:, he bounces and this is no longer a pppoe discussion
  anymore :) ]

> On Mon, 23 Feb 2004 10:26:13 -0800
> "David S. Miller" <davem@redhat.com> wrote:
> 
> > 1) IPV4 icmp sents sk_sndbuf of it's sockets to "2 * SK_WMEM_MAX", that's not
> >    what it really wants.  What it really wants is enough space to hold
> >    ~2 full sized IPV4 packets, roughly 2 * 64K + struct sk_buff overhead
> >    and thus that is what it should be using there.
> 
> Just sk_buff overhead for what MTU? 576? (would be a bit extreme)
> And in theory it could be one byte packets too.

Two full sized ICMP echo responses (64K) of data plus 2 struct sk_buff, for example.

> > 2) IPV6 icmp does the same as ipv4, except this value is even more wrong there
> >    especially considering jumbograms.  With current code, sending a jumbogram
> >    ipv6 icmp packet would simply fail, and I wonder if anyone has even tried
> >    this.
> 
> Isn't even ICMPv6 limited to the minimum guaranteed MTU (1000 something) like ICMPv4 is to
> 576 bytes?

What about ECHO?  I can't send an ICMPv6 jumbo sized ECHO and expect a fully quoted response
back?

Re: [PATCH] Increase snd/rcv buffers in pppoe

[Andi Kleen]

Sorry for the late answer.

On Mon, Feb 23, 2004 at 01:32:33PM -0800, David S. Miller wrote:
> > > 2) IPV6 icmp does the same as ipv4, except this value is even more wrong there
> > >    especially considering jumbograms.  With current code, sending a jumbogram
> > >    ipv6 icmp packet would simply fail, and I wonder if anyone has even tried
> > >    this.
> > 
> > Isn't even ICMPv6 limited to the minimum guaranteed MTU (1000 something) like ICMPv4 is to
> > 576 bytes?
> 
> What about ECHO?  I can't send an ICMPv6 jumbo sized ECHO and expect a fully quoted response
> back?

I don't think it should be only sized for big datagrams and other obscure cases.
Better is a reasonable default to fit at least 64K of data with a standard MTU like ~1.4K. 
Or may resize it on MTU change, but I'm not sure that's worth the effort. 

Are you working on this or should I prepare a new patch? 

-Andi

Re: [PATCH] Increase snd/rcv buffers in pppoe

[David S. Miller]

On 26 Feb 2004 20:49:00 +0100
Andi Kleen <ak@muc.de> wrote:

> I don't think it should be only sized for big datagrams and other obscure cases.
> Better is a reasonable default to fit at least 64K of data with a standard MTU like ~1.4K. 
> Or may resize it on MTU change, but I'm not sure that's worth the effort. 
> 
> Are you working on this or should I prepare a new patch? 

Let's just leave things how we've changed them, and when people complain
about jumbo MTU ipv6 icmp messages we'll address it.

Thanks.

Re: [PATCH] Increase snd/rcv buffers in pppoe

[Andi Kleen]

On Thu, Feb 26, 2004 at 12:42:45PM -0800, David S. Miller wrote:
> On 26 Feb 2004 20:49:00 +0100
> Andi Kleen <ak@muc.de> wrote:
> 
> > I don't think it should be only sized for big datagrams and other obscure cases.
> > Better is a reasonable default to fit at least 64K of data with a standard MTU like ~1.4K. 
> > Or may resize it on MTU change, but I'm not sure that's worth the effort. 
> > 
> > Are you working on this or should I prepare a new patch? 
> 
> Let's just leave things how we've changed them, and when people complain
> about jumbo MTU ipv6 icmp messages we'll address it.

Ah, I missed that you already checked it in.  Sorry for the noise.

I will test now if your change really fixes the DSL performance problem.

-Andi

Re: [PATCH] Increase snd/rcv buffers in pppoe

[Andi Kleen]

> I will test now if your change really fixes the DSL performance problem.

It seems to, but I guess that was by accident.

-Andi

Re: [PATCH] Increase snd/rcv buffers in pppoe

[David S. Miller]

On 26 Feb 2004 23:03:16 +0100
Andi Kleen <ak@muc.de> wrote:

> > I will test now if your change really fixes the DSL performance problem.
> 
> It seems to, but I guess that was by accident.

Nope, fully intentional.  It should end up doing the same thing your
original patch did, check the send buffer size the pppoe socket gets :)