From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=TxRGw/+I; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id 49BE95A0262 for ; Sat, 16 May 2026 17:46:33 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1778946392; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=154rZ31zYotHT2fRzkQjy6LKEOwSfCYFmvcQ6Pl846Q=; b=TxRGw/+IU5xfwAjwnXZjXUYTnXtdDLFY98sb1PPUtxHVXyvz930Clo85dETCToVUJcYzAl Z1J83E4pA1qzJ/z+poe3AW3WMl/NVblEITBm5PBEi2plFTyONLDwnZ7dz2CIayHnC5YeO6 XCwseESu3f2KaTS3+uhWpUS3a5FwTfQ= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-691-2HtvQDCONnekdXF6cCc-eQ-1; Sat, 16 May 2026 11:46:30 -0400 X-MC-Unique: 2HtvQDCONnekdXF6cCc-eQ-1 X-Mimecast-MFC-AGG-ID: 2HtvQDCONnekdXF6cCc-eQ_1778946389 Received: by mail-wr1-f72.google.com with SMTP id ffacd0b85a97d-44cc3c9b2feso658307f8f.1 for ; Sat, 16 May 2026 08:46:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778946389; x=1779551189; h=date:content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=154rZ31zYotHT2fRzkQjy6LKEOwSfCYFmvcQ6Pl846Q=; b=R1QmZ59FOOs2aX/F9ydz6vqNChrLUpMJQ0+umOi+vC+KUR9U8koo7VmZ7wHjd63+3p oJ0NfFhC4gYXTmh1fFVr8IqWRHAKb98F1jPptejql2uxP2/ZMI3mpDcfweWCzsBIWOzc EX3CGESsRQkcsrNl9QnETv8XrZ6wAVuhaMxdP55e2cdAppT71hrJaZkJmG+m9oEYnFYt Vw03URaVe630JTGRUzj3zqOlZjP7BdxzqZXvxv1MFq8UVH3QmrK10UDDqfL/1CcRNuPz 3fe7FoF/2Tij0luI6hPqEMzOXPStP4mIi2YftF6SHBCUM8iNcENNmBLcYzh7lJq6Y4Wf bSQA== X-Gm-Message-State: AOJu0YwuL/bMwrNGuA+QIZGDMQQa7U1QUpQKQ4fBqtHRAm61sTHkfAyc 557l2GMUgYCiUulnBtfsbkv8niG/EKKVCEN8Kv0CSB2UJ9i6tMzT+c82uHWnHhO/l8ByPr2pSqz t43CkvmjyiyDt7lMDYI0VmfYpyKanQrx8P8Cyg1SkAscWnebu0x300A== X-Gm-Gg: Acq92OHFVaP5hjPbqvFa1Lkbpm9dfVyUGM5h9LDugKHAzXqqq7J9mNZCR5vvtpDVZxt X384cmAnNAoott8InLiKOqRgutMDGqvP1+0pScG04JRLhXqyXq4h+wE2chvGMZbWqJXg1nDWmES NBy9HTh0bl1GuDllpYQQVR6+tFscy/DUjLgz0ihJiPSg0UZQIHM0N7yUeV6GvL4EK021wxOGBYa XaHbIG//B14QIUCgPoE2YJ5FLdHQxhz/G1BmgVDIScIKOPvuBFhFi1bC2MtwFYYoIn5levDYMVT e7RQy+hiC4tw4TOyPyk+p+BOq++NWxPGe5RbtCKwI3d2p7lEbat7Uvuws9nxjpY71LwBFVZtb2F NA6R45UFbbNZRVQpLI7AD39b63nzGz8qJ X-Received: by 2002:a5d:584c:0:b0:446:db72:e8ec with SMTP id ffacd0b85a97d-45e5c587281mr12066870f8f.23.1778946389240; Sat, 16 May 2026 08:46:29 -0700 (PDT) X-Received: by 2002:a5d:584c:0:b0:446:db72:e8ec with SMTP id ffacd0b85a97d-45e5c587281mr12066836f8f.23.1778946388667; Sat, 16 May 2026 08:46:28 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-45da0fe248dsm22772809f8f.30.2026.05.16.08.46.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 16 May 2026 08:46:28 -0700 (PDT) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 2/3] conf, tap, repair: Uniformly use non-blocking accept() on Unix sockets Message-ID: <20260516174627.290aae6e@elisabeth> In-Reply-To: References: <20260513041423.2446716-1-david@gibson.dropbear.id.au> <20260513041423.2446716-3-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Sat, 16 May 2026 17:46:27 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: qy9nDEHx-7dBhP7SFVNIXt95fwau0MtphEx3lIpJh8I_1778946389 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: FAEWQCUFB2MFZRPQIS5G3SSLYRUDMSJR X-Message-ID-Hash: FAEWQCUFB2MFZRPQIS5G3SSLYRUDMSJR X-MailFrom: sbrivio@redhat.com 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: On Wed, 13 May 2026 15:51:27 +1000 David Gibson 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 > > --- > > 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