From: Willy Tarreau <w@1wt.eu>
Date: Mon, 20 Nov 2023 09:44:21 +0000 (+0100)
Subject: BUG/MINOR: sock: mark abns sockets as non-suspendable and always unbind them
X-Git-Tag: v2.9-dev11~41
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=445fc1fe3a0dd6a9d43790f5848d1d22b0c0dcba;p=thirdparty%2Fhaproxy.git

BUG/MINOR: sock: mark abns sockets as non-suspendable and always unbind them

In 2.3, we started to get a cleaner socket unbinding mechanism with
commit f58b8db47 ("MEDIUM: receivers: add an rx_unbind() method in
the protocols"). This mechanism rightfully refrains from unbinding
when sockets are expected to be transferrable to another worker via
"expose-fd listeners", but this is not compatible with ABNS sockets,
which do not support reuseport, unbinding nor being renamed: in short
they will always prevent a new process from binding.

It turns out that this is not much visible because by pure accident,
GTUNE_SOCKET_TRANSFER is only set in the code dealing with master mode
and deamons, so it's never set in foreground mode nor in tests even if
present on the stats socket. However with master mode, it is now always
set even when not present on the stats socket, and will always conflict.

The only reasonable approach seems to consist in marking these abns
sockets as non-suspendable so that the generic sock_unbind() code can
decide to just unbind them regardless of GTUNE_SOCKET_TRANSFER.

This should carefully be backported as far as 2.4.
---

diff --git a/include/haproxy/receiver-t.h b/include/haproxy/receiver-t.h
index 4527b5f412..d9f280e46f 100644
--- a/include/haproxy/receiver-t.h
+++ b/include/haproxy/receiver-t.h
@@ -36,6 +36,7 @@
 #define RX_F_INHERITED          0x00000002  /* inherited FD from the parent process (fd@) or duped from another local receiver */
 #define RX_F_MWORKER            0x00000004  /* keep the FD open in the master but close it in the children */
 #define RX_F_MUST_DUP           0x00000008  /* this receiver's fd must be dup() from a reference; ignore socket-level ops here */
+#define RX_F_NON_SUSPENDABLE    0x00000010  /* this socket cannot be suspended hence must always be unbound */
 
 /* Bit values for rx_settings->options */
 #define RX_O_FOREIGN            0x00000001  /* receives on foreign addresses */
diff --git a/src/sock.c b/src/sock.c
index 7ec4d46006..7fcdc103fa 100644
--- a/src/sock.c
+++ b/src/sock.c
@@ -234,6 +234,7 @@ void sock_unbind(struct receiver *rx)
 {
 	/* There are a number of situations where we prefer to keep the FD and
 	 * not to close it (unless we're stopping, of course):
+	 *   - worker process unbinding from a worker's non-suspendable FD (ABNS) => close
 	 *   - worker process unbinding from a worker's FD with socket transfer enabled => keep
 	 *   - master process unbinding from a master's inherited FD => keep
 	 *   - master process unbinding from a master's FD => close
@@ -247,6 +248,7 @@ void sock_unbind(struct receiver *rx)
 
 	if (!stopping && !master &&
 	    !(rx->flags & RX_F_MWORKER) &&
+	    !(rx->flags & RX_F_NON_SUSPENDABLE) &&
 	    (global.tune.options & GTUNE_SOCKET_TRANSFER))
 		return;
 
diff --git a/src/sock_unix.c b/src/sock_unix.c
index 1f7da504df..ef749a53a6 100644
--- a/src/sock_unix.c
+++ b/src/sock_unix.c
@@ -340,6 +340,13 @@ int sock_unix_bind_receiver(struct receiver *rx, char **errmsg)
 	rx->fd = fd;
 	rx->flags |= RX_F_BOUND;
 
+	if (!path[0]) {
+		/* ABNS sockets do not support suspend, and they conflict with
+		 * other ones (no reuseport), so they must always be unbound.
+		 */
+		rx->flags |= RX_F_NON_SUSPENDABLE;
+	}
+
 	fd_insert(fd, rx->owner, rx->iocb, rx->bind_tgroup, rx->bind_thread);
 
 	/* for now, all regularly bound TCP listeners are exportable */