From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id BAC925A0278 for ; Mon, 19 Feb 2024 03:03:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1708308192; bh=AVPwl46STwIVvxHLEoiD0/Gbe2PvvMB37Q6t/8C9XOk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Fa7t+goO6MHxexy7Lmuyn36VYp2MuWxgdeRUQfdrhmiAj/TGIIR5yhYiKOETCdXjC R2TgngWcBOFqOPZ4WmkIYpzZoY9Z1ICLm9E7dqgvCADI9y29XAvcWPRu69jkW8B6pv rGyF2EY9VHIJc2XZsC9zRNBWqNFky5VLD/EnZGIwkgGhPn63iswgaeKMX+0Jhyroay P5QxP3Q9e543qdZ9rNqXMVXdf7P/XApYpaTBRTRjeX8m1bxxXK8hGXa0iHbqEXobob 8QdkDOtulhEurhKFyiCF6Ljq5fNGC0Y3X4Sp6+jteUwgEYeAIfC2uWWic/46frjKiy yC4EDTDXDTSRg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4TdQmc46Zfz4wbp; Mon, 19 Feb 2024 13:03:12 +1100 (AEDT) Date: Mon, 19 Feb 2024 12:50:16 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v2 09/22] tcp_splice: Simplify clean up logic Message-ID: References: <20240206011734.884138-1-david@gibson.dropbear.id.au> <20240206011734.884138-10-david@gibson.dropbear.id.au> <20240218215937.126b2839@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="FrDFuPXaONVHWVaI" Content-Disposition: inline In-Reply-To: <20240218215937.126b2839@elisabeth> Message-ID-Hash: LV43Q67MDPNTKK7JWP2JT7HD3A6L4ZAJ X-Message-ID-Hash: LV43Q67MDPNTKK7JWP2JT7HD3A6L4ZAJ 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: --FrDFuPXaONVHWVaI Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Feb 18, 2024 at 09:59:37PM +0100, Stefano Brivio wrote: > On Tue, 6 Feb 2024 12:17:21 +1100 > David Gibson wrote: >=20 > > Currently tcp_splice_flow_defer() contains specific logic to determine = if > > we're far enough initialised that we need to close pipes and/or sockets. > > This is potentially fragile if we change something about the order in w= hich > > we do things. We can simplify this by initialising the pipe and socket > > fields to -1 very early, then close()ing them if and only if they're non > > negative. > >=20 > > This lets us remove a special case cleanup if our connect() fails. This > > will already trigger a CLOSING event, and the socket fd in question is > > populated in the connection structure. Thus we can let the new cleanup > > logic handle it rather than requiring an explicit close(). > >=20 > > Signed-off-by: David Gibson > > --- > > tcp_splice.c | 27 +++++++++++---------------- > > 1 file changed, 11 insertions(+), 16 deletions(-) > >=20 > > diff --git a/tcp_splice.c b/tcp_splice.c > > index 40ecb5d4..f0343eb5 100644 > > --- a/tcp_splice.c > > +++ b/tcp_splice.c > > @@ -246,16 +246,14 @@ bool tcp_splice_flow_defer(union flow *flow) > > return false; > > =20 > > for (side =3D 0; side < SIDES; side++) { > > - if (conn->events & SPLICE_ESTABLISHED) { > > - /* Flushing might need to block: don't recycle them. */ > > - if (conn->pipe[side][0] !=3D -1) { > > - close(conn->pipe[side][0]); > > - close(conn->pipe[side][1]); > > - conn->pipe[side][0] =3D conn->pipe[side][1] =3D -1; > > - } > > + /* Flushing might need to block: don't recycle them. */ > > + if (conn->pipe[side][0] >=3D 0) { > > + close(conn->pipe[side][0]); > > + close(conn->pipe[side][1]); > > + conn->pipe[side][0] =3D conn->pipe[side][1] =3D -1; > > } > > =20 > > - if (side =3D=3D 0 || conn->events & SPLICE_CONNECT) { > > + if (conn->s[side] >=3D 0) { > > close(conn->s[side]); > > conn->s[side] =3D -1; > > } > > @@ -284,8 +282,6 @@ static int tcp_splice_connect_finish(const struct c= tx *c, > > int i =3D 0; > > =20 > > for (side =3D 0; side < SIDES; side++) { > > - conn->pipe[side][0] =3D conn->pipe[side][1] =3D -1; > > - > > for (; i < TCP_SPLICE_PIPE_POOL_SIZE; i++) { > > if (splice_pipe_pool[i][0] >=3D 0) { > > SWAP(conn->pipe[side][0], > > @@ -361,12 +357,8 @@ static int tcp_splice_connect(const struct ctx *c,= struct tcp_splice_conn *conn, > > } > > =20 > > if (connect(conn->s[1], sa, sl)) { > > - if (errno !=3D EINPROGRESS) { > > - int ret =3D -errno; > > - > > - close(sock_conn); > > - return ret; > > - } > > + if (errno !=3D EINPROGRESS) > > + return -errno; >=20 > Nit: a newline here would be nice. Done, thanks. --=20 David Gibson | 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 --FrDFuPXaONVHWVaI Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmXSs9cACgkQzQJF27ox 2GeKJg/+NWmQUgR+WUhmnr2hDR2yHzyflCrrlQK68izAnKLnwxmqNKStoFMqVSDQ jJl90y2H0EuiI/bPu7pMbgZ5whBhn0jW1OT3WcsTCuDhZ+7hRD3pvdrDKoQ2nt4W eGm/eppYATNplC56zwkCg2BBZew2sMQtmE1QjTuiTtYXATX63Ei+ZpjB6AcYenHG ZF9N0pZkxXWTEdosWS55cUBHFFK9dLceAWvLscGpwsi250ucvu+UHY5YqcI5n8+e 5HMt4ZDj2t83raROhSBSgziYWAWcvFHKuiXHROq9wvljqK7bMxdjiio5HU6AJano 7m08E4/H76Nf51Uldj/eHokWxF7VLYTDd10q4vwmgKqVEMBgi54RDvuA6VxfOKzh gQwUlNYG1dBbUZuWxPtTlOhxlOrYnHntYWkewRNX0aFsXFmjV9Sec9j1eiWkH+FJ UYD5d2RuOOtO0NpChOQ8rZuq0zC2ElWltsP70z0NxSZC9LVZ18btLUx5YYO1LdLU Fa+5LFgrJixhZVMKmwV74CV73SUtJZnE1YpuZ9ixTb2hgZXWHhr7JiBupj6+bzy5 Xt3wo+nmNh+/WUXy1bZL2ZEYqIoKthBga5aDdU9j5eR5q3P3FT1MFNOZuyFVj2TE BP6afLobgAGBDlSc+8rEOB236Gw8QAilG1Jm+oHJ4rJYnyGiiWI= =t4kv -----END PGP SIGNATURE----- --FrDFuPXaONVHWVaI--