From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202510 header.b=sOkRxx8S; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 82C645A061D for ; Wed, 05 Nov 2025 05:24:54 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202510; t=1762316692; bh=E4Vsd95S+XV4tXivtmDuKz0XrwpKiTcDPk3vB1OQTHk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=sOkRxx8SYcl2bhSJTmpS+bZ5hvwq1Gd1Vw5pl7dJ7HmJhFMpg8F1/ltjMVqgNJZfB MT+wTg0h3YRT6xRFCgdcIs7ES+tTXm7ekZYukekFp0+GboaJQ9y6A+FGeukjTCoZ2d s/WfitZEfgn4XX0U5y+ZMQBwazcUImxVSkcxHlNRAt1rbKi81S9Z+Ej1B0mwL4OlMj hluP6kBMgGYelUCxFvoABRPPN2+MWI6lemxm1pkCtOiHZEgws75h/MNa41nbD8eiXb 6rJUxRfkjWdUAXxt8/itRhQqNGrN/dFvSOMBBKViIaIB8Gz4znCw1wXGUpxTOHbGI8 xbZAidV9RR16w== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4d1XKc15rfz4wM7; Wed, 05 Nov 2025 15:24:52 +1100 (AEDT) Date: Wed, 5 Nov 2025 15:24:44 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v7 3/5] tcp: Resend SYN for inbound connections Message-ID: References: <20251031054242.7334-1-yuhuang@redhat.com> <20251031054242.7334-4-yuhuang@redhat.com> <20251104054233.1dec4eb6@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="n9fc7AjuYImzWz6S" Content-Disposition: inline In-Reply-To: <20251104054233.1dec4eb6@elisabeth> Message-ID-Hash: XQYP456X36BS4UZOT5M5HGDUMN6BHID5 X-Message-ID-Hash: XQYP456X36BS4UZOT5M5HGDUMN6BHID5 X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Yumei Huang , passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --n9fc7AjuYImzWz6S Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 =3D read_file_integer( > > + TCP_SYN_RETRIES, TCP_SYN_RETRIES_DEFAULT); > > + syn_linear_timeouts =3D read_file_integer( > > + TCP_SYN_LINEAR_TIMEOUTS, TCP_SYN_LINEAR_TIMEOUTS_DEFAULT); >=20 > I think this is a bit hard to read. Now: >=20 > tcp_syn_retries =3D read_file_integer(TCP_SYN_RETRIES, > TCP_SYN_RETRIES_DEFAULT); > syn_linear_timeouts =3D read_file_integer(TCP_SYN_LINEAR_TIMEOUTS, > TCP_SYN_LINEAR_TIMEOUTS_DEFAULT); >=20 > would be a bit closer to the coding style we're adopting, but I wonder: >=20 > - 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. > - the constants are defined here, in tcp.c, so they obviously refer to > TCP, so maybe SYN_LINEAR_TIMEOUTS is clear enough >=20 > - you don't really need to store both values in two different > variables, one is enough as you're assigning them right away. And: >=20 > v =3D read_file_int(SYN_RETRIES, SYN_RETRIES_DEFAULT); > c->tcp.tcp_syn_retries =3D MIN(v, UINT8_MAX); >=20 > v =3D read_file_int(SYN_LINEAR_TIMEOUTS, SYN_LINEAR_TIMEOUTS_DEFAULT); > c->tcp.syn_linear_timeouts =3D MIN(v, UINT8_MAX); >=20 > is four lines instead of six, and more readable if you ask me. >=20 > > + > > + c->tcp.tcp_syn_retries =3D MIN(tcp_syn_retries, UINT8_MAX); > > + c->tcp.syn_linear_timeouts =3D MIN(syn_linear_timeouts, UINT8_MAX); > > + > > + debug("Read sysctl values tcp_syn_retries: %"PRIu8", linear_timeouts:= %"PRIu8, > > + c->tcp.tcp_syn_retries, c->tcp.syn_linear_timeouts); > > +} > > + > > /** > > * tcp_init() - Get initial sequence, hash secret, initialise per-sock= et data > > * @c: Execution context > > @@ -2776,6 +2815,8 @@ int tcp_init(struct ctx *c) > > { > > ASSERT(!c->no_tcp); > > =20 > > + tcp_get_rto_params(c); > > + > > tcp_sock_iov_init(c); > > =20 > > memset(init_sock_pool4, 0xff, sizeof(init_sock_pool4)); > > diff --git a/tcp.h b/tcp.h > > index 234a803..befedde 100644 > > --- a/tcp.h > > +++ b/tcp.h > > @@ -59,12 +59,16 @@ union tcp_listen_epoll_ref { > > * @fwd_out: Port forwarding configuration for outbound packets > > * @timer_run: Timestamp of most recent timer run > > * @pipe_size: Size of pipes for spliced connections > > + * @tcp_syn_retries: SYN retries using exponential backoff timeout > > + * @syn_linear_timeouts: SYN retries before using exponential backoff = timeout > > */ > > struct tcp_ctx { > > struct fwd_ports fwd_in; > > struct fwd_ports fwd_out; > > struct timespec timer_run; > > size_t pipe_size; > > + uint8_t tcp_syn_retries; >=20 > Why 'tcp' again? >=20 > > + uint8_t syn_linear_timeouts; > > }; > > =20 > > #endif /* TCP_H */ >=20 > --=20 > Stefano >=20 --=20 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 --n9fc7AjuYImzWz6S Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmkK0YwACgkQzQJF27ox 2GdbNA/8DlfSNsPzFGYYgLMypSeKpfwYamxFHbmM5S+P/DlgIRypWr+uxzLm7FnH AZ5YCA1GWaAiRlwYSOdFD77a7FGS1DBKQhUxKk4GoWu9vc0PotXhj3Z82BkzGeAc oMCzQXgH3Y3nS+a4F7k4ij3B3rr0qf8K+qU7SFewcPA7S1yAmV0cHqWahBXxJQG+ h3a5O+xxJG1gRQPOr2+5u6whPhUZ7qgLmjm3s6gaaDQQ/3ZO0ZC3dQxvsnliJIj+ GppWAAGhTV5wG04IaeqlCI1w7g+RKD8GpLPF4BwNKXPkeKqvsTis7LsAPzlBsBf4 rfbxF0EyQGU60rs84Qc3/FpBOyZMEwZR9QD5HSqv288Z9JDE7ItxW+J8GS12/VAu +54ekBfu8xn2k7qfUToQSOukXJrRWRyQMB2qCCGZW+uAVU27T1WrKVjPTKZhr4Rg 0pFM54FAUkRhXyLDOFi84qeAoW45yQ7BPXiseJiqHZh5b4uYYssX2o+MrtAJw4AU OZfl5rsDTTd5h5Xqllix7qr5Fc7zz5za1848vry3qCblp/PyDNIw8tmtJ7Vji8x4 xG/11z6vCNkt727lG1WlxmnmDCGEF9UiyACGvmvHVrBNVlNOIctzWQvxTuaxuPXt M00vdNSfhI94GDW48aePu0G7/Cn8/kUSWzfhpMaBJsCF8IhhshM= =vg2E -----END PGP SIGNATURE----- --n9fc7AjuYImzWz6S--