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=202508 header.b=A0LThCsV; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id E99605A026F for ; Thu, 02 Oct 2025 05:02:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202508; t=1759374130; bh=1Hzrhcn323vjOaupiSqKi/Qd6nGndCJD8tEX9BePGHc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=A0LThCsVeZjw+UWOm82z5a2QpVS+rcimxR9CF9bT4rFzukEt6GU7IZTguATLwMjpr 3NdNygo8zQ6IDabBERLUJ/TUkQk6Cm/TS6z3pOi4yAEjniV2trnDdlj4q4qSNDZ7nA vc++to43VG2nHg5TwMqQ+GOsDGQYt0RU7eAH4vTI4poZVMUhAC/DewbJLDzkWUDWBO j4jqAVEXqf3Gm9polCfFJzAxXCHEHoRul3AwOFjKUwj+XgYOWTM1tIvcs/S2Bg0gBO 9uUIRxIbiT9qXOi7kdomKWfodKm+qxhW5uEAOB62MbXkw2XaoiEXnfc+PE6ElhjdHW Fa7fY4qgq7ZRg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ccc5t6slJz4wD8; Thu, 2 Oct 2025 13:02:10 +1000 (AEST) Date: Thu, 2 Oct 2025 13:02:09 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 3/4] tcp: Don't consider FIN flags with mismatching sequence Message-ID: References: <20251002000646.2136202-1-sbrivio@redhat.com> <20251002000646.2136202-4-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="MnYrxh3iCDVvJigW" Content-Disposition: inline In-Reply-To: Message-ID-Hash: TQR2IDBRFVOHFHTCOXXLI3KVU5YYG4UZ X-Message-ID-Hash: TQR2IDBRFVOHFHTCOXXLI3KVU5YYG4UZ 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: 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: --MnYrxh3iCDVvJigW Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Oct 02, 2025 at 12:52:31PM +1000, David Gibson wrote: > On Thu, Oct 02, 2025 at 02:06:45AM +0200, Stefano Brivio wrote: > > If a guest or container sends us a FIN segment but its sequence number > > doesn't match the highest sequence of data we *accepted* (not > > necessarily the highest sequence we received), that is, > > conn->seq_from_tap, plus any data we're accepting in the current > > batch, we should discard the flag (not necessarily the segment), > > because there's still data we need to receive (again) before the end > > of the stream. > >=20 > > If we consider those FIN flags as such, we'll end up in the > > situation described below. > >=20 > > Here, 192.168.10.102 is a HTTP server in a Podman container, and > > 192.168.10.44 is a client fetching approximately 121 KB of data from > > it: > >=20 > > 82 2.026811 192.168.10.102 =E2=86=92 192.168.10.44 54 TCP 55414 = =E2=86=92 44992 [FIN, ACK] Seq=3D121441 Ack=3D143 Win=3D65536 Len=3D0 > >=20 > > the server is done sending > >=20 > > 83 2.026898 192.168.10.44 =E2=86=92 192.168.10.102 54 TCP 44992 = =E2=86=92 55414 [ACK] Seq=3D143 Ack=3D114394 Win=3D216192 Len=3D0 > >=20 > > pasta (client) acknowledges a previous sequence, because of > > a short sendmsg() > >=20 > > 84 2.027324 192.168.10.44 =E2=86=92 192.168.10.102 54 TCP 44992 = =E2=86=92 55414 [FIN, ACK] Seq=3D143 Ack=3D114394 Win=3D216192 Len=3D0 > >=20 > > pasta (client) sends FIN, ACK as the client has no more data to > > send (a single GET request), while still acknowledging a previous > > sequence, because the retransmission didn't happen yet > >=20 > > 85 2.027349 192.168.10.102 =E2=86=92 192.168.10.44 54 TCP 55414 = =E2=86=92 44992 [ACK] Seq=3D121442 Ack=3D144 Win=3D65536 Len=3D0 > >=20 > > the server acknowledges the FIN, ACK > >=20 > > 86 2.224125 192.168.10.102 =E2=86=92 192.168.10.44 4150 TCP [TCP R= etransmission] 55414 =E2=86=92 44992 [ACK] Seq=3D114394 Ack=3D144 Win=3D655= 36 Len=3D4096 [TCP segment of a reassembled PDU] > >=20 > > and finally a retransmission comes, but as we wrongly switched to > > the CLOSE-WAIT state, > >=20 > > 87 2.224202 192.168.10.44 =E2=86=92 192.168.10.102 54 TCP 44992 = =E2=86=92 55414 [RST] Seq=3D144 Win=3D0 Len=3D0 > >=20 > > we consider frame #86 as an acknowledgement for the FIN segment we > > sent, and close the connection, while we still had to re-receive > > (and finally send) the missing data segment, instead. > >=20 > > Link: https://github.com/containers/podman/issues/27179 > > Signed-off-by: Stefano Brivio > > --- > > tcp.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > >=20 > > diff --git a/tcp.c b/tcp.c > > index 3f7dc82..5a7a607 100644 > > --- a/tcp.c > > +++ b/tcp.c > > @@ -1769,7 +1769,7 @@ static int tcp_data_from_tap(const struct ctx *c,= struct tcp_tap_conn *conn, > > } > > } > > =20 > > - if (th->fin) > > + if (th->fin && seq =3D=3D seq_from_tap) > > fin =3D 1; >=20 > Can a FIN segment also contain data? My quick googling suggests yes. > If so, doesn't this logic need to go after we process the data > processing, so that seq_from_tap points to the end of the packet's > data, rather than the beginning? (And the handling of zero-length > packets would also need revision to match). Following on from that, it seems to me like it would make sense for FIN segments to also participate in the 'keep' mechanism. It should work eventually, but I expect it would be smoother in the case that we get a final burst of packets in a stream out of order. --=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 --MnYrxh3iCDVvJigW Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmjd6zEACgkQzQJF27ox 2GcqwxAApBpI9pLeMz/IdZLFtzqeFn6hQL4DYHFeH/dg07h3nbe8RV83Zg2AkGoL rbWFwyXIVBAl4sdyYAKCqbMFU2m1i48ZlopxVb0xbM9OhIo7JCfYpoQCCotlIByf MQDptJkk9qveDy8T7gUis3UDiY9hQYSBHpQCHK2ElIdHcvu35zsljA+0G9aYRUc9 naGwkKwB5h65X5TR7lKIxzSi5GHhUfDGK1gLgxu3seqwbHy4ch7NhQqZ35Qewdzx EXyXXNZUbInn+I5jJRuLctMrTfgeung/hK0n7Ij6KxE5ANLB0keV/NraSzoAnjkE yglUbydclxVuH+jpN5JSlXc8oFuyfjQuuTcmXGzf/fYh2vMDBsaYAaLQoP+CLKHT 7TWKLn/ts54Hh42Yfo1EcF+CktGlKEwQzkMpbNz/y+4rQObDp8sLGsEd4aZhfQAt /8AlW/bO9hOhZXuHuiDsghsVvl5fiIbWOe564AAKggrTlrqdOkp62nruzZyNNoI0 rbbk2oZXrIKNhC8yYSM261x4a0s+yA5dEe+vglfufOLU9wwv72kqaJ44N7OwOCeB aMyB5OwUVdyvhs6AeaXt9X3oS3Q14VhZCl6Gtjo6zCajJX10JyniGL4bXyvhhO9L 7dTMFlenWkgSKJWYV3MmjpLleATrUbGi9i7w4vJah7ZHC9X/mfI= =Xa/d -----END PGP SIGNATURE----- --MnYrxh3iCDVvJigW--