Skip to content

Commit

Permalink
BUG/MINOR: sock: mark abns sockets as non-suspendable and always unbi…
Browse files Browse the repository at this point in the history
…nd them

In 2.3, we started to get a cleaner socket unbinding mechanism with
commit f58b8db ("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.

(cherry picked from commit 445fc1f)
Signed-off-by: Christopher Faulet <[email protected]>
  • Loading branch information
wtarreau authored and capflam committed Dec 5, 2023
1 parent a51c3e1 commit 3ecb341
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 0 deletions.
1 change: 1 addition & 0 deletions include/haproxy/receiver-t.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,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 */
Expand Down
2 changes: 2 additions & 0 deletions src/sock.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;

Expand Down
7 changes: 7 additions & 0 deletions src/sock_unix.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down

0 comments on commit 3ecb341

Please sign in to comment.