Re: [PATCH v8 6/6] tcp: Clamp the retry timeout

[Stefano Brivio <sbrivio@redhat.com>]

On Fri, 14 Nov 2025 14:35:12 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Nov 14, 2025 at 11:05:51AM +0800, Yumei Huang wrote:
> > On Fri, Nov 14, 2025 at 8:47 AM David Gibson
> > <david@gibson.dropbear.id.au> wrote:  
> > >
> > > On Fri, Nov 14, 2025 at 01:01:21AM +0100, Stefano Brivio wrote:  
> > > > On Mon, 10 Nov 2025 21:56:39 +1100
> > > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > > >  
> > > > > On Mon, Nov 10, 2025 at 05:31:37PM +0800, Yumei Huang wrote:  
> > > > >
> > > > > > @@ -2811,10 +2821,15 @@ void tcp_get_rto_params(struct ctx *c)
> > > > > >   v = read_file_integer(SYN_LINEAR_TIMEOUTS, SYN_LINEAR_TIMEOUTS_DEFAULT);
> > > > > >   c->tcp.syn_linear_timeouts = MIN(v, MAX_SYNCNT);
> > > > > >
> > > > > > + v = read_file_integer(RTO_MAX_MS, RTO_MAX_MS_DEFAULT);
> > > > > > + c->tcp.rto_max = MIN(DIV_ROUND_CLOSEST(v, 1000), INT_MAX);  
> > > > >
> > > > > Possibly we should verify this is => RTO_INIT.  
> > > >
> > > > As a sanity check, maybe, but I don't see any harmful effect if it's
> > > > < RTO_INIT, right? So I'm not sure if we should. No preference from my
> > > > side really.  
> > >
> > > Sorry, describing this as >= RTO_INIT was misleading.  What I'm
> > > concerned about here is if the kernel value is set to 400ms, we'll
> > > round it to... 0s.
> > >
> > > So, really what I'm concerned about is that we ensure this is > 0.  
> > 
> > That's a good point.  
> 
> Actually, thinking about it, I wonder if makes more sense to always
> round up to 1s, rather than to the nearest 1s.

I guess so, yes.

Using two seconds as timeout when the user configured 1400ms isn't
necessarily less correct than using one second, and it simplifies things
(we already have DIV_ROUND_UP()).

-- 
Stefano