net: more accurate skb truesize - regression on Microblaze

net: more accurate skb truesize - regression on Microblaze

[Michal Simek]

Hi Eric,

I have spent some time to investigate what it is causing regression on our network bechmark
and I have identified that your patch "net: more accurate skb truesize" is causing that.

I see regression especially on incoming tcp (RX) connection which is 20%. On TX I see regression till 5%.

In general microblaze systems are pretty sensitive for memory usage and working with it.
Increasing packet sizes has big impact on performance.
I was surprised that regression is so high.

 From microblaze point of view is more important to remove that performance regression
than having more accurate memory accounting. 20% performance regression is simple so high
for that.
Isn't there any other way to doing better memory accounting?
Or is there any workaround which would be possible to use?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: net: more accurate skb truesize - regression on Microblaze

[Eric Dumazet]

On Tue, 2012-04-10 at 10:10 +0200, Michal Simek wrote:
> Hi Eric,
> 
> I have spent some time to investigate what it is causing regression on our network bechmark
> and I have identified that your patch "net: more accurate skb truesize" is causing that.
> 
> I see regression especially on incoming tcp (RX) connection which is 20%. On TX I see regression till 5%.
> 
> In general microblaze systems are pretty sensitive for memory usage and working with it.
> Increasing packet sizes has big impact on performance.
> I was surprised that regression is so high.
> 
>  From microblaze point of view is more important to remove that performance regression
> than having more accurate memory accounting. 20% performance regression is simple so high
> for that.
> Isn't there any other way to doing better memory accounting?
> Or is there any workaround which would be possible to use?
> 
> Thanks,
> Michal
> 

Hi Michal

We are currently working on the issue.

TCP stack was a bit optimistic in the ideal skb->len / skb->truesize
ratio that happened to 'mostly work' before my patch, but not anymore
after it.

memory accounting was wrong, we really wanted to be accurate, or risk
OOM and crashes.

Now we must fix tcp.

In the meantime you could try :

echo 1 >/proc/sys/net/ipv4/tcp_adv_win_scale

BTW, some NIC drivers are known to provide fat skb in their rx path, and
need to be fixed as well. (Some others just lie about skb->truesize to
avoid the tcp slowdown, see my previous iwlwifi patch)

Re: net: more accurate skb truesize - regression on Microblaze

[Eric Dumazet]

On Tue, 2012-04-10 at 10:27 +0200, Eric Dumazet wrote:

> BTW, some NIC drivers are known to provide fat skb in their rx path, and
> need to be fixed as well. (Some others just lie about skb->truesize to
> avoid the tcp slowdown, see my previous iwlwifi patch)
> 
> 

What is the driver you currently use on your platform ?

Re: net: more accurate skb truesize - regression on Microblaze

[Michal Simek]

On 04/10/2012 10:27 AM, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 10:10 +0200, Michal Simek wrote:
>> Hi Eric,
>>
>> I have spent some time to investigate what it is causing regression on our network bechmark
>> and I have identified that your patch "net: more accurate skb truesize" is causing that.
>>
>> I see regression especially on incoming tcp (RX) connection which is 20%. On TX I see regression till 5%.
>>
>> In general microblaze systems are pretty sensitive for memory usage and working with it.
>> Increasing packet sizes has big impact on performance.
>> I was surprised that regression is so high.
>>
>>   From microblaze point of view is more important to remove that performance regression
>> than having more accurate memory accounting. 20% performance regression is simple so high
>> for that.
>> Isn't there any other way to doing better memory accounting?
>> Or is there any workaround which would be possible to use?
>>
>> Thanks,
>> Michal
>>
>
> Hi Michal
>
> We are currently working on the issue.
>
> TCP stack was a bit optimistic in the ideal skb->len / skb->truesize
> ratio that happened to 'mostly work' before my patch, but not anymore
> after it.
>
> memory accounting was wrong, we really wanted to be accurate, or risk
> OOM and crashes.
>
> Now we must fix tcp.
>
> In the meantime you could try :
>
> echo 1>/proc/sys/net/ipv4/tcp_adv_win_scale

