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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 12 Jan 2021 21:26:45 +0100
From:   Eric Dumazet <edumazet@...gle.com>
To:     Heath Caldwell <hcaldwel@...mai.com>
Cc:     netdev <netdev@...r.kernel.org>, Yuchung Cheng <ycheng@...gle.com>,
        Josh Hunt <johunt@...mai.com>, Ji Li <jli@...mai.com>
Subject: Re: [PATCH net-next 4/4] tcp: remove limit on initial receive window

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

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.

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

This is going nowhere. I will stop right now.