Re: TSecr != 0 check in inet_lro.c

[Octavian Purdila <opurdila@ixiacom.com>]

On Tuesday 25 August 2009 08:42:33 Eric Dumazet wrote:
> Octavian Purdila a écrit :
> > Hi,
> >
> > We are seeing a performance issue with TSO/LRO which we tracked down to
> > the TSecr !=0 check in lro_tcp_ip_check.
>
> ouch...
>
> > It happens when the LRO side's TSval wraps around and gets to 0. That
> > triggers the TSO side to send packets with TSecr set to 0, which means
> > that such packets won't be aggregated - and that will put a lot of burden
> > on the stack which will result in lots of drops.
>
> Probability of such event is 1 / 2^32 or so ?
>

Yes, its pretty low, but the timestamps are taken from jiffies and jiffies are 
initialized to -300*HZ so it will happen in 5 minutes after every reboot :)

> > I'm failing to understand the purpose of this check. Any hints? :)
>
> rfc1323 badly interpreted ?
>
> I remember tsecr=0 was forbidden by Linux, while apparently rfc is not
> so clear.
>
> rfc1323 : 3.2
>          The Timestamp Echo Reply field (TSecr) is only valid if the ACK
>          bit is set in the TCP header; if it is valid, it echos a times-
>          tamp value that was sent by the remote TCP in the TSval field
>          of a Timestamps option.  When TSecr is not valid, its value
>          must be zero.  The TSecr value will generally be from the most
>          recent Timestamp option that was received; however, there are
>          exceptions that are explained below.
>
> Note how this is not saying "a zero Tsecr value is not valid"

That is my understanding as well.

> I could not find why : "When TSecr is not valid, its value
> must be zero", and why we consider a zero value to be not meaningfull...
>
> ...
>
> So we dont have a bit saying we received a tsecr, we use the
> 'if saw_tstamp AND tsecr is not null' convention...

Alright, its starting to make sense. So, it looks like we can remove the check 
from inet_lro, and that may even reduce the probability of receiving a zero 
TSecr in the stack. Right?

Thanks for you help!

tavi