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=q7UyfUEl; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id A03E35A0619 for ; Fri, 24 Oct 2025 12:56:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202510; t=1761303381; bh=KIRXVpR8lobY/Fwxv782cMmxWhI7IZuZBnEvttAbmFo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=q7UyfUElSNCCTtY4OZbys7hx6d4IXUrjefX3+tNhK+ObLjx/sde8a+nFIWypLoUX3 UDWY0b86LJ+BgFCCFFEKG3ElwcmfSCh5fNilRZFde5NCCY6RM8Mwj8I70RfAsvtder rVw2YUVDIBnWqrcsmL0qqMqPyBwEqYWL3F872F4hA1DqJlzErdFTl9kfRUeX22xqFK im5i+Y5h+bNe0olRg2h4QV3NAB5dY2PQAo19iq90cXfWpBRbVBjKaaPkew3UYEoMTx W2Pm9+OY+3xUyJnNG35Du929tIkRkFgXnpWskyHKUmORVuDoyp83Q0g8IuMXVbtOiD 58TfeQB5gKsEA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ctKZs1WHmz4wCV; Fri, 24 Oct 2025 21:56:21 +1100 (AEDT) Date: Fri, 24 Oct 2025 21:55:58 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v6 3/4] tcp: Resend SYN for inbound connections Message-ID: References: <20251017062838.21041-1-yuhuang@redhat.com> <20251017062838.21041-4-yuhuang@redhat.com> <20251024010431.4329a843@elisabeth> <20251024103717.715fe49e@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="WTMk7Kbl/tN/y/t7" Content-Disposition: inline In-Reply-To: <20251024103717.715fe49e@elisabeth> Message-ID-Hash: 6DSEYSA7SQNCKO5RHB4IPRJI7VUXF5OM X-Message-ID-Hash: 6DSEYSA7SQNCKO5RHB4IPRJI7VUXF5OM 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: --WTMk7Kbl/tN/y/t7 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Oct 24, 2025 at 10:37:17AM +0200, Stefano Brivio wrote: > On Fri, 24 Oct 2025 14:30:09 +1100 > David Gibson wrote: > > On Fri, Oct 24, 2025 at 01:04:31AM +0200, Stefano Brivio wrote: > > > On Fri, 17 Oct 2025 14:28:37 +0800 > > > Yumei Huang wrote: [snip] > > > > @@ -2409,8 +2419,17 @@ void tcp_timer_handler(const struct ctx *c, = union epoll_ref ref) > > > > tcp_timer_ctl(c, conn); > > > > } else if (conn->flags & ACK_FROM_TAP_DUE) { > > > > if (!(conn->events & ESTABLISHED)) { > > > > - flow_dbg(conn, "handshake timeout"); > > > > - tcp_rst(c, conn); > > > > + if (conn->retries >=3D TCP_MAX_RETRIES || > > > > + conn->retries >=3D (c->tcp.tcp_syn_retries + > > > > + c->tcp.syn_linear_timeouts)) { > > > > + flow_dbg(conn, "handshake timeout"); > > > > + tcp_rst(c, conn); > > > > + } else { > > > > + flow_trace(conn, "SYN timeout, retry"); > > > > + tcp_send_flag(c, conn, SYN); > > > > + conn->retries++; =20 > > >=20 > > > I think I already raised this point on a previous revision: this needs > > > to be zeroed as the connection is established, but I don't see that in > > > the current version. =20 > >=20 > > Yes, you raised that, but then I realised it's already handled. I > > think I put that in the thread, not just direct to Yumei, but maybe > > not? Or it just got lost in the minutiae. >=20 > Yes, here: >=20 > https://archives.passt.top/passt-dev/aOxFRfJjPWy0ZW0M@zatzit >=20 > this is another example of what I meant about (potential) advantages of > a fully threaded (email) workflow. >=20 > In this case, I didn't review v2, which came before you could post this > to my comment on v1, but in a normal case, we could have settled this > earlier, once for all. Ah, right, that'd do it. > > When we receive a SYN-ACK, it will have th->ack_seq advanced a byte > > acknowledging the SYN. tcp_tap_handler() calls > > tcp_update_seqack_from_tap() in the !ESTABLISHED case which will see > > the new ack_seq and clear retries (retrans before this series). >=20 > It doesn't look obvious at all to me. Oh, it's definitely not obvious, but I'm pretty confident it's correct. Fwiw, I spotted this because I thought the explicit handling in v2 wasn't at quite the right point logically (though close enough to be fine in practice). I went looking for the precise right point - when we receive the SYN-ACK - and there it was, already handled. It does make a kind of logical sense. The RFCs don't generally treat SYN (or SYN-ACK, or FIN) retransmits any differently from data retransmits. We do treat them differently, but less so after this series, which is a good thing, I think. > We're unlikely to break it in the future, so I don't think it's fragile > in the long term, but... can one of you double check that it's actually > the case with a manual one-off test? Yeah, I guess that's wise. Easiest way is probably to add a temporary debug message here, and try it against a qemu guest that's temporarily suspended. Yumei, I can walk you through this, too. --=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 --WTMk7Kbl/tN/y/t7 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmj7WzgACgkQzQJF27ox 2GcwLQ/8DgIIqf1hzmNI6vh//5IdiaWtrIZRShxDDWbQuGxSE0mx1ycVKxyL8E8a immHNrN84i+tODKA1kmFwq9cZ1XyttPAduw7FSz4ysxiHSqExCBAOXFneGdVUHg/ GTuYB5YU2+ciPxt6YC1q6PN98RqWYRluw1Lehlw9k7tJItVB2eDkqA7yohCoZWwv Fj2V8uj3GWmWFjgAlSJ6Xok1rEN2SMgpiHsUVWppTOEndobi39kjt9Lcq24bHFz5 fmH0t2n33bEmMCvVSuOIPYRnA/BQralUTydA+qN1q8/rU1n95SXD1rPOROpcmM+b cGye8MGoPzdQyt2NnM2IiOuipqSs9FeGKJ1G5lHfBMpOqjPcbSHvE/Mx1lJ/6zJc 9v3tPo5X4QOYL3aELDWMQKv3JGDrP89HqxbREU5rsfy7jpb7VaF2bv0sQVbr27UW LMjbmpES7GGBg9/o2Wb/kyA+0FnNo/XUrakaDjd6nZjj5emoWCc4kZ9xzz7vCRnF 49AEck3xvROv6JeAgf8lv9HB8KSIVfvnJ2D+JtXXrqR3ToBgm2hFVIpzc5IAaER5 tgMNuGVCkAAPrQxYYX/ez/CLNCWJLquKo/D66SzUzcrJIuTQq9W69kEFY4RJs7/I VsrfNQySZZGws/bTNXADWKEqtLyLd44UNdonsTYbL/Bw4x1prQM= =wfkh -----END PGP SIGNATURE----- --WTMk7Kbl/tN/y/t7--