will retest it and let you know.

Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: net: more accurate skb truesize - regression on Microblaze

[Michal Simek]

On 04/10/2012 10:32 AM, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 10:27 +0200, Eric Dumazet wrote:
>
>> BTW, some NIC drivers are known to provide fat skb in their rx path, and
>> need to be fixed as well. (Some others just lie about skb->truesize to
>> avoid the tcp slowdown, see my previous iwlwifi patch)
>>
>>
>
> What is the driver you currently use on your platform ?

Using Xilinx ll_temac(in mainline) and axi_emac.


Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: net: more accurate skb truesize - regression on Microblaze

[Eric Dumazet]

On Tue, 2012-04-10 at 10:37 +0200, Michal Simek wrote:
> On 04/10/2012 10:32 AM, Eric Dumazet wrote:
> > On Tue, 2012-04-10 at 10:27 +0200, Eric Dumazet wrote:
> >
> >> BTW, some NIC drivers are known to provide fat skb in their rx path, and
> >> need to be fixed as well. (Some others just lie about skb->truesize to
> >> avoid the tcp slowdown, see my previous iwlwifi patch)
> >>
> >>
> >
> > What is the driver you currently use on your platform ?
> 
> Using Xilinx ll_temac(in mainline) and axi_emac.
> 

OK thanks, could you please give us the content
of /proc/sys/net/ipv4/tcp_rmem ?

Re: net: more accurate skb truesize - regression on Microblaze

[Michal Simek]

On 04/10/2012 10:45 AM, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 10:37 +0200, Michal Simek wrote:
>> On 04/10/2012 10:32 AM, Eric Dumazet wrote:
>>> On Tue, 2012-04-10 at 10:27 +0200, Eric Dumazet wrote:
>>>
>>>> BTW, some NIC drivers are known to provide fat skb in their rx path, and
>>>> need to be fixed as well. (Some others just lie about skb->truesize to
>>>> avoid the tcp slowdown, see my previous iwlwifi patch)
>>>>
>>>>
>>>
>>> What is the driver you currently use on your platform ?
>>
>> Using Xilinx ll_temac(in mainline) and axi_emac.
>>
>
> OK thanks, could you please give us the content
> of /proc/sys/net/ipv4/tcp_rmem ?

~ # cat /proc/sys/net/ipv4/tcp_rmem
4096    87380   130048


Let me know if you want to test anything.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: net: more accurate skb truesize - regression on Microblaze

[Eric Dumazet]

On Tue, 2012-04-10 at 11:11 +0200, Michal Simek wrote:

> ~ # cat /proc/sys/net/ipv4/tcp_rmem
> 4096    87380   130048

Are they default values, or tuned by admin ?

130048 bytes isnt enough to let TCP open its rcv window.

Re: net: more accurate skb truesize - regression on Microblaze

[Michal Simek]

On 04/10/2012 11:24 AM, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 11:11 +0200, Michal Simek wrote:
>
>> ~ # cat /proc/sys/net/ipv4/tcp_rmem
>> 4096    87380   130048
>
> Are they default values, or tuned by admin ?
>
> 130048 bytes isnt enough to let TCP open its rcv window.

yep. Default value after powerup. What's wrong with that?

Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: net: more accurate skb truesize - regression on Microblaze

[Eric Dumazet]

On Tue, 2012-04-10 at 11:29 +0200, Michal Simek wrote:
> On 04/10/2012 11:24 AM, Eric Dumazet wrote:
> > On Tue, 2012-04-10 at 11:11 +0200, Michal Simek wrote:
> >
> >> ~ # cat /proc/sys/net/ipv4/tcp_rmem
> >> 4096    87380   130048
> >
> > Are they default values, or tuned by admin ?
> >
> > 130048 bytes isnt enough to let TCP open its rcv window.
> 
> yep. Default value after powerup. What's wrong with that?
> 
> Michal
> 

I guess your tcp performance is driven by these numbers mostly.

receive window wont grow above 64K in these case. a tcpdump could
confirm the issue.

Do you have 130000 pages of memory ?

