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=202510 header.b=W95M2C4a; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 5CC675A061A for ; Wed, 22 Oct 2025 03:01:19 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202510; t=1761094876; bh=/ZLxzaGdDdB7jXBALGZMwDwHQQwlx+eXawwUED3h250=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=W95M2C4aLdFxrepds9oOpZtPg4819BUHMdyRi1DQ9kbzTGzCmxn3ayxymoqufbhM8 4iivKiC8LoDbDEUGGVdcrfDZQE/yktpI5lJYndqFBp2Uz7pfSOwDXZ5qsIfQhvXZmH 2IS7TEgy8iH0lKQY4F0+Du78B3e+kT65Ry1d2W1ga8gH3dZgcc0iO9IcH1QdA2HNzC ftonYrxCAW3XJN3a0siw1FaRSjKpczBedj/1GabqTpXzpZ0XHLmkK0zPTseOR1nlhJ xHMBgQiN8AzIgH+tKWkOuRY2EsYxt3RVoCngR4H9RAOgiumvXRPmofBPESJY/epPem n9OJN9EwHB+Sg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4crrT81pdMz4wCp; Wed, 22 Oct 2025 12:01:16 +1100 (AEDT) Date: Wed, 22 Oct 2025 11:58:48 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v4 2/7] epoll_ctl: Extract epoll operations Message-ID: References: <20251017103129.229412-1-lvivier@redhat.com> <20251017103129.229412-3-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="pOaijTMXBg5S7LD0" Content-Disposition: inline In-Reply-To: Message-ID-Hash: ABV4XBBRMLQ3VLNUQTEFRMAC7BGT22FB X-Message-ID-Hash: ABV4XBBRMLQ3VLNUQTEFRMAC7BGT22FB 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: --pOaijTMXBg5S7LD0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Oct 21, 2025 at 01:52:25PM +0200, Laurent Vivier wrote: > On 20/10/2025 03:20, David Gibson wrote: > > On Fri, Oct 17, 2025 at 12:31:24PM +0200, Laurent Vivier wrote: > > > Centralize epoll_add() and epoll_del() helper functions into new > > > epoll_ctl.c/h files. > > >=20 > > > This also moves the union epoll_ref definition from passt.h to > > > epoll_ctl.h where it's more logically placed. > > >=20 > > > The new epoll_add() helper simplifies adding file descriptors to epoll > > > by taking an epoll_ref and events, handling error reporting > > > consistently across all call sites. > > >=20 > > > Signed-off-by: Laurent Vivier > >=20 > > Concept looks good, some minor details in execution. > >=20 > > > --- > > > Makefile | 22 +++++++++++----------- > > > epoll_ctl.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ > > > epoll_ctl.h | 51 +++++++++++++++++++++++++++++++++++++++++++++++++= ++ > > > icmp.c | 4 +--- > > > passt.c | 2 +- > > > passt.h | 34 ---------------------------------- > > > pasta.c | 7 +++---- > > > repair.c | 14 +++++--------- > > > tap.c | 13 ++++--------- > > > tcp.c | 2 +- > > > tcp_splice.c | 2 +- > > > udp.c | 2 +- > > > udp_flow.c | 1 + > > > util.c | 22 +++------------------- > > > util.h | 4 +++- > > > vhost_user.c | 8 ++------ > > > vu_common.c | 2 +- > > > 17 files changed, 134 insertions(+), 101 deletions(-) > > > create mode 100644 epoll_ctl.c > > > create mode 100644 epoll_ctl.h > ... > > > @@ -1327,14 +1327,12 @@ static void tap_backend_show_hints(struct ctx= *c) > > > static void tap_sock_unix_init(const struct ctx *c) > > > { > > > union epoll_ref ref =3D { .type =3D EPOLL_TYPE_TAP_LISTEN }; > > > - struct epoll_event ev =3D { 0 }; > > > listen(c->fd_tap_listen, 0); > > > ref.fd =3D c->fd_tap_listen; > > > - ev.events =3D EPOLLIN | EPOLLET; > > > - ev.data.u64 =3D ref.u64; > > > - epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap_listen, &ev); > > > + > > > + epoll_add(c->epollfd, EPOLLIN | EPOLLET, &ref); > >=20 > > Preexisting, but we should probably check for errors here. > >=20 >=20 > To do what? die() ? err() to report the place where the error happens? The case I'm concerned about is this fails, but we carry on. Then we lose 1/Nth of our packets because we're not getting events for one queue, and weird failures follow. It takes us ages to debug, because we don't consider that the fd might have silently failed to make it into the epoll set. err() would be probably be enough to make this debuggable. But honestly, if we can't put our tap fds into the epoll, we're in a sufficiently bad state that die() makes sense. --=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 --pOaijTMXBg5S7LD0 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmj4LEgACgkQzQJF27ox 2GdLAhAAm+u5CSc+hgU/o0RlY7vRn70OtJK34r5BXos19EievEaDDKKEzUu475g0 uCWmF3dojqJyPiCOGyb8LB1iFufgJIIN0CRldOn+Vu/5ZszrQDrYE0E7kVqnyaxG MAkumHfEb1GuFLCEax+ABxqhrbEu97WlKH2mMo03432C7bxJKWFGjhGKuDAEk8lu jpPAch04O3eRjknF+77BUGLg93I/u88CSrfrMFyq07BOPlpTyprZaIP+5KuGl/T9 5V1fyZK6JERwfFDLk27KO5IWCocQMvuzkjobYiffDu+fun+GE2geqURj6wc9cs3H cHckN2jSajec4bNnOFuqW3lt/ZcVkN0E7hIgAec0Z/vjnu50B9Qp7YtYxjhOF5HA d4hris9PPTxGea9G0AFWVZ2dvZmcorPhoHsP/1K0VbbDTuPs7u5K9CVNoVuMA0X8 Nt1VBbjr9g8J2sYQREGe3cebKwU0w4KGk0gUm7hVYKZ7F8ROqXfn4nIaYRaAYZSO ynedFt/bjKUzh5Uj44FICbO7lOJPazAMZj6UXwYLc8BBmzLuoCdvhAG74yoSbn9i LSx8ptaZKrZWtrHopnomosGWoiRMRlvZPFIJvRZKy3fmd9LeTz0LDn8GtY4es+zp 1msKUzIuYR0pQjSBiDRF1aT2VkI7D6SmZ7eHHQbyIyfdfIyTxpY= =lG/h -----END PGP SIGNATURE----- --pOaijTMXBg5S7LD0--