Re: [PATCH] BUG/MINOR: when master-worker is in daemon mode, detach from tty

[William Lallemand]

On Wed, Nov 29, 2017 at 12:05:43AM +0100, PiBa-NL wrote:
> Hi List,
>

Hi Pieter,
 
> Made a patch that makes the master-worker detach from tty when it is 
> also combined with daemon mode to allow a script to start haproxy with 
> daemon mode, closing stdout so the calling process knows when to stop 
> reading from it and allow the master to properly daemonize.
> 

> diff --git a/src/haproxy.c b/src/haproxy.c
> index c3c8281..a811577 100644
> --- a/src/haproxy.c
> +++ b/src/haproxy.c
> @@ -2648,6 +2648,13 @@ int main(int argc, char **argv)
>   }
>  
>   if (global.mode & (MODE_DAEMON | MODE_MWORKER)) {
> + if ((!(global.mode & MODE_QUIET) || (global.mode & 
> MODE_VERBOSE)) &&
> + ((global.mode & (MODE_DAEMON | MODE_MWORKER)) == 
> (MODE_DAEMON | MODE_MWORKER))) {
> + /* detach from the tty, this is required to properly 
> daemonize. */
> + fclose(stdin); fclose(stdout); fclose(stderr);
> + global.mode &= ~MODE_VERBOSE;
> + global.mode |= MODE_QUIET; /* ensure that we won't say 
> anything from now */
> + }
>   struct proxy *px;
>   struct peers *curpeers;
>   int ret = 0;

I need to check that again later, in my opinion it should be done after the
pipe() so we don't inherit the 0 and 1 FDs in the pipe, we also need to rely on
setsid() to do a proper tty detach. This is already done in -D mode without -W, 
maybe
this part of the code should me moved elsewhere, but we have to be careful not
to break the daemon mode w/o mworker.


-- 
William Lallemand

Re: [PATCH] BUG/MINOR: Check if master-worker pipe getenv succeeded, also allow pipe fd 0 as valid.

[William Lallemand]

On Tue, Nov 28, 2017 at 11:58:50PM +0100, PiBa-NL wrote:
> Hi List, Willy / Willliam,
> 

Hi Pieter,

> A patch i came up with that might make it a little 'safer' with regard 
> to getenv and its return value or possible lack thereof.. I'm not sure 
> it it will ever happen. But if it does it wont fail on a null pointer or 
> empty string conversion to a long value.. Though a arithmetic conversion 
> error could still happen if the value is present but not a number..but 
> well that would be a really odd case.
> 

The getenv returning NULL should never happen, but the test is wrong, it should
have been a strtol with an errno check instead of an atol. However that's
overkill in this case, we just need to check the return value of getenv().

> There are a few things i'm not sure about though.
> 
> - What would/could possibly break if mworker_pipe values are left as -1 
> and the process continues and tries to use it?

That does not seems to be a good idea, because we try to register the fd in the
poller after that. We don't need to do this, it's better to quit the
master-worker if something this simple failed, because we can't trust the
process anymore.

> - wont the rd wr char* values leak memory?

No because you don't need to free the return value of a getenv, it's a pointer
to the environment.
 
> Anyhow the biggest part that should be noticed of the bug is the 
> sometimes wrongful alert when the fd is actually '0'...
> 

That's right, the check was not coherent there, we should check only if
getenv return NULL, if the env was modified between the master or the worker
there is a big problem on the system, so no need to check the converted value.

> If anything needs to be changed let me know.
> 
> Regards,
> 
> PiBa-NL / Pieter
> 
> 

> From 486d7c759af03f9193ae3e38005d8325ab069b37 Mon Sep 17 00:00:00 2001
> From: PiBa-NL <pba_...@yahoo.com>
> Date: Tue, 28 Nov 2017 23:22:14 +0100
> Subject: [PATCH] [PATCH] BUG/MINOR: Check if master-worker pipe getenv
>  succeeded, also allow pipe fd 0 as valid.
> 
> On FreeBSD in quiet mode the stdin/stdout/stderr are closed which lets the 
> mworker_pipe to use fd 0 and fd 1.
> ---
>  src/haproxy.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/haproxy.c b/src/haproxy.c
> index 891a021..c3c8281 100644
> --- a/src/haproxy.c
> +++ b/src/haproxy.c
> @@ -2688,9 +2688,15 @@ int main(int argc, char **argv)
>   free(msg);
>   }
>   } else {
> - mworker_pipe[0] = 
> atol(getenv("HAPROXY_MWORKER_PIPE_RD"));
> - mworker_pipe[1] = 
> atol(getenv("HAPROXY_MWORKER_PIPE_WR"));
> - if (mworker_pipe[0] <= 0 || mworker_pipe[1] <= 
> 0) {
> + mworker_pipe[0] = -1;
> + mworker_pipe[1] = -1;

We don't need to init to -1;


> + char* rd = getenv("HAPROXY_MWORKER_PIPE_RD");
> + char* wr = getenv("HAPROXY_MWORKER_PIPE_WR");

In my opinion we can simplify:

> + if (rd && wr && strlen(rd) > 0 && strlen(wr) > 
> 0) {
> + mworker_pipe[0] = atol(rd);
> + mworker_pipe[1] = atol(wr);
> + }
> + if (mworker_pipe[0] < 0 || mworker_pipe[1] < 0) 
> {
>   ha_warning("[%s.main()] Cannot get 
> master pipe FDs.\n", argv[0]);
>   }

By doing this, which is more secure, and assure us that it won't start with 
this kind of problem:

if (!rd || !wd) {
ha_alert("[%s.main()] Cannot get master 
pipe FDs.\n", argv[0]);
exit(1);
}
mworker_pipe[0] = atoi(rd);
mworker_pipe[1] = atoi(wr);


And we can do the same thing with the pipe return value:

ret = pipe(mworker_pipe);
if (ret < 0) {
ha_alert("[%s.main()] Cannot create 
master pipe.\n", argv[0]);
exit(1);
} 

This code will guarantee that the whole master-worker quit if there is a 
problem.

-- 
William Lallemand

Re: [BUG] haproxy 1.8-last/master-worker/peers

[William Lallemand]

On Tue, Nov 28, 2017 at 02:56:55PM +0100, William Lallemand wrote:
> On Tue, Nov 28, 2017 at 12:22:04PM +0100, Emmanuel Hocdet wrote:
> > ok, i should have something strange because it’s easy to reproduce in my 
> > environnement.
> > 
> > When i look lsof i see on master:
> > haproxy 21355 root4u  IPv4 39007164  0t0  TCP 10.101.20.4:943 
> > (LISTEN)
> > it’s link to self ip configuration "peer L6_2 10.101.20.4:943"
> > 
> > Master listen on peer? really?
> > on each reload (kill -USR2)  on more LISTEN appears
> > 
> 
> I can reproduce easily the problem, the peers listener is not closed since I 
> removed
> the deinit from the master-worker code, however that does not explain why the 
> old
> worker does not quit.
> 

Apparently this is reproducible without the master-worker, upon a reload with a 
local peer,
the previous process doesn't leave.

-- 
William Lallemand

Re: [BUG] haproxy 1.8-last/master-worker/peers

[William Lallemand]

On Tue, Nov 28, 2017 at 12:22:04PM +0100, Emmanuel Hocdet wrote:
> ok, i should have something strange because it’s easy to reproduce in my 
> environnement.
> 
> When i look lsof i see on master:
> haproxy 21355 root4u  IPv4 39007164  0t0  TCP 10.101.20.4:943 
> (LISTEN)
> it’s link to self ip configuration "peer L6_2 10.101.20.4:943"
> 
> Master listen on peer? really?
> on each reload (kill -USR2)  on more LISTEN appears
> 

I can reproduce easily the problem, the peers listener is not closed since I 
removed
the deinit from the master-worker code, however that does not explain why the 
old
worker does not quit.

-- 
William Lallemand

Re: cache issue : some data skipped in the middle of the content

[William Lallemand]

On Tue, Nov 28, 2017 at 10:56:56AM +0100, Cyril Bonté wrote:
> Hi all,
> 
> Late this night, I had an issue I didn't met during my previous tests, 
> related to the cache feature : sometimes, I had some corrupted images in the 
> browser.
> After adding some debug messages, it seems there's an issue in 
> cache_store_http_forward_data() when there is not enough contiguous space 
> available.
> For example, with an image of 6532 bytes :
> - first, the headers are captured = 287 bytes
> - then, it tries to fill the cache but can only read 2609 bytes, skipping the 
> first 287 bytes to ignore headers
> - cache_store_http_forward_data() has to be called a second time, that's 
> where I think the issue occurs : it will bypass 287 bytes of headers again, 
> while it shouldn't => only the last 3636 bytes will be read instead of the 
> remaining 3923 ones :
> [code]
>   /* Skip remaining headers to fill the cache */
>   b_adv(msg->chn->buf, st->hdrs_len);
>   ret = shctx_row_data_append(shctx,
>   st->first_block,
>   (unsigned char 
> *)bi_ptr(msg->chn->buf),
>   
> MIN(bi_contig_data(msg->chn->buf), len - st->hdrs_len));
>   /* Rewind the buffer to forward all data */
>   b_rew(msg->chn->buf, st->hdrs_len);
> [/code]
> 
> I won't be able to make more tests before this evening, but I hope there is 
> enough details ;-)
> 
> Also, I've not checked yet but I was wondering : is it specific to the cache 
> filter or can we find the same issue in other ones (compression for example) ?
> 
> Cyril

You're absolutely right, we spot this one before but it seems the patch didn't
make it to the master.

This should be specific to the cache, we don't rewind and store the headers in 
the other filters.

Patch attached.

-- 
William Lallemand
>From 82f39e7d1b408345aba5804608ee9ea4c4933d5a Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallem...@haproxy.com>
Date: Tue, 28 Nov 2017 11:33:02 +0100
Subject: [PATCH] BUG/MEDIUM: cache: bad computation of the remaining size
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The cache was not setting the hdrs_len to zero when we are called
in the http_forward_data with headers + body.

The consequence is to always try to store a size - the size of headers,
during the calls to http_forward_data even when it has already forwarded
the headers.

Thanks to Cyril Bonté for reporting this bug.

Must be backported to 1.8.
---
 src/cache.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/cache.c b/src/cache.c
index 06d7ce881..cdfd43d57 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -228,6 +228,7 @@ cache_store_http_forward_data(struct stream *s, struct filter *filter,
 			MIN(bi_contig_data(msg->chn->buf), len - st->hdrs_len));
 /* Rewind the buffer to forward all data */
 b_rew(msg->chn->buf, st->hdrs_len);
+st->hdrs_len = 0;
 if (ret)
 	goto disable_cache;
 			}
-- 
2.13.6

Re: [docs] about master-worker mode

[William Lallemand]

Hi,

On Mon, Nov 27, 2017 at 06:58:06AM -0200, Joao Morais wrote:
> 
> Hi, from nbproc doc[1]: "This requires the 'daemon' mode”, but this is also 
> the way to start more than one worker on master-worker mode, right?
>

You can have several workers with the master-worker in foreground mode, the doc
need to be updated there!
 
> Still on the same doc: "USING MULTIPLE PROCESSES IS HARDER TO DEBUG AND IS 
> REALLY DISCOURAGED”, is this still valid on master-worker? Both "harder to 
> debug" and "is really discouraged" parts.

It depends of your use case, HAProxy doesn't share stick-table, statistics and
can't do peers with nbproc, however you can do that with nbthread!


> From the blog post[2]: "Start the haproxy process with the command line 
> argument -W <# of processes>" - is this <# of procs> correct? Couldn’t use 
> this on the command line.
> 

That's an error, -W doesn't take a process argument, it's only a flag. The
master-worker only simplify the reload and the supervision of HAProxy.

> ~jm
> 
> [1] http://cbonte.github.io/haproxy-dconv/1.8/configuration.html#3.1-nbproc
> [2] https://www.haproxy.com/blog/whats-new-haproxy-1-8/
> 
> 

Thanks for the reports.

-- 
William Lallemand

Re: [PATCH v3 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[William Lallemand]

On Tue, Nov 21, 2017 at 11:28:55AM +0100, Willy Tarreau wrote:
> On Tue, Nov 21, 2017 at 11:19:41AM +0100, William Lallemand wrote:
> > I don't like the idea of the "daemon" keyword being ignored,
> 
> We already do that with -D which ignores "debug" for example. The command
> line purposely has precedence over the config.
> 
> > however, we could
> > exit with an error when we try to start with -Ws + -D or daemon.
> 
> Errors only cause gray hair to users, especially when they concern stuff
> that can be automatically recovered from. We could definitely consider
> that -Ws automatically implies MODE_FOREGROUND which will do exactly
> what -db does, but without having to think about it. After all, "-Ws"
> is "master-worker mode with systemd awareness" so it makes sense not
> to background in this case.
> 

That's right, I agree.

-- 
William Lallemand

Re: [PATCH v3 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[William Lallemand]

On Tue, Nov 21, 2017 at 11:12:45AM +0100, Lukas Tribus wrote:
> Hello,
> 
> 2017-11-21 8:39 GMT+01:00 William Lallemand <wlallem...@haproxy.com>:
> 
> That's not it, the hold-off timer is only a consequence of this
> problem. I believe the notification does not work in my case, which is
> why for systemd haproxy did not start correctly which is why systemd
> continues to restart haproxy.
> I found the root of the issue: the "daemon" keyword in the global
> configuration section. We need to remove "daemon" from the
> configuration for systemd to mode work correctly in notify mode.
> 
> We can override the "daemon" keyword in the configuration by passing
> -db to haproxy. Adding it to the unit file fixes the issue even if
> they keyword is in the configuration (so we pass "-Ws -db" ).
> 
> "-Ws -db" along with a note in the documentation that "daemon" may be
> ignored in systemd mode seems like a good compromise here? I will send
> an RFC patch shortly.
> 

You should never use systemd with the daemon mode, either in notify or default
mode, it's not the recommended setup.

I don't like the idea of the "daemon" keyword being ignored, however, we could
exit with an error when we try to start with -Ws + -D or daemon.

-- 
William Lallemand

Re: [PATCH v3 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[William Lallemand]

On Tue, Nov 21, 2017 at 07:16:19AM +0100, Willy Tarreau wrote:
> 
> I really don't like this. My fears with becoming more systemd-friendly
> was that we'd make users helpless when something decides not to work
> just to annoy them, and this reports seems to confirm this feeling :-/
> 
> Tim, have you seen this message about a "hold-off timer over" being
> displayed at the same second as the startup message ? Isn't there a
> timeout setting or something like this to place in the config file ?
> And is there a way to disable it so that people with huge configs
> are still allowed to load them ?
> 

There is indeed a timeout value, which is configurable in the unit file, the
default value is 100ms.

https://www.freedesktop.org/software/systemd/man/systemd.service.html#RestartSec=
 

There is also the start one there:

https://www.freedesktop.org/software/systemd/man/systemd.service.html#TimeoutStartSec=

I suggest we configure it to a greater value per default, it can be set to 
infinity too, but I don't like the idea.

-- 
William Lallemand

Re: haproxy-1.8-rc4 - FreeBSD 11.1 - master-worker daemon parent staying alive/process-owner

[William Lallemand]

Hi,

On Tue, Nov 21, 2017 at 02:13:24AM +0100, PiBa-NL wrote:
> Hi List,
> 
> I've got a startup script that essentially looks like the one below #1# 
> (simplified..)
> When configured with master-worker, the first parent process 2926 as 
> seen in #2# keeps running.

Yes, that's the expected behavior, the master-worker was designed in a way to
replace the systemd-wrapper, and the systemd way to run a daemon is to keep it
on the foreground and pipe it to systemd so it can catch the errors on the
standard ouput.

However, it was also designed for normal people who wants to daemonize,
so you can combine -W with -D which will daemonize the master.

> Doing the same without master-worker, the daemon properly detaches and 
> the parent exits returning possible warnings/errors..
> 
> When the second php exec line in #1# with "> /dev/null" is used instead 
> it does succeed.
> 
> While its running the stats page does get served by the workers..
> 
> To avoid a possible issue with polers(see my previous mail thread) ive 
> tried to add the -dk but still the first started parent process stays 
> alive..
> And if terminated with a ctrl+c it stops the other master-worker 
> processes with it.. as can be seen in #3# (was from a different attempt 
> so different processid's.).

Well, that's an expected behavior too, the master will forward the ctrl-c
signal to the workers and leave when all the workers are dead.

> 
> 'truss' output (again with different pids..): 
> https://0bin.net/paste/f2p8uRU1t2ebZjkL#iJOBdPnR8mCmRrtGGkEaqsmQXfbHmQ56vQHdseh1x8U
> 
> If desired i can gater the htop/truss/console output information from a 
> single run..
> 
> Any other info i can provide? Or should i change my script to not expect 
> any console output from haproxy? In my original script the 'exec' is 
> called with 2 extra parameters that return the console output and exit 
> status..
> p.s.
> how should configuration/startup errors be 'handled' when using 
> master-worker?

I'm not sure of getting the issue there, the errors are still displayed upon
startup like in any other haproxy mode, there is really no change here.
I assume your only problem with your script is the daemonize that you can
achieve by combining -W and -D.

> A kill -1 itself wont tell if a new configured bind cannot find the 
> interface address to bind to? and a -c before hand wont find such a problem.

Upon a reload (SIGUSR2 on the master) the master will try to parse the
configuration again and start the listeners. If it fails, the master will
reexec itself in a wait() mode, and won't kill the previous workers, the
parsing/bind error should be displayed on the standard output of the master.

> The end result that nothing is running and the error causing that 
> however should be 'caught' somehow for logging.?. should haproxy itself 
> log it to syslogs? but how will the startup script know to notify the 
> user of a failure?

Well, the master don't do syslog, because there might be no syslog in your
configuration. I think you should try the systemd way and log the standard
output.

> Would it be possible when starting haproxy with -sf  it would tell 
> if the (original?) master was successful in reloading the config / 
> starting new workers or how should this be done?

That may be badly documented but you are not supposed to use -sf with the 
master worker,
you just have to send the -USR2 signal to the master and it will parse again the
configuration, launch new workers and kill smoothly the previous ones.

Unfortunately signals are asynchronous, and we don't have a way yet to return
a bad exit code upon reload. But we might implement a synchronous
configuration notification in the future, using the admin socket for example.

> Currently a whole new set of master-worker processes seems to be take over..

Well, I supposed that's because you launched a new master-worker with -sf, it's
not supposed to be used that way but it should work too if you don't mind
having a new PID.


> Regards,
> PiBa-NL / Pieter
> 
 
Best Regards,

-- 
William Lallemand

Re: [PATCH v3 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[William Lallemand]

On Mon, Nov 20, 2017 at 03:58:35PM +0100, Tim Düsterhus wrote:
> From: Tim Duesterhus <t...@bastelstu.be>
> 
> This patch adds support for `Type=notify` to the systemd unit.
 
Looks good to me, thanks!

Willy can you merge it?

-- 
William Lallemand

Re: [PATCH v2 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[William Lallemand]

On Mon, Nov 20, 2017 at 12:55:06PM +0100, Tim Düsterhus wrote:
> William,
> 
> Am 20.11.2017 um 12:01 schrieb William Lallemand:
> >> The only difference I can see is that haproxy will fail to start for a
> >> different reason (use of undefined option, instead of not sending the
> >> READY=1 notification) if one uses the provided unit file *without*
> >> compiling with USE_SYSTEMD.
> >> Thus in my opinion a separate -Ws option will increase cognitive load 
> >> (which
> >> option should I use?) for next to no benefit.
> > 
> > 
> > Well, it's a matter of documentation, and a line to add in the usage() 
> > function
> > IMO.
> > 
> > This option ensures one thing, if the -Ws option is used in the unit file by
> > default, and if by any chance a binary was not built with the right
> > USE_SYSTEMD=1 option, it won't work at all preventing useless bug reports. 
> > We
> > can even put a Warning when trying to use this option when it's not built.
> > 
> 
> May I suggest the following: If haproxy is *not* compiled with the
> `USE_SYSTEMD` option it checks for the existence of the `NOTIFY_SOCKET`
> environment variable and refuses start up, if it is defined.
> 
> Then `Type=notify` will "just work" if haproxy is compiled with the
> option and will emit a proper error message if it is not.
> 

If you're suggesting of doing this with -W, it's not a good idea, sometimes you
just want to start HAProxy for tests or development independently of any init
system.

-- 
William Lallemand

Re: [PATCH v2 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[William Lallemand]

On Mon, Nov 20, 2017 at 11:39:53AM +0100, Tim Düsterhus wrote:
> William,
> 
> Am 20.11.2017 um 10:59 schrieb William Lallemand:
> > I think a good way to activate this feature will be to use it with a -Ws 
> > command
> > line option instead of changing the behavior of the -W one.
> > 
> > This way we can integrate the -Ws command in the unit file without changing 
> > the
> > behavior of the normal option.
> 
> Just to make sure you understood correctly: The patch does not really
> modify the behaviour of the normal option. `sd_notify(3)` is documented
> to just do nothing if the `NOTIFY_SOCKET` environment variable is not set:
> 
> > RETURN VALUE
> >On failure, these calls return a negative errno-style error code. If
> >$NOTIFY_SOCKET was not set and hence no status data could be sent, 0 
> > is
> >returned. *snip*

I hope it uses non-blocking sockets :-)

Well, in this case why not, but we will have to document properly how to
configure the systemd unit file depending of how it's built.


> The only difference I can see is that haproxy will fail to start for a
> different reason (use of undefined option, instead of not sending the
> READY=1 notification) if one uses the provided unit file *without*
> compiling with USE_SYSTEMD.
> Thus in my opinion a separate -Ws option will increase cognitive load (which
> option should I use?) for next to no benefit.


Well, it's a matter of documentation, and a line to add in the usage() function
IMO.

This option ensures one thing, if the -Ws option is used in the unit file by
default, and if by any chance a binary was not built with the right
USE_SYSTEMD=1 option, it won't work at all preventing useless bug reports. We
can even put a Warning when trying to use this option when it's not built.

-- 
William Lallemand

Re: [PATCH v2 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[William Lallemand]

Hi again,

On Sun, Nov 19, 2017 at 03:10:21AM +0100, Tim Düsterhus wrote:
> diff --git a/contrib/systemd/haproxy.service.in 
> b/contrib/systemd/haproxy.service.in
> index 81b4951df..895e3b036 100644
> --- a/contrib/systemd/haproxy.service.in
> +++ b/contrib/systemd/haproxy.service.in
> @@ -11,8 +11,9 @@ ExecStart=@SBINDIR@/haproxy -W -f $CONFIG -p $PIDFILE
>  ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q
>  ExecReload=/bin/kill -USR2 $MAINPID
>  KillMode=mixed
> +KillSignal=USR1

In my opinion this part is a problem,  it won't stop the process immediatly
but wait for session to be finished. It will cause the restart to behave like
the reload but with a PID change.



Cheers,

-- 
William Lallemand

Re: [PATCH v2 1/1] MEDIUM: mworker: Add systemd `Type=notify` support

[William Lallemand]

Hi Tim,

The change in the unit file seems legitimate, and the USE_SYSTEMD build option
is fine, however I think we can improve slighly the patch.

I think a good way to activate this feature will be to use it with a -Ws command
line option instead of changing the behavior of the -W one.

This way we can integrate the -Ws command in the unit file without changing the
behavior of the normal option.

Cheers,

-- 
William Lallemand

Re: [PATCH] BUG/MEDIUM: mworker: Fix re-exec when haproxy is started from PATH

[William Lallemand]

On Thu, Nov 16, 2017 at 05:30:05PM +0100, Tim Düsterhus wrote:
> William,
> 
> Am 15.11.2017 um 21:17 schrieb William Lallemand:
> > These problems have been fixed in the master with the following commits:
> > 
> > 75ea0a06b BUG/MEDIUM: mworker: does not close inherited FD
> > fade49d8f BUG/MEDIUM: mworker: does not deinit anymore
> > 2f8b31c2c BUG/MEDIUM: mworker: wait again for signals when execvp fail
> > 722d4ca0d MINOR: mworker: display an accurate error when the reexec fail
> > 
> > 
> > The master worker should be able to behave correctly on a execvp failure 
> > now :-) 
> > 
> 
> I took a look at your commits. While I don't know much about haproxy's
> internals they look good to me.
> 
> Just one thing: At the top of `static void mworker_reload(void)` the
> Environment is modified using:
> 
> > setenv("HAPROXY_MWORKER_REEXEC", "1", 1);
> 
> Is it necessary to reset that value in case of `execvp` failure? You
> don't seem to do so.
> 

It's not, this variable is only used at the start of the executable to know if
it's a restart or not, once it's started it should always be 1.

Cheers,

-- 
William Lallemand

Re: [PATCH] BUG/MEDIUM: mworker: Fix re-exec when haproxy is started from PATH

[William Lallemand]

Hi Tim,

On Tue, Nov 14, 2017 at 08:20:25PM +0100, Tim Düsterhus wrote:
> William,
> 
> Am 14.11.2017 um 15:03 schrieb William Lallemand:
> > Well, in my opinion the return value does not need to be checked because we
> > should never reach the code after the exec, that means that the code after 
> > the
> > exec is only reached during an exec error. But we could at least display the
> > error code of exec and return afterwards.
> 
> Displaying the error code basically would imply "checking the return
> value" for me.
> I did not mean to say that some recovery should be performed if `exec`
> fails. I am only saying that the error message should not be
> misleadingly claim that we "Cannot allocate memory" when e.g. the
> haproxy binary was deleted instead.
> 
> Personally I was confused whether I misconfigured the Docker container
> with a broken memory limit, when the actual issue was the bug this patch
> fixed. While *this* bug has been fixed `execvp` could possibly fail for
> other reasons in the future.
> 

These problems have been fixed in the master with the following commits:

75ea0a06b BUG/MEDIUM: mworker: does not close inherited FD
fade49d8f BUG/MEDIUM: mworker: does not deinit anymore
2f8b31c2c BUG/MEDIUM: mworker: wait again for signals when execvp fail
722d4ca0d MINOR: mworker: display an accurate error when the reexec fail


The master worker should be able to behave correctly on a execvp failure now 
:-) 

Thanks for the report.

Cheers,

-- 
William Lallemand

Re: [PATCH] BUG/MEDIUM: mworker: Fix re-exec when haproxy is started from PATH

[William Lallemand]

Hi Tim,

On Sun, Nov 12, 2017 at 05:39:18PM +0100, Tim Düsterhus wrote:
> From: Tim Duesterhus <t...@bastelstu.be>
> 
> If haproxy is started using the name of the binary only (i.e.
> not using a relative or absolute path) the `execv` in
> `mworker_reload` fails with `ENOENT`, because it does not
> examine the `PATH`:
> 
>   [WARNING] 315/161139 (7) : Reexecuting Master process
>   [WARNING] 315/161139 (7) : Cannot allocate memory
>   [WARNING] 315/161139 (7) : Failed to reexecute the master processs [7]
> 
> The error messages are misleading, because the return value of
> `execv` is not checked. This should be fixed in a separate commit.
>
> Once this happened the master process ignores any further
> signals sent by the administrator.
> 

Well, in my opinion the return value does not need to be checked because we
should never reach the code after the exec, that means that the code after the
exec is only reached during an exec error. But we could at least display the
error code of exec and return afterwards.

However, in the case of an exec failure, the master-worker should always work,
and so it should be able to receive signals.

I spot two other problems in this code path, I'll fix them shortly.

> Replace `execv` with `execvp` to establish the expected
> behaviour.

Willy, could you please apply this patch? thanks 

-- 
William Lallemand

[PATCH] MINOR: mworker: do not store child pid anymore in the pidfile

[William Lallemand]

The parent process supervises itself the children, we don't need to
store the children pids anymore in the pidfile in master-worker mode.
---
 src/haproxy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index bcbbad4a1..4d4bd3b26 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2649,7 +2649,7 @@ int main(int argc, char **argv)
else if (ret == 0) /* child breaks here */
break;
children[proc] = ret;
-   if (pidfd >= 0) {
+   if (pidfd >= 0 && !(global.mode & MODE_MWORKER)) {
char pidstr[100];
snprintf(pidstr, sizeof(pidstr), "%d\n", ret);
shut_your_big_mouth_gcc(write(pidfd, pidstr, 
strlen(pidstr)));
-- 
2.13.6

[PATCH 2/3] MINOR: mworker: allow pidfile in mworker + foreground

[William Lallemand]

This patch allows the use of the pidfile in master-worker mode without
using the background option.
---
 src/haproxy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index bbd26b82d..f12e903b2 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2499,7 +2499,7 @@ int main(int argc, char **argv)
}
 
/* open log & pid files before the chroot */
-   if (global.mode & MODE_DAEMON && global.pidfile != NULL) {
+   if ((global.mode & MODE_DAEMON || global.mode & MODE_MWORKER) && 
global.pidfile != NULL) {
unlink(global.pidfile);
pidfd = open(global.pidfile, O_CREAT | O_WRONLY | O_TRUNC, 
0644);
if (pidfd < 0) {
-- 
2.13.6

some mworker + pidfile patches

[William Lallemand]

A few patches which help using the pidfile in master-worker mode.

[PATCH 3/3] MINOR: mworker: write parent pid in the pidfile

[William Lallemand]

The first pid in the pidfile is now the parent, it's more convenient for
supervising the processus.

You can now reload haproxy in master-worker mode with convenient command
like: kill -USR2 $(head -1 /tmp/haproxy.pid)
---
 src/haproxy.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/haproxy.c b/src/haproxy.c
index f12e903b2..bcbbad4a1 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2631,6 +2631,13 @@ int main(int argc, char **argv)
}
}
 
+   /* if in master-worker mode, write the PID of the father */
+   if (global.mode & MODE_MWORKER) {
+   char pidstr[100];
+   snprintf(pidstr, sizeof(pidstr), "%d\n", getpid());
+   shut_your_big_mouth_gcc(write(pidfd, pidstr, 
strlen(pidstr)));
+   }
+
/* the father launches the required number of processes */
for (proc = 0; proc < global.nbproc; proc++) {
ret = fork();
-- 
2.13.6

Re: [ANNOUNCE] haproxy-1.8-rc1 : the last mile

[William Lallemand]

Hi Lukas,

On Wed, Nov 01, 2017 at 09:02:53PM +0100, Willy Tarreau wrote:
> Hi Lukas,
> 
> On Wed, Nov 01, 2017 at 08:43:19PM +0100, Lukas Tribus wrote:
> > Just upgrading the binary from -dev3 to -rc1 however broke my setup:
> > Turns out that the new object caching code breaks when another filter
> > (compression) is already enabled (at config parsing stage) - even when
> > object caching is not enabled itself:
> > 
> (...)
> > 
> > lukas@dev:~/haproxy$ ./haproxy -f ../haproxy.cfg
> > [ALERT] 304/203750 (6995) : Proxy 'http_in': unable to find the cache
> > '(null)' referenced by the filter 'cache'.
> > [ALERT] 304/203750 (6995) : Proxy 'bk_testbk': unable to find the
> > cache '(null)' referenced by the filter 'cache'.
> > [ALERT] 304/203750 (6995) : Fatal errors found in configuration.
> > lukas@dev:~/haproxy$
> > 
> > Now I'm going to disable compression and try the fun stuff :)
> 

That's a bug of the post parsing callback, it tries to use the cache with a
filter which is not a cache. I just fix it in the master. 


> Thanks for reporting, such type of early breakage is indeed expected
> after I stressed everyone to merge. We know that the inned parts work
> pretty well overall but some integration work is now needed.
> 
> You may have to explicitly use the compression filter by the way,
> though I have no idea how to do that but I think it's mentionned
> somewhere in the config manual. William was about to write some doc
> when I interrupted him to get his code, but he'll certainly get back
> to this soon.

It will need a configuration filter keyword for the cache, to define the
explicit order of the filters. 

The cache might not work after the compression in the current state of the
filter API.

-- 
William Lallemand

Re: [PATCH] Reset a few more counters on "clear counters"

[William Lallemand]

On Wed, Oct 18, 2017 at 07:06:19PM +0200, Lukas Tribus wrote:
> Hello!
> 
> 
> 2017-10-18 18:36 GMT+02:00 Willy Tarreau <w...@1wt.eu>:
> 
> > On Wed, Oct 18, 2017 at 04:29:19PM +0200, Olivier Houchard wrote:
> > > A few counters (namely, MaxSslRate, SslFrontendMaxKeyRate, and
> > > SslBackendMaxKeyRate) are not cleared as I think they should, when clear
> > > counters is used.
> > > The attached patch addresses that.
> >
> > Applied, thanks!
> > Willy
> >
> 
> Can we backport this one? There is at least one report for this in 1.7.
> 
> 
> 
> Thanks,
> Lukas

Sure, it will be backported.

-- 
William Lallemand

Re: Haproxy refuses new connections when doing a reload followed by a restart

[William Lallemand]

On Thu, Oct 12, 2017 at 05:50:52PM +0300, Apollon Oikonomopoulos wrote:
> > 
> > One helpful feature I read in the documentation is the usage of the
> > sd_notify(..  "READY=1").  It can be useful for configuration files that 
> > takes
> > time to process, for example those with a lot of ssl frontends. This signal
> > could be send once the children has been forked.
> > 
> > It's difficult to know when a reload is completely finished (old processes
> > killed) in case of long TCP sessions. So, if we use this system there is a 
> > risk
> > to trigger a timeout in systemd on the reload isn't it?
> 
> The Reload timeout is apparently controlled by TimeoutStartSec in 
> systemd.
> 
> > feature for the reload, it should be done after the fork of the new 
> > processes,
> > not after the leaving of the old processes, because the processes are 
> > ready to
> > receive traffic at this stage.
> 
> That's true. OTOH the problem with haproxy-systemd-wrapper is that once 
> it re-exec's itself it loses track of the old processes completely 
> (IIRC),

That's right, but we won't fix it in the wrapper, the current architecture
doesn't allow it easily, and it's not reasonable to backport the master-worker
in a stable branch. Those problems will be fixed with the master-worker in 1.8.

> combined with the fact that old processes may eat up a lot of 
> memory. There are cases where you would prefer breaking a long TCP 
> session after 30s if it would give you back 2GB of RSS, to having the 
> process lying around just for one client.

Sure, that can be done in the haproxy config file with the hard-stop-after 
keyword.

> > Are there really advantages to letting know systemd when a reload is 
> > finished
> > or when a process is ready?
> 
> Yes, there are. systemd will only perform a single operation on a unit 
> at a time, and will queue up the rest. When you inform systemd that 
> something (startup/reload) is in progress, it will not let any other 
> action happen until the first operation is finished. Now it's trivial to 
> issue a ton of reloads in a row that will leave a ton of old processes 
> lying around until they terminate.
 
I don't think you can, either with the master-worker or the wrapper, it was one
of the problems we had in the past.

The master-worker waits to be ready to handle the signals, and the wrapper waits
for a pipe to be closed on the children side to handle signals.

> The other advantage with Type=notify services is that systemd will wait 
> for READY=1 before starting units with After=haproxy (although HAProxy 
> is really a "leaf" kind of service).
> 

That's interesting, nobody ever ask for that, but I see a few cases where that
can be useful.

> Note that the dependency is really thin, and you can always make it a 
> compile-time option. 
> 
> Regards,
> Apollon

Cheers,

-- 
William Lallemand

Re: Haproxy refuses new connections when doing a reload followed by a restart

[William Lallemand]

Hi,

On Thu, Oct 12, 2017 at 01:19:58PM +0300, Apollon Oikonomopoulos wrote:
> The biggest issue here is that we are using a signal to trigger the 
> reload (which is a complex, non-atomic operation) and let things settle 
> on their own. Systemd assumes that as soon as the signal is delivered 
> (i.e.  the ExecReload command is done), the reload is finished, while in 
> our case the reload is finished when the old haproxy process is really 
> dead. Using a signal to trigger the reload is handy, so we could keep 
> that, but the wrapper would need some changes to make reloads more 
> robust:
> 
>  1. It should use sd_notify(3) to communicate the start/stop/reload 
> status to systemd (that would also mean converting the actual 
> service to Type=notify). This way no other operation will be 
> performed on the unit until the reload is finished and the process 
> group is in a known-good state.
> 
>  2. It should handle the old process better: apart from relying on the 
> new haproxy process for killing the old one, it should explicitly 
> SIGKILL it after a given timeout if it's not dead yet and make sure 
> reloads are timeboxed.
> 
> IIUC, in 1.8 the wrapper has been replaced by the master process which 
> seems to do point 2 above, but point 1 is something that should still be 
> handled IMHO.

One helpful feature I read in the documentation is the usage of the
sd_notify(..  "READY=1").  It can be useful for configuration files that takes
time to process, for example those with a lot of ssl frontends. This signal
could be send once the children has been forked.

It's difficult to know when a reload is completely finished (old processes
killed) in case of long TCP sessions. So, if we use this system there is a risk
to trigger a timeout in systemd on the reload isn't it? If we want to use this
feature for the reload, it should be done after the fork of the new processes,
not after the leaving of the old processes, because the processes are ready to
receive traffic at this stage.

However I'm not sure it's that useful, you can know when a process is ready
using the logs, and it will add specific code for systemd and a dependency.

Are there really advantages to letting know systemd when a reload is finished
or when a process is ready?

Cheers,

-- 
William Lallemand

Re: Haproxy refuses new connections when doing a reload followed by a restart

[William Lallemand]

On Fri, Oct 06, 2017 at 05:04:18PM +0200, Moemen MHEDHBI wrote:
> Hi Lukas,
> 
> 
> On 04/10/2017 22:01, Lukas Tribus wrote:
> > I guess the problem is that when a reload happens before a restart and 
> > pre-reload
> > systemd-wrapper process is still alive, systemd gets confused by that old 
> > process
> > and therefor, refrains from starting up the new instance.
> >
> > Or systemd doesn't get confused, sends SIGTERM to the old systemd-wrapper
> > process as well, but the wrapper doesn't handle SIGTERM after a SIGUSR1
> > (a hard stop WHILE we are already gracefully stopping).
> >
> >
> > Should the systemd-wrapper exit after distributing the graceful stop 
> > message to
> > processes? I don't think so, it sounds horribly.
> >
> > Should the systemd-wrapper expect a SIGTERM after a SIGUSR1 and sends the
> > TERM/INT to its childs? I think so, but I'm not 100% sure. Is that even the 
> > issue?
> >
> >
> >
> > We did get rid of the systemd-wrapper in haproxy 1.8-dev, and replaced it 
> > with a
> > master->worker solution, so I'd say there is a chance that this doesn't 
> > affect 1.8.
> >
> 
> A. It appears to me that it is not the wrapper that receives the SIGUSR1
> but the haproxy process.
> 
> B. Here is how I technically explain the "bug" (to be confirmed by the
> Devs) reported by Niels:
>  - During the reload:
>   1. A SIGUSR2 is sent to the systemd-wrapper
>   2. The wrapper sends SIGUSR1 to haproxy processes listed in the pid file.
>   3. A new haproxy process is listening for incoming connections and the
> pid file now contains only the pid of the new process.
> - Then when issuing a restart/stop:
>  1. A SIGTERM is sent to the systemd-wrapper
>  2. The wrapper sends SIGTERM to haproxy processes listed in the pid file.
>  3. Only the new haproxy process is stopped the other one is still there
> since it did not receive the SIGTERM
> - This why systemd is getting confused and after the timeout systemd
> gets done with this by sending a SIGTERM to all child process
> (killmode=mixed policy)
> 

During a reload the wrapper receive a SIGURS2 or a SIGHUP which causes it to
reexec itself without changing its PID, read the pid file and fork kind of a
master process with -sf.  This new master process will send the SIGUSR1 to the
previous processes, fork the new children and write their PID in the pid file.

During a restart, it's more simple, the wrapper will receive a SIGTERM or a
SIGINT, the wrapper will read the PID file, and forward the signal to those
processes. Once the processes are killed, the master will leave and the wrapper
too.


> C. I was able to verify this by doing the following:
>  1. After the reload I manually add the old process pid to the pidfile
>  2. Then When I hit restart, all process are stopped correctly.
> 
> So the question is ( @William ): when doing a soft stop should we
> preserve old process pid in the pidfile until the process terminates ?
> 

Unfortunately that's one of the problem of the current wrapper system, it's
more a hack than a real process supervisor. The wrapper does not handle the
PID, it only forwards the signals and read the pid file.

The problem with letting old pid in the pidfile, is that you don't know if it's 
still an haproxy process, so, if you ask for a restart, it will eventualy kill
something which has been forked between the reload and the restart.
And the list will grow indefinitely with each reload/restart.


The master-worker model should fix that kind of issue, because it's aware of
all PIDs, old and new.

You could try:

* To change the KillMode to the default, which should kill -SIGTERM all 
processes
on a stop or restart. But if I remember well, it leads to a bad exit code on
the systemd side and display an error.

* To reduce the timeout of the SIGTERM with TimeoutStopSec= in your unit file

-- 
William Lallemand

Re: graceful shutdown maximum time

[William Lallemand]

On Fri, Aug 18, 2017 at 02:26:29PM -0700, Justin Karneges wrote:
> Hi,
> 
> When HAProxy is shut down gracefully, my understanding is that it waits
> for all open connections to be closed before it will terminate. However,
> if the connections don't ever close then HAProxy may never shut down (or
> perhaps it takes a very long time, I'm not sure). This is mainly a
> problem with long-lived connections that remain continuously active, so
> timeouts won't trigger either.
> 
> Is there a way to configure HAProxy to have a maximum graceful shutdown
> time? For example it would stop listening for new connections
> immediately, and then after a specified amount of time it would close
> all connections and terminate, regardless of what kind of activity might
> be going on within those connections.
> 
> Otherwise my plan is to create a cron job that kills old haproxy
> processes on a delay. Would be nice if there were a better option
> though.
> 
> Thanks for any suggestion.
> 
> Justin
> 

The "hard-stop-after" keyword do what you want:

https://cbonte.github.io/haproxy-dconv/1.7/configuration.html#hard-stop-after


-- 
William Lallemand

[ANNOUNCE] haproxy-1.7.9

[William Lallemand]

Hi,

HAProxy 1.7.9 was released on 2017/08/18. It added 19 new commits
after version 1.7.8.

Among the fixes, there is the resolution of the wrong termination state which
was introduced in 1.7.5 and a lot of fixes for lua. 

Please find the usual URLs below :
   Site index   : http://www.haproxy.org/
   Discourse: http://discourse.haproxy.org/
   Sources  : http://www.haproxy.org/download/1.7/src/
   Git repository   : http://git.haproxy.org/git/haproxy-1.7.git/
   Git Web browsing : http://git.haproxy.org/?p=haproxy-1.7.git
   Changelog: http://www.haproxy.org/download/1.7/src/CHANGELOG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/


---
Complete changelog :
Adis Nezirovic (1):
  BUG/MINOR: lua: Fix bitwise logic for hlua_server_check_* functions.

Christopher Faulet (5):
  BUG/MINOR: http: Set the response error state in http_sync_res_state
  MINOR: http: Reorder/rewrite checks in http_resync_states
  MINOR: http: Switch requests/responses in TUNNEL mode only by checking 
txn flags
  BUG/MEDIUM: http: Switch HTTP responses in TUNNEL mode when body length 
is undefined
  BUG/MAJOR: http: Fix possible infinity loop in http_sync_(req|res)_state

Frédéric Lécaille (1):
  BUG/MINOR: peers: peer synchronization issue (with several peers 
sections).

Nenad Merdanovic (2):
  BUG/MINOR: lua: Fix Server.get_addr() port values
  BUG/MINOR: lua: Correctly use INET6_ADDRSTRLEN in Server.get_addr()

Thierry FOURNIER (4):
  BUG/MINOR: lua: In error case, the safe mode is not removed
  BUG/MINOR: lua: executes the function destroying the Lua session in safe 
mode
  BUG/MAJOR: lua/socket: resources not detroyed when the socket is aborted
  BUG/MEDIUM: lua: bad memory access

Willy Tarreau (5):
  DOC: update CONTRIBUTING regarding optional parts and message format
  DOC: update the list of OpenSSL versions in the README
  MINOR: tools: add a portable timegm() alternative
  BUILD: lua: replace timegm() with my_timegm() to fix build on Solaris 10
  BUG/MINOR: lua: always detach the tcp/http tasks before freeing them

ben51degrees (1):
  DOC: Updated 51Degrees git URL to point to a stable version.

-- 
William Lallemand

Re: server ports - server state file

[William Lallemand]

On Thu, Aug 03, 2017 at 02:32:55PM +0200, Willy Tarreau wrote:
> On Tue, Aug 01, 2017 at 09:00:20AM +0200, Frederic Lecaille wrote:
> > Hello Haproxy ML,
> > 
> > Here is a simple patch to add server ports to server state file.
> 
> I missed this one, but it's now merged. thanks Fred!
> Willy
> 

Thanks Fred, that will be really useful!

-- 
William Lallemand

[ANNOUNCE] haproxy-1.7.8

[William Lallemand]

Hi,

HAProxy 1.7.8 was released on 2017/07/07. It added 10 new commits after version
1.7.7. It fixes some major issues, a memory leak in the compression code, a
segfault when you dump a map on the CLI while trying to remove an entry and a
bug introduced by a fix in 1.7.5 that causes haproxy to ignore "timeout
http-keep-alive".

Users of 1.7 are encouraged to upgrade.

Please find the usual URLs below :
   Site index   : http://www.haproxy.org/
   Discourse: http://discourse.haproxy.org/
   Sources  : http://www.haproxy.org/download/1.7/src/
   Git repository   : http://git.haproxy.org/git/haproxy-1.7.git/
   Git Web browsing : http://git.haproxy.org/?p=haproxy-1.7.git
   Changelog: http://www.haproxy.org/download/1.7/src/CHANGELOG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/


---
Complete changelog :
Christopher Faulet (4):
  BUG/MAJOR: compression: Be sure to release the compression state in all 
cases
  BUG/MINOR: stream: Don't forget to remove CF_WAKE_ONCE flag on response 
channel
  BUG/MINOR: http: Don't reset the transaction if there are still data to 
send
  BUG/MEDIUM: filters: Be sure to call flt_end_analyze for both channels

Emeric Brun (4):
  BUG/MINOR: stream: flag TASK_WOKEN_RES not set if task in runqueue
  BUG/MAJOR: cli: fix custom io_release was crushed by NULL.
  BUG/MAJOR: map: fix segfault during 'show map/acl' on cli.
  BUG/MEDIUM: map/acl: fix unwanted flags inheritance.

Jarno Huuskonen (1):
  DOC: fix references to the section about time format.

Willy Tarreau (1):
  BUG/MINOR: http: properly handle all 1xx informational responses

-- 
William Lallemand

Re: LoadBalance whole subnet

[William Lallemand]

On Wed, Jun 21, 2017 at 08:05:20AM +0200, Aleksandar Lazic wrote:
> > Hi Aleksandar,
> 
> > Don't worry that's a mistake, Sarunas put cont...@haproxy.com in copy to his
> > mail which lead to this.
> 
> > Please don't continue this thread on the mailing list, thanks.
> 
> 
> Well, I assume I understand you.
> 

To clarify, I think I wasn't clear enough. I meant it's not necessary to
continue the thread with Anamarija and contact@ on the mailing list :-)

-- 
William Lallemand

Re: 1.7.6 redirect regression (commit 73d071ecc84e0f26ebe1b9576fffc1ed0357ef32)

[William Lallemand]

On Wed, Jun 21, 2017 at 12:30:47PM +0300, Jarno Huuskonen wrote:
> Hi Christopher,
> 
> On Wed, Jun 21, Christopher Faulet wrote:
> > This bug was fixed in 1.8 (see commit
> > 9f724edbd8d1cf595d4177c3612607f395b4380e "BUG/MEDIUM: http: Drop the
> > connection establishment when a redirect is performed"). I attached
> > the patch. Could you quickly check if it fixes your bug (it should
> > do so) ?
> > 
> > It was not backported in 1.7 because we thought it only affected the
> > 1.8. I will check with Willy.
> 
> Thanks, patch fixes the problem (with test config (I'll try to
> test with prod. config later)).
> 
> -Jarno
> 

Thanks for tests, I will backport it in the 1.7 branch.

-- 
William Lallemand

Re: master-worker and seamless reload (bug)

[William Lallemand]

On Mon, Jun 19, 2017 at 03:35:08PM +0200, Emmanuel Hocdet wrote:
> In case of upgrade from haproxy without -x option to -x, i suppose it will do 
> cleanly.
> I try to play with -x, multi-proc (add and remove), upgrade pre -x  without 
> master-worker and is painful.
> Perhaps i misunderstood (and i used multi -x). Now, i only test from legacy 
> to  -x with master-worker.
> 
> > I think that's just a matter of documentation, maybe I can write a longer
> > paragraph in the management.txt doc explaining how to upgrade for the legacy
> > mode, and what changed between this one and the master worker.
> 
> Yes, documentation must be sufficient to avoid bad usages. What about case of 
> multi -x?
> 

Well, the -x should be unique, you don't need several -x, one socket is enough
because you have access to every FD of every processes from it. I add a warning
about that.

I fixed the few issues you had in the attached patches.

Willy, could you merge them? thanks

-- 
William Lallemand
>From 9927f4a1e864638e16dcb3c61de23bcc8f256a91 Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallem...@haproxy.com>
Date: Mon, 19 Jun 2017 15:57:55 +0200
Subject: [PATCH 1/4] BUG/MEDIUM: fix segfault when no argument to -x option

This patch fixes a segfault in the command line parser.

When haproxy is launched with -x with no argument and -x is the latest
option in argv it segfaults.

Use usage() insteads of exit() on error.
---
 src/haproxy.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index d64058da..2fd387f3 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -1279,9 +1279,9 @@ static void init(int argc, char **argv)
 			else if (*flag == 'q')
 arg_mode |= MODE_QUIET;
 			else if (*flag == 'x') {
-if (argv[1][0] == '-') {
-	Alert("Unix socket path expected with the -x flag\n");
-	exit(1);
+if (argc <= 1 || argv[1][0] == '-') {
+	Alert("Unix socket path expected with the -x flag\n\n");
+	usage(progname);
 }
 old_unixsocket = argv[1];
 argv++;
-- 
2.13.0

>From 0dc2e1f497f475e541056e0769431528a6edb39c Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallem...@haproxy.com>
Date: Mon, 19 Jun 2017 16:37:19 +0200
Subject: [PATCH 2/4] MINOR: warning on multiple -x

Multiple use of the -x option is useless, emit a warning.
---
 src/haproxy.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/haproxy.c b/src/haproxy.c
index 2fd387f3..1eabb552 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -1283,7 +1283,10 @@ static void init(int argc, char **argv)
 	Alert("Unix socket path expected with the -x flag\n\n");
 	usage(progname);
 }
+if (old_unixsocket)
+	Warning("-x option already set, overwriting the value\n");
 old_unixsocket = argv[1];
+
 argv++;
 				argc--;
 			}
-- 
2.13.0

>From 17f2decfe5d667d77415d390c61f839db8470065 Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallem...@haproxy.com>
Date: Tue, 20 Jun 2017 11:20:23 +0200
Subject: [PATCH 3/4] MINOR: mworker: don't copy -x argument anymore in
 copy_argv()

Don't copy the -x argument anymore in copy_argv() since it's already
allocated in mworker_reload().

Make the copy_argv() more consistent when used with multiple arguments
to strip.

It prevents multiple -x on reload, which is not supported.
---
 src/haproxy.c | 36 +---
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index 1eabb552..cdb6066b 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -596,23 +596,12 @@ static void mworker_reload()
 	}
 	next_argv[next_argc] = NULL;
 
-	/* if -x was used, try to update the stat socket if not available anymore */
+	/* add the -x option with the stat socket */
 	if (cur_unixsocket) {
 
-		if (old_unixsocket) {
-
-			/* look for -x  */
-			for (j = 0; next_argv[j]; j++) {
-if (!strcmp(next_argv[j], "-x"))
-	next_argv[j + 1] = (char *)cur_unixsocket;
-			}
-		} else {
-			/* if -x is not specified but we know the socket, add -x with it */
-			next_argv[next_argc++] = "-x";
-			next_argv[next_argc++] = (char *)cur_unixsocket;
-			next_argv[next_argc++] = NULL;
-
-		}
+		next_argv[next_argc++] = "-x";
+		next_argv[next_argc++] = (char *)cur_unixsocket;
+		next_argv[next_argc++] = NULL;
 	}
 
 	deinit(); /* we don't want to leak FD there */
@@ -1101,7 +1090,7 @@ out:
 static char **copy_argv(int argc, char **argv)
 {
 	char **newargv;
-	int i, j;
+	int i = 0, j = 0;
 
 	newargv = calloc(argc + 2, sizeof(char *));
 	if (newargv == NULL) {
@@ -1109,19 +1098,20 @@ static char **copy_argv(int argc, char **argv)
 		return NULL;
 	}
 
-	for (i = 0, j = 0; i < argc; i++, j++) {
-		char *flag = *(argv + i) + 1;
-
-		/* -sf or -st */
-		if (*flag == 's' && (flag[1] == 'f' || flag[1] == 't')) {
-			/* list of pids to 

Re: LoadBalance whole subnet

[William Lallemand]

On Tue, Jun 20, 2017 at 12:49:32PM +0200, Aleksandar Lazic wrote:
> Hi Anamarija.
> 
> ?!
> 
> Do you plan to make the mailing list out of support?!
> 
> Best Regards
> aleks
> 

Hi Aleksandar,

Don't worry that's a mistake, Sarunas put cont...@haproxy.com in copy to his
mail which lead to this.

Please don't continue this thread on the mailing list, thanks.

-- 
William Lallemand

Re: master-worker and seamless reload (bug)

[William Lallemand]

On Mon, Jun 19, 2017 at 11:26:31AM +0200, Emmanuel Hocdet wrote:
> 
> Exactly, use case is to upgrade haproxy from a 1.6/1.7/1.8 compatibility  to 
> 1.8 with master worker.
> 

That's insteresting, I will do some tests in order to be able to do this 
properly.
 
> 
> It's much simpler than I thought (with mworker).
> Perhaps keep -x internal to mworker and not expose it to legacy mode could be 
> less
> confused for users.
> I will try my test upgrade without -x.
> 

If you want to upgrade one "legacy" process which was not a master worker, you
will still need it. 

However, if you already have a master worker process running, it will load the
new version of the binary from where it was first executed and then add the -x
itself when forking its new workers on a reload.

Some people still want to use the -x feature without the master-worker, and
that's useful in your usecase, if your socket path changed in the configuration
file.

I think that's just a matter of documentation, maybe I can write a longer
paragraph in the management.txt doc explaining how to upgrade for the legacy
mode, and what changed between this one and the master worker.

-- 
William Lallemand

Re: master-worker and seamless reload (bug)

[William Lallemand]

On Fri, Jun 16, 2017 at 05:28:51PM +0200, Emmanuel Hocdet wrote:
> Hi,
>

Hi Emmanuel,
 
> i try to play with that, but i’m a little confused with the behaviour.
> 
> In my test, i use alternatly haproxy upgrade and worker reload (via USR2)
> 
> start with upgrade:
> # /usr/sbin/haproxy -f /var/lib/haproxy/ssl/ssl.cfg -p 
> /var/run/haproxy_ssl.pid -D -W -n 131072 -L ssl_1 -x 
> /var/run/haproxy/ssl.sock1  -x /var/run/haproxy/ssl.sock2  -sf `cat 
> /var/run/haproxy_ssl.pid`
> [ALERT] 166/165607 (13616) : Current worker 13618 left with exit code 0
> [ALERT] 166/165607 (13616) : Current worker 13617 left with exit code 0
> [WARNING] 166/165607 (13616) : All workers are left. Leaving... (0)

I'm not sure of what you are trying to do, do you try to upgrade an HAProxy
which is not using the master worker mode to a master worker?

In master worker mode the reload is meant to be done only with the USR2 signal,
the binary will be reloaded upon this signal so you don't have to start another
process to upgrade the binary.

> # ps auxwww |grep ssl
> 
> root 13635  0.0  0.0  79132   776 pts/0S16:59   0:00 
> /usr/sbin/haproxy -f /var/lib/haproxy/ssl/ssl.cfg -p /var/run/haproxy_ssl.pid 
> -D -W -n 131072 -L ssl_1 -x /var/run/haproxy/ssl.sock1 -x 
> /var/run/haproxy/ssl.sock2 -sf 13617 13618
> haproxy  13636  0.0  0.0  79132   940 ?Ss   16:59   0:00 
> /usr/sbin/haproxy -f /var/lib/haproxy/ssl/ssl.cfg -p /var/run/haproxy_ssl.pid 
> -D -W -n 131072 -L ssl_1 -x /var/run/haproxy/ssl.sock1 -x 
> /var/run/haproxy/ssl.sock2 -sf 13617 13618
> haproxy  13637  0.0  0.0  79132   940 ?Ss   16:59   0:00 
> /usr/sbin/haproxy -f /var/lib/haproxy/ssl/ssl.cfg -p /var/run/haproxy_ssl.pid 
> -D -W -n 131072 -L ssl_1 -x /var/run/haproxy/ssl.sock1 -x 
> /var/run/haproxy/ssl.sock2 -sf 13617 13618
> 
> seems ok, try to reload via USR2 on master.
> first note: the pid of master is not log in /var/run (only childrens) and i 
> don’t see any option to set it in a file (to use it in a script)
> 

That's right, it was one of the probable evolution. I will probably log only
the pid of the master in future patches.

> # kill -USR2 13635
> # [WARNING] 166/165947 (13635) : Reexecuting Master process
> [WARNING] 166/170818 (13635) : Former worker 13636 left with exit code 0
> [WARNING] 166/170818 (13635) : Former worker 13637 left with exit code 0
> 

Looks like a normal behavior.

> # ps auxwww |grep ssl
> root 13635  0.0  2.1  79132 44424 pts/0S16:59   0:00 
> /usr/sbin/haproxy -f /var/lib/haproxy/ssl/ssl.cfg -p /var/run/haproxy_ssl.pid 
> -D -W -n 131072 -L ssl_1 -x /var/run/haproxy/ssl.sock2 -x 
> /var/run/haproxy/ssl.sock2 -sf 13636 13637 13617 13618
> haproxy  13652  0.0  0.0  79132  1188 ?Ss   17:08   0:00 
> /usr/sbin/haproxy -f /var/lib/haproxy/ssl/ssl.cfg -p /var/run/haproxy_ssl.pid 
> -D -W -n 131072 -L ssl_1 -x /var/run/haproxy/ssl.sock2 -x 
> /var/run/haproxy/ssl.sock2 -sf 13636 13637 13617 13618
> haproxy  13653  0.0  0.0  79132  1176 ?Ss   17:08   0:00 
> /usr/sbin/haproxy -f /var/lib/haproxy/ssl/ssl.cfg -p /var/run/haproxy_ssl.pid 
> -D -W -n 131072 -L ssl_1 -x /var/run/haproxy/ssl.sock2 -x 
> /var/run/haproxy/ssl.sock2 -sf 13636 13637 13617 13618
> root 13655  0.0  0.0  11124   696 pts/0S+   17:08   0:00 grep ssl
> 
> I now see  -x /var/run/haproxy/ssl.sock2 twice (sock1 is lost) and -sf have 4 
> pids instead of 2.
> 

Well, that's a bug, the code of the mworker doesn't manage several -x. 

However you don't need to use several unix sockets, the fd of all listeners are
exposed in any process using the "expose-fd listeners" keyword, even if the
listeners are bind on another process.

The master worker reexec the process adding a -x with what it found in the
configuration, you don't have to start it with -x at startup.

Regarding the PID, I think you have this behavior because you started the
daemon mode with -sf. 

> one last:
> # haproxy -x
> Segmentation fault

I'll fix that.

> 
> ++
> Manu
> 

Thanks for the tests!

-- 
William Lallemand

[ANNOUNCE] haproxy-1.7.6

[William Lallemand]

Hi,

HAProxy 1.7.6 was released on 2017/06/16. It added 37 new commits
after version 1.7.5.

As you may know, I'm now part of the stable release team of HAProxy along
with Willy and Cyril.

This is my first stable release which fixes a few major bugs:

- Olivier fixed a hang reported on FreeBSD. HAProxy was relying on an undefined
behavior in C to compute the timer which lead to various hangs every 49.7 days.
We now use the -fwrapv flag at compilation time to force the behavior of the
compiler. Binaries compiled with clang are more suited to be impacted by this
bug.

- Fred fixed a hang which is related to the DNS polling system. The fd of the
resolver was not unregistered but closed which lead to a hang of any new
connection using the same fd number. 

- Willy fixed a runtime segfault caused by cookies and tarpit rules.

- Fred fixed a segfault occuring upon reload when parsing a server state file
in the case one of the servers was deleted from the configuration file.

Please find the usual URLs below :
   Site index   : http://www.haproxy.org/
   Discourse: http://discourse.haproxy.org/
   Sources  : http://www.haproxy.org/download/1.7/src/
   Git repository   : http://git.haproxy.org/git/haproxy-1.7.git/
   Git Web browsing : http://git.haproxy.org/?p=haproxy-1.7.git
   Changelog: http://www.haproxy.org/download/1.7/src/CHANGELOG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/


---
Complete changelog :
Adam Spiers (1):
  DOC: stick-table is available in frontend sections

Andrew Rodland (1):
  BUG/MINOR: hash-balance-factor isn't effective in certain circumstances

Christopher Faulet (4):
  BUG/MINOR: http: Fix conditions to clean up a txn and to handle the next 
request
  BUG/MINOR: buffers: Fix bi/bo_contig_space to handle full buffers
  BUG/MINOR: acls: Set the right refflag when patterns are loaded from a map
  BUG/MINOR: http/filters: Be sure to wait if a filter loops in 
HTTP_MSG_ENDING

Frédéric Lécaille (5):
  BUG/MINOR: dns: Wrong address family used when creating IPv6 sockets.
  BUG/MINOR: server: missing default server 'resolvers' setting duplication.
  BUG/MAJOR: dns: Broken kqueue events handling (BSD systems).
  BUG/MEDIUM: peers: Peers CLOSE_WAIT issue.
  BUG/MAJOR: server: Segfault after parsing server state file.

Glenn Strauss (2):
  DOC: update sample code for PROXY protocol
  DOC: mention lighttpd 1.4.46 implements PROXY

Jarno Huuskonen (4):
  DOC: changed "block"(deprecated) examples to http-request deny
  DOC: add few comments to examples.
  DOC: add layer 4 links/cross reference to "block" keyword.
  DOC: errloc/errorloc302/errorloc303 missing status codes.

Jim Freeman (1):
  CLEANUP: logs: typo: simgle => single

Lukas Tribus (1):
  DOC: update RFC references

Nan Liu (1):
  BUG/MINOR: Makefile: fix compile error with USE_LUA=1 in ubuntu16.04

Olivier Houchard (2):
  BUG/MAJOR: Use -fwrapv.
  BUG/MINOR: server: don't use "proxy" when px is really meant.

Thierry FOURNIER (3):
  BUG/MEDIUM: lua: memory leak
  MINOR/DOC: lua: just precise one thing
  BUG/MEDIUM: lua: segfault if a converter or a sample doesn't return 
anything

Willy Tarreau (12):
  BUG/MINOR: config: missing goto out after parsing an incorrect ACL 
character
  BUG/MINOR: arg: don't try to add an argument on failed memory allocation
  BUG/MEDIUM: arg: ensure that we properly unlink unresolved arguments on 
error
  BUG/MEDIUM: acl: don't free unresolved args in prune_acl_expr()
  MINOR: lua: ensure the memory allocator is used all the time
  BUG/MEDIUM: acl: proprely release unused args in prune_acl_expr()
  MEDIUM: config: don't check config validity when there are fatal errors
  BUG/MINOR: checks: don't send proxy protocol with agent checks
  BUG/MAJOR: http: call manage_client_side_cookies() before erasing the 
buffer
  BUG/MEDIUM: unix: never unlink a unix socket from the file system
  scripts: create-release pass -n to tail
  SCRIPTS: create-release: enforce GIT_COMMITTER_{NAME|EMAIL} validity

-- 
William Lallemand

Re: exit error regression on haproxy1.8dev

[William Lallemand]

On Thu, Jun 08, 2017 at 01:49:15PM +0200, William Lallemand wrote:
>  
> You are right about the regression, however this part of the code should be
> called in daemon mode only, the regression was elsewhere :)
> 
> Fix attached.
> 

Willy, could you apply this one?

Thanks,

-- 
William Lallemand

[PATCH] BUG/MINOR: warning: ‘need_resend’ may be used uninitialized

[William Lallemand]

The commit 201c07f68 ("MAJOR/REORG: dns: DNS resolution task and
requester queues") introduces a warning during compilation:

src/dns.c: In function ‘dns_resolve_recv’:
src/dns.c:487:6: warning: ‘need_resend’ may be used uninitialized in this 
function [-Wmaybe-uninitialized]
   if (need_resend) {
  ^

This patch initialize the variable and remove the comment about it.
---
 src/dns.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/src/dns.c b/src/dns.c
index cb7de6ab..78ee62bc 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -322,7 +322,7 @@ void dns_resolve_recv(struct dgram_conn *dgram)
struct dns_query_item *query;
unsigned char buf[DNS_MAX_UDP_MESSAGE + 1];
unsigned char *bufend;
-   int fd, buflen, dns_resp, need_resend;
+   int fd, buflen, dns_resp, need_resend = 0;
unsigned short query_id;
struct eb32_node *eb;
struct lru64 *lru = NULL;
@@ -387,12 +387,6 @@ void dns_resolve_recv(struct dgram_conn *dgram)
 
dns_resp = dns_validate_dns_response(buf, bufend, resolution);
 
-   /* treat errors first
-* need_resend flag could be set to 0 by default before the 
'switch' and then
-* set to 1 only where needed, but I think it's better this way 
to make people
-* aware they have to think twice how to set this flag when 
updating this portion
-* of the code
-*/
switch (dns_resp) {
case DNS_RESP_VALID:
need_resend = 0;
-- 
2.13.0

[PATCH] BUG/MEDIUM: build without openssl broken

[William Lallemand]

The commit 872f9c213 ("MEDIUM: ssl: add basic support for OpenSSL crypto
engine") broke the build without openssl support.

The ssl_free_dh() function is not defined when USE_OPENSSL is not
defined and leads to a compilation failure.
---
 src/haproxy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index 60bd334b..9cbb3d5c 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2575,7 +2575,7 @@ int main(int argc, char **argv)
protocol_unbind_all();
mworker_wait();
}
-#ifndef OPENSSL_NO_DH
+#if defined(USE_OPENSSL) && !defined(OPENSSL_NO_DH)
ssl_free_dh();
 #endif
/* should never get there */
-- 
2.13.0

Re: exit error regression on haproxy1.8dev

[William Lallemand]

On Wed, Jun 07, 2017 at 02:13:58PM +0200, Emmanuel Hocdet wrote:
> 
> > Le 7 juin 2017 à 10:34, Emmanuel Hocdet <m...@gandi.net> a écrit :
> > 
> > Hi,
> > 
> > ok:
> >> haproxy -f test.cfg -W
> >> echo $?
> > 0
> > 
> > bad:
> >> haproxy -f test.cfg
> >> echo $?
> > 1
> > 
> > With bunch of modified code, i am a little lost to track the bug.
> > 
> > Manu
> > 
> > 
> 
 
You are right about the regression, however this part of the code should be
called in daemon mode only, the regression was elsewhere :)

Fix attached.

-- 
William Lallemand
>From 4171380e2ff3d562bfcf4f7aa5325d79ccda025b Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallem...@haproxy.com>
Date: Wed, 7 Jun 2017 15:04:47 +0200
Subject: [PATCH] BUG/MEDIUM: misplaced exit and wrong exit code

Commit cb11fd2 ("MEDIUM: mworker: wait mode on reload failure")
introduced a regression, when HAProxy is used in daemon mode, it exits 1
after forking its children.

HAProxy should exit(0), the exit(EXIT_FAILURE) was expected to be use
when the master fail in master-worker mode.

Thanks to Emmanuel Hocdet for reporting this bug. No backport needed.
---
 src/haproxy.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index 60bd334b..51cdf48e 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2574,12 +2574,13 @@ int main(int argc, char **argv)
 			if (global.mode & MODE_MWORKER) {
 protocol_unbind_all();
 mworker_wait();
+/* should never get there */
+exit(EXIT_FAILURE);
 			}
 #ifndef OPENSSL_NO_DH
 			ssl_free_dh();
 #endif
-			/* should never get there */
-			exit(EXIT_FAILURE);
+			exit(0); /* parent must leave */
 		}
 
 		/* child must never use the atexit function */
-- 
2.13.0

Re: exit error regression on haproxy1.8dev

[William Lallemand]

On Wed, Jun 07, 2017 at 02:13:58PM +0200, Emmanuel Hocdet wrote:
> 
> > Le 7 juin 2017 à 10:34, Emmanuel Hocdet <m...@gandi.net> a écrit :
> > 
> > Hi,
> > 
> > ok:
> >> haproxy -f test.cfg -W
> >> echo $?
> > 0
> > 
> > bad:
> >> haproxy -f test.cfg
> >> echo $?
> > 1
> > 
> > With bunch of modified code, i am a little lost to track the bug.
> > 
> > Manu
> > 
> > 
> 
 
You are right about the regression, however this part of the code should be
called in daemon mode only, the regression was elsewhere :)

Fix attached.

-- 
William Lallemand
>From 4171380e2ff3d562bfcf4f7aa5325d79ccda025b Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallem...@haproxy.com>
Date: Wed, 7 Jun 2017 15:04:47 +0200
Subject: [PATCH] BUG/MEDIUM: misplaced exit and wrong exit code

Commit cb11fd2 ("MEDIUM: mworker: wait mode on reload failure")
introduced a regression, when HAProxy is used in daemon mode, it exits 1
after forking its children.

HAProxy should exit(0), the exit(EXIT_FAILURE) was expected to be use
when the master fail in master-worker mode.

Thanks to Emmanuel Hocdet for reporting this bug. No backport needed.
---
 src/haproxy.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index 60bd334b..51cdf48e 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2574,12 +2574,13 @@ int main(int argc, char **argv)
 			if (global.mode & MODE_MWORKER) {
 protocol_unbind_all();
 mworker_wait();
+/* should never get there */
+exit(EXIT_FAILURE);
 			}
 #ifndef OPENSSL_NO_DH
 			ssl_free_dh();
 #endif
-			/* should never get there */
-			exit(EXIT_FAILURE);
+			exit(0); /* parent must leave */
 		}
 
 		/* child must never use the atexit function */
-- 
2.13.0

[PATCH v3 1/9] MEDIUM: mworker: replace systemd mode by master worker mode

[William Lallemand]

This commit remove the -Ds systemd mode in HAProxy in order to replace
it by a more generic master worker system. It aims to replace entirely
the systemd wrapper in the near future.

The master worker mode implements a new way of managing HAProxy
processes. The master is in charge of parsing the configuration
file and is responsible for spawning child processes.

The master worker mode can be invoked by using the -W flag.  It can be
used either in background mode (-D) or foreground mode. When used in
background mode, the master will fork to daemonize.

In master worker background mode, chroot, setuid and setgid are done in
each child rather than in the master process, because the master process
will still need access to filesystem to reload the configuration.
---
 include/types/global.h |   2 +-
 src/cfgparse.c |   5 ++
 src/haproxy.c  | 136 +
 src/listener.c |   4 +-
 4 files changed, 99 insertions(+), 48 deletions(-)

diff --git a/include/types/global.h b/include/types/global.h
index aeb82ea..ee8e95e 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -44,7 +44,7 @@
 #defineMODE_VERBOSE0x10
 #defineMODE_STARTING   0x20
 #defineMODE_FOREGROUND 0x40
-#defineMODE_SYSTEMD0x80
+#defineMODE_MWORKER0x80/* Master Worker */
 
 /* list of last checks to perform, depending on config options */
 #define LSTCHK_CAP_BIND0x0001  /* check that we can bind to 
any port */
diff --git a/src/cfgparse.c b/src/cfgparse.c
index 4c0e2d4..bdf55a8 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -624,6 +624,11 @@ int cfg_parse_global(const char *file, int linenum, char 
**args, int kwm)
goto out;
global.mode |= MODE_DAEMON;
}
+   else if (!strcmp(args[0], "master-worker")) {
+   if (alertif_too_many_args(0, file, linenum, args, _code))
+   goto out;
+   global.mode |= MODE_MWORKER;
+   }
else if (!strcmp(args[0], "debug")) {
if (alertif_too_many_args(0, file, linenum, args, _code))
goto out;
diff --git a/src/haproxy.c b/src/haproxy.c
index 6ecae25..0e8511b 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -348,6 +348,7 @@ static void usage(char *name)
"-dM[] poisons memory with  (defaults to 
0x50)\n"
"-V enters verbose mode (disables quiet mode)\n"
"-D goes daemon ; -C changes to  before loading 
files.\n"
+   "-W master-worker mode.\n"
"-q quiet mode : don't display messages\n"
"-c check mode : only check config files and exit\n"
"-n sets the maximum total # of connections (%d)\n"
@@ -921,11 +922,10 @@ static void init(int argc, char **argv)
arg_mode |= MODE_DEBUG;
else if (*flag == 'c')
arg_mode |= MODE_CHECK;
-   else if (*flag == 'D') {
+   else if (*flag == 'D')
arg_mode |= MODE_DAEMON;
-   if (flag[1] == 's')  /* -Ds */
-   arg_mode |= MODE_SYSTEMD;
-   }
+   else if (*flag == 'W')
+   arg_mode |= MODE_MWORKER;
else if (*flag == 'q')
arg_mode |= MODE_QUIET;
else if (*flag == 'x') {
@@ -1001,7 +1001,7 @@ static void init(int argc, char **argv)
}
 
global.mode = MODE_STARTING | /* during startup, we want most of the 
alerts */
-   (arg_mode & (MODE_DAEMON | MODE_SYSTEMD | MODE_FOREGROUND | 
MODE_VERBOSE
+   (arg_mode & (MODE_DAEMON | MODE_MWORKER | MODE_FOREGROUND | 
MODE_VERBOSE
 | MODE_QUIET | MODE_CHECK | MODE_DEBUG));
 
if (change_dir && chdir(change_dir) < 0) {
@@ -1330,24 +1330,24 @@ static void init(int argc, char **argv)
 
if (arg_mode & (MODE_DEBUG | MODE_FOREGROUND)) {
/* command line debug mode inhibits configuration mode */
-   global.mode &= ~(MODE_DAEMON | MODE_SYSTEMD | MODE_QUIET);
+   global.mode &= ~(MODE_DAEMON | MODE_QUIET);
global.mode |= (arg_mode & (MODE_DEBUG | MODE_FOREGROUND));
}
 
-   if (arg_mode & (MODE_DAEMON | MODE_SYSTEMD)) {
+   if (arg_mode & MODE_DAEMON) {
/* command line daemon mode inhibits foreground and debug modes 
mode */
global.mode &= ~(MODE_DEBUG | MODE_FOREGROUND);
-   global.mode |= (arg_mode & (MODE_DAEMON | MODE_SYSTEMD));
+   global.mode |= arg_mode & MODE_DAEMON;
}
 
global.mode |= (arg_mode & (MODE_QUIET | 

[PATCH v3 5/9] MEDIUM: mworker: exit-on-failure option

[William Lallemand]

This option exits every workers when one of the current workers die.

It allows you to monitor the master process in order to relaunch
everything on a failure.

For example it can be used with systemd and Restart=on-failure in a spec
file.
---
 include/types/global.h |  1 +
 src/cfgparse.c | 11 ++-
 src/haproxy.c  |  5 +
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/types/global.h b/include/types/global.h
index ee8e95e..cce3de7 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -63,6 +63,7 @@
 #define GTUNE_RESOLVE_DONTFAIL   (1<<7)
 
 #define GTUNE_SOCKET_TRANSFER   (1<<8)
+#define GTUNE_EXIT_ONFAILURE (1<<9)
 
 /* Access level for a stats socket */
 #define ACCESS_LVL_NONE 0
diff --git a/src/cfgparse.c b/src/cfgparse.c
index bdf55a8..76a4f31 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -625,8 +625,17 @@ int cfg_parse_global(const char *file, int linenum, char 
**args, int kwm)
global.mode |= MODE_DAEMON;
}
else if (!strcmp(args[0], "master-worker")) {
-   if (alertif_too_many_args(0, file, linenum, args, _code))
+   if (alertif_too_many_args(1, file, linenum, args, _code))
goto out;
+   if (*args[1]) {
+   if (!strcmp(args[1], "exit-on-failure")) {
+   global.tune.options |= GTUNE_EXIT_ONFAILURE;
+   } else {
+   Alert("parsing [%s:%d] : '%s' only supports 
'exit-on-failure' option.\n", file, linenum, args[0]);
+   err_code |= ERR_ALERT | ERR_FATAL;
+   goto out;
+   }
+   }
global.mode |= MODE_MWORKER;
}
else if (!strcmp(args[0], "debug")) {
diff --git a/src/haproxy.c b/src/haproxy.c
index 4e892c0..10ddf16 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -671,6 +671,11 @@ static void mworker_wait()
/* check if exited child was in the current children 
list */
if (current_child(exitpid)) {
Alert("Current worker %d left with exit code 
%d\n", exitpid, status);
+   if (status != 0 && status != 130 && status != 
143
+   && global.tune.options & 
GTUNE_EXIT_ONFAILURE) {
+   Alert("exit-on-failure: killing every 
workers with SIGTERM\n");
+   mworker_kill(SIGTERM);
+   }
} else {
Warning("Former worker %d left with exit code 
%d\n", exitpid, status);
delete_oldpid(exitpid);
-- 
2.10.2

[PATCH v3 8/9] MEDIUM: systemd: Type=forking in unit file

[William Lallemand]

Adding Type=forking in the unit file ensure better monitoring from
systemd. During a systemctl start the tool is able to return an error if
it didn't work with this option.
---
 contrib/systemd/haproxy.service.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/systemd/haproxy.service.in 
b/contrib/systemd/haproxy.service.in
index 05bb716..ca38d70 100644
--- a/contrib/systemd/haproxy.service.in
+++ b/contrib/systemd/haproxy.service.in
@@ -12,6 +12,7 @@ ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q
 ExecReload=/bin/kill -USR2 $MAINPID
 KillMode=mixed
 Restart=always
+Type=forking
 
 [Install]
 WantedBy=multi-user.target
-- 
2.10.2

[PATCH v3 4/9] MEDIUM: mworker: try to guess the next stats socket to use with -x

[William Lallemand]

In master worker mode, you can't specify the stats socket where you get
your listeners FDs on a reload, because the command line of the re-exec
is launched by the master.

To solve the problem, when -x is found on the command line, its
parameter is rewritten on a reexec with the first stats socket with the
capability to send sockets. It tries to reuse the original parameter if
it has this capability.
---
 src/haproxy.c | 73 +++
 1 file changed, 68 insertions(+), 5 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index c196d51..4e892c0 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -173,6 +173,8 @@ static int oldpids_sig; /* use USR1 or TERM */
 /* Path to the unix socket we use to retrieve listener sockets from the old 
process */
 static const char *old_unixsocket;
 
+static char *cur_unixsocket = NULL;
+
 /* this is used to drain data, and as a temporary buffer for sprintf()... */
 struct chunk trash = { };
 
@@ -512,6 +514,42 @@ int delete_oldpid(int pid)
return 0;
 }
 
+
+static void get_cur_unixsocket()
+{
+   /* if -x was used, try to update the stat socket if not available 
anymore */
+   if (global.stats_fe) {
+   struct bind_conf *bind_conf;
+
+   /* pass through all stats socket */
+   list_for_each_entry(bind_conf, _fe->conf.bind, 
by_fe) {
+   struct listener *l;
+
+   list_for_each_entry(l, _conf->listeners, by_bind) {
+
+   if (l->addr.ss_family == AF_UNIX &&
+   (bind_conf->level & ACCESS_FD_LISTENERS)) {
+   const struct sockaddr_un *un;
+
+   un = (struct sockaddr_un *)>addr;
+   /* priority to old_unixsocket */
+   if (!cur_unixsocket) {
+   cur_unixsocket = 
strdup(un->sun_path);
+   } else {
+   if (old_unixsocket && 
!strcmp(un->sun_path, old_unixsocket)) {
+   free(cur_unixsocket);
+   cur_unixsocket = 
strdup(old_unixsocket);
+   return;
+   }
+   }
+   }
+   }
+   }
+   }
+   if (!cur_unixsocket && old_unixsocket)
+   cur_unixsocket = strdup(old_unixsocket);
+}
+
 /*
  * When called, this function reexec haproxy with -sf followed by current
  * children PIDs and possibily old children PIDs if they didn't leave yet.
@@ -530,8 +568,8 @@ static void mworker_reload()
while (next_argv[next_argc])
next_argc++;
 
-   /* 1 for haproxy -sf */
-   next_argv = realloc(next_argv, (next_argc + 1 + global.nbproc + 
nb_oldpids + 1) * sizeof(char *));
+   /* 1 for haproxy -sf, 2 for -x /socket */
+   next_argv = realloc(next_argv, (next_argc + 1 + 2 + global.nbproc + 
nb_oldpids + 1) * sizeof(char *));
if (next_argv == NULL)
goto alloc_error;
 
@@ -555,6 +593,26 @@ static void mworker_reload()
msg = NULL;
}
next_argv[next_argc] = NULL;
+
+   /* if -x was used, try to update the stat socket if not available 
anymore */
+   if (cur_unixsocket) {
+
+   if (old_unixsocket) {
+
+   /* look for -x  */
+   for (j = 0; next_argv[j]; j++) {
+   if (!strcmp(next_argv[j], "-x"))
+   next_argv[j + 1] = (char 
*)cur_unixsocket;
+   }
+   } else {
+   /* if -x is not specified but we know the socket, add 
-x with it */
+   next_argv[next_argc++] = "-x";
+   next_argv[next_argc++] = (char *)cur_unixsocket;
+   next_argv[next_argc++] = NULL;
+
+   }
+   }
+
deinit(); /* we don't want to leak FD there */
Warning("Reexecuting Master process\n");
execv(next_argv[0], next_argv);
@@ -2216,11 +2274,16 @@ int main(int argc, char **argv)
}
 
if (old_unixsocket) {
-   if (get_old_sockets(old_unixsocket) != 0) {
-   Alert("Failed to get the sockets from the old 
process!\n");
-   exit(1);
+   if (strcmp("/dev/null", old_unixsocket) != 0) {
+   if (get_old_sockets(old_unixsocket) != 0) {
+   Alert("Failed to get the sockets from the old 
process!\n");
+   if (!(global.mode & MODE_MWORKER))
+ 

[PATCH v3 9/9] MAJOR: systemd-wrapper: get rid of the wrapper

[William Lallemand]

The master worker mode obsoletes the systemd-wrapper, to ensure that
nobody uses it anymore, the code has been removed.
---
 Makefile   |  18 +--
 contrib/systemd/haproxy.service.in |   2 +-
 src/haproxy-systemd-wrapper.c  | 319 -
 3 files changed, 3 insertions(+), 336 deletions(-)
 delete mode 100644 src/haproxy-systemd-wrapper.c

diff --git a/Makefile b/Makefile
index 5b5ecca..1c90a3e 100644
--- a/Makefile
+++ b/Makefile
@@ -61,8 +61,7 @@
 #  by "haproxy -vv" in CFLAGS.
 #   SILENT_DEFINE may be used to specify other defines which will not be
 # reported by "haproxy -vv".
-#   EXTRA   is used to force building or not building some extra tools. By
-#   default on Linux 2.6+, it contains "haproxy-systemd-wrapper".
+#   EXTRA   is used to force building or not building some extra tools.
 #   DESTDIR is not set by default and is used for installation only.
 #   It might be useful to set DESTDIR if you want to install haproxy
 #   in a sandbox.
@@ -175,7 +174,7 @@ ADDLIB =
 DEFINE =
 SILENT_DEFINE =
 
- extra programs to build (eg: haproxy-systemd-wrapper)
+ extra programs to build
 # Force this to enable building extra programs or to disable them.
 # It's automatically appended depending on the targets.
 EXTRA =
@@ -266,7 +265,6 @@ ifeq ($(TARGET),linux26)
   USE_TPROXY  = implicit
   USE_LIBCRYPT= implicit
   USE_FUTEX   = implicit
-  EXTRA  += haproxy-systemd-wrapper
   USE_DL  = implicit
 else
 ifeq ($(TARGET),linux2628)
@@ -282,7 +280,6 @@ ifeq ($(TARGET),linux2628)
   USE_FUTEX   = implicit
   USE_CPU_AFFINITY= implicit
   ASSUME_SPLICE_WORKS= implicit
-  EXTRA  += haproxy-systemd-wrapper
   USE_DL  = implicit
 else
 ifeq ($(TARGET),solaris)
@@ -835,7 +832,6 @@ ifneq ($(TRACE),)
 OBJS += src/trace.o
 endif
 
-WRAPPER_OBJS = src/haproxy-systemd-wrapper.o
 
 # Not used right now
 LIB_EBTREE = $(EBTREE_DIR)/libebtree.a
@@ -850,9 +846,6 @@ DEP = $(INCLUDES) .build_opts
 haproxy: $(OPTIONS_OBJS) $(EBTREE_OBJS) $(OBJS)
$(LD) $(LDFLAGS) -o $@ $^ $(LDOPTS)
 
-haproxy-systemd-wrapper: $(WRAPPER_OBJS)
-   $(LD) $(LDFLAGS) -o $@ $^ $(LDOPTS)
-
 $(LIB_EBTREE): $(EBTREE_OBJS)
$(AR) rv $@ $^
 
@@ -875,11 +868,6 @@ src/haproxy.o: src/haproxy.c $(DEP)
  -DBUILD_OPTIONS='"$(strip $(BUILD_OPTIONS))"' \
   -c -o $@ $<
 
-src/haproxy-systemd-wrapper.o: src/haproxy-systemd-wrapper.c $(DEP)
-   $(CC) $(COPTS) \
- -DSBINDIR='"$(strip $(SBINDIR))"' \
-  -c -o $@ $<
-
 src/dlmalloc.o: $(DLMALLOC_SRC) $(DEP)
$(CC) $(COPTS) -DDEFAULT_MMAP_THRESHOLD=$(DLMALLOC_THRES) -c -o $@ $<
 
@@ -915,14 +903,12 @@ uninstall:
done
-rmdir "$(DESTDIR)$(DOCDIR)"
rm -f "$(DESTDIR)$(SBINDIR)"/haproxy
-   rm -f "$(DESTDIR)$(SBINDIR)"/haproxy-systemd-wrapper
 
 clean:
rm -f *.[oas] src/*.[oas] ebtree/*.[oas] haproxy test .build_opts 
.build_opts.new
for dir in . src include/* doc ebtree; do rm -f $$dir/*~ $$dir/*.rej 
$$dir/core; done
rm -f haproxy-$(VERSION).tar.gz haproxy-$(VERSION)$(SUBVERS).tar.gz
rm -f haproxy-$(VERSION) haproxy-$(VERSION)$(SUBVERS) nohup.out gmon.out
-   rm -f haproxy-systemd-wrapper
 
 tags:
find src include \( -name '*.c' -o -name '*.h' \) -print0 | \
diff --git a/contrib/systemd/haproxy.service.in 
b/contrib/systemd/haproxy.service.in
index ca38d70..81b4951 100644
--- a/contrib/systemd/haproxy.service.in
+++ b/contrib/systemd/haproxy.service.in
@@ -7,7 +7,7 @@ After=network.target
 # socket if you want seamless reloads.
 Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
 ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q
-ExecStart=@SBINDIR@/haproxy-systemd-wrapper -f $CONFIG -p $PIDFILE
+ExecStart=@SBINDIR@/haproxy -W -f $CONFIG -p $PIDFILE
 ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q
 ExecReload=/bin/kill -USR2 $MAINPID
 KillMode=mixed
diff --git a/src/haproxy-systemd-wrapper.c b/src/haproxy-systemd-wrapper.c
deleted file mode 100644
index 457f5bd..000
--- a/src/haproxy-systemd-wrapper.c
+++ /dev/null
@@ -1,319 +0,0 @@
-/*
- * Wrapper to make haproxy systemd-compliant.
- *
- * Copyright 2013 Marc-Antoine Perennou 
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- *
- */
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#define REEXEC_FLAG "HAPROXY_SYSTEMD_REEXEC"
-#define SD_DEBUG "<7>"
-#define SD_NOTICE "<5>"
-
-static volatile sig_atomic_t caught_signal;
-
-static char *pid_file = "/run/haproxy.pid";
-static int wrapper_argc;
-static char **wrapper_argv;
-
-static void setup_signal_handler();
-static void 

Re: Replace the systemd-wrapper by the master worker mode

[William Lallemand]

Those minor problems have been corrected in this new patchset.
Please merge them if that's ok for you.

[PATCH v3 7/9] DOC: add documentation for the master-worker mode

[William Lallemand]

---
 doc/configuration.txt | 16 
 doc/management.txt| 15 +--
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index ad7d3a8..5fa49d3 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -803,6 +803,20 @@ lua-load 
   This global directive loads and executes a Lua file. This directive can be
   used multiple times.
 
+master-worker [exit-on-failure]
+  Master-worker mode. It is equivalent to the command line "-W" argument.
+  This mode will launch a "master" which will monitor the "workers". Using
+  this mode, you can reload HAProxy directly by sending a SIGUSR2 signal to
+  the master.  The master-worker mode is compatible either with the foreground
+  or daemon mode. It is recommended to use this mode with multiprocess and
+  systemd.
+  The "exit-on-failure" option allows the master to kill every workers and
+  exit when one of the current workers died. It is convenient to combine this
+  option with Restart=on-failure in a systemd unit file in order to relaunch
+  the whole process.
+
+  See alors "-W" in the management guide.
+
 nbproc 
   Creates  processes when going daemon. This requires the "daemon"
   mode. By default, only one process is created, which is the recommended mode
@@ -10431,6 +10445,8 @@ defer-accept
 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.
+  During a reload with the master-worker mode, the process is automatically
+  reexecuted adding -x and one of the stats socket with this option.
   See alors "-x" in the management guide.
 
 force-sslv3
diff --git a/doc/management.txt b/doc/management.txt
index 64d6a2d..df091bb 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -160,8 +160,6 @@ list of options is :
 configuration. It is recommended to always force it in any init script so
 that a faulty configuration doesn't prevent the system from booting.
 
-  -Ds : work in systemd mode. Only used by the systemd wrapper.
-
   -L  : change the local peer name to , which defaults to the local
 hostname. This is used only with peers replication.
 
@@ -171,6 +169,13 @@ list of options is :
   -V : enable verbose mode (disables quiet mode). Reverts the effect of "-q" or
 "quiet".
 
+  -W : master-worker mode. It is equivalent to the "master-worker" keyword in
+the "global" section of the configuration. This mode will launch a "master"
+which will monitor the "workers". Using this mode, you can reload HAProxy
+directly by sending a SIGUSR2 signal to the master.  The master-worker mode
+is compatible either with the foreground or daemon mode.  It is
+recommended to use this mode with multiprocess and systemd.
+
   -c : only performs a check of the configuration files and exits before trying
 to bind. The exit status is zero if everything is OK, or non-zero if an
 error is encountered.
@@ -419,6 +424,12 @@ reload or restart, so that they are sent at the latest 
possible moment and only
 if absolutely required. This is what is performed by the "-st" (hard) and "-sf"
 (graceful) options respectively.
 
+In master-worker mode, it is not needed to start a new haproxy process in
+order to reload the configuration. The master process reacts to the SIGUSR2
+signal by reexecuting itself with the -sf parameter followed by the PIDs of
+the workers. The master will then parse the configuration file and fork new
+workers.
+
 To understand better how these signals are used, it is important to understand
 the whole restart mechanism.
 
-- 
2.10.2

[PATCH v3 6/9] MEDIUM: mworker: workers exit when the master leaves

[William Lallemand]

This patch ensure that the children will exit when the master quits,
even if the master didn't send any signal.

The master and the workers are connected through a pipe, when the pipe
closes the children leave.
---
 src/haproxy.c | 60 +++
 1 file changed, 60 insertions(+)

diff --git a/src/haproxy.c b/src/haproxy.c
index 10ddf16..60bd334 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -203,6 +203,8 @@ int *children = NULL; /* store PIDs of children in master 
workers mode */
 static volatile sig_atomic_t caught_signal = 0;
 static char **next_argv = NULL;
 
+int mworker_pipe[2];
+
 /* list of the temporarily limited listeners because of lack of resource */
 struct list global_listener_queue = LIST_HEAD_INIT(global_listener_queue);
 struct task *global_listener_queue_task;
@@ -2225,6 +2227,37 @@ static struct task *manage_global_listener_queue(struct 
task *t)
return t;
 }
 
+void mworker_pipe_handler(int fd)
+{
+   char c;
+
+   while (read(fd, , 1) == -1) {
+   if (errno == EINTR)
+   continue;
+   if (errno == EAGAIN) {
+   fd_cant_recv(fd);
+   return;
+   }
+   break;
+   }
+
+   deinit();
+   exit(EXIT_FAILURE);
+   return;
+}
+
+void mworker_pipe_register(int pipefd[2])
+{
+   close(mworker_pipe[1]); /* close the write end of the master 
pipe in the children */
+
+   fcntl(mworker_pipe[0], F_SETFL, O_NONBLOCK);
+   fdtab[mworker_pipe[0]].owner = mworker_pipe;
+   fdtab[mworker_pipe[0]].iocb = mworker_pipe_handler;
+   fd_insert(mworker_pipe[0]);
+   fd_want_recv(mworker_pipe[0]);
+   }
+
+
 int main(int argc, char **argv)
 {
int err, retry;
@@ -2475,6 +2508,30 @@ int main(int argc, char **argv)
if (ret > 0)
exit(0);
}
+
+   if (global.mode & MODE_MWORKER) {
+   if ((getenv("HAPROXY_MWORKER_REEXEC") == NULL)) {
+   char *msg = NULL;
+   /* master pipe to ensure the master is still 
alive  */
+   ret = pipe(mworker_pipe);
+   if (ret < 0) {
+   Warning("[%s.main()] Cannot create 
master pipe.\n", argv[0]);
+   } else {
+   memprintf(, "%d", mworker_pipe[0]);
+   setenv("HAPROXY_MWORKER_PIPE_RD", msg, 
1);
+   memprintf(, "%d", mworker_pipe[1]);
+   setenv("HAPROXY_MWORKER_PIPE_WR", msg, 
1);
+   free(msg);
+   }
+   } else {
+   mworker_pipe[0] = 
atol(getenv("HAPROXY_MWORKER_PIPE_RD"));
+   mworker_pipe[1] = 
atol(getenv("HAPROXY_MWORKER_PIPE_WR"));
+   if (mworker_pipe[0] <= 0 || mworker_pipe[1] <= 
0) {
+   Warning("[%s.main()] Cannot get master 
pipe FDs.\n", argv[0]);
+   }
+   }
+   }
+
/* the father launches the required number of processes */
for (proc = 0; proc < global.nbproc; proc++) {
ret = fork();
@@ -2633,6 +2690,9 @@ int main(int argc, char **argv)
if (!dns_init_resolvers(1))
exit(1);
 
+   if (global.mode & MODE_MWORKER)
+   mworker_pipe_register(mworker_pipe);
+
protocol_enable_all();
/*
 * That's it : the central polling loop. Run until we stop.
-- 
2.10.2

[PATCH v3 3/9] MEDIUM: mworker: wait mode on reload failure

[William Lallemand]

In Master Worker mode, when the reloading of the configuration fail,
the process is exiting leaving the children without their father.

To handle this, we register an exit function with atexit(3), which is
reexecuting the binary in a special mode. This particular mode of
HAProxy don't reload the configuration, it only loops on wait().
---
 src/haproxy.c | 37 -
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index 1f40a12..c196d51 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -181,6 +181,8 @@ struct chunk trash = { };
  */
 char *swap_buffer = NULL;
 
+int atexit_flag = 0;
+
 int nb_oldpids = 0;
 const int zero = 0;
 const int one = 1;
@@ -592,6 +594,7 @@ static void mworker_wait()
 
if (exitpid == -1 && errno == ECHILD) {
Warning("All workers are left. Leaving... (%d)\n", 
status);
+   atexit_flag = 0;
exit(status); /* parent must leave using the latest 
status code known */
}
 
@@ -619,6 +622,22 @@ static void mworker_wait()
 }
 
 
+/*
+ * Reexec the process in failure mode, instead of exiting
+ */
+void reexec_on_failure()
+{
+   if (!atexit_flag)
+   return;
+
+   setenv("HAPROXY_MWORKER_WAIT_ONLY", "1", 1);
+
+   Warning("Reexecuting Master process in waitpid mode\n");
+   mworker_reload();
+
+   Warning("Failed to reexecute the master processs\n");
+}
+
 
 /*
  * upon SIGUSR1, let's have a soft stop. Note that soft_stop() broadcasts
@@ -1270,6 +1289,18 @@ static void init(int argc, char **argv)
(arg_mode & (MODE_DAEMON | MODE_MWORKER | MODE_FOREGROUND | 
MODE_VERBOSE
 | MODE_QUIET | MODE_CHECK | MODE_DEBUG));
 
+   /* Master workers wait mode */
+   if ((global.mode & MODE_MWORKER) && 
(getenv("HAPROXY_MWORKER_WAIT_ONLY") != NULL)) {
+
+   unsetenv("HAPROXY_MWORKER_WAIT_ONLY");
+   mworker_wait();
+   }
+
+   if ((global.mode & MODE_MWORKER) && (getenv("HAPROXY_MWORKER_REEXEC") 
!= NULL)) {
+   atexit_flag = 1;
+   atexit(reexec_on_failure);
+   }
+
if (change_dir && chdir(change_dir) < 0) {
Alert("Could not change to directory %s : %s\n", change_dir, 
strerror(errno));
exit(1);
@@ -2422,9 +2453,13 @@ int main(int argc, char **argv)
 #ifndef OPENSSL_NO_DH
ssl_free_dh();
 #endif
-   exit(0); /* parent must leave */
+   /* should never get there */
+   exit(EXIT_FAILURE);
}
 
+   /* child must never use the atexit function */
+   atexit_flag = 0;
+
/* Must chroot and setgid/setuid in the children */
/* chroot if needed */
if (global.chroot != NULL) {
-- 
2.10.2

[PATCH v3 2/9] MEDIUM: mworker: handle reload and signals

[William Lallemand]

The master-worker will reload itself on SIGUSR2/SIGHUP

It's inherited from the systemd wrapper, when the SIGUSR2 signal is
received, the master process will reexecute itself with the -sf flag
followed by the PIDs of the children.

In the systemd wrapper, the children were using a pipe to notify when
the config has been parsed and when the new process is ready. The goal
was to ensure that the process couldn't reload during the parsing of the
configuration, before signals were send to old process.

With the new mworker model, the master parses the configuration and is
aware of all the children. We don't need a pipe, but we need to block
those signals before the end of a reload, to ensure that the process
won't be killed during a reload.

The SIGUSR1 signal is forwarded to the children to soft-stop HAProxy.

The SIGTERM and SIGINT signals are forwarded to the children in order to
terminate them.
---
 src/haproxy.c | 303 --
 src/signal.c  |   4 +
 2 files changed, 277 insertions(+), 30 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index 0e8511b..1f40a12 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -194,6 +194,11 @@ char localpeer[MAX_HOSTNAME_LEN];
  */
 int shut_your_big_mouth_gcc_int = 0;
 
+int *children = NULL; /* store PIDs of children in master workers mode */
+
+static volatile sig_atomic_t caught_signal = 0;
+static char **next_argv = NULL;
+
 /* list of the temporarily limited listeners because of lack of resource */
 struct list global_listener_queue = LIST_HEAD_INIT(global_listener_queue);
 struct task *global_listener_queue_task;
@@ -389,6 +394,232 @@ static void usage(char *name)
 /*   more specific functions   ***/
 /*/
 
+/* sends the signal  to all pids found in . Returns the number of
+ * pids the signal was correctly delivered to.
+ */
+static int tell_old_pids(int sig)
+{
+   int p;
+   int ret = 0;
+   for (p = 0; p < nb_oldpids; p++)
+   if (kill(oldpids[p], sig) == 0)
+   ret++;
+   return ret;
+}
+
+/* return 1 if a pid is a current child otherwise 0 */
+
+int current_child(int pid)
+{
+   int i;
+
+   for (i = 0; i < global.nbproc; i++) {
+   if (children[i] == pid)
+   return 1;
+   }
+   return 0;
+}
+
+static void mworker_signalhandler(int signum)
+{
+   caught_signal = signum;
+}
+
+static void mworker_register_signals()
+{
+   struct sigaction sa;
+   /* Here we are not using the haproxy async way
+   for signals because it does not exists in
+   the master */
+   memset(, 0, sizeof(struct sigaction));
+   sa.sa_handler = _signalhandler;
+   sigaction(SIGHUP, , NULL);
+   sigaction(SIGUSR1, , NULL);
+   sigaction(SIGUSR2, , NULL);
+   sigaction(SIGINT, , NULL);
+   sigaction(SIGTERM, , NULL);
+}
+
+static void mworker_block_signals()
+{
+   sigset_t set;
+
+   sigemptyset();
+   sigaddset(, SIGUSR1);
+   sigaddset(, SIGUSR2);
+   sigaddset(, SIGHUP);
+   sigaddset(, SIGINT);
+   sigaddset(, SIGTERM);
+   sigprocmask(SIG_SETMASK, , NULL);
+}
+
+static void mworker_unblock_signals()
+{
+   sigset_t set;
+
+   sigemptyset();
+   sigaddset(, SIGUSR1);
+   sigaddset(, SIGUSR2);
+   sigaddset(, SIGHUP);
+   sigaddset(, SIGINT);
+   sigaddset(, SIGTERM);
+   sigprocmask(SIG_UNBLOCK, , NULL);
+}
+
+static void mworker_unregister_signals()
+{
+   signal(SIGINT, SIG_DFL);
+   signal(SIGTERM, SIG_DFL);
+   signal(SIGHUP,  SIG_IGN);
+   signal(SIGUSR1, SIG_IGN);
+   signal(SIGUSR2, SIG_IGN);
+}
+
+/*
+ * Send signal to every known children.
+ */
+
+static void mworker_kill(int sig)
+{
+   int i;
+
+   tell_old_pids(sig);
+   if (children) {
+   for (i = 0; i < global.nbproc; i++)
+   kill(children[i], sig);
+   }
+}
+
+/*
+ * remove a pid forom the olpid array and decrease nb_oldpids
+ * return 1 pid was found otherwise return 0
+ */
+
+int delete_oldpid(int pid)
+{
+   int i;
+
+   for (i = 0; i < nb_oldpids; i++) {
+   if (oldpids[i] == pid) {
+   oldpids[i] = oldpids[nb_oldpids - 1];
+   oldpids[nb_oldpids - 1] = 0;
+   nb_oldpids--;
+   return 1;
+   }
+   }
+   return 0;
+}
+
+/*
+ * When called, this function reexec haproxy with -sf followed by current
+ * children PIDs and possibily old children PIDs if they didn't leave yet.
+ */
+static void mworker_reload()
+{
+   int next_argc = 0;
+   int j;
+   char *msg = NULL;
+
+   mworker_block_signals();
+   mworker_unregister_signals();
+   setenv("HAPROXY_MWORKER_REEXEC", "1", 1);
+
+   /* compute length  */
+   while (next_argv[next_argc])
+ 

Re: Download site horrendously slow

[William Lallemand]

On Wed, May 31, 2017 at 04:15:18PM +, Skarbek, John wrote:
> Hey guys,
> 
> Just an FYI, I'm not sure how you host the downloads for haproxy, but for the 
> past couple of days, they've been horridly slow.
> 

You are right, I'd just test it and the download of the tarball fluctuate
between 1 and 2 kB/s.


-- 
William Lallemand

Re: [PATCH 6/9] MEDIUM: mworker: workers exit when the master leaves

[William Lallemand]

On Tue, May 30, 2017 at 12:39:32PM +0200, Willy Tarreau wrote:
> [...]
> 
> The master, not intercepting this signal, would die, closing the pipe.
> The worker would be woken up on the detection of this closure, and while
> trying to perform the read() would get the signal in turn, causing the
> read() to return EINTR and to stop polling on this fd instead of exiting.
>

You are absolutely right, I'll fix that.

> 
> Regarding the environment variable names, it's preferable to prepend
> "HAPROXY" in front of them as we've been doing for all other ones to
> avoid namespace conflicts with anything used in other environments (I've
> ween places where you had to run "env|grep" to find your variable). I've
> seen another one called "WAIT_ONLY" in one of the first patches and
> which should equally be renamed.
> 

Will do.

> Otherwise the series looks quite good, it would be nice to get some
> feedback especially from systemd hostages^Wusers (my comments above
> will not affect their experience unless they're really unlucky).
> 
> Thanks!
> Willy
> 

I'll send you another batch with the fixes, and maybe some cleanup. In the
meanwhile people can still try them.

-- 
William Lallemand

[PATCH 9/9] MAJOR: systemd-wrapper: get rid of the wrapper

[William Lallemand]

The master worker mode obsoletes the systemd-wrapper, to ensure that
nobody uses it anymore, the code has been removed.
---
 Makefile   |  18 +--
 contrib/systemd/haproxy.service.in |   2 +-
 src/haproxy-systemd-wrapper.c  | 319 -
 3 files changed, 3 insertions(+), 336 deletions(-)
 delete mode 100644 src/haproxy-systemd-wrapper.c

diff --git a/Makefile b/Makefile
index 5b5ecca..1c90a3e 100644
--- a/Makefile
+++ b/Makefile
@@ -61,8 +61,7 @@
 #  by "haproxy -vv" in CFLAGS.
 #   SILENT_DEFINE may be used to specify other defines which will not be
 # reported by "haproxy -vv".
-#   EXTRA   is used to force building or not building some extra tools. By
-#   default on Linux 2.6+, it contains "haproxy-systemd-wrapper".
+#   EXTRA   is used to force building or not building some extra tools.
 #   DESTDIR is not set by default and is used for installation only.
 #   It might be useful to set DESTDIR if you want to install haproxy
 #   in a sandbox.
@@ -175,7 +174,7 @@ ADDLIB =
 DEFINE =
 SILENT_DEFINE =
 
- extra programs to build (eg: haproxy-systemd-wrapper)
+ extra programs to build
 # Force this to enable building extra programs or to disable them.
 # It's automatically appended depending on the targets.
 EXTRA =
@@ -266,7 +265,6 @@ ifeq ($(TARGET),linux26)
   USE_TPROXY  = implicit
   USE_LIBCRYPT= implicit
   USE_FUTEX   = implicit
-  EXTRA  += haproxy-systemd-wrapper
   USE_DL  = implicit
 else
 ifeq ($(TARGET),linux2628)
@@ -282,7 +280,6 @@ ifeq ($(TARGET),linux2628)
   USE_FUTEX   = implicit
   USE_CPU_AFFINITY= implicit
   ASSUME_SPLICE_WORKS= implicit
-  EXTRA  += haproxy-systemd-wrapper
   USE_DL  = implicit
 else
 ifeq ($(TARGET),solaris)
@@ -835,7 +832,6 @@ ifneq ($(TRACE),)
 OBJS += src/trace.o
 endif
 
-WRAPPER_OBJS = src/haproxy-systemd-wrapper.o
 
 # Not used right now
 LIB_EBTREE = $(EBTREE_DIR)/libebtree.a
@@ -850,9 +846,6 @@ DEP = $(INCLUDES) .build_opts
 haproxy: $(OPTIONS_OBJS) $(EBTREE_OBJS) $(OBJS)
$(LD) $(LDFLAGS) -o $@ $^ $(LDOPTS)
 
-haproxy-systemd-wrapper: $(WRAPPER_OBJS)
-   $(LD) $(LDFLAGS) -o $@ $^ $(LDOPTS)
-
 $(LIB_EBTREE): $(EBTREE_OBJS)
$(AR) rv $@ $^
 
@@ -875,11 +868,6 @@ src/haproxy.o: src/haproxy.c $(DEP)
  -DBUILD_OPTIONS='"$(strip $(BUILD_OPTIONS))"' \
   -c -o $@ $<
 
-src/haproxy-systemd-wrapper.o: src/haproxy-systemd-wrapper.c $(DEP)
-   $(CC) $(COPTS) \
- -DSBINDIR='"$(strip $(SBINDIR))"' \
-  -c -o $@ $<
-
 src/dlmalloc.o: $(DLMALLOC_SRC) $(DEP)
$(CC) $(COPTS) -DDEFAULT_MMAP_THRESHOLD=$(DLMALLOC_THRES) -c -o $@ $<
 
@@ -915,14 +903,12 @@ uninstall:
done
-rmdir "$(DESTDIR)$(DOCDIR)"
rm -f "$(DESTDIR)$(SBINDIR)"/haproxy
-   rm -f "$(DESTDIR)$(SBINDIR)"/haproxy-systemd-wrapper
 
 clean:
rm -f *.[oas] src/*.[oas] ebtree/*.[oas] haproxy test .build_opts 
.build_opts.new
for dir in . src include/* doc ebtree; do rm -f $$dir/*~ $$dir/*.rej 
$$dir/core; done
rm -f haproxy-$(VERSION).tar.gz haproxy-$(VERSION)$(SUBVERS).tar.gz
rm -f haproxy-$(VERSION) haproxy-$(VERSION)$(SUBVERS) nohup.out gmon.out
-   rm -f haproxy-systemd-wrapper
 
 tags:
find src include \( -name '*.c' -o -name '*.h' \) -print0 | \
diff --git a/contrib/systemd/haproxy.service.in 
b/contrib/systemd/haproxy.service.in
index ca38d70..81b4951 100644
--- a/contrib/systemd/haproxy.service.in
+++ b/contrib/systemd/haproxy.service.in
@@ -7,7 +7,7 @@ After=network.target
 # socket if you want seamless reloads.
 Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
 ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q
-ExecStart=@SBINDIR@/haproxy-systemd-wrapper -f $CONFIG -p $PIDFILE
+ExecStart=@SBINDIR@/haproxy -W -f $CONFIG -p $PIDFILE
 ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q
 ExecReload=/bin/kill -USR2 $MAINPID
 KillMode=mixed
diff --git a/src/haproxy-systemd-wrapper.c b/src/haproxy-systemd-wrapper.c
deleted file mode 100644
index 457f5bd..000
--- a/src/haproxy-systemd-wrapper.c
+++ /dev/null
@@ -1,319 +0,0 @@
-/*
- * Wrapper to make haproxy systemd-compliant.
- *
- * Copyright 2013 Marc-Antoine Perennou 
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- *
- */
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#define REEXEC_FLAG "HAPROXY_SYSTEMD_REEXEC"
-#define SD_DEBUG "<7>"
-#define SD_NOTICE "<5>"
-
-static volatile sig_atomic_t caught_signal;
-
-static char *pid_file = "/run/haproxy.pid";
-static int wrapper_argc;
-static char **wrapper_argv;
-
-static void setup_signal_handler();
-static void 

[PATCH 7/9] DOC: add documentation for the master-worker mode

[William Lallemand]

---
 doc/configuration.txt | 16 
 doc/management.txt| 15 +--
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index ad7d3a8..5fa49d3 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -803,6 +803,20 @@ lua-load 
   This global directive loads and executes a Lua file. This directive can be
   used multiple times.
 
+master-worker [exit-on-failure]
+  Master-worker mode. It is equivalent to the command line "-W" argument.
+  This mode will launch a "master" which will monitor the "workers". Using
+  this mode, you can reload HAProxy directly by sending a SIGUSR2 signal to
+  the master.  The master-worker mode is compatible either with the foreground
+  or daemon mode. It is recommended to use this mode with multiprocess and
+  systemd.
+  The "exit-on-failure" option allows the master to kill every workers and
+  exit when one of the current workers died. It is convenient to combine this
+  option with Restart=on-failure in a systemd unit file in order to relaunch
+  the whole process.
+
+  See alors "-W" in the management guide.
+
 nbproc 
   Creates  processes when going daemon. This requires the "daemon"
   mode. By default, only one process is created, which is the recommended mode
@@ -10431,6 +10445,8 @@ defer-accept
 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.
+  During a reload with the master-worker mode, the process is automatically
+  reexecuted adding -x and one of the stats socket with this option.
   See alors "-x" in the management guide.
 
 force-sslv3
diff --git a/doc/management.txt b/doc/management.txt
index 64d6a2d..df091bb 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -160,8 +160,6 @@ list of options is :
 configuration. It is recommended to always force it in any init script so
 that a faulty configuration doesn't prevent the system from booting.
 
-  -Ds : work in systemd mode. Only used by the systemd wrapper.
-
   -L  : change the local peer name to , which defaults to the local
 hostname. This is used only with peers replication.
 
@@ -171,6 +169,13 @@ list of options is :
   -V : enable verbose mode (disables quiet mode). Reverts the effect of "-q" or
 "quiet".
 
+  -W : master-worker mode. It is equivalent to the "master-worker" keyword in
+the "global" section of the configuration. This mode will launch a "master"
+which will monitor the "workers". Using this mode, you can reload HAProxy
+directly by sending a SIGUSR2 signal to the master.  The master-worker mode
+is compatible either with the foreground or daemon mode.  It is
+recommended to use this mode with multiprocess and systemd.
+
   -c : only performs a check of the configuration files and exits before trying
 to bind. The exit status is zero if everything is OK, or non-zero if an
 error is encountered.
@@ -419,6 +424,12 @@ reload or restart, so that they are sent at the latest 
possible moment and only
 if absolutely required. This is what is performed by the "-st" (hard) and "-sf"
 (graceful) options respectively.
 
+In master-worker mode, it is not needed to start a new haproxy process in
+order to reload the configuration. The master process reacts to the SIGUSR2
+signal by reexecuting itself with the -sf parameter followed by the PIDs of
+the workers. The master will then parse the configuration file and fork new
+workers.
+
 To understand better how these signals are used, it is important to understand
 the whole restart mechanism.
 
-- 
2.10.2

[PATCH 2/9] MEDIUM: mworker: handle reload and signals

[William Lallemand]

The master-worker will reload itself on SIGUSR2/SIGHUP

It's inherited from the systemd wrapper, when the SIGUSR2 signal is
received, the master process will reexecute itself with the -sf flag
followed by the PIDs of the children.

In the systemd wrapper, the children were using a pipe to notify when
the config has been parsed and when the new process is ready. The goal
was to ensure that the process couldn't reload during the parsing of the
configuration, before signals were send to old process.

With the new mworker model, the master parses the configuration and is
aware of all the children. We don't need a pipe, but we need to block
those signals before the end of a reload, to ensure that the process
won't be killed during a reload.

The SIGUSR1 signal is forwarded to the children to soft-stop HAProxy.

The SIGTERM and SIGINT signals are forwarded to the children in order to
terminate them.
---
 src/haproxy.c | 304 --
 src/signal.c  |   4 +
 2 files changed, 279 insertions(+), 29 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index 0e8511b..2454dfd 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -158,6 +158,8 @@ struct global global = {
 
 /*/
 
+#define REEXEC_FLAG "HAPROXY_MWORKER_REEXEC"
+
 int stopping;  /* non zero means stopping in progress */
 int killed;/* non zero means a hard-stop is triggered */
 int jobs = 0;   /* number of active jobs (conns, listeners, active tasks, ...) 
*/
@@ -194,6 +196,11 @@ char localpeer[MAX_HOSTNAME_LEN];
  */
 int shut_your_big_mouth_gcc_int = 0;
 
+int *children = NULL; /* store PIDs of children in master workers mode */
+
+static volatile sig_atomic_t caught_signal = 0;
+static char **next_argv = NULL;
+
 /* list of the temporarily limited listeners because of lack of resource */
 struct list global_listener_queue = LIST_HEAD_INIT(global_listener_queue);
 struct task *global_listener_queue_task;
@@ -389,6 +396,232 @@ static void usage(char *name)
 /*   more specific functions   ***/
 /*/
 
+/* sends the signal  to all pids found in . Returns the number of
+ * pids the signal was correctly delivered to.
+ */
+static int tell_old_pids(int sig)
+{
+   int p;
+   int ret = 0;
+   for (p = 0; p < nb_oldpids; p++)
+   if (kill(oldpids[p], sig) == 0)
+   ret++;
+   return ret;
+}
+
+/* return 1 if a pid is a current child otherwise 0 */
+
+int current_child(int pid)
+{
+   int i;
+
+   for (i = 0; i < global.nbproc; i++) {
+   if (children[i] == pid)
+   return 1;
+   }
+   return 0;
+}
+
+static void mworker_signalhandler(int signum)
+{
+   caught_signal = signum;
+}
+
+static void mworker_register_signals()
+{
+   struct sigaction sa;
+   /* Here we are not using the haproxy async way
+   for signals because it does not exists in
+   the master */
+   memset(, 0, sizeof(struct sigaction));
+   sa.sa_handler = _signalhandler;
+   sigaction(SIGHUP, , NULL);
+   sigaction(SIGUSR1, , NULL);
+   sigaction(SIGUSR2, , NULL);
+   sigaction(SIGINT, , NULL);
+   sigaction(SIGTERM, , NULL);
+}
+
+static void mworker_block_signals()
+{
+   sigset_t set;
+
+   sigemptyset();
+   sigaddset(, SIGUSR1);
+   sigaddset(, SIGUSR2);
+   sigaddset(, SIGHUP);
+   sigaddset(, SIGINT);
+   sigaddset(, SIGTERM);
+   sigprocmask(SIG_SETMASK, , NULL);
+}
+
+static void mworker_unblock_signals()
+{
+   sigset_t set;
+
+   sigemptyset();
+   sigaddset(, SIGUSR1);
+   sigaddset(, SIGUSR2);
+   sigaddset(, SIGHUP);
+   sigaddset(, SIGINT);
+   sigaddset(, SIGTERM);
+   sigprocmask(SIG_UNBLOCK, , NULL);
+}
+
+static void mworker_unregister_signals()
+{
+   signal(SIGINT, SIG_DFL);
+   signal(SIGTERM, SIG_DFL);
+   signal(SIGHUP,  SIG_IGN);
+   signal(SIGUSR1, SIG_IGN);
+   signal(SIGUSR2, SIG_IGN);
+}
+
+/*
+ * Send signal to every known children.
+ */
+
+static void mworker_kill(int sig)
+{
+   int i;
+
+   tell_old_pids(sig);
+   if (children) {
+   for (i = 0; i < global.nbproc; i++)
+   kill(children[i], sig);
+   }
+}
+
+/*
+ * remove a pid forom the olpid array and decrease nb_oldpids
+ * return 1 pid was found otherwise return 0
+ */
+
+int delete_oldpid(int pid)
+{
+   int i;
+
+   for (i = 0; i < nb_oldpids; i++) {
+   if (oldpids[i] == pid) {
+   oldpids[i] = oldpids[nb_oldpids - 1];
+   oldpids[nb_oldpids - 1] = 0;
+   nb_oldpids--;
+   return 1;
+   }
+   }
+   return 0;
+}
+
+/*
+ * When called, this function reexec haproxy with -sf followed by current
+ * 

[PATCH 6/9] MEDIUM: mworker: workers exit when the master leaves

[William Lallemand]

This patch ensure that the children will exit when the master quits,
even if the master didn't send any signal.

The master and the workers are connected through a pipe, when the pipe
closes the children leave.
---
 src/haproxy.c | 55 +++
 1 file changed, 55 insertions(+)

diff --git a/src/haproxy.c b/src/haproxy.c
index d23bf3a..01681d4 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -205,6 +205,8 @@ int *children = NULL; /* store PIDs of children in master 
workers mode */
 static volatile sig_atomic_t caught_signal = 0;
 static char **next_argv = NULL;
 
+int mworker_pipe[2];
+
 /* list of the temporarily limited listeners because of lack of resource */
 struct list global_listener_queue = LIST_HEAD_INIT(global_listener_queue);
 struct task *global_listener_queue_task;
@@ -2227,6 +2229,34 @@ static struct task *manage_global_listener_queue(struct 
task *t)
return t;
 }
 
+void mworker_pipe_handler(int fd)
+{
+   char c;
+
+   if (read(fd, , 1) > -1) {
+   fd_delete(fd);
+   deinit();
+   exit(EXIT_FAILURE);
+   } else {
+   /* should never happened */
+   fd_delete(fd);
+   }
+
+   return;
+}
+
+void mworker_pipe_register(int pipefd[2])
+{
+   close(mworker_pipe[1]); /* close the write end of the master 
pipe in the children */
+
+   fcntl(mworker_pipe[0], F_SETFL, O_NONBLOCK);
+   fdtab[mworker_pipe[0]].owner = mworker_pipe;
+   fdtab[mworker_pipe[0]].iocb = mworker_pipe_handler;
+   fd_insert(mworker_pipe[0]);
+   fd_want_recv(mworker_pipe[0]);
+   }
+
+
 int main(int argc, char **argv)
 {
int err, retry;
@@ -2478,6 +2508,28 @@ int main(int argc, char **argv)
exit(0);
}
 
+   if (global.mode & MODE_MWORKER) {
+   if ((getenv(REEXEC_FLAG) == NULL)) {
+   char *msg = NULL;
+   /* master pipe to ensure the master is still 
alive  */
+   ret = pipe(mworker_pipe);
+   if (ret < 0) {
+   Warning("[%s.main()] Cannot create 
master pipe.\n", argv[0]);
+   } else {
+   memprintf(, "%d", mworker_pipe[0]);
+   setenv("MWORKER_PIPE_RD", msg, 1);
+   memprintf(, "%d", mworker_pipe[1]);
+   setenv("MWORKER_PIPE_WR", msg, 1);
+   free(msg);
+   }
+   } else {
+   mworker_pipe[0] = 
atol(getenv("MWORKER_PIPE_RD"));
+   mworker_pipe[1] = 
atol(getenv("MWORKER_PIPE_WR"));
+   if (mworker_pipe[0] <= 0 || mworker_pipe[1] <= 
0) {
+   Warning("[%s.main()] Cannot get master 
pipe FDs.\n", argv[0]);
+   }
+   }
+   }
 
/* the father launches the required number of processes */
for (proc = 0; proc < global.nbproc; proc++) {
@@ -2637,6 +2689,9 @@ int main(int argc, char **argv)
if (!dns_init_resolvers(1))
exit(1);
 
+   if (global.mode & MODE_MWORKER)
+   mworker_pipe_register(mworker_pipe);
+
protocol_enable_all();
/*
 * That's it : the central polling loop. Run until we stop.
-- 
2.10.2

[PATCH 8/9] MEDIUM: systemd: Type=forking in unit file

[William Lallemand]

Adding Type=forking in the unit file ensure better monitoring from
systemd. During a systemctl start the tool is able to return an error if
it didn't work with this option.
---
 contrib/systemd/haproxy.service.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/systemd/haproxy.service.in 
b/contrib/systemd/haproxy.service.in
index 05bb716..ca38d70 100644
--- a/contrib/systemd/haproxy.service.in
+++ b/contrib/systemd/haproxy.service.in
@@ -12,6 +12,7 @@ ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q
 ExecReload=/bin/kill -USR2 $MAINPID
 KillMode=mixed
 Restart=always
+Type=forking
 
 [Install]
 WantedBy=multi-user.target
-- 
2.10.2

[PATCH 3/9] MEDIUM: mworker: wait mode on reload failure

[William Lallemand]

In Master Worker mode, when the reloading of the configuration fail,
the process is exiting leaving the children without their father.

To handle this, we register an exit function with atexit(3), which is
reexecuting the binary in a special mode. This particular mode of
HAProxy don't reload the configuration, it only loops on wait().
---
 src/haproxy.c | 37 -
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index 2454dfd..61114b6 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -183,6 +183,8 @@ struct chunk trash = { };
  */
 char *swap_buffer = NULL;
 
+int atexit_flag = 0;
+
 int nb_oldpids = 0;
 const int zero = 0;
 const int one = 1;
@@ -594,6 +596,7 @@ static void mworker_wait()
 
if (exitpid == -1 && errno == ECHILD) {
Warning("All workers are left. Leaving... (%d)\n", 
status);
+   atexit_flag = 0;
exit(status); /* parent must leave using the latest 
status code known */
}
 
@@ -621,6 +624,22 @@ static void mworker_wait()
 }
 
 
+/*
+ * Reexec the process in failure mode, instead of exiting
+ */
+void reexec_on_failure()
+{
+   if (!atexit_flag)
+   return;
+
+   setenv("WAIT_ONLY", "1", 1);
+
+   Warning("Reexecuting Master process in waitpid mode\n");
+   mworker_reload();
+
+   Warning("Failed to reexecute the master processs\n");
+}
+
 
 /*
  * upon SIGUSR1, let's have a soft stop. Note that soft_stop() broadcasts
@@ -1272,6 +1291,18 @@ static void init(int argc, char **argv)
(arg_mode & (MODE_DAEMON | MODE_MWORKER | MODE_FOREGROUND | 
MODE_VERBOSE
 | MODE_QUIET | MODE_CHECK | MODE_DEBUG));
 
+   /* Master workers wait mode */
+   if ((global.mode & MODE_MWORKER) && (getenv("WAIT_ONLY") != NULL)) {
+
+   unsetenv("WAIT_ONLY");
+   mworker_wait();
+   }
+
+   if ((global.mode & MODE_MWORKER) && (getenv(REEXEC_FLAG) != NULL)) {
+   atexit_flag = 1;
+   atexit(reexec_on_failure);
+   }
+
if (change_dir && chdir(change_dir) < 0) {
Alert("Could not change to directory %s : %s\n", change_dir, 
strerror(errno));
exit(1);
@@ -2425,9 +2456,13 @@ int main(int argc, char **argv)
 #ifndef OPENSSL_NO_DH
ssl_free_dh();
 #endif
-   exit(0); /* parent must leave */
+   /* should never get there */
+   exit(EXIT_FAILURE);
}
 
+   /* child must never use the atexit function */
+   atexit_flag = 0;
+
/* Must chroot and setgid/setuid in the children */
/* chroot if needed */
if (global.chroot != NULL) {
-- 
2.10.2

[PATCH 5/9] MEDIUM: mworker: exit-on-failure option

[William Lallemand]

This option exits every workers when one of the current workers die.

It allows you to monitor the master process in order to relaunch
everything on a failure.

For example it can be used with systemd and Restart=on-failure in a spec
file.
---
 include/types/global.h |  1 +
 src/cfgparse.c | 11 ++-
 src/haproxy.c  |  5 +
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/types/global.h b/include/types/global.h
index ee8e95e..cce3de7 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -63,6 +63,7 @@
 #define GTUNE_RESOLVE_DONTFAIL   (1<<7)
 
 #define GTUNE_SOCKET_TRANSFER   (1<<8)
+#define GTUNE_EXIT_ONFAILURE (1<<9)
 
 /* Access level for a stats socket */
 #define ACCESS_LVL_NONE 0
diff --git a/src/cfgparse.c b/src/cfgparse.c
index bdf55a8..76a4f31 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -625,8 +625,17 @@ int cfg_parse_global(const char *file, int linenum, char 
**args, int kwm)
global.mode |= MODE_DAEMON;
}
else if (!strcmp(args[0], "master-worker")) {
-   if (alertif_too_many_args(0, file, linenum, args, _code))
+   if (alertif_too_many_args(1, file, linenum, args, _code))
goto out;
+   if (*args[1]) {
+   if (!strcmp(args[1], "exit-on-failure")) {
+   global.tune.options |= GTUNE_EXIT_ONFAILURE;
+   } else {
+   Alert("parsing [%s:%d] : '%s' only supports 
'exit-on-failure' option.\n", file, linenum, args[0]);
+   err_code |= ERR_ALERT | ERR_FATAL;
+   goto out;
+   }
+   }
global.mode |= MODE_MWORKER;
}
else if (!strcmp(args[0], "debug")) {
diff --git a/src/haproxy.c b/src/haproxy.c
index 57a5db6..d23bf3a 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -673,6 +673,11 @@ static void mworker_wait()
/* check if exited child was in the current children 
list */
if (current_child(exitpid)) {
Alert("Current worker %d left with exit code 
%d\n", exitpid, status);
+   if (status != 0 && status != 130 && status != 
143
+   && global.tune.options & 
GTUNE_EXIT_ONFAILURE) {
+   Alert("exit-on-failure: killing every 
workers with SIGTERM\n");
+   mworker_kill(SIGTERM);
+   }
} else {
Warning("Former worker %d left with exit code 
%d\n", exitpid, status);
delete_oldpid(exitpid);
-- 
2.10.2

[PATCH 4/9] MEDIUM: mworker: try to guess the next stats socket to use with -x

[William Lallemand]

In master worker mode, you can't specify the stats socket where you get
your listeners FDs on a reload, because the command line of the re-exec
is launched by the master.

To solve the problem, when -x is found on the command line, its
parameter is rewritten on a reexec with the first stats socket with the
capability to send sockets. It tries to reuse the original parameter if
it has this capability.
---
 src/haproxy.c | 74 +++
 1 file changed, 69 insertions(+), 5 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index 61114b6..57a5db6 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -175,6 +175,8 @@ static int oldpids_sig; /* use USR1 or TERM */
 /* Path to the unix socket we use to retrieve listener sockets from the old 
process */
 static const char *old_unixsocket;
 
+static char *cur_unixsocket = NULL;
+
 /* this is used to drain data, and as a temporary buffer for sprintf()... */
 struct chunk trash = { };
 
@@ -514,6 +516,42 @@ int delete_oldpid(int pid)
return 0;
 }
 
+
+static void get_cur_unixsocket()
+{
+   /* if -x was used, try to update the stat socket if not available 
anymore */
+   if (global.stats_fe) {
+   struct bind_conf *bind_conf;
+
+   /* pass through all stats socket */
+   list_for_each_entry(bind_conf, _fe->conf.bind, 
by_fe) {
+   struct listener *l;
+
+   list_for_each_entry(l, _conf->listeners, by_bind) {
+
+   if (l->addr.ss_family == AF_UNIX &&
+   (bind_conf->level & ACCESS_FD_LISTENERS)) {
+   const struct sockaddr_un *un;
+
+   un = (struct sockaddr_un *)>addr;
+   /* priority to old_unixsocket */
+   if (!cur_unixsocket) {
+   cur_unixsocket = 
strdup(un->sun_path);
+   } else {
+   if (old_unixsocket && 
!strcmp(un->sun_path, old_unixsocket)) {
+   free(cur_unixsocket);
+   cur_unixsocket = 
strdup(old_unixsocket);
+   return;
+   }
+   }
+   }
+   }
+   }
+   }
+   if (!cur_unixsocket && old_unixsocket)
+   cur_unixsocket = strdup(old_unixsocket);
+}
+
 /*
  * When called, this function reexec haproxy with -sf followed by current
  * children PIDs and possibily old children PIDs if they didn't leave yet.
@@ -532,8 +570,8 @@ static void mworker_reload()
while (next_argv[next_argc])
next_argc++;
 
-   /* 1 for haproxy -sf */
-   next_argv = realloc(next_argv, (next_argc + 1 + global.nbproc + 
nb_oldpids + 1) * sizeof(char *));
+   /* 1 for haproxy -sf, 2 for -x /socket */
+   next_argv = realloc(next_argv, (next_argc + 1 + 2 + global.nbproc + 
nb_oldpids + 1) * sizeof(char *));
if (next_argv == NULL)
goto alloc_error;
 
@@ -557,6 +595,26 @@ static void mworker_reload()
msg = NULL;
}
next_argv[next_argc] = NULL;
+
+   /* if -x was used, try to update the stat socket if not available 
anymore */
+   if (cur_unixsocket) {
+
+   if (old_unixsocket) {
+
+   /* look for -x  */
+   for (j = 0; next_argv[j]; j++) {
+   if (!strcmp(next_argv[j], "-x"))
+   next_argv[j + 1] = (char 
*)cur_unixsocket;
+   }
+   } else {
+   /* if -x is not specified but we know the socket, add 
-x with it */
+   next_argv[next_argc++] = "-x";
+   next_argv[next_argc++] = (char *)cur_unixsocket;
+   next_argv[next_argc++] = NULL;
+
+   }
+   }
+
deinit(); /* we don't want to leak FD there */
Warning("Reexecuting Master process\n");
execv(next_argv[0], next_argv);
@@ -2218,11 +2276,16 @@ int main(int argc, char **argv)
}
 
if (old_unixsocket) {
-   if (get_old_sockets(old_unixsocket) != 0) {
-   Alert("Failed to get the sockets from the old 
process!\n");
-   exit(1);
+   if (strcmp("/dev/null", old_unixsocket) != 0) {
+   if (get_old_sockets(old_unixsocket) != 0) {
+   Alert("Failed to get the sockets from the old 
process!\n");
+   if (!(global.mode & MODE_MWORKER))
+ 

[PATCH 1/9] MEDIUM: mworker: replace systemd mode by master worker mode

[William Lallemand]

This commit remove the -Ds systemd mode in HAProxy in order to replace
it by a more generic master worker system. It aims to replace entirely
the systemd wrapper in the near future.

The master worker mode implements a new way of managing HAProxy
processes. The master is in charge of parsing the configuration
file and is responsible for spawning child processes.

The master worker mode can be invoked by using the -W flag.  It can be
used either in background mode (-D) or foreground mode. When used in
background mode, the master will fork to daemonize.

In master worker background mode, chroot, setuid and setgid are done in
each child rather than in the master process, because the master process
will still need access to filesystem to reload the configuration.
---
 include/types/global.h |   2 +-
 src/cfgparse.c |   5 ++
 src/haproxy.c  | 136 +
 src/listener.c |   4 +-
 4 files changed, 99 insertions(+), 48 deletions(-)

diff --git a/include/types/global.h b/include/types/global.h
index aeb82ea..ee8e95e 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -44,7 +44,7 @@
 #defineMODE_VERBOSE0x10
 #defineMODE_STARTING   0x20
 #defineMODE_FOREGROUND 0x40
-#defineMODE_SYSTEMD0x80
+#defineMODE_MWORKER0x80/* Master Worker */
 
 /* list of last checks to perform, depending on config options */
 #define LSTCHK_CAP_BIND0x0001  /* check that we can bind to 
any port */
diff --git a/src/cfgparse.c b/src/cfgparse.c
index 4c0e2d4..bdf55a8 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -624,6 +624,11 @@ int cfg_parse_global(const char *file, int linenum, char 
**args, int kwm)
goto out;
global.mode |= MODE_DAEMON;
}
+   else if (!strcmp(args[0], "master-worker")) {
+   if (alertif_too_many_args(0, file, linenum, args, _code))
+   goto out;
+   global.mode |= MODE_MWORKER;
+   }
else if (!strcmp(args[0], "debug")) {
if (alertif_too_many_args(0, file, linenum, args, _code))
goto out;
diff --git a/src/haproxy.c b/src/haproxy.c
index 6ecae25..0e8511b 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -348,6 +348,7 @@ static void usage(char *name)
"-dM[] poisons memory with  (defaults to 
0x50)\n"
"-V enters verbose mode (disables quiet mode)\n"
"-D goes daemon ; -C changes to  before loading 
files.\n"
+   "-W master-worker mode.\n"
"-q quiet mode : don't display messages\n"
"-c check mode : only check config files and exit\n"
"-n sets the maximum total # of connections (%d)\n"
@@ -921,11 +922,10 @@ static void init(int argc, char **argv)
arg_mode |= MODE_DEBUG;
else if (*flag == 'c')
arg_mode |= MODE_CHECK;
-   else if (*flag == 'D') {
+   else if (*flag == 'D')
arg_mode |= MODE_DAEMON;
-   if (flag[1] == 's')  /* -Ds */
-   arg_mode |= MODE_SYSTEMD;
-   }
+   else if (*flag == 'W')
+   arg_mode |= MODE_MWORKER;
else if (*flag == 'q')
arg_mode |= MODE_QUIET;
else if (*flag == 'x') {
@@ -1001,7 +1001,7 @@ static void init(int argc, char **argv)
}
 
global.mode = MODE_STARTING | /* during startup, we want most of the 
alerts */
-   (arg_mode & (MODE_DAEMON | MODE_SYSTEMD | MODE_FOREGROUND | 
MODE_VERBOSE
+   (arg_mode & (MODE_DAEMON | MODE_MWORKER | MODE_FOREGROUND | 
MODE_VERBOSE
 | MODE_QUIET | MODE_CHECK | MODE_DEBUG));
 
if (change_dir && chdir(change_dir) < 0) {
@@ -1330,24 +1330,24 @@ static void init(int argc, char **argv)
 
if (arg_mode & (MODE_DEBUG | MODE_FOREGROUND)) {
/* command line debug mode inhibits configuration mode */
-   global.mode &= ~(MODE_DAEMON | MODE_SYSTEMD | MODE_QUIET);
+   global.mode &= ~(MODE_DAEMON | MODE_QUIET);
global.mode |= (arg_mode & (MODE_DEBUG | MODE_FOREGROUND));
}
 
-   if (arg_mode & (MODE_DAEMON | MODE_SYSTEMD)) {
+   if (arg_mode & MODE_DAEMON) {
/* command line daemon mode inhibits foreground and debug modes 
mode */
global.mode &= ~(MODE_DEBUG | MODE_FOREGROUND);
-   global.mode |= (arg_mode & (MODE_DAEMON | MODE_SYSTEMD));
+   global.mode |= arg_mode & MODE_DAEMON;
}
 
global.mode |= (arg_mode & (MODE_QUIET | 

Replace the systemd-wrapper by the master worker mode

[William Lallemand]

The master worker mode replaces the systemd wrapper, it does not need a
separated binary anymore, everything is builtin.

This mode will launch a "master" which will monitor the "workers". Using this
mode, you can reload HAProxy directly by sending a SIGUSR2 signal to the
master. The master-worker mode is compatible either with the foreground or
daemon mode. It is recommended to use this mode with multiprocess and/or 
systemd.

The master take advantage of the seamless reload feature (-x on the command
line), it will try to add automatically this option during a reload when a
stats socket exposes its listeners. The seamless reload is transparent.

Re: [RFC][PATCHES] seamless reload

[William Lallemand]

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_ADMIN3
+#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(, "user ");
-		else if (bind_conf->level == ACCESS_LVL_OPER)
-			chunk_appendf(, "operator ");
-		else if (bind_conf->level == ACCESS_LVL_ADMIN)
+		if ((bind_conf->level & ACCESS_LVL_MASK) == ACCESS_LVL_ADMIN)
 			chunk_appendf(, "admin ");
+		else if ((bind_conf->level & ACCESS_LVL_MASK) == ACCESS_LVL_OPER)
+			chunk_appendf(, "operator ");
+		else if ((bind_conf->level & ACCESS_LVL_MASK) == ACCESS_LVL_USER)
+			chunk_appendf(, "user ");
 		else
 			chunk_appendf(, "  ");
 
@@ -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(ar

Re: haproxy doesn't restart after segfault on systemd

[William Lallemand]

Hi again,

On Fri, May 19, 2017 at 11:45:18AM +0200, William Lallemand wrote:
> Hi,
> 
> On Thu, May 18, 2017 at 05:25:26PM -0400, Patrick Hemmer wrote:
> > So we had an incident today where haproxy segfaulted and our site went
> > down. Unfortunately we did not capture a core, and the segfault message
> > logged to dmesg just showed it inside libc. So there's likely not much
> > we can do here. We'll be making changes to ensure we capture a core in
> > the future.
> > 
> > However the issue I am reporting that is reproducible (on version 1.7.5)
> > is that haproxy did not auto restart, which would have minimized the
> > downtime to the site. We use nbproc > 1, so we have multiple haproxy
> > processes running, and when one of them dies, neither the
> > "haproxy-master" process or the "haproxy-systemd-wrapper" process exits,
> > which prevents systemd from starting the service back up.
> > 
> 
> In fact the systemd wrapper was designed as a hack to allow a 'systemd 
> reload'.
> Without it, we were having systemd thinking that everything crashed.
> 
> There wasn't any big architecture change with this feature, the systemd 
> wrapper
> is not aware at all of which haproxy processes are started, and the
> haproxy-master only do a waitpid once he's started.
> 
> In future versions of HAProxy, the systemd-wrapper will disappear and will be
> merged within the haproxy binary, it will allow to have a single binary which
> works in a master-worker mode.
> 
> > While I think this behavior would be fine, a possible alternative would
> > be for the "haproxy-master" process to restart the dead worker without
> > having to kill all the other processes.
> 
> It's not easy to relaunch only one process once everything is parsed and 
> ready,
> because a lot of stuff is free-ed. The master already lost the configuration 
> at
> this step. I don't see how we can implement this in a proper way.
> 
> > 
> > Another possible action would be to leave the workers running, but
> > signal them to stop accepting new connections, and then let the
> > "haproxy-master" exit so systemd will restart it.
> > 
> 
> Something feasible could be to have a configuration keyword which allow the
> master to reload when the number of active processes is below the value.
> 
> But it can be dangerous, for example if you have a RAM problem on your server,
> and the HAProxy processes keep being OOM, it will reload each time and will be
> an infinite loop. So I don't know if that's a good idea.
> 
> And you don't want your stats to disappear without asking a reload yourself.
> 
> 
> > But in any case, I think we need some way of handling this so that site
> > interruption is minimal.
> > 

I had a discussion with Willy about that, and we though that it's not a good
idea to let HAProxy restart by itself , because it will reduce the amount of
bug report and can cause other problems.

However, we though about another way to do that, we can add an option to the
master process, which can kill the master and every active processes when one
of the process segv. It will allow systemd to be notified of the crash, and you
will be able to restart everything with it, using Restart=always for example.

-- 
William Lallemand

Re: haproxy doesn't restart after segfault on systemd

[William Lallemand]

Hi,

On Thu, May 18, 2017 at 05:25:26PM -0400, Patrick Hemmer wrote:
> So we had an incident today where haproxy segfaulted and our site went
> down. Unfortunately we did not capture a core, and the segfault message
> logged to dmesg just showed it inside libc. So there's likely not much
> we can do here. We'll be making changes to ensure we capture a core in
> the future.
> 
> However the issue I am reporting that is reproducible (on version 1.7.5)
> is that haproxy did not auto restart, which would have minimized the
> downtime to the site. We use nbproc > 1, so we have multiple haproxy
> processes running, and when one of them dies, neither the
> "haproxy-master" process or the "haproxy-systemd-wrapper" process exits,
> which prevents systemd from starting the service back up.
> 

In fact the systemd wrapper was designed as a hack to allow a 'systemd reload'.
Without it, we were having systemd thinking that everything crashed.

There wasn't any big architecture change with this feature, the systemd wrapper
is not aware at all of which haproxy processes are started, and the
haproxy-master only do a waitpid once he's started.

In future versions of HAProxy, the systemd-wrapper will disappear and will be
merged within the haproxy binary, it will allow to have a single binary which
works in a master-worker mode.

> While I think this behavior would be fine, a possible alternative would
> be for the "haproxy-master" process to restart the dead worker without
> having to kill all the other processes.

It's not easy to relaunch only one process once everything is parsed and ready,
because a lot of stuff is free-ed. The master already lost the configuration at
this step. I don't see how we can implement this in a proper way.

> 
> Another possible action would be to leave the workers running, but
> signal them to stop accepting new connections, and then let the
> "haproxy-master" exit so systemd will restart it.
> 

Something feasible could be to have a configuration keyword which allow the
master to reload when the number of active processes is below the value.

But it can be dangerous, for example if you have a RAM problem on your server,
and the HAProxy processes keep being OOM, it will reload each time and will be
an infinite loop. So I don't know if that's a good idea.

And you don't want your stats to disappear without asking a reload yourself.


> But in any case, I think we need some way of handling this so that site
> interruption is minimal.
> 
> -Patrick

-- 
William Lallemand

[PATCH v1 RFC 4/5] MEDIUM: mworkers: don't refork on daemon mode

[William Lallemand]

The master won't change PID anymore in master workers + daemon mode.
---
 src/haproxy.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index 5e3028d..e0b6462 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -157,6 +157,8 @@ struct global global = {
 
 /*/
 
+#define REEXEC_FLAG "HAPROXY_MWORKERS_REEXEC"
+
 int stopping;  /* non zero means stopping in progress */
 int killed;/* non zero means a hard-stop is triggered */
 int jobs = 0;   /* number of active jobs (conns, listeners, active tasks, ...) 
*/
@@ -444,6 +446,8 @@ static void master_reload()
signal(SIGINT,  SIG_DFL);
signal(SIGTERM, SIG_DFL);
 
+   setenv(REEXEC_FLAG, "1", 1);
+
/* compute length  */
while (next_argv[next_argc])
next_argc++;
@@ -2081,6 +2085,7 @@ int main(int argc, char **argv)
char errmsg[100];
int pidfd = -1;
 
+
init(argc, argv);
signal_register_fct(SIGQUIT, dump, SIGQUIT);
signal_register_fct(SIGUSR1, sig_soft_stop, SIGUSR1);
@@ -2306,8 +2311,9 @@ int main(int argc, char **argv)
 * process live in background before forking children
 */
 
-   // TODO: don't refork if it's a reexec
-   if ((global.mode & MODE_MWORKER) && (global.mode & 
MODE_DAEMON)) {
+   if ((getenv(REEXEC_FLAG) == NULL)
+   && (global.mode & MODE_MWORKER)
+   && (global.mode & MODE_DAEMON)) {
ret = fork();
if (ret < 0) {
Alert("[%s.main()] Cannot fork.\n", argv[0]);
-- 
2.10.2

[PATCH v1 RFC 1/5] MEDIUM: mworkers: replace systemd mode by master workers mode

[William Lallemand]

This commit remove the -Ds systemd mode in HAProxy in order to replace
it by a more generic master workers system. It aims to replace entirely
the systemd wrapper in the near future.

The master workers mode implements a new way of managing HAProxy
processes. The master is in charge of parsing the configuration
file and is responsible for spawning child processes.

The master workers mode can be invoked by using the -W flag.  It can be
used either in background mode (-D) or foreground mode. When used in
background mode, the master will fork to daemonize.

In master workers background mode, chroot, setuid and setgid are done in
each child rather than in the master process, because the master process
will still need access to filesystem to reload the configuration.
---
 include/types/global.h |   2 +-
 src/haproxy.c  | 133 +
 src/listener.c |   4 +-
 3 files changed, 93 insertions(+), 46 deletions(-)

diff --git a/include/types/global.h b/include/types/global.h
index 57b969d..4af1212 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -44,7 +44,7 @@
 #defineMODE_VERBOSE0x10
 #defineMODE_STARTING   0x20
 #defineMODE_FOREGROUND 0x40
-#defineMODE_SYSTEMD0x80
+#defineMODE_MWORKER0x80/* Master Worker */
 
 /* list of last checks to perform, depending on config options */
 #define LSTCHK_CAP_BIND0x0001  /* check that we can bind to 
any port */
diff --git a/src/haproxy.c b/src/haproxy.c
index 02f90a8..8c73613 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -921,11 +921,10 @@ static void init(int argc, char **argv)
arg_mode |= MODE_DEBUG;
else if (*flag == 'c')
arg_mode |= MODE_CHECK;
-   else if (*flag == 'D') {
+   else if (*flag == 'D')
arg_mode |= MODE_DAEMON;
-   if (flag[1] == 's')  /* -Ds */
-   arg_mode |= MODE_SYSTEMD;
-   }
+   else if (*flag == 'W')
+   arg_mode |= MODE_MWORKER;
else if (*flag == 'q')
arg_mode |= MODE_QUIET;
else if (*flag == 'x') {
@@ -1001,7 +1000,7 @@ static void init(int argc, char **argv)
}
 
global.mode = MODE_STARTING | /* during startup, we want most of the 
alerts */
-   (arg_mode & (MODE_DAEMON | MODE_SYSTEMD | MODE_FOREGROUND | 
MODE_VERBOSE
+   (arg_mode & (MODE_DAEMON | MODE_MWORKER | MODE_FOREGROUND | 
MODE_VERBOSE
 | MODE_QUIET | MODE_CHECK | MODE_DEBUG));
 
if (change_dir && chdir(change_dir) < 0) {
@@ -1330,24 +1329,24 @@ static void init(int argc, char **argv)
 
if (arg_mode & (MODE_DEBUG | MODE_FOREGROUND)) {
/* command line debug mode inhibits configuration mode */
-   global.mode &= ~(MODE_DAEMON | MODE_SYSTEMD | MODE_QUIET);
+   global.mode &= ~(MODE_DAEMON | MODE_QUIET);
global.mode |= (arg_mode & (MODE_DEBUG | MODE_FOREGROUND));
}
 
-   if (arg_mode & (MODE_DAEMON | MODE_SYSTEMD)) {
+   if (arg_mode & MODE_DAEMON) {
/* command line daemon mode inhibits foreground and debug modes 
mode */
global.mode &= ~(MODE_DEBUG | MODE_FOREGROUND);
-   global.mode |= (arg_mode & (MODE_DAEMON | MODE_SYSTEMD));
+   global.mode |= arg_mode & MODE_DAEMON;
}
 
global.mode |= (arg_mode & (MODE_QUIET | MODE_VERBOSE));
 
-   if ((global.mode & MODE_DEBUG) && (global.mode & (MODE_DAEMON | 
MODE_SYSTEMD | MODE_QUIET))) {
-   Warning(" mode incompatible with ,  and 
. Keeping  only.\n");
-   global.mode &= ~(MODE_DAEMON | MODE_SYSTEMD | MODE_QUIET);
+   if ((global.mode & MODE_DEBUG) && (global.mode & (MODE_DAEMON | 
MODE_QUIET))) {
+   Warning(" mode incompatible with  and . 
Keeping  only.\n");
+   global.mode &= ~(MODE_DAEMON | MODE_QUIET);
}
 
-   if ((global.nbproc > 1) && !(global.mode & (MODE_DAEMON | 
MODE_SYSTEMD))) {
+   if ((global.nbproc > 1) && !(global.mode & (MODE_DAEMON | 
MODE_MWORKER))) {
if (!(global.mode & (MODE_FOREGROUND | MODE_DEBUG)))
Warning(" is only meaningful in daemon mode. 
Setting limit to 1 process.\n");
global.nbproc = 1;
@@ -2020,7 +2019,7 @@ int main(int argc, char **argv)
}
 
/* open log & pid files before the chroot */
-   if (global.mode & (MODE_DAEMON | MODE_SYSTEMD) && global.pidfile != 
NULL) {
+   if (global.mode & MODE_DAEMON && global.pidfile != NULL) {
unlink(global.pidfile);
pidfd = open(global.pidfile, O_CREAT | O_WRONLY 

[PATCH v1 RFC 2/5] MEDIUM: mworkers: reload processes on SIGUSR2

[William Lallemand]

This behavior allows to reload HAProxy using the master works mode.

It's inherited from the systemd wrapper, when the SIGUSR2 signal is
received, the master process will reexecute itself with the -sf flag
followed by the PIDs of the children.
---
 src/haproxy.c | 135 +++---
 1 file changed, 130 insertions(+), 5 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index 8c73613..d2b5d4c 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -193,6 +193,11 @@ char localpeer[MAX_HOSTNAME_LEN];
  */
 int shut_your_big_mouth_gcc_int = 0;
 
+int *children; /* store PIDs of children in master workers mode */
+
+static volatile sig_atomic_t caught_signal = 0;
+static char **next_argv = NULL;
+
 /* list of the temporarily limited listeners because of lack of resource */
 struct list global_listener_queue = LIST_HEAD_INIT(global_listener_queue);
 struct task *global_listener_queue_task;
@@ -387,6 +392,80 @@ static void usage(char *name)
 /*   more specific functions   ***/
 /*/
 
+/* return 1 if a pid is a current child otherwise 0 */
+
+int current_child(int pid)
+{
+   int i;
+
+   for (i = 0; i < global.nbproc; i++) {
+   if (children[i] == pid)
+   return 1;
+   }
+   return 0;
+}
+
+
+/*
+ * When called, this function add -sf followed by current children PIDs and
+ * possibily old children PIDs if they didn't leave yet.
+ */
+
+
+static void master_reload()
+{
+   int next_argc = 0;
+   int j;
+
+   signal(SIGUSR1, SIG_IGN);
+   signal(SIGUSR2, SIG_IGN);
+   signal(SIGHUP,  SIG_IGN);
+   signal(SIGINT,  SIG_DFL);
+   signal(SIGTERM, SIG_DFL);
+
+   /* compute length  */
+   while (next_argv[next_argc])
+   next_argc++;
+
+   /* 1 for haproxy -sf */
+   next_argv = realloc(next_argv, (next_argc + 1 + global.nbproc + 1) * 
sizeof(char *));
+   if (next_argv == NULL)
+   goto alloc_error;
+
+   if (children) {
+   char *msg = NULL;
+
+   /* add -sf *  to argv */
+   next_argv[next_argc++] = "-sf";
+   for (j = 0; j < global.nbproc; next_argc++,j++) {
+   next_argv[next_argc] = memprintf(, "%d", 
children[j]);
+   if (next_argv[next_argc] == NULL)
+   goto alloc_error;
+   msg = NULL;
+   }
+   next_argv[next_argc] = NULL;
+   }
+   Warning("Reexecuting Master process [%d]\n", pid);
+   execv(next_argv[0], next_argv);
+
+alloc_error:
+   Warning("Cannot allocate memory\n");
+   Warning("Failed to reexecute the master processs [%d]\n", pid);
+   return;
+}
+
+/*
+ * In master-workers mode, SIGUSR2 on the master will reexec itself to ensure
+ * that the binary is up to date in memory, launch new processes and kill the
+ * old ones.
+ */
+
+static void sig_master_usr2(int signum)
+{
+   Warning("Received signal USR2, reloading...\n");
+   caught_signal = signum;
+}
+
 /*
  * upon SIGUSR1, let's have a soft stop. Note that soft_stop() broadcasts
  * a signal zero to all subscribers. This means that it's as easy as
@@ -776,6 +855,39 @@ out:
 }
 
 /*
+ * copy and cleanup the current argv
+ * Remove the -sf /-st parameters
+ * Return an allocated copy of argv
+ */
+
+static char **copy_argv(int argc, char **argv)
+{
+   char **newargv;
+   int i, j;
+
+   newargv = calloc(argc + 1, sizeof(char *));
+   if (newargv == NULL) {
+   Warning("Cannot allocate memory\n");
+   return NULL;
+   }
+
+   for (i = 0, j = 0; i < argc; i++, j++) {
+   char *flag = *(argv + i) + 1;
+
+   /* -sf or -st */
+   if (*flag == 's' && (flag[1] == 'f' || flag[1] == 't')) {
+   /* list of pids to finish ('f') or terminate ('t') */
+   i++;
+   while (i < argc && argv[i][0] != '-') {
+   i++;
+   }
+   }
+   newargv[j] = argv[i];
+   }
+   return newargv;
+}
+
+/*
  * This function initializes all the necessary variables. It only returns
  * if everything is OK. If something fails, it exits.
  */
@@ -792,6 +904,8 @@ static void init(int argc, char **argv)
struct proxy *px;
struct post_check_fct *pcf;
 
+   next_argv = copy_argv(argc, argv);
+
chunk_init(, malloc(global.tune.bufsize), global.tune.bufsize);
alloc_trash_buffers(global.tune.bufsize);
 
@@ -2099,10 +2213,10 @@ int main(int argc, char **argv)
struct proxy *px;
struct peers *curpeers;
int ret = 0;
-   int *children = calloc(global.nbproc, sizeof(int));
int proc;
char 

[PATCH v1 RFC 5/5] MEDIUM: mworkers: wait mode on reload failure

[William Lallemand]

In Master Workers mode, when the reloading of the configuration fail,
the process is exiting leaving the children without their father.

To handle this, we register an exit function with atexit(3), which is
reexecuting the binary in a special mode. This particular mode of
HAProxy don't reload the configuration, it only loops on wait().
---
 src/haproxy.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/src/haproxy.c b/src/haproxy.c
index e0b6462..8a815f1 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -553,6 +553,19 @@ static void mworkers_wait()
 }
 
 
+/*
+ * Reexec the process in failure mode, instead of exiting
+ */
+void reexec_on_failure()
+{
+   setenv("WAIT_ONLY", "1", 1);
+
+   Warning("Reexecuting Master process in waitpid mode\n");
+   master_reload();
+
+   Warning("Failed to reexecute the master processs\n");
+}
+
 
 /*
  * upon SIGUSR1, let's have a soft stop. Note that soft_stop() broadcasts
@@ -1205,6 +1218,17 @@ static void init(int argc, char **argv)
(arg_mode & (MODE_DAEMON | MODE_MWORKER | MODE_FOREGROUND | 
MODE_VERBOSE
 | MODE_QUIET | MODE_CHECK | MODE_DEBUG));
 
+   /* Master workers wait mode */
+   if ((global.mode & MODE_MWORKER) && (getenv("WAIT_ONLY") != NULL)) {
+
+   unsetenv("WAIT_ONLY");
+   mworkers_wait();
+   }
+
+   if ((global.mode & MODE_MWORKER) && (getenv(REEXEC_FLAG) != NULL)) {
+   atexit(reexec_on_failure);
+   }
+
if (change_dir && chdir(change_dir) < 0) {
Alert("Could not change to directory %s : %s\n", change_dir, 
strerror(errno));
exit(1);
-- 
2.10.2

RFC: Master Workers model

[William Lallemand]

This is a work in progress set of patches.

The current systemd model works this way:

- systemd-wrapper
  ` haproxy master
 ` haproxy child
 ` haproxy child
 ` haproxy child

The master workers model will allow you to have the same architecture, but
without the systemd wrapper at the top.

In practice:

* You won't need an additional binary
* It will work either in background or foreground mode (systemd compatibility)
* You can send directly the USR2 signal to the master to reload
* Use the same exit codes as the systemd-wrapper
* The master manage the parsing of the configuration file
* The master is aware of current and previous children

Near future:

* (TODO) Children will exit at the exit of the master
* (TODO) Check if the children are ready on a reload
* (TODO) Transparent use of the new -x option for seamless reload

Ideas/unplanned/distant future:

* Stop using kill to reload and use a synchronous method to return a status 
code to the init.
* Have a poll loop in the master for signals and stuff.

Don't hesitate to comment. The documentation will come with future patches.

[PATCH v1 RFC 3/5] MEDIUM: mworkers: old PIDs awereness

[William Lallemand]

The master process is now able to manage old processes when they didn't
quit on a reload. The master won't exit if every current processes has
left but there are still old process.

There is now a notification message when a child exits, either an old or
a current process.

When every workers have left, the master leaves with an appropriate
return code (which is the same as the systemd wrapper).
---
 src/haproxy.c | 124 +-
 1 file changed, 96 insertions(+), 28 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index d2b5d4c..5e3028d 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -193,7 +193,7 @@ char localpeer[MAX_HOSTNAME_LEN];
  */
 int shut_your_big_mouth_gcc_int = 0;
 
-int *children; /* store PIDs of children in master workers mode */
+int *children = NULL; /* store PIDs of children in master workers mode */
 
 static volatile sig_atomic_t caught_signal = 0;
 static char **next_argv = NULL;
@@ -407,6 +407,26 @@ int current_child(int pid)
 
 
 /*
+ * remove a pid forom the olpid array and decrease nb_oldpids
+ * return 1 pid was found otherwise return 0
+ */
+
+int delete_oldpid(int pid)
+{
+   int i;
+
+   for (i = 0; i < nb_oldpids; i++) {
+   if (oldpids[i] == pid) {
+   oldpids[i] = oldpids[nb_oldpids - 1];
+   oldpids[nb_oldpids - 1] = 0;
+   nb_oldpids--;
+   return 1;
+   }
+   }
+   return 0;
+}
+
+/*
  * When called, this function add -sf followed by current children PIDs and
  * possibily old children PIDs if they didn't leave yet.
  */
@@ -416,6 +436,7 @@ static void master_reload()
 {
int next_argc = 0;
int j;
+   char *msg = NULL;
 
signal(SIGUSR1, SIG_IGN);
signal(SIGUSR2, SIG_IGN);
@@ -428,24 +449,31 @@ static void master_reload()
next_argc++;
 
/* 1 for haproxy -sf */
-   next_argv = realloc(next_argv, (next_argc + 1 + global.nbproc + 1) * 
sizeof(char *));
+   next_argv = realloc(next_argv, (next_argc + 1 + global.nbproc + 
nb_oldpids + 1) * sizeof(char *));
if (next_argv == NULL)
goto alloc_error;
 
-   if (children) {
-   char *msg = NULL;
 
-   /* add -sf *  to argv */
+   /* add -sf *  to argv */
+   if (children || nb_oldpids > 0)
next_argv[next_argc++] = "-sf";
+   if (children) {
for (j = 0; j < global.nbproc; next_argc++,j++) {
next_argv[next_argc] = memprintf(, "%d", 
children[j]);
if (next_argv[next_argc] == NULL)
goto alloc_error;
msg = NULL;
}
-   next_argv[next_argc] = NULL;
}
-   Warning("Reexecuting Master process [%d]\n", pid);
+   /* copy old process PIDs */
+   for (j = 0; j < nb_oldpids; next_argc++,j++) {
+   next_argv[next_argc] = memprintf(, "%d", oldpids[j]);
+   if (next_argv[next_argc] == NULL)
+   goto alloc_error;
+   msg = NULL;
+   }
+   next_argv[next_argc] = NULL;
+   Warning("Reexecuting Master process\n");
execv(next_argv[0], next_argv);
 
 alloc_error:
@@ -459,7 +487,6 @@ alloc_error:
  * that the binary is up to date in memory, launch new processes and kill the
  * old ones.
  */
-
 static void sig_master_usr2(int signum)
 {
Warning("Received signal USR2, reloading...\n");
@@ -467,6 +494,63 @@ static void sig_master_usr2(int signum)
 }
 
 /*
+ * Wait for every children to exit
+ */
+
+static void mworkers_wait()
+{
+   int exitpid = -1;
+   int status = 0;
+   struct sigaction sa;
+
+   /* Here we are not using the haproxy async way
+   for signals because it does not exists in
+   the master */
+   memset(, 0, sizeof(struct sigaction));
+   sa.sa_handler = _master_usr2;
+   sigaction(SIGUSR2, , NULL);
+
+   while (1) {
+
+   while (((exitpid = wait()) == -1) && errno == EINTR) {
+   int sig = caught_signal;
+   if (sig) {
+   caught_signal = 0;
+   master_reload();
+   }
+   }
+
+   if (exitpid == -1 && errno == ECHILD) {
+   Warning("All workers are left. Leaving... (%d)\n", 
status);
+   exit(status); /* parent must leave using the latest 
status code known */
+   }
+
+   if (WIFEXITED(status))
+   status = WEXITSTATUS(status);
+   else if (WIFSIGNALED(status))
+   status = 128 + WTERMSIG(status);
+   else if (WIFSTOPPED(status))
+   status = 128 + WSTOPSIG(status);
+   else
+   status = 255;
+
+  

Re: 'set-dst' and 'set-dst-port' tcp/http actions

[William Lallemand]

On Tue, May 31, 2016 at 05:28:13PM -0700, Derek Brown wrote:
> I'll give them a try.  Are they to be applied to the latest 1.7-dev branch?
> 

That's right.

-- 
William Lallemand

'set-dst' and 'set-dst-port' tcp/http actions

[William Lallemand]

Hello,

Here a set of patches implementing http/tcp set-{dst,src}[-port].

The feature can be useful to connect to a IP/port which is defined in a map.

Regards,

-- 
William Lallemand
>From 4f44c85bd02830d690539b269a4669b6bd251e44 Mon Sep 17 00:00:00 2001
From: William Lallemand <wlallem...@irq6.net>
Date: Wed, 25 May 2016 01:48:42 +0200
Subject: [PATCH 1/4] MEDIUM: tcp: add 'set-src' to 'tcp-request connection'

The 'set-src' action was not available for tcp actions The action code
has been converted into a function in proto_tcp.c to be used for both
'http-request' and 'tcp-request connection' actions.

Both http and tcp keywords are registered in proto_tcp.c
---
 doc/configuration.txt  | 15 +++
 include/types/action.h |  1 -
 src/proto_http.c   | 61 +++--
 src/proto_tcp.c| 73 --
 4 files changed, 90 insertions(+), 60 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index c4df56e..f94885f 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -8674,6 +8674,21 @@ tcp-request connection  [{if | unless} ]
 an error occurs, this action silently fails and the actions evaluation
 continues.
 
+- set-src  :
+  Is used to set the source IP address to the value of specified
+  expression. Useful if you want to mask source IP for privacy.
+  If you want to provide an IP from a HTTP header use "http-request
+  set-src"
+
+ Is a standard HAProxy expression formed by a sample-fetch
+   followed by some converters.
+
+  Example:
+
+ tcp-request connection set-src src,ipmask(24)
+
+  When set-src is successful, the source port is set to 0.
+
 - "silent-drop" :
 This stops the evaluation of the rules and makes the client-facing
 connection suddenly disappear using a system-dependant way that tries
diff --git a/include/types/action.h b/include/types/action.h
index b97f9bf..742252e 100644
--- a/include/types/action.h
+++ b/include/types/action.h
@@ -80,7 +80,6 @@ enum act_name {
 	/* http request actions. */
 	ACT_HTTP_REQ_TARPIT,
 	ACT_HTTP_REQ_AUTH,
-	ACT_HTTP_REQ_SET_SRC,
 
 	/* tcp actions */
 	ACT_TCP_EXPECT_PX,
diff --git a/src/proto_http.c b/src/proto_http.c
index d79a782..6686631 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -3554,26 +3554,6 @@ resume_execution:
 			}
 			break;
 
-		case ACT_HTTP_REQ_SET_SRC:
-			if ((cli_conn = objt_conn(sess->origin)) && conn_ctrl_ready(cli_conn)) {
-struct sample *smp;
-
-smp = sample_fetch_as_type(px, sess, s, SMP_OPT_DIR_REQ|SMP_OPT_FINAL, rule->arg.expr, SMP_T_ADDR);
-
-if (smp) {
-	if (smp->data.type == SMP_T_IPV4) {
-		((struct sockaddr_in *)_conn->addr.from)->sin_family = AF_INET;
-		((struct sockaddr_in *)_conn->addr.from)->sin_addr.s_addr = smp->data.u.ipv4.s_addr;
-		((struct sockaddr_in *)_conn->addr.from)->sin_port = 0;
-	} else if (smp->data.type == SMP_T_IPV6) {
-		((struct sockaddr_in6 *)_conn->addr.from)->sin6_family = AF_INET6;
-		memcpy(&((struct sockaddr_in6 *)_conn->addr.from)->sin6_addr, >data.u.ipv6, sizeof(struct in6_addr));
-		((struct sockaddr_in6 *)_conn->addr.from)->sin6_port = 0;
-	}
-}
-			}
-			break;
-
 		/* other flags exists, but normaly, they never be matched. */
 		default:
 			break;
@@ -9204,39 +9184,6 @@ struct act_rule *parse_http_req_cond(const char **args, const char *file, int li
 		proxy->conf.lfs_line = proxy->conf.args.line;
 
 		cur_arg += 2;
-	} else if (strncmp(args[0], "set-src", 7) == 0) {
-		struct sample_expr *expr;
-		unsigned int where;
-		char *err = NULL;
-
-		cur_arg = 1;
-		proxy->conf.args.ctx = ARGC_HRQ;
-
-		expr = sample_parse_expr((char **)args, _arg, file, linenum, , >conf.args);
-		if (!expr) {
-			Alert("parsing [%s:%d] : error detected in %s '%s' while parsing 'http-request %s' rule : %s.\n",
-			  file, linenum, proxy_type_str(proxy), proxy->id, args[0], err);
-			free(err);
-			goto out_err;
-		}
-
-		where = 0;
-		if (proxy->cap & PR_CAP_FE)
-			where |= SMP_VAL_FE_HRQ_HDR;
-		if (proxy->cap & PR_CAP_BE)
-			where |= SMP_VAL_BE_HRQ_HDR;
-
-		if (!(expr->fetch->val & where)) {
-			Alert("parsing [%s:%d] : error detected in %s '%s' while parsing 'http-request %s' rule :"
-			  " fetch method '%s' extracts information from '%s', none of which is available here.\n",
-			  file, linenum, proxy_type_str(proxy), proxy->id, args[0],
-			  args[cur_arg-1], sample_src_names(expr->fetch->use));
-			free(expr);
-			goto out_err;
-		}
-
-		rule->arg.expr = expr;
-		rule->action = ACT_HTTP_REQ_SET_SRC;
 	} else if (((custom = action_http_req_custom(args[0])) != NULL)) {
 		char *errmsg = NULL;
 		cur_arg = 1;
@@ -9253,8 +92

Re: dynamically choosing back-end port

[William Lallemand]

Hi Derek,

I have the same need and I'm working on the 'http-request set-dst' and
'http-request set-dst-port' actions to cover it.

It would work in the same way as 'http-request set-src' do with the evaluation
of an expression.


On Thu, May 12, 2016 at 12:15:48PM -0700, Derek Brown wrote:
> Hi-
> 
> I'm wondering if you need any additional information, or if I can provide
> any clarification, to get a response to my query.
> 
> Thanks, in advance
> Derek
> 

-- 
William Lallemand

Re: Complete rewrite of HAProxy in Lua

[William Lallemand]

On Wed, Apr 01, 2015 at 10:43:47AM +0200, Willy Tarreau wrote:
 In the end, of the current HAProxy will only remain the Lua engine, and
 probably by then we'll find even better ones so that haproxy will be
 distributed as a Lua library to use anywhere, maybe even on IoT devices
 if that makes sense (anyone ever dreamed of having haproxy in their
 watches ?).

That's good news, we'll finally get ride of these filthy pointers!
 

-- 
William Lallemand

Re: Compile ZLIB in OpenBSD 5.4

[William Lallemand]

On Fri, 4 Apr 2014 18:36:10 -0300
Jorge Severino severino.jo...@gmail.com wrote:

 root@haproxy01 $ make TARGET=openbsd CPU=native USE_ZLIB=1
 ZLIB_INC=/usr/include ZLIB_LIB=/usr/lib
 *** Parse error in /tmp/haproxy-1.5-dev22: Missing dependency operator
 (Makefile:202)
 *** Parse error: Need an operator in 'else' (Makefile:206)
 *** Parse error: Missing dependency operator (Makefile:207)
 *** Parse error: Need an operator in 'else' (Makefile:213)
 [...]

You should install and use GNU Make, not the BSD one.


-- 
William Lallemand

Re: Log format with odd number of quotes. Bug or conf error?

[William Lallemand]

On Mon, 10 Mar 2014 17:30:27 -0400
Julien Vehent jul...@linuxwall.info wrote:

 Thanks Thierry ! It is trivial indeed, but like you I was concerned 
 about triggering another issue by removing it...
 
 What about the second issue I'm seeing? Should request/response headers 
 be completely absent from the logs when empty? And is there a way to 
 force a  or - sign when they are empty, to play nice with log parsers?
 
 Thanks,
 Julien
 

It's a bug, indeed, and thierry's patch is the right way to fix it.


-- 
William Lallemand

Re: junk at end of syslog packets in 1.5dev19

[William Lallemand]

On Thu, Aug 29, 2013 at 10:50:09PM +0200, Lukas Tribus wrote:
 The behavior also depends on the syslog facility; I used syslog in my tests.
 
 
 With the current git tree I see the following behavior:
 ntp and local7: syslog ends with \n -- correct behavior!
 ftp and user: syslog ends with \n + 2 random characters
 kern: syslog ends with \n + 4 random characters
 
 It seems 2a4a44f0f9f caused the initial problem and bfb099c3b3f1 amplified it.
 
 
 
 Regards,
 
 Lukas   

Hello Lukas,

Thanks for your analyze, the bug was indeed caused by the number of digits in 
the facility.

Here's a patch with the real size to send :)


-- 
William Lallemand
From fcf527a2c629b696e8e2fa2993feb5be90b9ef04 Mon Sep 17 00:00:00 2001
From: William Lallemand wlallem...@exceliance.fr
Date: Fri, 30 Aug 2013 14:17:46 +0200
Subject: [PATCH] BUG/MINOR: log: junk at the end of syslog packet

With a facily of 2 or 1 digit, the send size was wrong and bytes with
unknown value were sent.
The size was calculated using the start of the buffer and not the start
of the data which varies with the number of digits of the facility.

This bug was reported by Samuel Stoller and reported by Lukas Tribus.
---
 src/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/log.c b/src/log.c
index 369dc34..ad9fa73 100644
--- a/src/log.c
+++ b/src/log.c
@@ -847,7 +847,7 @@ void __send_log(struct proxy *p, int level, char *message, size_t size)
 		} while (fac_level  log_ptr  dataptr);
 		*log_ptr = '';
 
-		sent = sendto(*plogfd, log_ptr, size + log_ptr - dataptr,
+		sent = sendto(*plogfd, log_ptr, size - (log_ptr - dataptr),
 			  MSG_DONTWAIT | MSG_NOSIGNAL,
 			  (struct sockaddr *)logsrv-addr, get_addr_len(logsrv-addr));
 		if (sent  0) {
-- 
1.8.1.5

Re: junk at end of syslog packets in 1.5dev19

[William Lallemand]

On Thu, Aug 22, 2013 at 10:18:46AM +0200, Lukas Tribus wrote:
  Is this happen randomly or can you pin point this to specifc requests, maybe
  errors/timeouts? How can we reproduce this?
 
 Nevermind, its easily reproducible (just generate some syslog messages).
 
 The whole thing seems random: most of the times, the syslog msg ends
 with \n\0\0, other times with \n\n\0 or even \n\n.
 
 
 I've tracked this down to:
 

Hello,

I can't reproduce the bug, how did you compile HAProxy?

What is your configuration file?

Thanks,

-- 
William Lallemand

Re: haproxy duplicate http_request_counter values (BUG)

[William Lallemand]

On Tue, Aug 20, 2013 at 04:14:05PM -0400, Patrick Hemmer wrote:
 I see 2 ways of handling this.
 1) Move the code that populates the session unique_id member to
 http_process_req_common (or to http_wait_for_request where it's
 allocated). This will let requests terminated by an `errorfile`
 directive log out a request ID.
 2) Initialize the unique_id member upon allocation.
 
 I've attached a patch which does option 2, but I'm not sure if option 1
 would be preferable so that even `errorfile` requests will get a request ID.
 
 -Patrick

Hello Patrick,

Thanks for reporting the bug, I implemented something more relevant, the
unique-id is now generated when a request failed.

-- 
William Lallemand
From 6c2adb543c54df657e37836fc484a7f4e97ef7e1 Mon Sep 17 00:00:00 2001
From: William Lallemand wlallem...@exceliance.fr
Date: Wed, 28 Aug 2013 15:44:19 +0200
Subject: [PATCH] BUG/MEDIUM: unique_id: junk in log on empty unique_id

When a request fail, the unique_id was allocated but not generated.
The string was not initialized and junk was printed in the log with %ID.

This patch changes the behavior of the unique_id. The unique_id is now
generated when a request failed.

This bug was reported by Patrick Hemmer.
---
 src/log.c| 10 +-
 src/proto_http.c |  9 +
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/src/log.c b/src/log.c
index 369dc34..f1fe40c 100644
--- a/src/log.c
+++ b/src/log.c
@@ -1488,8 +1488,10 @@ int build_logline(struct session *s, char *dst, size_t maxsize, struct list *lis
 break;
 
 			case LOG_FMT_UNIQUEID: // %ID
+ret = NULL;
 src = s-unique_id;
-ret = lf_text(tmplog, src, maxsize - (tmplog - dst), tmp);
+if (src)
+	ret = lf_text(tmplog, src, maxsize - (tmplog - dst), tmp);
 if (ret == NULL)
 	goto out;
 tmplog = ret;
@@ -1541,6 +1543,12 @@ void sess_log(struct session *s)
 			level = LOG_ERR;
 	}
 
+	/* if unique-id was not generated */
+	if (!s-unique_id  !LIST_ISEMPTY(s-fe-format_unique_id)) {
+		if ((s-unique_id = pool_alloc2(pool2_uniqueid)) != NULL)
+			build_logline(s, s-unique_id, UNIQUEID_LEN, s-fe-format_unique_id);
+	}
+
 	tmplog = update_log_hdr();
 	size = tmplog - logline;
 	size += build_logline(s, tmplog, sizeof(logline) - size, s-fe-logformat);
diff --git a/src/proto_http.c b/src/proto_http.c
index 8d6eaf5..6ab2676 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -2635,9 +2635,6 @@ int http_wait_for_request(struct session *s, struct channel *req, int an_bit)
 		}
 	}
 
-	if (!LIST_ISEMPTY(s-fe-format_unique_id))
-		s-unique_id = pool_alloc2(pool2_uniqueid);
-
 	/* 4. We may have to convert HTTP/0.9 requests to HTTP/1.0 */
 	if (unlikely(msg-sl.rq.v_l == 0)  !http_upgrade_v09_to_v10(txn))
 		goto return_bad_req;
@@ -3950,8 +3947,12 @@ int http_process_request(struct session *s, struct channel *req, int an_bit)
 
 	/* add unique-id if header-unique-id is specified */
 
-	if (!LIST_ISEMPTY(s-fe-format_unique_id))
+	if (!LIST_ISEMPTY(s-fe-format_unique_id)) {
+		if ((s-unique_id = pool_alloc2(pool2_uniqueid)) == NULL)
+			goto return_bad_req;
+		s-unique_id[0] = '\0';
 		build_logline(s, s-unique_id, UNIQUEID_LEN, s-fe-format_unique_id);
+	}
 
 	if (s-fe-header_unique_id  s-unique_id) {
 		chunk_printf(trash, %s: %s, s-fe-header_unique_id, s-unique_id);
-- 
1.8.1.5

Re: Tilde in haproxy 1.5 log

[William Lallemand]

On Tue, Jan 08, 2013 at 05:14:05PM +0100, Baptiste wrote:
 sorry, posted too quicly.
 you can use the log-format tool to properly format your log line.
 
 that said, I'm not sure that you can remove this char.
 

Hello,

You can remove the ~ char using log-format.

The default variable used in the log is %ft which add a ~ in SSL. You can use
%f, it's the frontend without ~.

http://cbonte.github.com/haproxy-dconv/configuration-1.5.html#8.2.4
 

-- 
William Lallemand

Re: http compression with haproxy possible

[William Lallemand]

Hi,

Like Baptiste says, compression is on his way, and it's almost ready!
But I can't give you a release date for the moment.

There is no really interceptor concept, indeed, compression and SSL are not
at the same level, encryption is HTTP over SSL where compression is gzip over 
HTTP.

As an HAProxy contributor, I can see the HAProxy core evolving and becoming
better and better. It becomes easier to implement that kind of feature.

On Thu, Sep 20, 2012 at 10:46:30PM +0200, Baptiste wrote:
 Hi Paul,
 
 At Exceliance, we already spent some time on (HTTP) compression in
 HAProxy, and as far as I know, it's almost ready, but I prefer letting
 the dev give you more details on this.
 
 cheers
 
 On Thu, Sep 20, 2012 at 10:40 PM, Paul Svensson paul.svens...@ymail.com 
 wrote:
  Dear haproxy team,
 
  I know you're busy with ssl support in haproxy. That's a real good feature!
  From my understanding the conecpt of ssl de/encryption in haproxy may allow
  other interceptors, such as compression. In case I'm right where's the best
  point to start? Any http header analysis could be done with acls, so a
  single additional option would be required.
 
  I'm looking forward for you opinion
  Paul
 

-- 
William Lallemand

Re: Haproxy not logging captured request headers with custom log format

[William Lallemand]

On Fri, Sep 14, 2012 at 05:25:02PM +0530, Gagandeep Singh wrote:
 Hi William*
 *
 I tried commenting out option  httplog, but the logs are still not showing
 the captured request headers.
 The Haproxy version is:
 
 HAProxy version 1.5-dev8, released 2012/03/24
 

Try with the latest version (-dev12) some bugs were fixed concerning log-format.


-- 
William Lallemand

Re: Haproxy not logging captured request headers with custom log format

[William Lallemand]

On Thu, Sep 13, 2012 at 11:22:16PM +0530, Gagandeep Singh wrote:
 Hi all

Hi,

 
 were getting printed. I wanted to log the unique-id-header, but could not
 find any way of doing so.
 So i copy pasted the log-format mentioned on the
 1.5/doc/configuration.txthttp://haproxy.1wt.eu/download/1.5/doc/configuration.txt,
 and unique id started getting logged. Unfortunately, now the captured
 request headers are not getting logged. After some thought, i though't id
 put some delimiters around the %hrl and %hsl and i realized that these are
 empty strings.
 
 Very weird. Here is my config, plzz help :(
 
 listen server
   bind *:80
   mode http



   option  httplog
You shouldn't use this option with a log-format string, because it sets a
default string.



   no option logasap   # disable early logging of HTTP requests so that
   option  http-server-close
 total transfer time is logged
   option  forwardfor
   capture request  headerX-Forwarded-For  len 500
   capture request  headerHost len 500
   capture request  headerX-Request-UIDlen 500
   rspadd  X-Haproxy-Backend:\ server
 
   # Generate the X-Haproxy-Unique-ID and log it to make it easy to track
 requests
   log-format %Ci:%Cp\ [id=%ID]\ [%t]\ %f\ %b/%s\ %Tq/%Tw/%Tc/%Tr/%Tt\ %st\
 %B\ %cc\ %cs\ %tsc\ %ac/%fc/%bc/%sc/%rc\ %sq/%bq\ *{%hrl}\ {%hsl}*\ %{+Q}r
   unique-id-format %{+X}o\ %Ci:%Cp_%Fi:%Fp_%Ts_%rt:%pid
   unique-id-header X-Haproxy-Unique-ID
 
 
 Here is the log i see:
 Sep 13 17:28:57 localhost haproxy[11979]:
 10.161.27.218:41592[id=0AA11BDA:A278_0AA15B71:0050_505217C5_0014:2ECB]
 [13/Sep/2012:17:28:37.567] server www-example-com-healthz/-
 19998/0/2/1/+20001 200 +326 - -  10/10/1/1/0 0/0 *{} {}* GET
 /testing/healthz?merchant=www.example.comsource=elb HTTP/1.1
 
 
 Help will be greatly appreciated.
 

I don't have any problem with your configuration, which version of HAProxy are
you using ?

-- 
William Lallemand

Re: log format different and CAPTURE_LEN settings

[William Lallemand]

On Thu, Aug 09, 2012 at 03:16:07PM +0200, Aleksandar Lazic wrote:
 Hi,


Hello,

 [...]
 
 As you can see I have not 'option logasap' but get the '+'-sign?!
  
 Please can anybody help me to find the error, thanks.
 
 Best regards
 Aleks
 

It looks like a bug with the option unique-id-format.

Can you try this patch ?


-- 
William Lallemand
From 7d40e9f6d3f8f1c5ce09e264226a1e5e369d70a0 Mon Sep 17 00:00:00 2001
From: William Lallemand wlallem...@exceliance.fr
Date: Thu, 9 Aug 2012 16:41:35 +0200
Subject: [PATCH] BUG/MINOR: to_log erased with unique-id-format

curproxy-to_log was reset to LW_INIT when using unique-id-format,
so logs looked like option logasap
---
 src/log.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/log.c b/src/log.c
index 2a3cd16..b1f532a 100644
--- a/src/log.c
+++ b/src/log.c
@@ -309,7 +309,7 @@ void parse_logformat_string(char *str, struct proxy *curproxy, struct list *list
 	struct logformat_node *tmplf, *back;
 	int options = 0;
 
-	curproxy-to_log = LW_INIT;
+	curproxy-to_log |= LW_INIT;
 
 	/* flush the list first. */
 	list_for_each_entry_safe(tmplf, back, list_format, list) {
-- 
1.7.9.5