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

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

On Tue, Jan 12, 2021 at 8:25 PM Heath Caldwell [off-list ref] wrote:

quoted

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

quoted

On Tue, Jan 12, 2021 at 5:02 PM Heath Caldwell [off-list ref] wrote:

quoted

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

quoted

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.

Not clear to me at least.

If they were bug fixes, a FIxes: tag would be provided.

The underlying bugs that are addressed in patches 1 and 3 are present in
1da177e4c3f4 ("Linux-2.6.12-rc2") which looks to be the earliest parent
commit in the repository.  What should I do for a Fixes: tag in this
case?

quoted

You are a first time contributor to linux TCP stack, so better make
sure your claims are solid.

I fear that I may not have expressed the problems and solutions in a
manner that imparted the ideas well.

Maybe I added too much detail in the description for patch 1, which may
have obscured the problem: val is capped to sysctl_rmem_max *before* it
is doubled (resulting in the possibility for sk_rcvbuf to be set to
2*sysctl_rmem_max, rather than it being capped at sysctl_rmem_max).

This is fine. This has been done forever. Your change might break applications.

In what way might applications be broken?

It seems to be a very strange position to allow a configured maximum to
be violated because of obscure precedent.

It does not seem to be a supportable position to allow an application to
violate an installation's configuration because of a chance that the
application may behave differently if a setsockopt() call fails.  What
if a system administrator decides to reduce sysctl_rmem_max to half of
the current default?

I would advise documenting this fact, since existing behavior will be kept
in many linux hosts for years to come.

quoted

Maybe I was not explicit enough in the description for patch 3: space is
expanded into sock_net(sk)->ipv4.sysctl_tcp_rmem[2] and sysctl_rmem_max
without first shrinking them to discount the overhead.

quoted

quoted

Patches 2 and 4 might be arguable, though.

So we have to pick up whatever pleases us ?

I have been treating all of these changes together because they all kind
of work together to provide a consistent model and configurability for
the initial receive window.

Patches 1 and 3 address bugs.

Maybe, but will break applications.

How might patch 3 break an application?  It merely will reduce the
window scale value to something lower but still capable of representing
the largest window that a particular connection might advertise.

quoted

Patch 2 addresses an inconsistency in how overhead is treated specially
for TCP sockets.
Patch 4 addresses the 64KB limit which has been imposed.

For very good reasons.

What are the reasons?

This is going nowhere. I will stop right now.

That is a shame :(.