Re: [PATCH net-next 4/4] tcp: remove limit on initial receive window

On 2021-01-12 09:30 (+0100), Eric Dumazet [off-list ref] wrote:

I think the whole patch series is an attempt to badly break TCP stack.

Can you explain the concern that you have about how these changes might
break the TCP stack?

Patches 1 and 3 fix clear bugs.

Patches 2 and 4 might be arguable, though.

Is you objection primarily about the limit removed by patch 4?

Hint : 64K is really the max allowed by TCP standards. Yes, this is
sad, but this is it.

Do you mean the limit imposed by the size of the "Window Size" header
field?  This limitation is directly addressed by the check in
__tcp_transmit_skb():

	if (likely(!(tcb->tcp_flags & TCPHDR_SYN))) {
		th->window      = htons(tcp_select_window(sk));
		tcp_ecn_send(sk, skb, th, tcp_header_size);
	} else {
		/* RFC1323: The window in SYN & SYN/ACK segments
		 * is never scaled.
		 */
		th->window	= htons(min(tp->rcv_wnd, 65535U));
	}

and checking (and capping it there) allows for the field to not overflow
while also not artificially restricting the size of the window which
will later be advertised (once window scaling is negotiated).

I will not spend hours of work running  packetdrill tests over your
changes, but I am sure they are now quite broken.

If you believe auto tuning is broken, fix it properly, without trying
to change all the code so that you can understand it.

The removal of the limit specifically addresses the situation where auto
tuning cannot work: on the initial burst.  There is no way to know
whether an installation desires to receive a larger first burst unless
it is specifically configured - and this limit prevents such
configuration.

I strongly advise you read RFC 7323 before doing any changes in TCP
stack, and asking us to spend time reviewing your patches.

Can you point out the part of the RFC which would be violated by
initially (that is, the first packet after the SYN) advertising a window
larger than 64KB?

If you want to do research, this is fine, but please do not break
production TCP stack.

Thank you.