public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH] flow: close socket fd on error
@ 2025-05-12 16:47 Laurent Vivier
  2025-05-12 21:20 ` Stefano Brivio
  0 siblings, 1 reply; 3+ messages in thread
From: Laurent Vivier @ 2025-05-12 16:47 UTC (permalink / raw)
  To: passt-dev; +Cc: Laurent Vivier

In eea8a76caf85 ("flow: fix podman issue #26073"), we unregister
the fd from epoll_ctl() in case of error, but we also need to close it.

As flowside_sock_l4() also calls sock_l4_sa() via flowside_sock_splice()
we can do it unconditionally.

Fixes: eea8a76caf85 ("flow: fix podman issue #26073")
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 udp_flow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/udp_flow.c b/udp_flow.c
index b3a13b7993d5..4c6b3c2ca8da 100644
--- a/udp_flow.c
+++ b/udp_flow.c
@@ -88,8 +88,8 @@ static int udp_flow_sock(const struct ctx *c,
 	if (flowside_connect(c, s, pif, side) < 0) {
 		int rc = -errno;
 
-		if (pif == PIF_HOST)
-			epoll_del(c, s);
+		epoll_del(c, s);
+		close(s);
 
 		flow_dbg_perror(uflow, "Couldn't connect flow socket");
 		return rc;
-- 
@@ -88,8 +88,8 @@ static int udp_flow_sock(const struct ctx *c,
 	if (flowside_connect(c, s, pif, side) < 0) {
 		int rc = -errno;
 
-		if (pif == PIF_HOST)
-			epoll_del(c, s);
+		epoll_del(c, s);
+		close(s);
 
 		flow_dbg_perror(uflow, "Couldn't connect flow socket");
 		return rc;
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] flow: close socket fd on error
  2025-05-12 16:47 [PATCH] flow: close socket fd on error Laurent Vivier
@ 2025-05-12 21:20 ` Stefano Brivio
  2025-05-14  6:05   ` David Gibson
  0 siblings, 1 reply; 3+ messages in thread
From: Stefano Brivio @ 2025-05-12 21:20 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: passt-dev

On Mon, 12 May 2025 18:47:00 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> In eea8a76caf85 ("flow: fix podman issue #26073"), we unregister
> the fd from epoll_ctl() in case of error, but we also need to close it.
> 
> As flowside_sock_l4() also calls sock_l4_sa() via flowside_sock_splice()
> we can do it unconditionally.

The previous PIF_HOST check made sense to me because it excluded
PIF_TAP, not so much because it excluded PIF_SPLICE.

But anyway, we'll never reach here with anything that's not PIF_HOST
nor PIF_TAP (kind of by definition), and a spurious epoll_del() is
surely better than the alternative in any case...

> Fixes: eea8a76caf85 ("flow: fix podman issue #26073")
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Applied, thanks. New release coming soon.

-- 
Stefano


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] flow: close socket fd on error
  2025-05-12 21:20 ` Stefano Brivio
@ 2025-05-14  6:05   ` David Gibson
  0 siblings, 0 replies; 3+ messages in thread
From: David Gibson @ 2025-05-14  6:05 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: Laurent Vivier, passt-dev

[-- Attachment #1: Type: text/plain, Size: 1272 bytes --]

On Mon, May 12, 2025 at 11:20:50PM +0200, Stefano Brivio wrote:
> On Mon, 12 May 2025 18:47:00 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
> > In eea8a76caf85 ("flow: fix podman issue #26073"), we unregister
> > the fd from epoll_ctl() in case of error, but we also need to close it.
> > 
> > As flowside_sock_l4() also calls sock_l4_sa() via flowside_sock_splice()
> > we can do it unconditionally.
> 
> The previous PIF_HOST check made sense to me because it excluded
> PIF_TAP, not so much because it excluded PIF_SPLICE.

Right.  pif_is_socket() exists for exactly that case..

> But anyway, we'll never reach here with anything that's not PIF_HOST
> nor PIF_TAP (kind of by definition), and a spurious epoll_del() is
> surely better than the alternative in any case...

..but in this case it's not relevant because you have to already have
a socket by this point.

> 
> > Fixes: eea8a76caf85 ("flow: fix podman issue #26073")
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> 
> Applied, thanks. New release coming soon.
> 

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-05-14  6:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-05-12 16:47 [PATCH] flow: close socket fd on error Laurent Vivier
2025-05-12 21:20 ` Stefano Brivio
2025-05-14  6:05   ` David Gibson

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).