Re: [PATCH 1/2] syncookies: do not store rcv_wscale in tcp timestamp

[Hagen Paul Pfeifer <hagen@jauu.net>]

* Florian Westphal | 2010-06-24 09:53:04 [+0200]:

>> I speculate that this behavior was and is not correct. I suppose that their is
>> a race between the SYN/ACK where we initial force a particular window scale
>> and the next time where we recalculate the window via tcp_select_initial_window().
>
>Yes, but it is not the only one. We also do a route lookup to get the
>current window size.

Is this critical in any sense? The window size is a pure passive variable, we
do not announce any window information to the peer. I don't get the problem
for the window size.

>> If the user change net.core.rmem_max or net.ipv4.tcp_rmem in between this
>> time, the recalculated window scale (rcv_wscale) can be smaller. But the
>> receiver still operates with the initial window scale and can overshot the
>> granted window - and bang.
>
>Why "bang"?
>Sure, its not nice, but is it such a severe problem that we have to keep
>rcv_wscale around in the timestamp?

IMHO if we want a 100% race free behavior yes.

>>  or disable window scaling and don't transmit any scaling option
>> when SYN cookies are active.
>
>No, I would rather see this patch rejected than that.

Sure? SYN cookies are only active if we suffer under acute memory pressure. We
are likely not in the ability to backup large buffer space - why not limit
the window to 2^16 bytes which is sufficient for 99.9999% of the use case?
I do not identify any _real_ advantages to speed up a connection when the
server is under fire.

Another solution is to accept this patch and assume that this race is
rather artificial nature and does not happened. Sure, the race is very
unlikely[TM].

HGN