Re: [PATCH v7 3/5] tcp: Resend SYN for inbound connections

10 Nov 2025


      On Mon, Nov 10, 2025 at 10:52:05AM +0800, Yumei Huang wrote:
...
On Fri, Nov 7, 2025 at 6:05 PM Stefano Brivio  wrote:
...
On Fri, 7 Nov 2025 17:56:54 +0800
Yumei Huang  wrote:
...
On Wed, Nov 5, 2025 at 3:01 PM Stefano Brivio  wrote:
...
On Wed, 5 Nov 2025 15:24:44 +1100
David Gibson  wrote:
...
On Tue, Nov 04, 2025 at 05:42:33AM +0100, Stefano Brivio wrote:
...
On Fri, 31 Oct 2025 13:42:40 +0800
Yumei Huang  wrote:
[snip]
> +/**
> + * tcp_get_rto_params() - Get host kernel RTO parameters
> + * @c:           Execution context
> +*/
> +void tcp_get_rto_params(struct ctx *c)
> +{
> + intmax_t tcp_syn_retries, syn_linear_timeouts;
> +
> + tcp_syn_retries = read_file_integer(
> +         TCP_SYN_RETRIES, TCP_SYN_RETRIES_DEFAULT);
> + syn_linear_timeouts = read_file_integer(
> +         TCP_SYN_LINEAR_TIMEOUTS, TCP_SYN_LINEAR_TIMEOUTS_DEFAULT);
I think this is a bit hard to read. Now:
tcp_syn_retries = read_file_integer(TCP_SYN_RETRIES,
                                        TCP_SYN_RETRIES_DEFAULT);
    syn_linear_timeouts = read_file_integer(TCP_SYN_LINEAR_TIMEOUTS,
                                            TCP_SYN_LINEAR_TIMEOUTS_DEFAULT);
would be a bit closer to the coding style we're adopting, but I wonder:
- does read_file_integer() really need to have "integer" in the name?
...
In a language where integers are called 'int', perhaps
  read_file_int() is clear enough?
I think the idea is that read_file_integer() can be used for any
(signed) integer type (with range checking performed after the call).
read_file_int() might suggest it reads exactly an 'int', not anything
bigger or smaller.
Oh, I see. It could be read_file_num() then, even if it's slightly less
accurate, or it can even remain read_file_integer(). If something like
this:
v = read_file_integer(SYN_RETRIES, SYN_RETRIES_DEFAULT);
        c->tcp.tcp_syn_retries = MIN(v, UINT8_MAX);
v = read_file_integer(SYN_LINEAR_TIMEOUTS, SYN_LINEAR_TIMEOUTS_DEFAULT);
        c->tcp.syn_linear_timeouts = MIN(v, UINT8_MAX);
fits 80 columns, I'm taking it as a sign that the function name isn't
exceedingly long. No particular preference from me.
The longest line just exceeds 80 columns.
No, why? It's 80 columns, look:
v = read_file_integer(SYN_LINEAR_TIMEOUTS, SYN_LINEAR_TIMEOUTS_DEFAULT);
01234567890123456789012345678901234567890123456789012345678901234567890123456789
You are right. Somehow I counted 81 columns. Then I will keep the name
as it is.  (To me, read_file_num() sounds like it can read any kind of
number.)
I also slightly prefer read_file_integer().

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson