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=202512 header.b=BCqc8CRM; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id B694B5A004E for ; Mon, 19 Jan 2026 05:51:10 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202512; t=1768798267; bh=7Q82eHpdu++3To7kAYlpEVogPvi/I+trAJvYEWacKyc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=BCqc8CRMsl++gjMgblHqDNYU05fN01purDlwuly8IJZew6I1nf0DjeEr6MzQZRgz9 5a1Ca6D4YK0/k3Gfhw/6sDOxZW0i5FSYyeTuzsugD9r56TMdQImpjE+y310KaDmlVJ F2DvmNp863Hmm79rwQHgTSMTKu7JzSlVXy6ZlF7OKn0uYOJxb23IEYUxLBAbx5Dva1 xB5RlKz0rcWO/DEJTXFKTooh5hNrbYfUklbkL2GEv1vFMyeBBY1Zi9pY+2zgDsyfRc SAZSbwdcbKUUn7Jpcles+4jj3fZNBo4C99oGjlpFnqSxda9sh3zAwlYegT/n5uZiKq l1V1B4vUwaBqg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4dvdMH0thJz4wGx; Mon, 19 Jan 2026 15:51:07 +1100 (AEDT) Date: Mon, 19 Jan 2026 15:48:19 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH 2/3] tcp: Register fds with epoll at flow creation Message-ID: References: <20260116155223.2717168-1-lvivier@redhat.com> <20260116155223.2717168-3-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="/KJ4ogy+EEeo+XSt" Content-Disposition: inline In-Reply-To: <20260116155223.2717168-3-lvivier@redhat.com> Message-ID-Hash: TXHP6CEHPMGRP7VEEKGG5ABAFC25XKOE X-Message-ID-Hash: TXHP6CEHPMGRP7VEEKGG5ABAFC25XKOE 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: --/KJ4ogy+EEeo+XSt Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jan 16, 2026 at 04:52:22PM +0100, Laurent Vivier wrote: > Register connection sockets with epoll using empty events > (events=3D0) in tcp_conn_from_tap(), tcp_tap_conn_from_sock() > and tcp_flow_repair_socket(). >=20 > This allows tcp_epoll_ctl() to always use EPOLL_CTL_MOD, removing > the need to check whether fds are already registered. As a result, the > conditional ADD/MOD logic is no longer needed, simplifying the function. >=20 > Signed-off-by: Laurent Vivier Couple of queries, but the concept looks good. > --- > flow.c | 1 + > tcp.c | 36 ++++++++++++++---------------------- > 2 files changed, 15 insertions(+), 22 deletions(-) >=20 > diff --git a/flow.c b/flow.c > index cefe6c8b5b24..532339ce7fe1 100644 > --- a/flow.c > +++ b/flow.c > @@ -357,6 +357,7 @@ static void flow_set_state(struct flow_common *f, enu= m flow_state state) > * > * Return: true if flow is registered with epoll, false otherwise > */ > +/* cppcheck-suppress unusedFunction */ > bool flow_in_epoll(const struct flow_common *f) > { > return f->epollid !=3D EPOLLFD_ID_INVALID; > diff --git a/tcp.c b/tcp.c > index 1db861705ddb..d9bca041dea8 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -528,37 +528,22 @@ static uint32_t tcp_conn_epoll_events(uint8_t event= s, uint8_t conn_flags) > static int tcp_epoll_ctl(struct tcp_tap_conn *conn) > { > uint32_t events; > - int m; > =20 > if (conn->events =3D=3D CLOSED) { > - if (flow_in_epoll(&conn->f)) { > - int epollfd =3D flow_epollfd(&conn->f); > + int epollfd =3D flow_epollfd(&conn->f); > =20 > - epoll_del(epollfd, conn->sock); > - if (conn->timer !=3D -1) > - epoll_del(epollfd, conn->timer); > - } > + epoll_del(epollfd, conn->sock); > + if (conn->timer !=3D -1) > + epoll_del(epollfd, conn->timer); > =20 > return 0; > } > =20 > events =3D tcp_conn_epoll_events(conn->events, conn->flags); > =20 > - if (flow_in_epoll(&conn->f)) { > - m =3D EPOLL_CTL_MOD; > - } else { > - flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); > - m =3D EPOLL_CTL_ADD; > - } > - > - if (flow_epoll_set(&conn->f, m, events, conn->sock, > - !TAPSIDE(conn)) < 0) { > - int ret =3D -errno; > - > - if (m =3D=3D EPOLL_CTL_ADD) > - flow_epollid_clear(&conn->f); > - return ret; > - } > + if (flow_epoll_set(&conn->f, EPOLL_CTL_MOD, events, conn->sock, > + !TAPSIDE(conn)) < 0) > + return -errno; > =20 > return 0; > } > @@ -1710,6 +1695,8 @@ static void tcp_conn_from_tap(const struct ctx *c, = sa_family_t af, > conn->sock =3D s; > conn->timer =3D -1; > conn->listening_sock =3D -1; > + flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); > + flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, s, !TAPSIDE(conn)); Do we need to handle errors here? Because this is conn_from_tap(), we know that !TAPSIDE() will always be TGTSIDE in this case. > conn_event(c, conn, TAP_SYN_RCVD); > =20 > conn->wnd_to_tap =3D WINDOW_DEFAULT; > @@ -2433,6 +2420,8 @@ static void tcp_tap_conn_from_sock(const struct ctx= *c, union flow *flow, > conn->sock =3D s; > conn->timer =3D -1; > conn->ws_to_tap =3D conn->ws_from_tap =3D 0; > + flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); > + flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, s, !TAPSIDE(conn)); Same comments as above, except here we know it's INISIDE. > conn_event(c, conn, SOCK_ACCEPTED); > =20 > hash =3D flow_hash_insert(c, TAP_SIDX(conn)); > @@ -3825,6 +3814,9 @@ int tcp_flow_migrate_target(struct ctx *c, int fd) > return 0; > } > =20 > + flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); > + flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, conn->sock, !TAPSIDE(conn)); > + > flow_hash_insert(c, TAP_SIDX(conn)); > FLOW_ACTIVATE(conn); > =20 > --=20 > 2.52.0 >=20 --=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 --/KJ4ogy+EEeo+XSt Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmltt5IACgkQzQJF27ox 2GcFOQ/+IXrx/yqG1hrSZpgMjxk//wYmq/dXzzTIChIe65xzESFHxwMhp/XyC/p5 czN86v118adtK2V64Xm1tcgNom6bU8SevT0pBTXt5OFAJ+tj8JJVk+ubr1/5dfUf wMP4iEmj3OfCZUWBHPvQ7Icxl05MJW1PrciIdiqNi1rWYrHjLe0lsghtJPErmqH1 xvvGomzkPIY8tmDs9Bd0EC1F/T+LFYq5oV0SZAeke2XMbrrrHbSS6ZCl/2cya/PR 2GVOEiWnrm+h97ZFTUXIbkk0bp/UXVWqa2I0IASDG0zThrfwvQvkV75qnCLyABKR hH1DNUbqmnMCiSHDJrAz1hr3zOusnu7DCHjedqmSOf+KFH9pdm5cLsCOV0Mt0UoY sg9eb9+xa1wBJs+TCx5RljAo2bnxdjAwExRe+fmHD5o1eKkKwlpQqPCeDGJUcS/O nS8+NTGJHPwSZ61j6XVHmkcnEx5HB2PHUHFoLRIfL55fw0aqTDIkNJyXaZDfEB/R s4uZs5c0IlBDxIarfG8uMmteGzJQMQv6qJXQz5vg6gZIeVTN/7aFHZq1+AP7DnF1 Ef3Ms9VQVmdYyjoZewkbX8rqrNKl7r0XR33oom85HdVKBihiq7gzMBICqtXLo5BV H0kYWey8qO+yxny6LbWbd+YeAvjA2fSyhS5/hdN5gHqQxCVK7dY= =XesG -----END PGP SIGNATURE----- --/KJ4ogy+EEeo+XSt--