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 8889D5A0271 for ; Wed, 9 Aug 2023 08:30:17 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1691562613; bh=8rpgroFHhf2cPVWG+Kf5I7EdyesUmxk7vGcTwbsc4kA=; h=Date:From:To:Subject:References:In-Reply-To:From; b=BIWKeuSdfefIhqB1XpZxSKlrs4fnHI/fPAgcNnPeq38oQDmRraI3Oub5RCQe8Z4EW a1yxEE92G4XvcaDagr3bk8BKY7TYXn8dJxhQpbP64UDmj5G6o/FqGvnwxqTV73T5SB y8hWlagdNL8xwVGMqs7qZAq0f4+awlguiZgn3Xp0= Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4RLKtF744lz4wxQ; Wed, 9 Aug 2023 16:30:13 +1000 (AEST) Date: Wed, 9 Aug 2023 16:29:54 +1000 From: David Gibson To: Stefano Brivio , passt-dev@passt.top Subject: Re: [PATCH 7/9] epoll: Split handling of listening TCP sockets into their own handler Message-ID: References: <20230807134631.1400119-1-david@gibson.dropbear.id.au> <20230807134631.1400119-8-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="F+Ne0AgJdSVi0AmW" Content-Disposition: inline In-Reply-To: <20230807134631.1400119-8-david@gibson.dropbear.id.au> Message-ID-Hash: NTLI6XYXBI2QGV5JBI2JZ6E64BDOFBOJ X-Message-ID-Hash: NTLI6XYXBI2QGV5JBI2JZ6E64BDOFBOJ 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 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: --F+Ne0AgJdSVi0AmW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Aug 07, 2023 at 11:46:29PM +1000, David Gibson wrote: > tcp_sock_handler() handles both listening TCP sockets, and connected TCP > sockets, but what it needs to do in those cases has essentially nothing in > common. Therefore, give listening sockets their own epoll_type value and > dispatch directly to their own handler from the top level. This also lets > us remove the listen field from tcp_epoll_ref, since the information it > carries is implicit in the epoll_type. >=20 > Signed-off-by: David Gibson Stefano, I've realized an additional change that belongs with this patch, so I'll be making a second spin. Go ahead and review, but don't apply just yet. > --- > passt.c | 8 ++++++-- > passt.h | 4 +++- > tcp.c | 31 +++++++++---------------------- > tcp.h | 8 ++++---- > util.c | 2 +- > 5 files changed, 23 insertions(+), 30 deletions(-) >=20 > diff --git a/passt.c b/passt.c > index c750fad..c32981d 100644 > --- a/passt.c > +++ b/passt.c > @@ -56,7 +56,8 @@ > char pkt_buf[PKT_BUF_BYTES] __attribute__ ((aligned(PAGE_SIZE))); > =20 > char *epoll_type_str[EPOLL_TYPE_MAX+1] =3D { > - [EPOLL_TYPE_TCP] =3D "TCP socket", > + [EPOLL_TYPE_TCP] =3D "connected TCP socket", > + [EPOLL_TYPE_TCP_LISTEN] =3D "listening TCP socket", > [EPOLL_TYPE_TCP_TIMER] =3D "TCP timer", > [EPOLL_TYPE_UDP] =3D "UDP socket", > [EPOLL_TYPE_ICMP] =3D "ICMP socket", > @@ -323,7 +324,10 @@ loop: > break; > case EPOLL_TYPE_TCP: > if (!c.no_tcp) > - tcp_sock_handler(&c, ref, eventmask, &now); > + tcp_sock_handler(&c, ref, eventmask); > + break; > + case EPOLL_TYPE_TCP_LISTEN: > + tcp_listen_handler(&c, ref, &now); > break; > case EPOLL_TYPE_TCP_TIMER: > tcp_timer_handler(&c, ref); > diff --git a/passt.h b/passt.h > index fc1efdb..176bc85 100644 > --- a/passt.h > +++ b/passt.h > @@ -47,8 +47,10 @@ union epoll_ref; > enum epoll_type { > /* Special value to indicate an invalid type */ > EPOLL_TYPE_NONE =3D 0, > - /* TCP sockets */ > + /* Connected TCP sockets */ > EPOLL_TYPE_TCP, > + /* Listening TCP sockets */ > + EPOLL_TYPE_TCP_LISTEN, > /* timerfds used for TCP timers */ > EPOLL_TYPE_TCP_TIMER, > /* UDP sockets */ > diff --git a/tcp.c b/tcp.c > index 98761a2..c237393 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -2765,22 +2765,20 @@ static void tcp_tap_conn_from_sock(struct ctx *c,= union epoll_ref ref, > } > =20 > /** > - * tcp_conn_from_sock() - Handle new connection request from listening s= ocket > + * tcp_listen_handler() - Handle new connection request from listening s= ocket > * @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) > +void tcp_listen_handler(struct ctx *c, union epoll_ref ref, > + const struct timespec *now) > { > struct sockaddr_storage sa; > union tcp_conn *conn; > socklen_t sl; > int s; > =20 > - ASSERT(ref.tcp.listen); > - > - if (c->tcp.conn_count >=3D TCP_MAX_CONNS) > + if (c->no_tcp || c->tcp.conn_count >=3D TCP_MAX_CONNS) > return; > =20 > sl =3D sizeof(sa); > @@ -2926,19 +2924,10 @@ static void tcp_tap_sock_handler(struct ctx *c, s= truct tcp_tap_conn *conn, > * @c: Execution context > * @ref: epoll reference > * @events: epoll events bitmap > - * @now: Current timestamp > */ > -void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t event= s, > - const struct timespec *now) > +void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t event= s) > { > - union tcp_conn *conn; > - > - if (ref.tcp.listen) { > - tcp_conn_from_sock(c, ref, now); > - return; > - } > - > - conn =3D tc + ref.tcp.index; > + union tcp_conn *conn =3D tc + ref.tcp.index; > =20 > if (conn->c.spliced) > tcp_splice_sock_handler(c, &conn->splice, ref.fd, events); > @@ -2960,7 +2949,7 @@ static int tcp_sock_init_af(const struct ctx *c, in= t af, in_port_t port, > const struct in_addr *addr, const char *ifname) > { > in_port_t idx =3D port + c->tcp.fwd_in.delta[port]; > - union tcp_epoll_ref tref =3D { .listen =3D 1, .index =3D idx }; > + union tcp_epoll_ref tref =3D { .index =3D idx }; > int s; > =20 > s =3D sock_l4(c, af, IPPROTO_TCP, addr, ifname, port, tref.u32); > @@ -3020,8 +3009,7 @@ int tcp_sock_init(const struct ctx *c, sa_family_t = af, const void *addr, > static void tcp_ns_sock_init4(const struct ctx *c, in_port_t port) > { > in_port_t idx =3D port + c->tcp.fwd_out.delta[port]; > - union tcp_epoll_ref tref =3D { .listen =3D 1, .outbound =3D 1, > - .index =3D idx }; > + union tcp_epoll_ref tref =3D { .outbound =3D 1, .index =3D idx }; > struct in_addr loopback =3D { htonl(INADDR_LOOPBACK) }; > int s; > =20 > @@ -3045,8 +3033,7 @@ static void tcp_ns_sock_init4(const struct ctx *c, = in_port_t port) > static void tcp_ns_sock_init6(const struct ctx *c, in_port_t port) > { > in_port_t idx =3D port + c->tcp.fwd_out.delta[port]; > - union tcp_epoll_ref tref =3D { .listen =3D 1, .outbound =3D 1, > - .index =3D idx }; > + union tcp_epoll_ref tref =3D { .outbound =3D 1, .index =3D idx }; > int s; > =20 > ASSERT(c->mode =3D=3D MODE_PASTA); > diff --git a/tcp.h b/tcp.h > index 8eb7782..8189ac0 100644 > --- a/tcp.h > +++ b/tcp.h > @@ -14,8 +14,9 @@ > struct ctx; > =20 > void tcp_timer_handler(struct ctx *c, union epoll_ref ref); > -void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t event= s, > - const struct timespec *now); > +void tcp_listen_handler(struct ctx *c, union epoll_ref ref, > + const struct timespec *now); > +void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t event= s); > int tcp_tap_handler(struct ctx *c, int af, const void *addr, > const struct pool *p, const struct timespec *now); > int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr, > @@ -37,8 +38,7 @@ void tcp_update_l2_buf(const unsigned char *eth_d, cons= t unsigned char *eth_s, > */ > union tcp_epoll_ref { > struct { > - uint32_t listen:1, > - outbound:1, > + uint32_t outbound:1, > index:20; > }; > uint32_t u32; > diff --git a/util.c b/util.c > index 2cac7ba..d965f48 100644 > --- a/util.c > +++ b/util.c > @@ -120,7 +120,7 @@ int sock_l4(const struct ctx *c, int af, uint8_t prot= o, > =20 > switch (proto) { > case IPPROTO_TCP: > - ref.type =3D EPOLL_TYPE_TCP; > + ref.type =3D EPOLL_TYPE_TCP_LISTEN; > break; > case IPPROTO_UDP: > ref.type =3D EPOLL_TYPE_UDP; --=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 --F+Ne0AgJdSVi0AmW Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmTTMkUACgkQzQJF27ox 2Ge+4hAArOxq4qgt2gi+BAIuep+jj/6kJWneTiBLDTvBsDQRwXNjhs9rPOkIf3xK +u9IrkL/uQYTsWgj4UQ9dU0sTTHM68ENJ35qUc+AuWml3hVBNqYcfkot1C4tudH0 iVg2Z2ogMRIpzjWQrizIDow2FtqJ0eilhsHad1Ubz42jphdPOqF5+16iKzkR9Ika JnnbuX7CIn4hGxONLYrZNZaacalvFT50QfL3wgJzgwUQ+tBIcQQZsbTYu0JwHJuU RLggR2IDLygYxg+hXPVPoY0HlPJj6zG5ml5EvHeWUJ+4GU+XRlqJZJhxamiCrcvM UfYv7J2Cc+AgoY/JZLrkxnthpcOsGl7DzrmI2DE1f2D44x7CXAjhP6odsVykLXqE BpF8WeO0R+R0nCwybjnG1U5tQD9V/2LP5jeJv3AMOqu8u02yu/DD1XHRONEvAdgW A2J74G2QtH9xzwIcXe6i0n9ROtffRVfHtWyfcastZm9me6gmI+5nf9qvyPSjSXA2 nYk+H9aucwY8UKUtW67Sg8Hfbe/09iB8o4Lm0RVaNBcfyUd21NdNsRzPebl+YOf4 5Lcl24r7JqhXS/+VMj0QXISenQn5KjYGcwFYQwgef+qwrzAf23tD+sCm0bs3EnyQ dEYgYvb5p3psDA8wgVqVUCkSwsdBR29A2eDKLqecwEeuoR5mGeg= =6oCX -----END PGP SIGNATURE----- --F+Ne0AgJdSVi0AmW--