netdev - Re: [PATCH net-next] tcp: introduce rto_max

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CANn89iK7hDCGQsGiX5rD6S29u1u8k5za-SOBaxY59S=C+BgaKA@mail.gmail.com>
Date: Mon, 15 Jul 2024 23:57:15 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: Jason Xing <kerneljasonxing@...il.com>
Cc: kuba@...nel.org, pabeni@...hat.com, davem@...emloft.net, 
	dsahern@...nel.org, ncardwell@...gle.com, corbet@....net, 
	netdev@...r.kernel.org, Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next] tcp: introduce rto_max_us sysctl knob

On Mon, Jul 15, 2024 at 11:42 PM Jason Xing <kerneljasonxing@...il.com> wrote:
>
> Hello Eric,
>
> On Mon, Jul 15, 2024 at 10:40 PM Eric Dumazet <edumazet@...gle.com> wrote:
> >
> > On Sun, Jul 14, 2024 at 8:31 PM Jason Xing <kerneljasonxing@...il.com> wrote:
> > >
> > > From: Jason Xing <kernelxing@...cent.com>
> > >
> > > As we all know, the algorithm of rto is exponential backoff as RFC
> > > defined long time ago.
> >
> > This is weak sentence. Please provide RFC numbers instead.
>
> RFC 6298. I will update it.
>
> >
> > > After several rounds of repeatedly transmitting
> > > a lost skb, the expiry of rto timer could reach above 1 second within
> > > the upper bound (120s).
> >
> > This is confusing. What do you mean exactly ?
>
> I will rewrite this part. What I was trying to say is that waiting
> more than 1 second is not very friendly to some applications,
> especially the expiry time can reach 120 seconds which is too long.

Says who ? I think this needs IETF approval.

>
> >
> > >
> > > Waiting more than one second to retransmit for some latency-sensitive
> > > application is a little bit unacceptable nowadays, so I decided to
> > > introduce a sysctl knob to allow users to tune it. Still, the maximum
> > > value is 120 seconds.
> >
> > I do not think this sysctl needs usec resolution.
>
> Are you suggesting using jiffies is enough? But I have two reasons:
> 1) Keep the consistency with rto_min_us
> 2) If rto_min_us can be set to a very small value, why not rto_max?

rto_max is usually 3 order of magnitude higher than rto_min

For HZ=100, using jiffies for rto_min would not allow rto_min < 10 ms.
Some of us use 5 msec rto_min.

jiffies is plain enough for rto_max.


>
> What do you think?

I think you missed many many details really.

Look at all TCP_RTO_MAX instances in net/ipv4/tcp_timer.c and ask
yourself how many things
will break if we allow a sysctl value with 1 second for rto_max.

>
> >
> > Also storing this sysctl once, and converting it millions of times per
> > second to jiffies is not good.
> > I suggest you use proc_dointvec_jiffies() instead in the sysctl handler.
> >
> > Minimal value of one jiffies makes also no sense. We can not predict
> > if some distros/users
> > might (ab)use this sysctl.
>
> Okay. If the final solution is using jiffies, I will accordingly adjust it.
>
> >
> > You forgot to update
> > Documentation/networking/net_cachelines/netns_ipv4_sysctl.rst
>
> Oh sorry, I forgot.
>
> > This means the location you chose for the new sysctl is pretty much
> > random and not reflcting
> > this is used in one fast path.
>
> I will investigate its proper location...
>
> >
> > I suggest you wait for net-next being reopened, we are all busy
> > attending netdev 0x18 conference.
>
> Roger that. Thanks for your suggestions.
>
> Thanks,
> Jason