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=202602 header.b=OARZeEQC; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 5115E5A0271 for ; Wed, 13 May 2026 07:51:35 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1778651490; bh=t3DnCjhqxTaB/4QpKgwTL6qsEdV6/DSxBE16bF3dvuw=; h=Date:From:To:Subject:References:In-Reply-To:From; b=OARZeEQCeFp4Ti9N1K2kBFKnGeL9RIHc0zOQjMGUa5QY7Mi3V+wyoA3QEdPztVbkJ pn8yBrW3kYl0uPR1t5lYvnv06UEChjBjMvmtKpYyiJaEQeLWP8G5JM32DGvPKUfFbP kfSiArost8RcBqB6ZvHAuRxLAdD8YVjq1sUSHps/TCn6VYijEx7KSajArdS6zDOBsx G7cJ83PLrHC+uSu5el5dPq3pGcZ25qQxxMV4aDytPpgksNIjfLrb/YWwzlMFDPymNA +dJcooAyPRc9rDc1G8Iu2WszDYq/dat+MIrxiT3TsxLy17kBmCzCe0zCPz4e3WF5SH wjdMgJpQIkpFA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4gFjJL5SWDz4wHr; Wed, 13 May 2026 15:51:30 +1000 (AEST) Date: Wed, 13 May 2026 15:51:27 +1000 From: David Gibson To: Stefano Brivio , passt-dev@passt.top Subject: Re: [PATCH 2/3] conf, tap, repair: Uniformly use non-blocking accept() on Unix sockets Message-ID: References: <20260513041423.2446716-1-david@gibson.dropbear.id.au> <20260513041423.2446716-3-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="/XrxryAVMh5Sj5w9" Content-Disposition: inline In-Reply-To: <20260513041423.2446716-3-david@gibson.dropbear.id.au> Message-ID-Hash: HZIQYC4K32543MO6PPMU7DTXMYDLESRM X-Message-ID-Hash: HZIQYC4K32543MO6PPMU7DTXMYDLESRM 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: --/XrxryAVMh5Sj5w9 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, May 13, 2026 at 02:14:22PM +1000, David Gibson wrote: > sock_unix(), which creates a listening Unix socket, doesn't set the > SOCK_NONBLOCK flag, meaning that accept() will block if called with no > pending connections. Generally, this doesn't matter because we only > accept() once we've received an epoll event indicating there's a pending > connection request. >=20 > Control connections (pesto) are an exception, because the way we queue > connections requires that we call accept() when we close one connection to > see if there's another one waiting. We rely on an EAGAIN here to know th= at > there's nothing waiting. To handle these we have an explicit fcntl() to > enable NONBLOCK on the control listening socket. >=20 > However, always using non-blocking accept() for Unix sockets would make > things a bit more uniform, and should be a bit less fragile in the case > that we ever somehow got a spurious connection event. So, alter > sock_unix() to always use the SOCK_NONBLOCK flag. Remove the control > socket's special case fcntl(), and adjust the error handling on each > Unix socket accept() for the new behaviour. As a bonus the last adds > reporting for accept() errors on tap socket connections. I didn't realise it, but adding that reporting also removes a valid, if fairly minor coverity warning (at least with coverity 2026.3.0). > we will need non-blocking accept() for the upcoming control/configuration > socket. Always add SOCK_NONBLOCK, which is more robust and in keeping wi= th > the normal non-blocking style of passt. Oops. This paragraph is left over from a previous version. Can you remove on merge, if there's no other reason to respin? >=20 > Signed-off-by: David Gibson > --- > conf.c | 4 +--- > repair.c | 4 ++-- > tap.c | 5 +++++ > util.c | 2 +- > 4 files changed, 9 insertions(+), 6 deletions(-) >=20 > diff --git a/conf.c b/conf.c > index 029b9c7c..dec43fca 100644 > --- a/conf.c > +++ b/conf.c > @@ -1091,8 +1091,6 @@ static void conf_open_files(struct ctx *c) > die_perror("Couldn't open control socket %s", > c->control_path); > } > - if (fcntl(c->fd_control_listen, F_SETFL, O_NONBLOCK)) > - die_perror("Couldn't set O_NONBLOCK on control socket"); > } else { > c->fd_control_listen =3D -1; > } > @@ -2087,7 +2085,7 @@ retry: > fd =3D accept4(c->fd_control_listen, NULL, NULL, SOCK_CLOEXEC); > if (fd < 0) { > if (errno !=3D EAGAIN) > - warn_perror("accept4() on configuration listening socket"); > + warn_perror("Error accept()ing configuration socket"); > return; > } > =20 > diff --git a/repair.c b/repair.c > index 3e0e3e0a..42c4ae97 100644 > --- a/repair.c > +++ b/repair.c > @@ -101,8 +101,8 @@ int repair_listen_handler(struct ctx *c, uint32_t eve= nts) > =20 > if ((c->fd_repair =3D accept4(c->fd_repair_listen, NULL, NULL, > SOCK_CLOEXEC)) < 0) { > - rc =3D errno; > - debug_perror("accept4() on TCP_REPAIR helper listening socket"); > + if ((rc =3D errno) !=3D EAGAIN) > + warn_perror("Error accept()ing repair helper"); > return rc; > } > =20 > diff --git a/tap.c b/tap.c > index e7cac9df..fda2da9b 100644 > --- a/tap.c > +++ b/tap.c > @@ -1491,6 +1491,11 @@ void tap_listen_handler(struct ctx *c, uint32_t ev= ents) > } > =20 > c->fd_tap =3D accept4(c->fd_tap_listen, NULL, NULL, SOCK_CLOEXEC); > + if (c->fd_tap < 0) { > + if (errno !=3D EAGAIN) > + warn_perror("Error accepting tap client"); > + return; > + } > =20 > if (!getsockopt(c->fd_tap, SOL_SOCKET, SO_PEERCRED, &ucred, &len)) > info("accepted connection from PID %i", ucred.pid); > diff --git a/util.c b/util.c > index 73c9d51d..204391c7 100644 > --- a/util.c > +++ b/util.c > @@ -238,7 +238,7 @@ int sock_l4_dualstack_any(const struct ctx *c, enum e= poll_type type, > */ > int sock_unix(char *sock_path) > { > - int fd =3D socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0); > + int fd =3D socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, = 0); > struct sockaddr_un addr =3D { > .sun_family =3D AF_UNIX, > }; > --=20 > 2.54.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 --/XrxryAVMh5Sj5w9 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmoEEV4ACgkQzQJF27ox 2Gcf6g//bT31ZKvvAVaDMKfeYH5PbG2/Jk4idVYzgKEVyi4n2lOvXq4ETmHZZHb/ xsVzTGPoxqcEcG5lgvfwmUu/HJOt9/acIXDESk+/eClRRKouuruSjAIbegAJJyzE B7e0PnjH2ot5JlroQQxgmSMxLt3Htp32yFZ+6p25bJA1laSIELIVS7L5zdnSek08 7WC5ENydVofxkvAMDkkEvELkJwGKerQtTLsMk5xBn6OUmxQ+agQH8F4/usMFB3fx QatsZmDplSoZiw+oOrdSUGHBl5ckDT7h0S1islTs7RszT4ZfoeRDicNwZAnbvXei 7Wn76jQ70mYvODRXxxei4Zq4J6VXm/sBJt5rxGDTTYV97XcQix3Y0EK7sU9E4Bot xQEscqg2o77TiQ/5vtx4mSrRCaZHQoheM9FLxFcPGvtDCZBkohr2v/AQ0ByFRhe4 hu+Q0j6F8kJ5XLHeKFDeJrdORX3tRS7oZ52crdhE3suzTjaYHWiFe0M052Np3CzP SJ9JUG3NX46fcQmU4YIDp2xlyPsEyiy61DPg5nrlc+jUvPR98MpSY6C6iv8n9tFi oSMFZvHu/bANaYJ4nXacFalnG9EIQndEWYzfu/IErktYJP8/5kMG1ZFBWf/+Navf HrM4oFMkymrdm8En+VqYM4CRVpCXdmv+cr2MoBaJM3X+Lfh2A6o= =8v9N -----END PGP SIGNATURE----- --/XrxryAVMh5Sj5w9--