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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CANn89iLXgGT2NL5kg7LQrzCFT_n7GJzb9FExdOD3fRNFEc1z0A@mail.gmail.com>
Date: Mon, 15 Jul 2024 07:40:44 -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 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.

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

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

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.

You forgot to update
Documentation/networking/net_cachelines/netns_ipv4_sysctl.rst
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 suggest you wait for net-next being reopened, we are all busy
attending netdev 0x18 conference.