Seems the net/ipv4/tcp.c code is wrong (or the comment is wrong)

Since its not 1/128 but 1/1024 ....


        /* Set per-socket limits to no more than 1/128 the pressure threshold */
        limit = nr_free_buffer_pages() << (PAGE_SHIFT - 10);
        limit = max(limit, 128UL);
        max_share = min(4UL*1024*1024, limit);

        sysctl_tcp_wmem[0] = SK_MEM_QUANTUM;
        sysctl_tcp_wmem[1] = 16*1024;
        sysctl_tcp_wmem[2] = max(64*1024, max_share);

        sysctl_tcp_rmem[0] = SK_MEM_QUANTUM;
        sysctl_tcp_rmem[1] = 87380;
        sysctl_tcp_rmem[2] = max(87380, max_share);

Re: net: more accurate skb truesize - regression on Microblaze

[Michal Simek]

On 04/10/2012 11:38 AM, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 11:29 +0200, Michal Simek wrote:
>> On 04/10/2012 11:24 AM, Eric Dumazet wrote:
>>> On Tue, 2012-04-10 at 11:11 +0200, Michal Simek wrote:
>>>
>>>> ~ # cat /proc/sys/net/ipv4/tcp_rmem
>>>> 4096    87380   130048
>>>
>>> Are they default values, or tuned by admin ?
>>>
>>> 130048 bytes isnt enough to let TCP open its rcv window.
>>
>> yep. Default value after powerup. What's wrong with that?
>>
>> Michal
>>
>
> I guess your tcp performance is driven by these numbers mostly.
>
> receive window wont grow above 64K in these case. a tcpdump could
> confirm the issue.
>
> Do you have 130000 pages of memory ?

128MB memory and 4k page which is 32k pages.

Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: net: more accurate skb truesize - regression on Microblaze

[Eric Dumazet]

On Tue, 2012-04-10 at 11:38 +0200, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 11:29 +0200, Michal Simek wrote:
> > On 04/10/2012 11:24 AM, Eric Dumazet wrote:
> > > On Tue, 2012-04-10 at 11:11 +0200, Michal Simek wrote:
> > >
> > >> ~ # cat /proc/sys/net/ipv4/tcp_rmem
> > >> 4096    87380   130048
> > >
> > > Are they default values, or tuned by admin ?
> > >
> > > 130048 bytes isnt enough to let TCP open its rcv window.
> > 
> > yep. Default value after powerup. What's wrong with that?
> > 
> > Michal
> > 
> 
> I guess your tcp performance is driven by these numbers mostly.
> 
> receive window wont grow above 64K in these case. a tcpdump could
> confirm the issue.
> 
> Do you have 130000 pages of memory ?
> 
> Seems the net/ipv4/tcp.c code is wrong (or the comment is wrong)
> 
> Since its not 1/128 but 1/1024 ....
> 
> 
>         /* Set per-socket limits to no more than 1/128 the pressure threshold */
>         limit = nr_free_buffer_pages() << (PAGE_SHIFT - 10);
>         limit = max(limit, 128UL);
>         max_share = min(4UL*1024*1024, limit);
> 
>         sysctl_tcp_wmem[0] = SK_MEM_QUANTUM;
>         sysctl_tcp_wmem[1] = 16*1024;
>         sysctl_tcp_wmem[2] = max(64*1024, max_share);
> 
>         sysctl_tcp_rmem[0] = SK_MEM_QUANTUM;
>         sysctl_tcp_rmem[1] = 87380;
>         sysctl_tcp_rmem[2] = max(87380, max_share);
> 

OK there is a bug introduced in commit
c43b874d5d714f271b80d4c3f49e05d0cbf51ed2
(tcp: properly initialize tcp memory limits)

I'll send a patch to change : 

limit = nr_free_buffer_pages() << (PAGE_SHIFT - 10);

back to

limit = nr_free_buffer_pages() << (PAGE_SHIFT - 7);


You could try this :)

[PATCH] tcp: restore correct limit

[Eric Dumazet]

