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=liePwOnA; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id E1CF55A061A for ; Thu, 09 Oct 2025 05:44:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202510; t=1759981439; bh=hIGgZuCPcBZteHMCR84XE7UJy69ICJvw9PRU8cu9EIg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=liePwOnADHSXZx1TCWDLC1DbTTtlDY6SXLj6NtR7g7Zg7YnPnti0Y6Kl8OjTincd/ ToydCzqPAFQxKaLEiHrf93CtWOAUAd//v0UWnDsUsKwcL51lDsD6sAoNRs2L0fG0qH 4Tx0NFfwOurfcs5rtYMzqvpi0Fmri6zIzkiZQgak3adkXvd1HjjTWKzEWucy8da/+8 7ggmEz+uK/AQQcAklKE9yfqIGGz3UHhaKGiV+RFgFjBK6YSNbUs2HNsiGIHrKv51qs GlGwq0VcEyjEHemlYVeoCDyEmDBGRWSiXSusKengIqT/Y40OIEu7MwV9T/mSz9VEvl r2fxGuJ/iLmDg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4chwhv4XHlz4wCQ; Thu, 9 Oct 2025 14:43:59 +1100 (AEDT) Date: Thu, 9 Oct 2025 13:43:00 +1100 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-4-sbrivio@redhat.com> <20251002135104.1a662029@elisabeth> <20251007003254.57ce5e26@elisabeth> <20251008004249.6dc894fa@elisabeth> <20251008120127.75aa7585@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="2XVuIKstYS2p0Xo2" Content-Disposition: inline In-Reply-To: <20251008120127.75aa7585@elisabeth> Message-ID-Hash: VORSGP4Z36QO7QJUFWPYTN5C77HO47OP X-Message-ID-Hash: VORSGP4Z36QO7QJUFWPYTN5C77HO47OP 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: --2XVuIKstYS2p0Xo2 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Oct 08, 2025 at 12:01:27PM +0200, Stefano Brivio wrote: > On Wed, 8 Oct 2025 11:41:23 +1100 > David Gibson wrote: >=20 > > On Wed, Oct 08, 2025 at 12:42:49AM +0200, Stefano Brivio wrote: > > > On Tue, 7 Oct 2025 10:34:01 +1100 > > > David Gibson wrote: =20 > > > > On Tue, Oct 07, 2025 at 12:32:54AM +0200, Stefano Brivio wrote: =20 > > > > > On Fri, 3 Oct 2025 13:43:32 +1000 > > > > > David Gibson wrote: =20 > > [snip] > > > > > > @@ -1820,6 +1817,8 @@ static int tcp_data_from_tap(const struct= ctx *c, struct tcp_tap_conn *conn, > > > > > > break; > > > > > > seq_from_tap +=3D size; > > > > > > iov_i +=3D count; > > > > > > + if (th->fin) > > > > > > + fin =3D 1; > > > > > > =20 > > > > > > if (keep =3D=3D i) > > > > > > keep =3D -1; > > > > > >=20 > > > > > >=20 > > > > > > We'd need to double check that the "accept data segment" path i= s safe > > > > > > with len =3D=3D 0, of course. =20 > > > > >=20 > > > > > For sure it's not before d2c33f45f7be ("tcp: Convert > > > > > tcp_data_from_tap() to use iov_tail"), because we might add > > > > > zero-length segments to the tcp_iov array, and that would make > > > > > backporting an otherwise simple and critical fix to slightly older > > > > > versions rather complicated. =20 > > > >=20 > > > > Kinda. It's not that complicated to deal with that case, by wrappi= ng > > > > the actual data processing in an `if (len) { ... }` =20 > > >=20 > > > That's needed for sure, but do we risk looping forever on a particular > > > batch of FIN segments without data with a given series of sequence > > > numbers? > > > > > > Right now the loop terminates because the sequence moves forward. I'm > > > not sure what happens without data while moving 'keep' around. Maybe = it > > > takes a few minutes to figure out (I haven't tried) but I wouldn't ca= ll > > > that trivial. =20 > >=20 > > That should be fine, because we need to advance the sequence by one > > for the FIN anyway, so we will move forwards. I might have forgotten > > that in my quick example, which is a bug, but an easy one to fix. >=20 > But we don't, in that loop, and there's a specific reason for it. FIN > segments are a special case in that, if you receive more than one, you > don't advance the sequence by more than one, even if the segments > themselves are in the expected sequence. Ok. *thinks*. So, if we both set fin =3D 1 and advance the sequence inside an if (th->fin && !fin), that should do the trick, yes? > If you want to move the sequence increase into that loop (which might > make sense with some extra care), perhaps it's worth doing that > together with the whole https://bugs.passt.top/show_bug.cgi?id=3D125 at > this point. Yeah, maybe. --=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 --2XVuIKstYS2p0Xo2 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmjnISYACgkQzQJF27ox 2GcKQQ/5AeqRylU0TqfX8v+Jo/oK/ESqtcGn+Aoq9vMV3tPYY6IfdFy6ZBslAuls g8wrGLgcCtf/vorDMXLSLYgbATO7mYKKGNUC2LeW5VGyx+OLPCEzIaMPlb37iKLV yy65f2SD1w4eMzrNrczEA2hxDb/xCyDmWaQVyTO+/T4mRzmpNwrRRwSPftYGjw1O 4DelInVb4bLCzq8a6Kl4H3kytQ8D9zErgn+1mxl9wknDAClOMYxvuHovOuteKT14 pbctnd+z00YmzhVZsrfS+b8t2YamhD+yDAjbobHasxCuPUh6ZYbUvFbQu5ZpIEUk 3vha8SdnJ0WYmAKIodhla/UjW6wtpMaC8Hob9W1H8OHkk/BZxhqthfOJgngL7f1j 6P/6fjlZ81DxGvV/0prDZ4RQtoElSRGfP0+uDge+XOodcab44FUTtpn27excxtPN 2up+e7YDxw05Np+kVSLHjRgCVBhKZb8TpZk9VjASGKdjJZkbJlbQ94F/Ggxzk74u nxm9gmRJjhildn7i9d2czHT+fnjQurUXOnEwyJ/l+M+5berst8PIkiqcJWoaBsME 8qmxgOXZOlRWDYf2Wbt76a8Sj0lxjulXilmywDD9QpXTKYFrD1e4ZKLpgumd+aFr NSEVVHc20FtSXZ6Y4khTWveKvHxeLpqeHUXuesDnQw31GW2QgoU= =13fV -----END PGP SIGNATURE----- --2XVuIKstYS2p0Xo2--