Re: [PATCH 2/3] conf, tap, repair: Uniformly use non-blocking accept() on Unix sockets

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 13 May 2026 15:51:27 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> 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.
> > 
> > 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 that
> > there's nothing waiting.  To handle these we have an explicit fcntl() to
> > enable NONBLOCK on the control listening socket.
> > 
> > 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 with
> > 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?

I think the comments I'm raising (the one below and the one to 3/3)
actually warrant a respin at this point.

> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  conf.c   | 4 +---
> >  repair.c | 4 ++--
> >  tap.c    | 5 +++++
> >  util.c   | 2 +-
> >  4 files changed, 9 insertions(+), 6 deletions(-)
> > 
> > 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 = -1;
> >  	}
> > @@ -2087,7 +2085,7 @@ retry:
> >  	fd = accept4(c->fd_control_listen, NULL, NULL, SOCK_CLOEXEC);
> >  	if (fd < 0) {
> >  		if (errno != EAGAIN)
> > -			warn_perror("accept4() on configuration listening socket");
> > +			warn_perror("Error accept()ing configuration socket");
> >  		return;
> >  	}
> >  
> > 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 events)
> >  
> >  	if ((c->fd_repair = accept4(c->fd_repair_listen, NULL, NULL,
> >  				    SOCK_CLOEXEC)) < 0) {
> > -		rc = errno;
> > -		debug_perror("accept4() on TCP_REPAIR helper listening socket");
> > +		if ((rc = errno) != EAGAIN)
> > +			warn_perror("Error accept()ing repair helper");

See repair_wait() for the reason why this particular listening socket
needs to be blocking (with a timeout).

> >  		return rc;
> >  	}
> >  
> > 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 events)
> >  	}
> >  
> >  	c->fd_tap = accept4(c->fd_tap_listen, NULL, NULL, SOCK_CLOEXEC);
> > +	if (c->fd_tap < 0) {
> > +		if (errno != EAGAIN)
> > +			warn_perror("Error accepting tap client");
> > +		return;
> > +	}
> >  
> >  	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 epoll_type type,
> >   */
> >  int sock_unix(char *sock_path)
> >  {
> > -	int fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
> > +	int fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0);
> >  	struct sockaddr_un addr = {
> >  		.sun_family = AF_UNIX,
> >  	};
> > -- 
> > 2.54.0

-- 
Stefano