Commit c43b874d5d714f (tcp: properly initialize tcp memory limits)
added a regression on machines with low amount of memory, since sockets
cant use 1/128 of memory but 1/1024

Fix this to match comment and previous behavior.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Glauber Costa <glommer@parallels.com>
---
 net/ipv4/tcp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 5d54ed3..67d726e 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3302,7 +3302,7 @@ void __init tcp_init(void)
 
 	tcp_init_mem(&init_net);
 	/* Set per-socket limits to no more than 1/128 the pressure threshold */
-	limit = nr_free_buffer_pages() << (PAGE_SHIFT - 10);
+	limit = nr_free_buffer_pages() << (PAGE_SHIFT - 7);
 	limit = max(limit, 128UL);
 	max_share = min(4UL*1024*1024, limit);
 

Re: net: more accurate skb truesize - regression on Microblaze

[Michal Simek]

On 04/10/2012 11:52 AM, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 11:38 +0200, Eric Dumazet wrote:
>> On Tue, 2012-04-10 at 11:29 +0200, Michal Simek wrote:
>>> On 04/10/2012 11:24 AM, Eric Dumazet wrote:
>>>> On Tue, 2012-04-10 at 11:11 +0200, Michal Simek wrote:
>>>>
>>>>> ~ # cat /proc/sys/net/ipv4/tcp_rmem
>>>>> 4096    87380   130048
>>>>
>>>> Are they default values, or tuned by admin ?
>>>>
>>>> 130048 bytes isnt enough to let TCP open its rcv window.
>>>
>>> yep. Default value after powerup. What's wrong with that?
>>>
>>> Michal
>>>
>>
>> I guess your tcp performance is driven by these numbers mostly.
>>
>> receive window wont grow above 64K in these case. a tcpdump could
>> confirm the issue.
>>
>> Do you have 130000 pages of memory ?
>>
>> Seems the net/ipv4/tcp.c code is wrong (or the comment is wrong)
>>
>> Since its not 1/128 but 1/1024 ....
>>
>>
>>          /* Set per-socket limits to no more than 1/128 the pressure threshold */
>>          limit = nr_free_buffer_pages()<<  (PAGE_SHIFT - 10);
>>          limit = max(limit, 128UL);
>>          max_share = min(4UL*1024*1024, limit);
>>
>>          sysctl_tcp_wmem[0] = SK_MEM_QUANTUM;
>>          sysctl_tcp_wmem[1] = 16*1024;
>>          sysctl_tcp_wmem[2] = max(64*1024, max_share);
>>
>>          sysctl_tcp_rmem[0] = SK_MEM_QUANTUM;
>>          sysctl_tcp_rmem[1] = 87380;
>>          sysctl_tcp_rmem[2] = max(87380, max_share);
>>
>
> OK there is a bug introduced in commit
> c43b874d5d714f271b80d4c3f49e05d0cbf51ed2
> (tcp: properly initialize tcp memory limits)
>
> I'll send a patch to change :
>
> limit = nr_free_buffer_pages()<<  (PAGE_SHIFT - 10);
>
> back to
>
> limit = nr_free_buffer_pages()<<  (PAGE_SHIFT - 7);
>
>
> You could try this :)

sure.

~ # cat /proc/sys/net/ipv4/tcp_rmem
4096    87380   1040384

Regression is till 5% which is much better on hw design without csum support.

I will also test it with csum support and let you know.

Michal



-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: [PATCH] tcp: restore correct limit

[Michal Simek]

On 04/10/2012 12:03 PM, Eric Dumazet wrote:
> Commit c43b874d5d714f (tcp: properly initialize tcp memory limits)
> added a regression on machines with low amount of memory, since sockets
> cant use 1/128 of memory but 1/1024
>
> Fix this to match comment and previous behavior.
>
> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
> Cc: Jason Wang<jasowang@redhat.com>
> Cc: Glauber Costa<glommer@parallels.com>
> ---
>   net/ipv4/tcp.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 5d54ed3..67d726e 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -3302,7 +3302,7 @@ void __init tcp_init(void)
>
>   	tcp_init_mem(&init_net);
>   	/* Set per-socket limits to no more than 1/128 the pressure threshold */
> -	limit = nr_free_buffer_pages()<<  (PAGE_SHIFT - 10);
> +	limit = nr_free_buffer_pages()<<  (PAGE_SHIFT - 7);
>   	limit = max(limit, 128UL);
>   	max_share = min(4UL*1024*1024, limit);
>

