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 EB6BE5A005E for ; Thu, 17 Nov 2022 09:59:39 +0100 (CET) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4NCYks5jSgz4xZ3; Thu, 17 Nov 2022 19:59:33 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1668675573; bh=ELpIYx955RJ4/6vXuTALo4G8FkomQ/J5LCaEq7JfpSQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=B2569kJPWO3Ypl3Ju0DSSG2uwI7WCf/bx15SbUOZc30W+O2n21kw3GFJkd8q9IgzF G3l2hEi6tQP5K3vjmjaUvxL4cfdr3wNBr2/903saco7fBGOOr2t0qYryZVghh2gj+n dj2GCtvAlOisoUTFCXDroCZcXzl9EPUyKVl5nXJ0= Date: Thu, 17 Nov 2022 19:58:28 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 15/32] tcp: Unify part of spliced and non-spliced conn_from_sock path Message-ID: References: <20221116044212.3876516-1-david@gibson.dropbear.id.au> <20221116044212.3876516-16-david@gibson.dropbear.id.au> <20221117005358.324ca3a0@elisabeth> <20221117083029.3a062bea@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="+YtlAfVG4KRJisXB" Content-Disposition: inline In-Reply-To: <20221117083029.3a062bea@elisabeth> Message-ID-Hash: JYQAJ2FP6D3T7IRTESQA6HKZ6H3D7PDV X-Message-ID-Hash: JYQAJ2FP6D3T7IRTESQA6HKZ6H3D7PDV 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.3 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: --+YtlAfVG4KRJisXB Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Nov 17, 2022 at 08:30:29AM +0100, Stefano Brivio wrote: > On Thu, 17 Nov 2022 12:37:04 +1100 > David Gibson wrote: >=20 > > On Thu, Nov 17, 2022 at 12:53:58AM +0100, Stefano Brivio wrote: > > > On Wed, 16 Nov 2022 15:41:55 +1100 > > > David Gibson wrote: > > > =20 > > > > In tcp_sock_handler() we split off to handle spliced sockets before > > > > checking anything else. However the first steps of the "new connec= tion" > > > > path for each case are the same: allocate a connection entry and ac= cept() > > > > the connection. > > > >=20 > > > > Remove this duplication by making tcp_conn_from_sock() handle both = spliced > > > > and non-spliced cases, with help from more specific tcp_tap_conn_fr= om_sock > > > > and tcp_splice_conn_from_sock functions for the later stages which = differ. > > > >=20 > > > > Signed-off-by: David Gibson > > > > --- > > > > tcp.c | 68 ++++++++++++++++++++++++++++++++++--------------= ---- > > > > tcp_splice.c | 58 +++++++++++++++++++++++--------------------- > > > > tcp_splice.h | 4 ++++ > > > > 3 files changed, 80 insertions(+), 50 deletions(-) > > > >=20 > > > > diff --git a/tcp.c b/tcp.c > > > > index 72d3b49..e66a82a 100644 > > > > --- a/tcp.c > > > > +++ b/tcp.c > > > > @@ -2753,28 +2753,19 @@ static void tcp_connect_finish(struct ctx *= c, struct tcp_tap_conn *conn) > > > > } > > > > =20 > > > > /** > > > > - * tcp_conn_from_sock() - Handle new connection request from liste= ning socket > > > > + * tcp_tap_conn_from_sock() - Initialize state for non-spliced con= nection > > > > * @c: Execution context > > > > * @ref: epoll reference of listening socket > > > > + * @conn: connection structure to initialize > > > > + * @s: Accepted socket > > > > + * @sa: Peer socket address (from accept()) > > > > * @now: Current timestamp > > > > */ > > > > -static void tcp_conn_from_sock(struct ctx *c, union epoll_ref ref, > > > > - const struct timespec *now) > > > > +static void tcp_tap_conn_from_sock(struct ctx *c, union epoll_ref = ref, > > > > + struct tcp_tap_conn *conn, int s, > > > > + struct sockaddr *sa, > > > > + const struct timespec *now) > > > > { > > > > - struct sockaddr_storage sa; > > > > - struct tcp_tap_conn *conn; > > > > - socklen_t sl; > > > > - int s; > > > > - > > > > - if (c->tcp.conn_count >=3D TCP_MAX_CONNS) > > > > - return; > > > > - > > > > - sl =3D sizeof(sa); > > > > - s =3D accept4(ref.r.s, (struct sockaddr *)&sa, &sl, SOCK_NONBLOCK= ); > > > > - if (s < 0) > > > > - return; > > > > - > > > > - conn =3D CONN(c->tcp.conn_count++); > > > > conn->c.spliced =3D false; > > > > conn->sock =3D s; > > > > conn->timer =3D -1; > > > > @@ -2784,7 +2775,7 @@ static void tcp_conn_from_sock(struct ctx *c,= union epoll_ref ref, > > > > if (ref.r.p.tcp.tcp.v6) { > > > > struct sockaddr_in6 sa6; > > > > =20 > > > > - memcpy(&sa6, &sa, sizeof(sa6)); > > > > + memcpy(&sa6, sa, sizeof(sa6)); > > > > =20 > > > > if (IN6_IS_ADDR_LOOPBACK(&sa6.sin6_addr) || > > > > IN6_ARE_ADDR_EQUAL(&sa6.sin6_addr, &c->ip6.addr_seen) || > > > > @@ -2813,7 +2804,7 @@ static void tcp_conn_from_sock(struct ctx *c,= union epoll_ref ref, > > > > } else { > > > > struct sockaddr_in sa4; > > > > =20 > > > > - memcpy(&sa4, &sa, sizeof(sa4)); > > > > + memcpy(&sa4, sa, sizeof(sa4)); > > > > =20 > > > > memset(&conn->a.a4.zero, 0, sizeof(conn->a.a4.zero)); > > > > memset(&conn->a.a4.one, 0xff, sizeof(conn->a.a4.one)); > > > > @@ -2846,6 +2837,37 @@ static void tcp_conn_from_sock(struct ctx *c= , union epoll_ref ref, > > > > tcp_get_sndbuf(conn); > > > > } > > > > =20 > > > > +/** > > > > + * tcp_conn_from_sock() - Handle new connection request from liste= ning socket > > > > + * @c: Execution context > > > > + * @ref: epoll reference of listening socket > > > > + * @now: Current timestamp > > > > + */ > > > > +static void tcp_conn_from_sock(struct ctx *c, union epoll_ref ref, > > > > + const struct timespec *now) > > > > +{ > > > > + struct sockaddr_storage sa; > > > > + union tcp_conn *conn; > > > > + socklen_t sl; > > > > + int s; > > > > + > > > > + if (c->tcp.conn_count >=3D TCP_MAX_CONNS) > > > > + return; > > > > + > > > > + sl =3D sizeof(sa); > > > > + s =3D accept4(ref.r.s, (struct sockaddr *)&sa, &sl, SOCK_NONBLOCK= ); =20 > > >=20 > > > Combined with 16/32 I'm not sure this is simplifying much -- it looks= a > > > bit unnatural there to get the peer address not "directly" from > > > accept4(). On the other hand you drop a few lines -- I'm fine with > > > it either way. =20 > >=20 > > Um.. I'm not really sure what you're getting at here. >=20 > By "directly" I mean assigned by accept4() in the same function, > instead of accept4() being done in the caller. >=20 > That is, if I now look at tcp_tap_conn_from_sock() we have 'sa' there > which comes as an argument, not directly a couple of lines above from > accept4(), which would be quicker to review. Right, but this is unavoidable. This patch is a preliminary to deciding whether to take the spliced or non-spliced route based on the address we get from accept4(). > On the other hand the function comment says "from accept()", so it's > not much effort to figure that out either. --=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 --+YtlAfVG4KRJisXB Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEoULxWu4/Ws0dB+XtgypY4gEwYSIFAmN195QACgkQgypY4gEw YSIeGQ/+Knn+rsta/wRlX149irlAijc295v6LRzdJjLcvuR60eLP4gTTX8EPCmQz fFo5CzSW83sH5E7nOgMmFarFHRbS9tRliseJ/E2pz8htOFPtZBt3i/odNR5cK7wV mkNTEk7HEUiahPu8c3SPxkcveqpduIA62bAaII8SRPxcKUFx33OPnqvhAv6GnQsH h/5WaP38Rzwo3kNTqaK9G0kzoONXx+Krrzji6pvv0aIlQIB3wqKuqsmyiv7+n5tS R5lfCpL3JsEMeYc5UqSycDRmyyjCGPV3AqzaKbBNgx9e/7OVHnTci1mdKMu1P8xm zsTDV4w7DPmZKkso6Y4U/opDUfL5SA4q3eHPWxr2U0R+EoRvPpz/cvlnWD+T7XS4 DgDv4r7o4c3ruScxAAg7Miau8hHA9y6aBcAjxho/elHyTGiqxBcj5nsFBUaDULvk jlvqFO/UZiduz1wVLyWnLyY7LxXH7zFaPeALpVPcbHnX/qStCRy1UDm1UTpSrlPG LuQEb8H019wtZw+PLI4m+YM3ZFb3O6TYRXX73eALiPDk1zPYZO79Q8pWXKaamAQj uzJV655yvXUDiKOqGBGknxCA6Gbie8PO18/Vv36Ocw92UhFMn13TcmzgoT2rt9pd CcuPvfjlcPYhXqS3zIX8FCIXp2FqWZ78RB5vmUX/bZmJYO6SBjw= =Z9zk -----END PGP SIGNATURE----- --+YtlAfVG4KRJisXB--