Hi guys,
On Fri, May 12, 2017 at 04:26:01PM +0200, Willy Tarreau wrote:
> In fact William is currently working on the master-worker model to get rid
> of the systemd-wrapper and found some corner cases between this and your
> patchset. Nothing particularly difficult, just the fact that he'll need
> to pass the path to the previous socket to the new processes during reloads.
>
> During this investigation it was found that we'd need to be able to say
> that a process possibly has no stats socket and that the next one will not
> be able to retrieve the FDs. Such information cannot be passed from the
> command line since it's a consequence of the config parsing. Thus we thought
> it would make sense to have a per-socket option to say whether or not it
> would be usable for offering the listening file descriptors, just like we
> currently have an administrative level on them (I even seem to remember
> that Olivier first asked if we wouldn't need to do this). And suddenly a
> few benefits appear when doing this :
> - security freaks not willing to expose FDs over the socket would simply
> not enable them ;
>
> - we could restrict the number of processes susceptible of exposing the
> FDs simply by playing with the "process" directive on the socket ; that
> could also save some system-wide FDs ;
>
> - the master process could reliably find the socket's path in the conf
> (the first one with this new directive enabled), even if it's changed
> between reloads ;
>
> - in the default case (no specific option) we wouldn't change the existing
> behaviour so it would not make existing reloads worse.
>
The attached patches provide the "expose-fd listeners" stats socket option and
remove the "no-unused-socket" global option.
It behaves exactly has Willy explain above minus the master process :-)
ps: Master Worker patches are coming soon!
--
William Lallemand
>From 5567d977f722e862c6ba56d65118e094ac28735c Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallem...@haproxy.com>
Date: Wed, 24 May 2017 00:57:40 +0200
Subject: [PATCH 1/3] cli: add ACCESS_LVL_MASK to store the access level
The current level variable use only 2 bits for storing the 3 access
level (user, oper and admin).
This patch add a bitmask which allows to use the remaining bits for
other usage.
---
include/types/global.h | 2 ++
src/cli.c | 32 ++++++++++++++++++--------------
src/stats.c | 2 +-
src/stick_table.c | 4 ++--
4 files changed, 23 insertions(+), 17 deletions(-)
diff --git a/include/types/global.h b/include/types/global.h
index 57b969d..cd5fda3 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -69,6 +69,8 @@
#define ACCESS_LVL_USER 1
#define ACCESS_LVL_OPER 2
#define ACCESS_LVL_ADMIN 3
+#define ACCESS_LVL_MASK 0x3
+
/* SSL server verify mode */
enum {
diff --git a/src/cli.c b/src/cli.c
index 55baee3..cdbaf2b 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -217,7 +217,8 @@ static int stats_parse_global(char **args, int section_type, struct proxy *curpx
}
bind_conf = bind_conf_alloc(global.stats_fe, file, line, args[2], xprt_get(XPRT_RAW));
- bind_conf->level = ACCESS_LVL_OPER; /* default access level */
+ bind_conf->level &= ~ACCESS_LVL_MASK;
+ bind_conf->level |= ACCESS_LVL_OPER; /* default access level */
if (!str2listener(args[2], global.stats_fe, bind_conf, file, line, err)) {
memprintf(err, "parsing [%s:%d] : '%s %s' : %s\n",
@@ -383,7 +384,7 @@ int cli_has_level(struct appctx *appctx, int level)
struct stream_interface *si = appctx->owner;
struct stream *s = si_strm(si);
- if (strm_li(s)->bind_conf->level < level) {
+ if ((strm_li(s)->bind_conf->level & ACCESS_LVL_MASK) < level) {
appctx->ctx.cli.msg = stats_permission_denied_msg;
appctx->st0 = CLI_ST_PRINT;
return 0;
@@ -790,12 +791,12 @@ static int cli_io_handler_show_cli_sock(struct appctx *appctx)
} else
continue;
- if (bind_conf->level == ACCESS_LVL_USER)
- chunk_appendf(&trash, "user ");
- else if (bind_conf->level == ACCESS_LVL_OPER)
- chunk_appendf(&trash, "operator ");
- else if (bind_conf->level == ACCESS_LVL_ADMIN)
+ if ((bind_conf->level & ACCESS_LVL_MASK) == ACCESS_LVL_ADMIN)
chunk_appendf(&trash, "admin ");
+ else if ((bind_conf->level & ACCESS_LVL_MASK) == ACCESS_LVL_OPER)
+ chunk_appendf(&trash, "operator ");
+ else if ((bind_conf->level & ACCESS_LVL_MASK) == ACCESS_LVL_USER)
+ chunk_appendf(&trash, "user ");
else
chunk_appendf(&trash, " ");
@@ -1000,13 +1001,16 @@ static int bind_parse_level(char **args, int cur_arg, struct proxy *px, struct b
return ERR_ALERT | ERR_FATAL;
}
- if (!strcmp(args[cur_arg+1], "user"))
- conf->level = ACCESS_LVL_USER;
- else if (!strcmp(args[cur_arg+1], "operator"))
- conf->level = ACCESS_LVL_OPER;
- else if (!strcmp(args[cur_arg+1], "admin"))
- conf->level = ACCESS_LVL_ADMIN;
- else {
+ if (!strcmp(args[cur_arg+1], "user")) {
+ conf->level &= ~ACCESS_LVL_MASK;
+ conf->level |= ACCESS_LVL_USER;
+ } else if (!strcmp(args[cur_arg+1], "operator")) {
+ conf->level &= ~ACCESS_LVL_MASK;
+ conf->level |= ACCESS_LVL_OPER;
+ } else if (!strcmp(args[cur_arg+1], "admin")) {
+ conf->level &= ~ACCESS_LVL_MASK;
+ conf->level |= ACCESS_LVL_ADMIN;
+ } else {
memprintf(err, "'%s' only supports 'user', 'operator', and 'admin' (got '%s')",
args[cur_arg], args[cur_arg+1]);
return ERR_ALERT | ERR_FATAL;
diff --git a/src/stats.c b/src/stats.c
index 8f73b7d..71230d0 100644
--- a/src/stats.c
+++ b/src/stats.c
@@ -1957,7 +1957,7 @@ int stats_dump_proxy_to_buffer(struct stream_interface *si, struct proxy *px, st
if (uri)
flags = uri->flags;
- else if (strm_li(s)->bind_conf->level >= ACCESS_LVL_OPER)
+ else if ((strm_li(s)->bind_conf->level & ACCESS_LVL_MASK) >= ACCESS_LVL_OPER)
flags = ST_SHLGNDS | ST_SHNODE | ST_SHDESC;
else
flags = ST_SHNODE | ST_SHDESC;
diff --git a/src/stick_table.c b/src/stick_table.c
index a03a824..8cc7dd2 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -2253,7 +2253,7 @@ static int table_dump_head_to_buffer(struct chunk *msg, struct stream_interface
/* any other information should be dumped here */
- if (target && strm_li(s)->bind_conf->level < ACCESS_LVL_OPER)
+ if (target && (strm_li(s)->bind_conf->level & ACCESS_LVL_MASK) < ACCESS_LVL_OPER)
chunk_appendf(msg, "# contents not dumped due to insufficient privileges\n");
if (bi_putchk(si_ic(si), msg) == -1) {
@@ -2667,7 +2667,7 @@ static int cli_io_handler_table(struct appctx *appctx)
return 0;
if (appctx->ctx.table.target &&
- strm_li(s)->bind_conf->level >= ACCESS_LVL_OPER) {
+ (strm_li(s)->bind_conf->level & ACCESS_LVL_MASK) >= ACCESS_LVL_OPER) {
/* dump entries only if table explicitly requested */
eb = ebmb_first(&appctx->ctx.table.proxy->table.keys);
if (eb) {
--
2.10.2
>From 599d792bcf858ed863f5e6cb4c3923164ffac5a5 Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallem...@haproxy.com>
Date: Fri, 26 May 2017 17:42:10 +0200
Subject: [PATCH 2/3] MINOR: cli: add 'expose-fd listeners' to pass listeners
FDs
This patch changes the stats socket rights for allowing the sending of
listening sockets.
The previous behavior was to allow any unix stats socket with admin
level to send sockets. It's not possible anymore, you have to set this
option to activate the socket sending.
Example:
stats socket /var/run/haproxy4.sock mode 666 expose-fd listeners level user process 4
---
doc/configuration.txt | 5 +++++
doc/management.txt | 3 ++-
include/types/global.h | 1 +
src/cli.c | 24 ++++++++++++++++++++++--
4 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index edd9e79..f9bdf2a 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -10413,6 +10413,11 @@ defer-accept
an established connection while the proxy will only see it in SYN_RECV. This
option is only supported on TCPv4/TCPv6 sockets and ignored by other ones.
+expose-fd listeners
+ This option is only usable with the stats socket. It gives your stats socket
+ the capability to pass listeners FD to another HAProxy process.
+ See alors "-x" in the management guide.
+
force-sslv3
This option enforces use of SSLv3 only on SSL connections instantiated from
this listener. SSLv3 is generally less expensive than the TLS counterparts
diff --git a/doc/management.txt b/doc/management.txt
index d1ea1f6..565813e 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -280,7 +280,8 @@ list of options is :
-x <unix_socket> : connect to the specified socket and try to retrieve any
listening sockets from the old process, and use them instead of trying to
bind new ones. This is useful to avoid missing any new connection when
- reloading the configuration on Linux.
+ reloading the configuration on Linux. The capability must be enable on the
+ stats socket using "expose-fd listeners" in your configuration.
A safe way to start HAProxy from an init file consists in forcing the daemon
mode, storing existing pids to a pid file and using this pid file to notify
diff --git a/include/types/global.h b/include/types/global.h
index cd5fda3..aeb82ea 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -71,6 +71,7 @@
#define ACCESS_LVL_ADMIN 3
#define ACCESS_LVL_MASK 0x3
+#define ACCESS_FD_LISTENERS 0x4 /* expose listeners FDs on stats socket */
/* SSL server verify mode */
enum {
diff --git a/src/cli.c b/src/cli.c
index cdbaf2b..55115d6 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -993,6 +993,24 @@ static int cli_parse_set_ratelimit(char **args, struct appctx *appctx, void *pri
return 1;
}
+/* parse the "expose-fd" argument on the bind lines */
+static int bind_parse_expose_fd(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
+{
+ if (!*args[cur_arg + 1]) {
+ memprintf(err, "'%s' : missing fd type", args[cur_arg]);
+ return ERR_ALERT | ERR_FATAL;
+ }
+ if (!strcmp(args[cur_arg+1], "listeners")) {
+ conf->level |= ACCESS_FD_LISTENERS;
+ } else {
+ memprintf(err, "'%s' only supports 'listeners' (got '%s')",
+ args[cur_arg], args[cur_arg+1]);
+ return ERR_ALERT | ERR_FATAL;
+ }
+
+ return 0;
+}
+
/* parse the "level" argument on the bind lines */
static int bind_parse_level(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
{
@@ -1026,6 +1044,7 @@ static int _getsocks(char **args, struct appctx *appctx, void *private)
unsigned char *tmpbuf = NULL;
struct cmsghdr *cmsg;
struct stream_interface *si = appctx->owner;
+ struct stream *s = si_strm(si);
struct connection *remote = objt_conn(si_opposite(si)->end);
struct msghdr msghdr;
struct iovec iov;
@@ -1057,7 +1076,7 @@ static int _getsocks(char **args, struct appctx *appctx, void *private)
setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, (void *)&tv, sizeof(tv));
iov.iov_base = &tot_fd_nb;
iov.iov_len = sizeof(tot_fd_nb);
- if (!cli_has_level(appctx, ACCESS_LVL_ADMIN))
+ if (!(strm_li(s)->bind_conf->level & ACCESS_FD_LISTENERS))
goto out;
memset(&msghdr, 0, sizeof(msghdr));
/*
@@ -1224,7 +1243,8 @@ static struct cfg_kw_list cfg_kws = {ILH, {
}};
static struct bind_kw_list bind_kws = { "STAT", { }, {
- { "level", bind_parse_level, 1 }, /* set the unix socket admin level */
+ { "level", bind_parse_level, 1 }, /* set the unix socket admin level */
+ { "expose-fd", bind_parse_expose_fd, 1 }, /* set the unix socket expose fd rights */
{ NULL, NULL, 0 },
}};
--
2.10.2
>From 82dd696315fc0f5119b2227001443e1fa7a0b70b Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallem...@haproxy.com>
Date: Fri, 26 May 2017 18:19:55 +0200
Subject: [PATCH 3/3] MEDIUM: proxy: zombify proxies only when the expose-fd
socket is bound
When HAProxy is running with multiple processes and some listeners
arebound to processes, the unused sockets were not closed in the other
processes. The aim was to be able to send those listening sockets using
the -x option.
However to ensure the previous behavior which was to close those
sockets, we provided the "no-unused-socket" global option.
This patch changes this behavior, it will close unused sockets which are
not in the same process as an expose-fd socket, making the
"no-unused-socket" option useless.
The "no-unused-socket" option was removed in this patch.
---
doc/configuration.txt | 7 -------
src/cfgparse.c | 5 -----
src/haproxy.c | 19 ++++++++++++++++++-
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index f9bdf2a..bd9a99f 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -587,7 +587,6 @@ The following keywords are supported in the "global" section :
- nosplice
- nogetaddrinfo
- noreuseport
- - no-unused-socket
- spread-checks
- server-state-base
- server-state-file
@@ -1251,12 +1250,6 @@ noreuseport
Disables the use of SO_REUSEPORT - see socket(7). It is equivalent to the
command line argument "-dR".
-no-unused-socket
- By default, each haproxy process keeps all sockets opened, event those that
- are only used by another processes, so that any process can provide all the
- sockets, to make reloads seamless. This option disables this, and close all
- unused sockets, to save some file descriptors.
-
spread-checks <0..50, in percent>
Sometimes it is desirable to avoid sending agent and health checks to
servers at exact intervals, for instance when many logical servers are
diff --git a/src/cfgparse.c b/src/cfgparse.c
index f167321..4c0e2d4 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -659,11 +659,6 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
goto out;
global.tune.options &= ~GTUNE_USE_REUSEPORT;
}
- else if (!strcmp(args[0], "no-unused-socket")) {
- if (alertif_too_many_args(0, file, linenum, args, &err_code))
- goto out;
- global.tune.options &= ~GTUNE_SOCKET_TRANSFER;
- }
else if (!strcmp(args[0], "quiet")) {
if (alertif_too_many_args(0, file, linenum, args, &err_code))
goto out;
diff --git a/src/haproxy.c b/src/haproxy.c
index 261b213..5ccebd1 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -859,7 +859,6 @@ static void init(int argc, char **argv)
#if defined(SO_REUSEPORT)
global.tune.options |= GTUNE_USE_REUSEPORT;
#endif
- global.tune.options |= GTUNE_SOCKET_TRANSFER;
pid = getpid();
progname = *argv;
@@ -2165,6 +2164,24 @@ int main(int argc, char **argv)
exit(0); /* parent must leave */
}
+ /* pass through every cli socket, and check if it's bound to
+ * the current process and if it exposes listeners sockets.
+ * Caution: the GTUNE_SOCKET_TRANSFER is now set after the fork.
+ * */
+
+ if (global.stats_fe) {
+ struct bind_conf *bind_conf;
+
+ list_for_each_entry(bind_conf, &global.stats_fe->conf.bind, by_fe) {
+ if (bind_conf->level & ACCESS_FD_LISTENERS) {
+ if (!bind_conf->bind_proc || bind_conf->bind_proc & (1UL << proc)) {
+ global.tune.options |= GTUNE_SOCKET_TRANSFER;
+ break;
+ }
+ }
+ }
+ }
+
/* we might have to unbind some proxies from some processes */
px = proxy;
while (px != NULL) {
--
2.10.2