hw design with csum is also much better.
Tested-by: Michal Simek <monstr@monstr.eu>

Thanks for help,
Michal





-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: [PATCH] tcp: restore correct limit

[Jason Wang]

On 04/10/2012 06:14 PM, Michal Simek wrote:
> On 04/10/2012 12:03 PM, Eric Dumazet wrote:
>> Commit c43b874d5d714f (tcp: properly initialize tcp memory limits)
>> added a regression on machines with low amount of memory, since sockets
>> cant use 1/128 of memory but 1/1024
>>
>> Fix this to match comment and previous behavior.
>>
>> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
>> Cc: Jason Wang<jasowang@redhat.com>
>> Cc: Glauber Costa<glommer@parallels.com>
>> ---
>>   net/ipv4/tcp.c |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>> index 5d54ed3..67d726e 100644
>> --- a/net/ipv4/tcp.c
>> +++ b/net/ipv4/tcp.c
>> @@ -3302,7 +3302,7 @@ void __init tcp_init(void)
>>
>>       tcp_init_mem(&init_net);
>>       /* Set per-socket limits to no more than 1/128 the pressure 
>> threshold */
>> -    limit = nr_free_buffer_pages()<<  (PAGE_SHIFT - 10);
>> +    limit = nr_free_buffer_pages()<<  (PAGE_SHIFT - 7);
>>       limit = max(limit, 128UL);
>>       max_share = min(4UL*1024*1024, limit);
>>
>
> hw design with csum is also much better.
> Tested-by: Michal Simek <monstr@monstr.eu>
>
> Thanks for help,
> Michal
>
>
>
>
>
Hi Michal and Eric:

Which version of kernel did you test, did you try the newest kernel? The 
reason I use (PAGE_SHIFT - 10) is in the commit before 3dc43e3, the 
limit were calculated with:

     limit = nr_free_buffer_pages() / 8;
     limit = max(limit, 128UL);
...
     limit = ((unsigned long)sysctl_tcp_mem[1]) << (PAGE_SHIFT - 7);

So the rmem should be ok. But there's a defect (which I think does 
affect the regression) of my patch would could cause limit that we 
should shift after comparing with 128UL like:

     limit = nr_free_buffer_pages() / 8;
     limit = max(limit, 128UL) << (PAGE_SHIFT - 7);

Is anything I miss?

Thanks

Re: [PATCH] tcp: restore correct limit

[Michal Simek]

On 04/10/2012 12:29 PM, Jason Wang wrote:
> On 04/10/2012 06:14 PM, Michal Simek wrote:
>> On 04/10/2012 12:03 PM, Eric Dumazet wrote:
>>> Commit c43b874d5d714f (tcp: properly initialize tcp memory limits)
>>> added a regression on machines with low amount of memory, since sockets
>>> cant use 1/128 of memory but 1/1024
>>>
>>> Fix this to match comment and previous behavior.
>>>
>>> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
>>> Cc: Jason Wang<jasowang@redhat.com>
>>> Cc: Glauber Costa<glommer@parallels.com>
>>> ---
>>> net/ipv4/tcp.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>>> index 5d54ed3..67d726e 100644
>>> --- a/net/ipv4/tcp.c
>>> +++ b/net/ipv4/tcp.c
>>> @@ -3302,7 +3302,7 @@ void __init tcp_init(void)
>>>
>>> tcp_init_mem(&init_net);
>>> /* Set per-socket limits to no more than 1/128 the pressure threshold */
>>> - limit = nr_free_buffer_pages()<< (PAGE_SHIFT - 10);
>>> + limit = nr_free_buffer_pages()<< (PAGE_SHIFT - 7);
>>> limit = max(limit, 128UL);
>>> max_share = min(4UL*1024*1024, limit);
>>>
>>
>> hw design with csum is also much better.
>> Tested-by: Michal Simek <monstr@monstr.eu>
>>
>> Thanks for help,
>> Michal
>>
>>
>>
>>
>>
> Hi Michal and Eric:
>
> Which version of kernel did you test, did you try the newest kernel? The reason I use (PAGE_SHIFT - 10) is in the commit before 3dc43e3, the limit were calculated with:

