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 D37075A0278 for ; Mon, 19 Feb 2024 03:03:17 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1708308192; bh=6XqDuD4XpNmbb68rKZWg7TS9p1B5tMuxK8wrlONKAwA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=OySSEl3XZ4N5BxO0xjuFg9gFZPLd6glv4jIbuhCaoTcB3d9054RDE//zLk54S3xod RoZdImkAnxcoHVIv5zJCxABjakXm/5qEXsVxsYDY8bJ3M9fdcKtDZWoHWWGSZa+OMX wo8ZSo+OgDHYEcG0nu7v7O8SjLT+HNJmTo6lm18seQPQbYBPsbvI4e5rQeeNVQ6NCW omTZpyCve1jA3CI8VOzVJPzWjxrsVNYlJQuzp8uCpAgjv3vx5UttzJf3WKnE71c0MA 7ck3zdWM+3hy4xEWUNFzdmWAlE8V1oFrGzDK3PiNwc/zvo6O4dEzClM1l+UTDNSVBU 1hEiuLoT6JzcQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4TdQmc4K3Tz4wcH; Mon, 19 Feb 2024 13:03:12 +1100 (AEDT) Date: Mon, 19 Feb 2024 12:51:50 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v2 12/22] tcp, tcp_splice: Helpers for getting sockets from the pools Message-ID: References: <20240206011734.884138-1-david@gibson.dropbear.id.au> <20240206011734.884138-13-david@gibson.dropbear.id.au> <20240218220029.57acacf8@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="vrPNhXemTVKgysUj" Content-Disposition: inline In-Reply-To: <20240218220029.57acacf8@elisabeth> Message-ID-Hash: 4Y2KADRPMR7UY5QCHBQMRJJ46YBZEPIO X-Message-ID-Hash: 4Y2KADRPMR7UY5QCHBQMRJJ46YBZEPIO 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: --vrPNhXemTVKgysUj Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Feb 18, 2024 at 10:00:29PM +0100, Stefano Brivio wrote: > On Tue, 6 Feb 2024 12:17:24 +1100 > David Gibson wrote: >=20 > > We maintain pools of ready-to-connect sockets in both the original and > > (for pasta) guest namespace to reduce latency when starting new TCP > > connections. If we exhaust those pools we have to take a higher > > latency path to get a new socket. > >=20 > > Currently we open-code that fallback in the places we need it. To > > improve clarity encapsulate that into helper functions. > >=20 > > Signed-off-by: David Gibson > > --- > > tcp.c | 29 ++++++++++++++++++++++++----- > > tcp_conn.h | 2 +- > > tcp_splice.c | 46 +++++++++++++++++++++++++--------------------- > > 3 files changed, 50 insertions(+), 27 deletions(-) > >=20 > > diff --git a/tcp.c b/tcp.c > > index e15b932f..c06d1cc4 100644 > > --- a/tcp.c > > +++ b/tcp.c > > @@ -1792,7 +1792,7 @@ int tcp_conn_pool_sock(int pool[]) > > * > > * Return: socket number on success, negative code if socket creation = failed > > */ > > -int tcp_conn_new_sock(const struct ctx *c, sa_family_t af) > > +static int tcp_conn_new_sock(const struct ctx *c, sa_family_t af) > > { > > int s; > > =20 > > @@ -1811,6 +1811,27 @@ int tcp_conn_new_sock(const struct ctx *c, sa_fa= mily_t af) > > return s; > > } > > =20 > > +/** > > + * tcp_conn_sock() - Obtain a connectable socket in the host/init name= space > > + * @c: Execution context > > + * @af: Address family (AF_INET or AF_INET6) > > + * > > + * Return: Socket fd on success, -errno on failure > > + */ > > +int tcp_conn_sock(const struct ctx *c, sa_family_t af) > > +{ > > + int *pool =3D af =3D=3D AF_INET6 ? init_sock_pool6 : init_sock_pool4; > > + int s; > > + > > + if ((s =3D tcp_conn_pool_sock(pool)) >=3D 0) > > + return s; > > + > > + /* If the pool is empty we just open a new one without refilling the > > + * pool to keep latency down. > > + */ > > + return tcp_conn_new_sock(c, af); > > +} > > + > > /** > > * tcp_conn_tap_mss() - Get MSS value advertised by tap/guest > > * @conn: Connection pointer > > @@ -1909,7 +1930,6 @@ static void tcp_conn_from_tap(struct ctx *c, sa_f= amily_t af, > > const struct tcphdr *th, const char *opts, > > size_t optlen, const struct timespec *now) > > { > > - int *pool =3D af =3D=3D AF_INET6 ? init_sock_pool6 : init_sock_pool4; > > struct sockaddr_in addr4 =3D { > > .sin_family =3D AF_INET, > > .sin_port =3D th->dest, > > @@ -1931,9 +1951,8 @@ static void tcp_conn_from_tap(struct ctx *c, sa_f= amily_t af, > > if (!(flow =3D flow_alloc())) > > return; > > =20 > > - if ((s =3D tcp_conn_pool_sock(pool)) < 0) > > - if ((s =3D tcp_conn_new_sock(c, af)) < 0) > > - goto cancel; > > + if ((s =3D tcp_conn_sock(c, af)) < 0) > > + goto cancel; > > =20 > > if (!c->no_map_gw) { > > if (af =3D=3D AF_INET && IN4_ARE_ADDR_EQUAL(daddr, &c->ip4.gw)) > > diff --git a/tcp_conn.h b/tcp_conn.h > > index 20c7cb8b..e55edafe 100644 > > --- a/tcp_conn.h > > +++ b/tcp_conn.h > > @@ -159,7 +159,7 @@ bool tcp_flow_defer(union flow *flow); > > bool tcp_splice_flow_defer(union flow *flow); > > void tcp_splice_timer(const struct ctx *c, union flow *flow); > > int tcp_conn_pool_sock(int pool[]); > > -int tcp_conn_new_sock(const struct ctx *c, sa_family_t af); > > +int tcp_conn_sock(const struct ctx *c, sa_family_t af); > > void tcp_sock_refill_pool(const struct ctx *c, int pool[], sa_family_t= af); > > void tcp_splice_refill(const struct ctx *c); > > =20 > > diff --git a/tcp_splice.c b/tcp_splice.c > > index 576fe9be..609f3242 100644 > > --- a/tcp_splice.c > > +++ b/tcp_splice.c > > @@ -90,7 +90,7 @@ static const char *tcp_splice_flag_str[] __attribute(= (__unused__)) =3D { > > }; > > =20 > > /* Forward declaration */ > > -static int tcp_sock_refill_ns(void *arg); > > +static int tcp_conn_sock_ns(const struct ctx *c, sa_family_t af); > > =20 > > /** > > * tcp_splice_conn_epoll_events() - epoll events masks for given state > > @@ -380,36 +380,19 @@ static int tcp_splice_connect(const struct ctx *c= , struct tcp_splice_conn *conn, > > static int tcp_splice_new(const struct ctx *c, struct tcp_splice_conn = *conn, > > in_port_t port, uint8_t pif) > > { > > + sa_family_t af =3D CONN_V6(conn) ? AF_INET6 : AF_INET; > > int s =3D -1; > > =20 > > - /* If the pool is empty we take slightly different approaches > > - * for init or ns sockets. For init sockets we just open a > > - * new one without refilling the pool to keep latency down. > > - * For ns sockets, we're going to incur the latency of > > - * entering the ns anyway, so we might as well refill the > > - * pool. > > - */ > > if (pif =3D=3D PIF_SPLICE) { > > - int *p =3D CONN_V6(conn) ? init_sock_pool6 : init_sock_pool4; > > - sa_family_t af =3D CONN_V6(conn) ? AF_INET6 : AF_INET; > > - > > port +=3D c->tcp.fwd_out.delta[port]; > > =20 > > - s =3D tcp_conn_pool_sock(p); > > - if (s < 0) > > - s =3D tcp_conn_new_sock(c, af); > > + s =3D tcp_conn_sock(c, af); > > } else { > > - int *p =3D CONN_V6(conn) ? ns_sock_pool6 : ns_sock_pool4; > > - > > ASSERT(pif =3D=3D PIF_HOST); > > =20 > > port +=3D c->tcp.fwd_in.delta[port]; > > =20 > > - /* If pool is empty, refill it first */ > > - if (p[TCP_SOCK_POOL_SIZE-1] < 0) > > - NS_CALL(tcp_sock_refill_ns, c); > > - > > - s =3D tcp_conn_pool_sock(p); > > + s =3D tcp_conn_sock_ns(c, af); > > } > > =20 > > if (s < 0) { > > @@ -709,6 +692,27 @@ static int tcp_sock_refill_ns(void *arg) > > return 0; > > } > > =20 > > +/** > > + * tcp_conn_sock_ns() - Obtain a connectable socket in the namespace > > + * @c: Execution context > > + * @af: Address family (AF_INET or AF_INET6) > > + * > > + * Return: Socket fd in the namespace on success, -errno on failure > > + */ > > +static int tcp_conn_sock_ns(const struct ctx *c, sa_family_t af) > > +{ > > + int *p =3D af =3D=3D AF_INET6 ? ns_sock_pool6 : ns_sock_pool4; > > + > > + /* If the pool is empty we have to incur the latency of entering the = ns. > > + * Therefore, we might as well refill the whole pool while we're at i= t, > > + * which differs from tcp_conn_sock(). > > + */ > > + if (p[TCP_SOCK_POOL_SIZE-1] < 0) >=20 > Nit, for consistency (but yes, it was already like this): >=20 > if (p[TCP_SOCK_POOL_SIZE - 1] < 0) Adjusted, 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 --vrPNhXemTVKgysUj Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmXStDUACgkQzQJF27ox 2GfOIA//RbCHGYOxUbQKF5Sr3Wob1GwDFR32gme1flORre7mbA1b2UOjPj/iHd9n htpBpUUZeGPdy3KJ5IXMGg2Lq5wxHSM8IAxnkAwOM0fYfxt3Du2w4MoSTjWImyXn 83Q7v097VgdauvKyXQEi/NszFpDQTcdnJc505I8lAkuKJx3ft1bUycuBaPxDYP5K LeHLXB7TJtA7c9WaLmWovwkPa4QZBaynNXYX0l2ndOw3r9pz4uua2ETL36p7+C8z coX1NYgQYwpe60n2jZgpeFWqMgkJlu8Edizbm/8EFimVwkG6UPfjroLsXKzTsfw6 baPe9ZXsevhV67OY0MRng9a1HtH8Y+DxEyc5XAyayjGFQlta5Ha1TnklWf/76u3W +tdIZHElunTrGDkCuslP1mF0K72YnkDKIYMELVKvW/XtNVRo202FbN+8HOZWEH2g n1jMZXrS6SfAnLlVGQrMBwOPOfCLRILTuo6Dmkmr+MujnDdjb+H0P0bHdJAdwDyG gif6sFaRzaOs82KD5/AA1cN9ZHSNSschRbmj+RZXtBISN5kEiFAXZTMWKjEnSbJw H9SBcIFihewaHyZLWD+OecNm/9JbcUKGvfytFXNAdBRSN8WD4Wynk8dW8qw2n9Jw wRkzBClTGNHyoF2k/bHdXxixsk91lNGoeZ0gryR7nN3MAwzEQTs= =N+Q/ -----END PGP SIGNATURE----- --vrPNhXemTVKgysUj--