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

[Heath Caldwell <hcaldwel@akamai.com>]

On 2021-01-12 21:26 (+0100), Eric Dumazet <edumazet@google.com> wrote:
> On Tue, Jan 12, 2021 at 8:25 PM Heath Caldwell <hcaldwel@akamai.com> wrote:
> >
> > On 2021-01-12 18:05 (+0100), Eric Dumazet <edumazet@google.com> wrote:
> > > On Tue, Jan 12, 2021 at 5:02 PM Heath Caldwell <hcaldwel@akamai.com> wrote:
> > > >
> > > > On 2021-01-12 09:30 (+0100), Eric Dumazet <edumazet@google.com> 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.
> > >
> > > 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?
> >
> > > 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.
> 
> >
> > 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.
> >
> > > > 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.

> > 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 :(.