I have tested it on 3.4.0-rc2.

Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: [PATCH] tcp: restore correct limit

[Eric Dumazet]

On Tue, 2012-04-10 at 18:29 +0800, Jason Wang wrote:
> On 04/10/2012 06:14 PM, Michal Simek wrote:
> > On 04/10/2012 12:03 PM, Eric Dumazet wrote:
> >> Commit c43b874d5d714f (tcp: properly initialize tcp memory limits)
> >> added a regression on machines with low amount of memory, since sockets
> >> cant use 1/128 of memory but 1/1024
> >>
> >> Fix this to match comment and previous behavior.
> >>
> >> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
> >> Cc: Jason Wang<jasowang@redhat.com>
> >> Cc: Glauber Costa<glommer@parallels.com>
> >> ---
> >>   net/ipv4/tcp.c |    2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> >> index 5d54ed3..67d726e 100644
> >> --- a/net/ipv4/tcp.c
> >> +++ b/net/ipv4/tcp.c
> >> @@ -3302,7 +3302,7 @@ void __init tcp_init(void)
> >>
> >>       tcp_init_mem(&init_net);
> >>       /* Set per-socket limits to no more than 1/128 the pressure 
> >> threshold */
> >> -    limit = nr_free_buffer_pages()<<  (PAGE_SHIFT - 10);
> >> +    limit = nr_free_buffer_pages()<<  (PAGE_SHIFT - 7);
> >>       limit = max(limit, 128UL);
> >>       max_share = min(4UL*1024*1024, limit);
> >>
> >
> > hw design with csum is also much better.
> > Tested-by: Michal Simek <monstr@monstr.eu>
> >
> > Thanks for help,
> > Michal
> >
> >
> >
> >
> >
> Hi Michal and Eric:
> 
> Which version of kernel did you test, did you try the newest kernel? The 
> reason I use (PAGE_SHIFT - 10) is in the commit before 3dc43e3, the 
> limit were calculated with:
> 
>      limit = nr_free_buffer_pages() / 8;
>      limit = max(limit, 128UL);
> ...
>      limit = ((unsigned long)sysctl_tcp_mem[1]) << (PAGE_SHIFT - 7);
> 
> So the rmem should be ok. But there's a defect (which I think does 
> affect the regression) of my patch would could cause limit that we 
> should shift after comparing with 128UL like:
> 
>      limit = nr_free_buffer_pages() / 8;
>      limit = max(limit, 128UL) << (PAGE_SHIFT - 7);
> 
> Is anything I miss?
> 

Yes, probably.

Maybe you should check what was the situation on 2.6 kernels.

Your commit did not completely fix the 4acb41903b2 one

I dont feel its necessary to put in the changelog the complete bug
history, since your commit does the needed tracking.

[PATCH v2] tcp: restore correct limit

[Eric Dumazet]

Commit c43b874d5d714f (tcp: properly initialize tcp memory limits) tried
to fix a regression added in commits 4acb4190 & 3dc43e3,
but still get it wrong.

Result is machines with low amount of memory have too small tcp_rmem[2]
value and slow tcp receives : Per socket limit being 1/1024 of memory
instead of 1/128 in old kernels, so rcv window is capped to small
values.

Fix this to match comment and previous behavior.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Glauber Costa <glommer@parallels.com>
---
v2: remove one useless "limit = max(limit, 128UL);" line

 net/ipv4/tcp.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 5d54ed3..7758a83 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3302,8 +3302,7 @@ void __init tcp_init(void)
 
 	tcp_init_mem(&init_net);
 	/* Set per-socket limits to no more than 1/128 the pressure threshold */
