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=Vf6MVaVm; 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 CA40A5A026D for ; Wed, 17 Jun 2026 01:09:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1781651362; 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=ComKV9L1IZ8MOgGpEv03mZsOisolPFes+Z5acDY0zc8=; b=Vf6MVaVmX3xVsfVzDnBsblGfWScWc79CXs10+7UruQMmLl13nJJ3y4kmqPi6m7BHBytKmE KVowk0ERv0EXcqP4qmD4PloBrXuCZu5aSdkX1YyVga9z6RoIqeHeWkFsoq6aEuu+Ey5hEu xryd+MK2UYr1J9sfec3Nfb89TOareis= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-455-tnmxMrnKPtGPUBTFmvrtWA-1; Tue, 16 Jun 2026 19:09:21 -0400 X-MC-Unique: tnmxMrnKPtGPUBTFmvrtWA-1 X-Mimecast-MFC-AGG-ID: tnmxMrnKPtGPUBTFmvrtWA_1781651360 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-45eee3f9f03so4483392f8f.1 for ; Tue, 16 Jun 2026 16:09:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781651360; x=1782256160; 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=ComKV9L1IZ8MOgGpEv03mZsOisolPFes+Z5acDY0zc8=; b=FDj0ozknBYlj+4nBvVkHcQRIsDH8vVFQSRB0aNnffpS0xL4iYC35qZu891w7X/kwOj YQfw1NHuw01xcWceo+u8oxmuidNG48ElOASbrpGleGuhxS0E+2tQL/PZcQloB5KMtXF+ Gw08FZmr25dDvw+3zS99JvaGOtWs2k5fCvSmMEwAYUaJYrPlVk0NoYPvd73/djwSkZLJ YKOeleQBGFBevShnd65aAEhQAaEZBOABMc3kUBNNxMrHqyd3c/A6CEomo2M8H3o2PfhM tk7u7KcFTS6+acMJigl8earIdm3HPEfSy4wqP0YONRAz2B/yqMMssqZ8y8ZnuPN7MYIY fwag== X-Gm-Message-State: AOJu0YzSaqQpldSQh0vXlbJUHWWeAIXJBNNQaYPUNySfsYcKVJDyJsHi PkSkqDgWUQ0U5eY2o+ZtP/2AIr7Q70hi/JosdUDoNIcwfaO9TEyaQC1OPzNLfOmCJpTrxtzyQ92 Gew3G58r2vEadERIP6GEnXJy3HqAg8E/7022WtzVFuo7Zok7MOJpcrPLBxT0cFg== X-Gm-Gg: AfdE7cn1YOC5yEvb1TeLHFJtlEHL8PrAYAB2Vfavj/wj0mKOgxYSo1c76m5OBdyznqA 127k6VyrDfwkrsRHYO0T6pB7MR/faYlXHE3KaKOIubce2k4M0JnnJGFPuYoTJZhMqiDMYQTiGyH YZk3+1vFd+zSw5gtl268AwWFMmRkA4T6RwXrPmUIjSo0/8ZtbZ3ej6yT8OYHNaimpy6dfvDvU7b g89bpttbid+J4SlNU+26IF4NDL+pTfmkORZddoyl8hwquCR635C22HmVSxU2F8fvvpzSY5iyzC/ 9xaCmRPgITejsDc8Y9vWx85Hxo3YOyJ8VLG8Cl/gTlVqqvd1+mlYmn0vyAvubn7n3rTWVa6QTlk 8J+1bWG5O9e58gBlR5RTr8pDEX1oFrNktColpQNeglriWXGHFCw== X-Received: by 2002:a5d:4a91:0:b0:45e:e50e:3bc with SMTP id ffacd0b85a97d-4623788f72amr1816712f8f.29.1781651360002; Tue, 16 Jun 2026 16:09:20 -0700 (PDT) X-Received: by 2002:a5d:4a91:0:b0:45e:e50e:3bc with SMTP id ffacd0b85a97d-4623788f72amr1816683f8f.29.1781651359442; Tue, 16 Jun 2026 16:09:19 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-4606f2c3fcfsm43060415f8f.26.2026.06.16.16.09.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 16 Jun 2026 16:09:18 -0700 (PDT) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 3/4] flow: Safer errno handling in flowside_connect() callers Message-ID: <20260617010918.12f447a5@elisabeth> In-Reply-To: <20260609023226.86058-4-david@gibson.dropbear.id.au> References: <20260609023226.86058-1-david@gibson.dropbear.id.au> <20260609023226.86058-4-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: Wed, 17 Jun 2026 01:09:18 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: Tc8a9bFzrKs2F4FRQTkGq9G4GOvElwnfukjPtyhM9v8_1781651360 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: CTKCJ4YR3KHXFPAPN4RQU64R6ELP45YM X-Message-ID-Hash: CTKCJ4YR3KHXFPAPN4RQU64R6ELP45YM 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 Tue, 9 Jun 2026 12:32:25 +1000 David Gibson wrote: > flowside_connect() behaves much like connect(2) itself, returning -1 on > error with errno set to the error code. One of the callers, in > udp_flow_sock(), uses the errno code with flow_dbg_perror() *after* it's > called epoll_del() and close() either of which could clobber errno. > > Change flowside_connect() to use the more regular convention for internal > functions: return a negative errno code on error, rather than just -1. > Save it in the callers and use that rather than raw errno to print the > message. > > Signed-off-by: David Gibson > --- > flow.c | 6 ++++-- > tcp.c | 4 ++-- > udp_flow.c | 7 +++---- > 3 files changed, 9 insertions(+), 8 deletions(-) > > diff --git a/flow.c b/flow.c > index dd92bad7..98828430 100644 > --- a/flow.c > +++ b/flow.c > @@ -259,7 +259,7 @@ int flowside_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif, > * > * Connect @s to the endpoint address and port from @tgt. > * > - * Return: 0 on success, negative on error > + * Return: 0 on success, negative error code on error > */ > int flowside_connect(const struct ctx *c, int s, > uint8_t pif, const struct flowside *tgt) > @@ -267,7 +267,9 @@ int flowside_connect(const struct ctx *c, int s, > union sockaddr_inany sa; > > pif_sockaddr(c, &sa, pif, &tgt->eaddr, tgt->eport); > - return connect(s, &sa.sa, socklen_inany(&sa)); > + if (connect(s, &sa.sa, socklen_inany(&sa)) < 0) > + return -errno; > + return 0; This looks like a good idea nevertheless, but: > } > > /** flow_log__ - Log flow-related message, internal helper > diff --git a/tcp.c b/tcp.c > index 6fba865f..81813643 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -3744,8 +3744,8 @@ static int tcp_flow_repair_connect(const struct ctx *c, > > rc = flowside_connect(c, conn->sock, PIF_HOST, tgt); > if (rc) { > - rc = -errno; > - flow_perror(conn, "Failed to connect migrated socket"); > + flow_err(conn, "Failed to connect migrated socket: %s", > + strerror_(-rc)); ...wouldn't it be more convenient to establish that flowside_connect() behaves like connect() also by setting errno (by updating the comment to flowside_connect() accordingly) and use that fact here to keep calling flow_perror(), so that we don't need to open code the strerror_() call? > return rc; > } > > diff --git a/udp_flow.c b/udp_flow.c > index 35417bc4..6edfa65a 100644 > --- a/udp_flow.c > +++ b/udp_flow.c > @@ -88,13 +88,12 @@ static int udp_flow_sock(const struct ctx *c, > return rc; > } > > - if (flowside_connect(c, s, pif, side) < 0) { > - rc = -errno; > - > + if ((rc = flowside_connect(c, s, pif, side)) < 0) { > epoll_del(flow_epollfd(&uflow->f), s); > close(s); > > - flow_dbg_perror(uflow, "Couldn't connect flow socket"); > + flow_dbg(uflow, "Couldn't connect flow socket: %s", > + strerror_(-rc)); For the same reason, couldn't we just move the existing flow_dbg_perror() call before epoll_del() instead? > return rc; > } > uflow->s[sidei] = s; -- Stefano