netdev - Re: [PATCH net-next 0/2] tcp: add sysctl_tcp_rto_min

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: Wed, 29 May 2024 16:43:51 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: Kevin Yang <yyd@...gle.com>, Paolo Abeni <pabeni@...hat.com>, 
	David Miller <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 0/2] tcp: add sysctl_tcp_rto_min_us

Hello Eric,

On Wed, May 29, 2024 at 3:39 PM Eric Dumazet <edumazet@...gle.com> wrote:
>
> On Wed, May 29, 2024 at 9:00 AM Jason Xing <kerneljasonxing@...il.com> wrote:
> >
> > On Wed, May 29, 2024 at 2:43 PM Jason Xing <kerneljasonxing@...il.com> wrote:
> > >
> > > Hello Kevin,
> > >
> > > On Wed, May 29, 2024 at 1:13 AM Kevin Yang <yyd@...gle.com> wrote:
> > > >
> > > > Adding a sysctl knob to allow user to specify a default
> > > > rto_min at socket init time.
> > >
> > > I wonder what the advantage of this new sysctl knob is since we have
> > > had BPF or something like that to tweak the rto min already?
> > >
> > > There are so many places/parameters of the TCP stack that can be
> > > exposed to the user side and adjusted by new sysctls...
> > >
> > > Thanks,
> > > Jason
> > >
> > > >
> > > > After this patch series, the rto_min will has multiple sources:
> > > > route option has the highest precedence, followed by the
> > > > TCP_BPF_RTO_MIN socket option, followed by this new
> > > > tcp_rto_min_us sysctl.
> > > >
> > > > Kevin Yang (2):
> > > >   tcp: derive delack_max with tcp_rto_min helper
> > > >   tcp: add sysctl_tcp_rto_min_us
> > > >
> > > >  Documentation/networking/ip-sysctl.rst | 13 +++++++++++++
> > > >  include/net/netns/ipv4.h               |  1 +
> > > >  net/ipv4/sysctl_net_ipv4.c             |  8 ++++++++
> > > >  net/ipv4/tcp.c                         |  3 ++-
> > > >  net/ipv4/tcp_ipv4.c                    |  1 +
> > > >  net/ipv4/tcp_output.c                  | 11 ++---------
> > > >  6 files changed, 27 insertions(+), 10 deletions(-)
> > > >
> > > > --
> > > > 2.45.1.288.g0e0cd299f1-goog
> > > >
> > > >
> >
> > Oh, I think you should have added Paolo as well.
> >
> > +Paolo Abeni
>
> Many cloud customers do not have any BPF expertise.
> If they use existing BPF programs (added by a product), they might not
> have the ability to change it.
>
> We tried advising them to use route attributes, after
> commit bbf80d713fe75cfbecda26e7c03a9a8d22af2f4f ("tcp: derive
> delack_max from rto_min")
>
> Alas, dhcpd was adding its own routes, without the "rto_min 5"
> attribute, then systemd came...
> Lots of frustration, lots of wasted time, for something that has been
> used for more than a decade
> in Google DC.
>
> With a sysctl, we could have saved months of SWE, and helped our
> customers sooner.

I'm definitely aware of the importance of this kind of sysctl knob.
Many years ago (around 6 or 7 years ago), we already implemented
similar things in the private kernel.

For a long time, netdev guys often proposed the question as I did in
the previous email. I'm not against it, just repeating the same
question and asking ourselves again: is it really necessary? We still
have a lot of places to tune/control by introducing new sysctl.

For a long time, there have been plenty of papers studying different
combinations of different parameters in TCP stack so that they can
serve one particular case well.

Do we also need to expose remaining possible parameters to the user
side? Just curious...

Thanks,
Jason