-	limit = nr_free_buffer_pages() << (PAGE_SHIFT - 10);
-	limit = max(limit, 128UL);
+	limit = nr_free_buffer_pages() << (PAGE_SHIFT - 7);
 	max_share = min(4UL*1024*1024, limit);
 
 	sysctl_tcp_wmem[0] = SK_MEM_QUANTUM;

Re: net: more accurate skb truesize - regression on Microblaze

[Eric Dumazet]

On Tue, 2012-04-10 at 10:37 +0200, Michal Simek wrote:
> On 04/10/2012 10:32 AM, Eric Dumazet wrote:
> > On Tue, 2012-04-10 at 10:27 +0200, Eric Dumazet wrote:
> >
> >> BTW, some NIC drivers are known to provide fat skb in their rx path, and
> >> need to be fixed as well. (Some others just lie about skb->truesize to
> >> avoid the tcp slowdown, see my previous iwlwifi patch)
> >>
> >>
> >
> > What is the driver you currently use on your platform ?
> 
> Using Xilinx ll_temac(in mainline) and axi_emac.
> 

ll_temac allocates XTE_MAX_JUMBO_FRAME_SIZE frames, even for MTU=1500 ?

Trying to allocate smaller packets would definitely help your tcp
receiver performance and overall reliability  (order-0 allocations
instead of order-2)

Re: net: more accurate skb truesize - regression on Microblaze

[Michal Simek]

On 04/10/2012 01:32 PM, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 10:37 +0200, Michal Simek wrote:
>> On 04/10/2012 10:32 AM, Eric Dumazet wrote:
>>> On Tue, 2012-04-10 at 10:27 +0200, Eric Dumazet wrote:
>>>
>>>> BTW, some NIC drivers are known to provide fat skb in their rx path, and
>>>> need to be fixed as well. (Some others just lie about skb->truesize to
>>>> avoid the tcp slowdown, see my previous iwlwifi patch)
>>>>
>>>>
>>>
>>> What is the driver you currently use on your platform ?
>>
>> Using Xilinx ll_temac(in mainline) and axi_emac.
>>
>
> ll_temac allocates XTE_MAX_JUMBO_FRAME_SIZE frames, even for MTU=1500 ?

yes.

>
> Trying to allocate smaller packets would definitely help your tcp
> receiver performance and overall reliability  (order-0 allocations
> instead of order-2)

There is long history for ll_temac and axi emac drivers. and if you compare
them they are very similar with similar bugs.
In our distribution we are using older ll_temac driver where this is fixed.
Performance is much higher if you allocate buffers for actual mtu.

Thanks,
Michal

P.S.: Our plan is to start to use mainline drivers and fixing them to get
the same performance as we have with old one. It is one my todo list.


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: net: more accurate skb truesize - regression on Microblaze

[Eric Dumazet]

On Tue, 2012-04-10 at 13:38 +0200, Michal Simek wrote:
> On 04/10/2012 01:32 PM, Eric Dumazet wrote:
> >
> > ll_temac allocates XTE_MAX_JUMBO_FRAME_SIZE frames, even for MTU=1500 ?
> 
> yes.
> 
> >
> > Trying to allocate smaller packets would definitely help your tcp
> > receiver performance and overall reliability  (order-0 allocations
> > instead of order-2)
> 
> There is long history for ll_temac and axi emac drivers. and if you compare
> them they are very similar with similar bugs.
> In our distribution we are using older ll_temac driver where this is fixed.
> Performance is much higher if you allocate buffers for actual mtu.
> 
> Thanks,
> Michal
> 
> P.S.: Our plan is to start to use mainline drivers and fixing them to get
> the same performance as we have with old one. It is one my todo list.

Ah good, because this 9000+pad versus 1500+pad allocation is way more
problematic than the (small) change on skb->truesize you bisected ;)

Re: net: more accurate skb truesize - regression on Microblaze

[Michal Simek]

On 04/10/2012 01:50 PM, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 13:38 +0200, Michal Simek wrote:
>> On 04/10/2012 01:32 PM, Eric Dumazet wrote:
>>>
>>> ll_temac allocates XTE_MAX_JUMBO_FRAME_SIZE frames, even for MTU=1500 ?
>>
>> yes.
>>
>>>
>>> Trying to allocate smaller packets would definitely help your tcp
>>> receiver performance and overall reliability  (order-0 allocations
>>> instead of order-2)
>>
>> There is long history for ll_temac and axi emac drivers. and if you compare
>> them they are very similar with similar bugs.
>> In our distribution we are using older ll_temac driver where this is fixed.
>> Performance is much higher if you allocate buffers for actual mtu.
>>
>> Thanks,
>> Michal
>>
>> P.S.: Our plan is to start to use mainline drivers and fixing them to get
>> the same performance as we have with old one. It is one my todo list.
>
> Ah good, because this 9000+pad versus 1500+pad allocation is way more
> problematic than the (small) change on skb->truesize you bisected ;)

yeah I know. Currently for me is the most important thing to have stable
driver which is ancient one and start to work on mainline to add there all features
and do more testing before we will add it to customers.

I have found it because I have compared the same driver with different kernel
versions and there was performance regression. Comparing with old one will show
different results. :-)

I will keep you in the loop when I start to fix mainline driver.

Thanks,
Michal






-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: net: more accurate skb truesize - regression on Microblaze

[David Miller]

From: Michal Simek <monstr@monstr.eu>
Date: Tue, 10 Apr 2012 13:38:33 +0200

> There is long history for ll_temac and axi emac drivers. and if you
> compare
> them they are very similar with similar bugs.
> In our distribution we are using older ll_temac driver where this is
> fixed.
> Performance is much higher if you allocate buffers for actual mtu.

You place an enormous burdon upon us when you ask us to analyze your
performance problems and bugs when using out of tree drivers.  Please
do not do this.

Re: net: more accurate skb truesize - regression on Microblaze

[David Miller]

From: Michal Simek <monstr@monstr.eu>
Date: Tue, 10 Apr 2012 15:49:17 +0200

> I reported problem with performance regression on the same drvier
> between one kernel version and the newer version. Even with pointer
> to specific patch which I have identified as source of the problem.
> It has nothing to do with performance of driver itself because
> identified patch directly points to generic network code not to
> mainline driver and performance problem in it.

The size and attributes of the SKBs allocated is a clear important
variable in this bug, and you've just described how the SKBs allocated
are of completely different sizes in the driver you're using compared
to the one in the tree.

You were very fortunate that Eric was able to track down the cause
even with this unknown variable, but in other cases this can lead to
wild goose chases which waste developer time.

So, again, please do not do this in the future, reproduce your
problems with upstream sources only.

Re: net: more accurate skb truesize - regression on Microblaze

[David Miller]

From: Michal Simek <monstr@monstr.eu>
Date: Tue, 10 Apr 2012 16:04:18 +0200

> I didn't report problem on any private held driver. I think you have
> misunderstand our later discussion about ll_temac driver where are
> some problems which at least

Thanks for the clarification.

Re: net: more accurate skb truesize - regression on Microblaze

[Michal Simek]

On 04/10/2012 04:14 PM, David Miller wrote:
> From: Michal Simek<monstr@monstr.eu>
> Date: Tue, 10 Apr 2012 16:04:18 +0200
>
>> I didn't report problem on any private held driver. I think you have
>> misunderstand our later discussion about ll_temac driver where are
>> some problems which at least
>
> Thanks for the clarification.

No worries.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: [PATCH v2] tcp: restore correct limit

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 10 Apr 2012 12:56:42 +0200

> Commit c43b874d5d714f (tcp: properly initialize tcp memory limits) tried
> to fix a regression added in commits 4acb4190 & 3dc43e3,
> but still get it wrong.
> 
> Result is machines with low amount of memory have too small tcp_rmem[2]
> value and slow tcp receives : Per socket limit being 1/1024 of memory
> instead of 1/128 in old kernels, so rcv window is capped to small
> values.
> 
> Fix this to match comment and previous behavior.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Glauber Costa <glommer@parallels.com>

Applied and queued up for -stable, thanks Eric.