Re: [PR] BUILD: extend travis-ci matrix

2019-05-06 Thread Willy Tarreau
Hi Lukas,

On Mon, May 06, 2019 at 06:51:57PM +0200, Lukas Tribus wrote:
> There is room for improvement here. Can you confirm that attaching a
> patch file per commit to the email would fix this usability issue?

I'd say yes, provided the attachments are prefixed with a sequence
number, like git-format-patch does. However I think we should set
reasonable limits to 10 patches or less to save all list members
from being bombed if someone sends a huge (or even bogus) PR. I
think that till a handful of patches (let's say two hands :-)) it's
still possible to let various participants give their opinion on
different patches. When you see patch 137/375 it's unlikely that
anyone will have a look at it so then falling back to the current
mode would work better. Similarly I think that if a patch is too
large the series should not be forwarded. I'm well aware that
"too large" is a bit vague. The idea is that few people if any
will spend their time reviewing a 1000-line patch. But this could
be applied to the whole series if easier. In the end I think that
such thresholds will serve as a hint for reviewers about what to
expect when seeing the announce.

> I just finally fixed the script for authors with non-ascii names (it
> crashed previously), I can add the file attachments to my todo list,
> if useful.

This could definitely be useful. But don't spend too much valuable time
on this, as this is not the source of contribs I'd like to encourage
the most considering the amount of fixes they usually require before
being merged. By the way I noticed that something changed recently on
Github, the curl command receives a redirect and now needs -L to follow
it.

Thanks!
Willy



gcc address sanitizer finding

2019-05-06 Thread Илья Шипицин
Hello,

I built haproxy using gcc-9

make CC=gcc V=1 TARGET=linux2628 USE_ZLIB=1 USE_PCRE=1 USE_PCRE_JIT=1
USE_LUA=1 USE_OPENSSL=1 DEBUG_CFLAGS="-g -fsanitize=address"
LDFLAGS="-lasan"

finding is:

***  h10.1 debug|==8326==ERROR: LeakSanitizer: detected memory leaks
***  h10.1 debug|
***  h10.1 debug|Direct leak of 24 byte(s) in 1 object(s) allocated
from:
***  h10.1 debug|#0 0x7f33138ec1a8 in __interceptor_malloc
(/lib64/libasan.so.5+0x10e1a8)
***  h10.1 debug|#1 0x7f33136f3b1b  (/lib64/libssl.so.1.1+0x33b1b)
***  h10.1 debug|
***  h10.1 debug|Direct leak of 1 byte(s) in 1 object(s) allocated from:
***  h10.1 debug|#0 0x7f33138749dd in strdup
(/lib64/libasan.so.5+0x969dd)
***  h10.1 debug|#1 0x817241 in mworker_env_to_proc_list
src/mworker.c:167
***  h10.1 debug|
***  h10.1 debug|SUMMARY: AddressSanitizer: 25 byte(s) leaked in 2
allocation(s).


[PATCH 1/1] BUILD: travis-ci bugfixes and improvements

2019-05-06 Thread chipitsine
From: Ilya Shipitsin 

Call missing scripts/build-ssl.sh (which actually builds SSL variants)
Enable OpenSSL, LibreSSL builds caching, it saves a bunch of time
LibreSSL builds are not allowed to fail anymore
Add openssl to osx builds
---
 .travis.yml | 18 +++---
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 87331fc2..f9a13586 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -13,6 +13,11 @@ addons:
   apt:
 packages: [ liblua5.3-dev ]
 
+cache:
+  directories:
+  - download-cache
+  - ${HOME}/opt
+
 matrix:
   include:
   - os: linux
@@ -44,22 +49,13 @@ matrix:
 env: TARGET=linux2628 FLAGS=
   - os: osx
 compiler: clang
-env: TARGET=generic FLAGS=
-  allow_failures:
-  - os: linux
-compiler: gcc
-env: TARGET=linux2628 LIBRESSL_VERSION=2.9.1
-  - os: linux
-compiler: gcc
-env: TARGET=linux2628 LIBRESSL_VERSION=2.8.3
-  - os: linux
-compiler: gcc
-env: TARGET=linux2628 LIBRESSL_VERSION=2.7.5
+env: TARGET=generic FLAGS="USE_OPENSSL=1" OPENSSL_VERSION=1.1.1b
 
 install:
   - git clone https://github.com/VTest/VTest.git ../vtest
   # Special flags due to: https://github.com/vtest/VTest/issues/12
   - make -C ../vtest FLAGS="-O2 -s -Wall"
+  - scripts/build-ssl.sh > build-ssl.log 2>&1 || (cat build-ssl.log && exit 1)
 
 script:
   - make CC=$CC V=1 TARGET=$TARGET $FLAGS
-- 
2.20.1




[PATCH 0/1] travis-ci improvements and bugfixes

2019-05-06 Thread chipitsine
From: Ilya Shipitsin 

Added scripts/build-ssl.sh (without that script we were using openssl-1.0.2)
LibreSSL builds are not allowed to fail anymore
Enabled OpenSSL, LibreSSL build caching
Enabled openssl for osx builds

Ilya Shipitsin (1):
  BUILD: travis-ci bugfixes and improvements

 .travis.yml | 18 +++---
 1 file changed, 7 insertions(+), 11 deletions(-)

-- 
2.20.1




Re: [PR] BUILD: extend travis-ci matrix

2019-05-06 Thread Lukas Tribus
Hello Willy,


On Mon, 6 May 2019 at 08:06, Willy Tarreau  wrote:
>
> Hi Ilya,
>
> > I made another PR
> >
> > https://github.com/haproxy/haproxy/pull/92
>
> Thank you.
>
> > (I really like automatic PR to mailing list routing)
>
> Well, it was the only workable workaround we have when people send PRs.
> Sadly we can't block them. Apparently only mirror repositories can block
> PRs and github doesn't accept to create them anymore. But when there are
> multiple patches in the PR it's not usable as-is anymore, it only serves
> as a notification that someone sent something.
>
> For me, PRs tremendously increase the amount of manipulations, which is
> why I tend to postpone them until I think this will not divert me too
> much from what I am doing. I just timed the operations on this small
> batch and it went from ~15 seconds to review and merge your 4 patches
> if they were in mails (just pressing Enter in mutt to review them,
> possibly "e" to quickly edit if needed, and "A" to apply), then "g" to
> ack the merge, to around 3 minutes to :

There is room for improvement here. Can you confirm that attaching a
patch file per commit to the email would fix this usability issue?

I just finally fixed the script for authors with non-ascii names (it
crashed previously), I can add the file attachments to my todo list,
if useful.



Lukas



Re: [PR] BUILD: extend travis-ci matrix

2019-05-06 Thread Илья Шипицин
I have just noticed that "scripts/build-ssl.sh" is missing in .travis.yml
so, unfortunately, we are running all builds against xenial openssl-1.0.2
(no openssl-1.1.X, no libressl...)

I'm not sure when "scripts/build-ssl.sh" disappeared. I'll send new patch
soon

пн, 6 мая 2019 г. в 11:05, Willy Tarreau :

> Hi Ilya,
>
> > I made another PR
> >
> > https://github.com/haproxy/haproxy/pull/92
>
> Thank you.
>
> > (I really like automatic PR to mailing list routing)
>
> Well, it was the only workable workaround we have when people send PRs.
> Sadly we can't block them. Apparently only mirror repositories can block
> PRs and github doesn't accept to create them anymore. But when there are
> multiple patches in the PR it's not usable as-is anymore, it only serves
> as a notification that someone sent something.
>
> For me, PRs tremendously increase the amount of manipulations, which is
> why I tend to postpone them until I think this will not divert me too
> much from what I am doing. I just timed the operations on this small
> batch and it went from ~15 seconds to review and merge your 4 patches
> if they were in mails (just pressing Enter in mutt to review them,
> possibly "e" to quickly edit if needed, and "A" to apply), then "g" to
> ack the merge, to around 3 minutes to :
>   - copy the link
>   - open a browser
>   - paste the link into the browser's URL bar
>   - locate the links to the commits in the page
>   - open each of them in a separate tab
>   - visit each tab in turn, and for each of them, press Ctrl-L and
> append ".patch" at the end of the URL, then press enter
>   - after the patches are loaded, visit all of them again, to look at the
> dates and figure in what order to apply them
>   - then for each of them :
>   - review the patch
>   - Ctrl-S to save the patch
>   - remember the file name (commit ID) and press Enter
>   - Alt-tab to switch to the xterm in the development directory
>   - git am ~/.patch
>   - rm -f ~/.patch
>   - Alt-tab to switch back to browser again
>   - Ctrl-W to close the tab
>   - close browser window
>   - back to mutt to ack merging and rant about my hate of this workflow
>
> => This is way longer and more error-prone than the first variant. And
>this doesn't even include commenting if I disagree on something, which
>additionally requires copy-pasting the relevant parts of the patch
>into the response message. There is a reason why most projects
>developed on github are so low quality with so horrible commits
>and zero maintenance : it encourages laziness and dirtiness. It
>costs so much effort to fix minor issues and encourage people to
>improve, compared to lazily click on "merge" that you simply think
>"bah..." and you merge it as-is, making it impossible to review them
>later when trying to make a maintenance version. So you end up with
>continuous development made of a pile of junk patches :-/
>
> Anyway now I've finally applied your patches. In the future, if you
> want to help with a smoother process, please either attach your patches
> or use git-send-email. The most efficient workflow is one patch per mail,
> like git-send-email does, as it eases reviews (which can also be done by
> multiple persons). If you attach them, make sure to use the file names
> created by git-format-patch so that there's no doubt when saving files
> for manual editing for example. Also it's important to indicate the
> intent with a patch or a patch series, i.e. "please merge this", "please
> don't merge this, it's just for discussing", "it should be mergeable once
> the makefile is fixed", "I think it's OK for merging but I'd prefer an
> extra review first", "how about we'd do this", etc. I'm all for being
> accommodating with submissions and slightly edit them if needed because
> I know that if the work has to be 100% on either side it can total more
> time than 90/10 or even 95/5 (but I never edit signed patches beyond
> fixing merging issues though). I'm just very careful about the time
> spent on my side because I know I'm already a bottleneck, so each extra
> minute added here delays everything else.
>
> Thanks!
> Willy
>


Editor's Picks - Ecommerce lifeboat for global brands | 7 payment gateways for SMEs | Jio to route mobile calls via landline | B2B invoices on govt portal by September | Cognizant slowdown | BigBasket

2019-05-06 Thread TradeBriefs



Re: [PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

2019-05-06 Thread William Lallemand
On Mon, May 06, 2019 at 02:20:32PM +0200, Vincent Bernat wrote:
> However, many people prefer /etc/default and /etc/sysconfig to systemd
> overrides. And for distribution, it enables a smoother transition. For
> Debian, we would still add the EnvironmentFile directive. You could
> still be compatible with both styles of distribution with:
> 
> EnvironmentFile=-/etc/default/haproxy
> EnvironmentFile=-/etc/sysconfig/haproxy

Oh that's right, I forgot that the - was checking if the file exists, looks like
a good solution.

-- 
William Lallemand



Re: QAT intermittent healthcheck errors

2019-05-06 Thread Emeric Brun
Hi Marcin,

On 5/6/19 3:31 PM, Emeric Brun wrote:
> Hi Marcin,
> 
> On 5/6/19 3:15 PM, Marcin Deranek wrote:
>> Hi Emeric,
>>
>> On 5/3/19 5:54 PM, Emeric Brun wrote:
>>> Hi Marcin,
>>>
>>> On 5/3/19 4:56 PM, Marcin Deranek wrote:
 Hi Emeric,

 On 5/3/19 4:50 PM, Emeric Brun wrote:

> I've a testing platform here but I don't use the usdm_drv but the 
> qat_contig_mem and I don't reproduce this issue (I'm using QAT 1.5, as 
> the doc says to use with my chip) .

 I see. I use qat 1.7 and qat-engine 0.5.40.

> Anyway, could you re-compile a haproxy's binary if I provide you a 
> testing patch?

 Sure, that should not be a problem.
>>>
>>> The patch in attachment.
>>
>> As I use HAProxy 1.8 I had to adjust the patch (see attachment for end 
>> result). Unfortunately after applying the patch there is no change in 
>> behavior: we still leak /dev/usdm_drv descriptors and have "stuck" HAProxy 
>> instances after reload..
>> Regards,
> 
> 
> Ok, the patch adds a ENGINE_finish() call before the reload. I was supposing 
> that the ENGINE_finish would perform the close of the fd because on the 
> application side there is no different way to interact with the engine.
> 
> Unfortunately, this is not the case. So I will ask to the intel guys what we 
> supposed to do to close this fd.


I've just written to my contact at intel.

Just for note: I'm using hardware supported with QAT 1.5 in this version tu 
usdm_drv was not present and I use the other option qat_contig_mem which seems 
to not cause such fd leak.

Perhaps to switch to this one would be a work-around if you want to continue to 
perform test waiting for intel's guy reply.

R,
Emeric



Re: QAT intermittent healthcheck errors

2019-05-06 Thread Emeric Brun
Hi Marcin,

On 5/6/19 3:15 PM, Marcin Deranek wrote:
> Hi Emeric,
> 
> On 5/3/19 5:54 PM, Emeric Brun wrote:
>> Hi Marcin,
>>
>> On 5/3/19 4:56 PM, Marcin Deranek wrote:
>>> Hi Emeric,
>>>
>>> On 5/3/19 4:50 PM, Emeric Brun wrote:
>>>
 I've a testing platform here but I don't use the usdm_drv but the 
 qat_contig_mem and I don't reproduce this issue (I'm using QAT 1.5, as the 
 doc says to use with my chip) .
>>>
>>> I see. I use qat 1.7 and qat-engine 0.5.40.
>>>
 Anyway, could you re-compile a haproxy's binary if I provide you a testing 
 patch?
>>>
>>> Sure, that should not be a problem.
>>
>> The patch in attachment.
> 
> As I use HAProxy 1.8 I had to adjust the patch (see attachment for end 
> result). Unfortunately after applying the patch there is no change in 
> behavior: we still leak /dev/usdm_drv descriptors and have "stuck" HAProxy 
> instances after reload..
> Regards,


Ok, the patch adds a ENGINE_finish() call before the reload. I was supposing 
that the ENGINE_finish would perform the close of the fd because on the 
application side there is no different way to interact with the engine.

Unfortunately, this is not the case. So I will ask to the intel guys what we 
supposed to do to close this fd.

R,
Emeric 




Re: recent LibreSSL regressions

2019-05-06 Thread Olivier Houchard
Hi Ilya,

On Mon, May 06, 2019 at 12:54:56AM +0500, Илья Шипицин wrote:
> hello,
> 
> when I first sent LibreSSL patches (it was 27th April 2019), reg-tests were
> ok.
> I suspect recent 0RTT patches could break LibreSSL things
> 
> can someone have a look ?
> 
> https://travis-ci.org/chipitsine/haproxy-1/builds/528535120
> 
> thanks!

You're right indeed, it was definitively broken for libressl, it should be
fixed with commit 4cd2af4e5d785c42d2924492df987a7cd5832e23

Regards,

Olivier



Re: HAProxy 1.9.6 unresponsive

2019-05-06 Thread Willy Tarreau
On Sun, May 05, 2019 at 09:40:02AM +0200, Willy Tarreau wrote:
> With this said, after studying the code a little bit more, I'm seeing a
> potential case where if we'd have a trailers entry in the HTX buffer but
> no end of message, we could loop forever there not consuming this block.
> I have no idea if this is possible in an HTX message, I'll ask Christopher
> tomorrow. In any case we need to address this one way or another, possibly
> reporting an error instead if required. Thus I'm postponing 1.9.8 for
> tomorrow.

So the case is indeed possible and at the moment all we can do is try to
minimize the probability to produce it :-(  The issue is caused by the
moment we've received the end of trailsers but not the end of the mesage.
>From the H2 protocol perspective if we've sent the END_STREAM flag, the
stream is closed, and a closed stream gets detached and cannot receive
new traffic, so at best we'll occasionally close too early and report
client failures at the upper layers while everything went OK. We cannot
send trailers without the END_STREAM flag since no frame may follow.
Abusing CONTINUATION is out of question here as this would require to
completely freeze the whole connection (including control frames) for
the time it takes to get this final EOM block. I thought about simply
reporting an error when we're in this situation between trailers and EOM
but it will mean that occasionally some chunked responses of sizes close
to 16N kB with trailers may err out, which is not acceptable either.

For 2.0 we approximately see what needs to be modified to address this
situation, but that will not be trivial and not backportable.

For 1.9 I'm still trying to figure what the "best" solution is. I may
finally end up marking the stream as closed as soon as we see the
trailers pushed down. I'm just unsure right now about all the possible
consequences and need to study the edge cases. Also I fear that this
will be something hard to unroll later, so I'm still studying.

Willy



Re: [PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

2019-05-06 Thread Vincent Bernat
 ❦  6 mai 2019 13:46 +02, William Lallemand :

>> /etc/default is a debianism. Other distros use different directories, 
>> such as RedHat which uses /etc/sysconfig
>> 
>> -Patrick
>
> Hi Patrick,
>
> I don't think that's a problem, most distribution use their own unit file
> anyway, people should edit this file before using it.

One of the promise of systemd was to be able to share unit files accross
distribution. For this, systemd wants to discourage the use of
/etc/default and /etc/sysconfig (there was even a discussion about
deprecating EnvironmentFile). You can still use the EXTRAOPTS approach
by using:

ExecStart=/usr/sbin/haproxy -Ws -f $CONFIG $EXTRAOPTS

And let people override EXTRAOPTS with an override:

Environment=EXTRAOPTS=somethingmore

However, many people prefer /etc/default and /etc/sysconfig to systemd
overrides. And for distribution, it enables a smoother transition. For
Debian, we would still add the EnvironmentFile directive. You could
still be compatible with both styles of distribution with:

EnvironmentFile=-/etc/default/haproxy
EnvironmentFile=-/etc/sysconfig/haproxy
-- 
Use the "telephone test" for readability.
- The Elements of Programming Style (Kernighan & Plauger)


signature.asc
Description: PGP signature


Re: [PATCH v2 0/2] systemd: Make use of master socket in systemd unit

2019-05-06 Thread William Lallemand
On Mon, May 06, 2019 at 01:08:35PM +0200, Tim Düsterhus wrote:
> List,
> 
> I apologize for the stupid threading of the v2 patch. When I want to
> send a number of patches in-reply-to an existing thread: Should I put
> the --in-reply-to onto git format-patch or should I specify it in git
> send-email?
> 
> This time I added it to git format-patch, but it did not make the actual
> patches a reply to the --cover-letter. Will it work properly when
> specifying it with git send-email?

I never used it with git format-patch, I usually do a `git send-email --compose 
--in-reply-to=`

-- 
William Lallemand



Re: [PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

2019-05-06 Thread William Lallemand
On Mon, May 06, 2019 at 07:15:11AM -0400, Patrick Hemmer wrote:
> 
> /etc/default is a debianism. Other distros use different directories, 
> such as RedHat which uses /etc/sysconfig
> 
> -Patrick

Hi Patrick,

I don't think that's a problem, most distribution use their own unit file
anyway, people should edit this file before using it.

-- 
William Lallemand



Re: [PATCH v2 0/2] systemd: Make use of master socket in systemd unit

2019-05-06 Thread William Lallemand
Tim,

On Mon, May 06, 2019 at 01:00:51PM +0200, Tim Duesterhus wrote:
> > Regarding the overriding of ExecStart, I disagree. In my opinion this is a
> > confusing solution for the user, when doing that the user won't have the 
> > update
> > of the unit file in the package. Lots of people still prefer /etc/default/ 
> > and
> > /etc/sysconfig/ to add some options.
> 
> I'm not sure whether I understand what you mean by "won't have the update".
> With systemd it's fairly easy to just override the ExecStart= with a drop-in
> file. Anything else is not modified. As an example: On my server I use this to
> disable the PID file, because I think it's useless with systemd.
> 
> # /lib/systemd/system/haproxy.service.d/no-pidfile.conf
> [Service]
> ExecStart=
> ExecStart=/usr/sbin/haproxy -Ws -f $CONFIG
> 

I was talking about the distribution pushing an update to the unit file.
For example if they want to add an option to the command line.

In this particular case, as you override the ExecStart, you won't benefit from
the new options on the ExecStart line in the new systemd unit.

> > In this case we could indeed use the EXTRAOPTS variable, and add
> > "EXTRAOPTS=-S /run/haproxy-master.sock" at the end of the Environment line.
> 
> Okay, I've taken the patch from the Debian project and added Apollon (the 
> author)
> and Vincent (the Debian developer I most frequently see on the list) to the 
> Cc.
> Please object if you are unhappy with the changes as they are!
> 
>   MINOR: systemd: Use the variables from /etc/default/haproxy
>

The EXTRAOPTS part seems relevant, however the EnvironmentFile line is specific
to each distribution, but they could still patch it in the other distributions.

In my opinion this file is more like an example and it must be adapted to each
distribution.

-- 
William Lallemand



Re: [PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

2019-05-06 Thread Patrick Hemmer




*From:* Tim Duesterhus [mailto:t...@bastelstu.be]
*Sent:* Monday, May 6, 2019, 07:00 EDT
*To:* haproxy@formilux.org
*Cc:* Apollon Oikonomopoulos , 
wlallem...@haproxy.com, w...@1wt.eu, ber...@debian.org, Tim Duesterhus 

*Subject:* [PATCH v2 1/2] MINOR: systemd: Use the variables from 
/etc/default/haproxy



From: Apollon Oikonomopoulos 

This will allow seamless upgrades from the sysvinit system while respecting
any changes the users may have made. It will also make local configuration
easier than overriding the systemd unit file.

Note by Tim:

This GPL-2 licensed patch was taken from the Debian project at [1].

It was slightly modified to cleanly apply, because HAProxy's default unit
file does not include rsyslog.service as an 'After' dependency. Also the
subject line was modified to include the proper subsystem and severity.

This patch may be backported to 1.9.

[1] 
https://salsa.debian.org/haproxy-team/haproxy/blob/master/debian/patches/haproxy.service-use-environment-variables.patch

Co-authored-by: Tim Duesterhus 
---
  contrib/systemd/haproxy.service.in | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/contrib/systemd/haproxy.service.in 
b/contrib/systemd/haproxy.service.in
index 74e66e302..138701223 100644
--- a/contrib/systemd/haproxy.service.in
+++ b/contrib/systemd/haproxy.service.in
@@ -3,10 +3,11 @@ Description=HAProxy Load Balancer
  After=network.target
  
  [Service]

+EnvironmentFile=-/etc/default/haproxy
  Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
-ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q
-ExecStart=@SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE
-ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q
+ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q $EXTRAOPTS
+ExecStart=@SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE $EXTRAOPTS
+ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q $EXTRAOPTS
  ExecReload=/bin/kill -USR2 $MAINPID
  KillMode=mixed
  Restart=always


/etc/default is a debianism. Other distros use different directories, 
such as RedHat which uses /etc/sysconfig


-Patrick


Re: [PATCH v2 0/2] systemd: Make use of master socket in systemd unit

2019-05-06 Thread Tim Düsterhus
List,

I apologize for the stupid threading of the v2 patch. When I want to
send a number of patches in-reply-to an existing thread: Should I put
the --in-reply-to onto git format-patch or should I specify it in git
send-email?

This time I added it to git format-patch, but it did not make the actual
patches a reply to the --cover-letter. Will it work properly when
specifying it with git send-email?

Best regards
Tim Düsterhus



[PATCH v2 2/2] MINOR: systemd: Make use of master socket in systemd unit

2019-05-06 Thread Tim Duesterhus
Unless the EXTRAOPTS variable is overriden in /etc/default/haproxy
the unit file will use the master socket by default.

This patch may be backported to 1.9 and depends on
MINOR: systemd: Use the variables from /etc/default/haproxy.
---
 contrib/systemd/haproxy.service.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/systemd/haproxy.service.in 
b/contrib/systemd/haproxy.service.in
index 138701223..44c369cfd 100644
--- a/contrib/systemd/haproxy.service.in
+++ b/contrib/systemd/haproxy.service.in
@@ -4,7 +4,7 @@ After=network.target
 
 [Service]
 EnvironmentFile=-/etc/default/haproxy
-Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
+Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid" 
"EXTRAOPTS=-S /run/haproxy-master.sock"
 ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q $EXTRAOPTS
 ExecStart=@SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE $EXTRAOPTS
 ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q $EXTRAOPTS
-- 
2.21.0




[PATCH v2 0/2] systemd: Make use of master socket in systemd unit

2019-05-06 Thread Tim Duesterhus
William,

Am 06.05.19 um 11:08 schrieb William Lallemand:
> This socket gives full admin access to HAProxy without being in the
> configuration, so it might surprise the user if it's there after an upgrade, 
> it
> should really be configurable. But I agree that it could be nice to have it
> by default.

Fair enough. I guess it could have been acceptable without a backport
(thus 2.0 only).

> Regarding the overriding of ExecStart, I disagree. In my opinion this is a
> confusing solution for the user, when doing that the user won't have the 
> update
> of the unit file in the package. Lots of people still prefer /etc/default/ and
> /etc/sysconfig/ to add some options.

I'm not sure whether I understand what you mean by "won't have the update".
With systemd it's fairly easy to just override the ExecStart= with a drop-in
file. Anything else is not modified. As an example: On my server I use this to
disable the PID file, because I think it's useless with systemd.

# /lib/systemd/system/haproxy.service.d/no-pidfile.conf
[Service]
ExecStart=
ExecStart=/usr/sbin/haproxy -Ws -f $CONFIG

> In this case we could indeed use the EXTRAOPTS variable, and add
> "EXTRAOPTS=-S /run/haproxy-master.sock" at the end of the Environment line.

Okay, I've taken the patch from the Debian project and added Apollon (the 
author)
and Vincent (the Debian developer I most frequently see on the list) to the Cc.
Please object if you are unhappy with the changes as they are!

Best regards
Tim Duesterhus

Apollon Oikonomopoulos (1):
  MINOR: systemd: Use the variables from /etc/default/haproxy

Tim Duesterhus (1):
  MINOR: systemd: Make use of master socket in systemd unit

 contrib/systemd/haproxy.service.in | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

-- 
2.21.0




[PATCH v2 1/2] MINOR: systemd: Use the variables from /etc/default/haproxy

2019-05-06 Thread Tim Duesterhus
From: Apollon Oikonomopoulos 

This will allow seamless upgrades from the sysvinit system while respecting
any changes the users may have made. It will also make local configuration
easier than overriding the systemd unit file.

Note by Tim:

This GPL-2 licensed patch was taken from the Debian project at [1].

It was slightly modified to cleanly apply, because HAProxy's default unit
file does not include rsyslog.service as an 'After' dependency. Also the
subject line was modified to include the proper subsystem and severity.

This patch may be backported to 1.9.

[1] 
https://salsa.debian.org/haproxy-team/haproxy/blob/master/debian/patches/haproxy.service-use-environment-variables.patch

Co-authored-by: Tim Duesterhus 
---
 contrib/systemd/haproxy.service.in | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/contrib/systemd/haproxy.service.in 
b/contrib/systemd/haproxy.service.in
index 74e66e302..138701223 100644
--- a/contrib/systemd/haproxy.service.in
+++ b/contrib/systemd/haproxy.service.in
@@ -3,10 +3,11 @@ Description=HAProxy Load Balancer
 After=network.target
 
 [Service]
+EnvironmentFile=-/etc/default/haproxy
 Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
-ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q
-ExecStart=@SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE
-ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q
+ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q $EXTRAOPTS
+ExecStart=@SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE $EXTRAOPTS
+ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q $EXTRAOPTS
 ExecReload=/bin/kill -USR2 $MAINPID
 KillMode=mixed
 Restart=always
-- 
2.21.0




Re: reg-tests are broken when running osx + openssl

2019-05-06 Thread Frederic Lecaille

On 5/6/19 11:25 AM, Frederic Lecaille wrote:

On 5/6/19 9:54 AM, Илья Шипицин wrote:

there's some random failure

https://travis-ci.com/haproxy/haproxy/jobs/197824840

looks like test is not stable


According to the log, S13 and S37 syslog servers have the same port: 42464


Obviously this is a vtest bug. I could sometimes reproduce it. I cannot 
reproduce anymore with this commit: : 
https://github.com/haproxyFred/VTest/commit/17871365a14d81234426ac203da3af30bb46e506


I have sent a PR for vtest for this issue: 
https://github.com/vtest/VTest/pull/13.


Fred.




cygwin compilation error

2019-05-06 Thread Gil Bahat
Hi,

is cygwin still supported anymore? the target seems to be present in the
Makefiles and I'd love to be able to use it. I'm running into what seems to
be a workable linker error:

$ make TARGET=cygwin
  LD  haproxy
src/http_act.o:http_act.c:(.rdata+0x340): multiple definition of
`.weak.ist_uc.'
src/ev_poll.o:ev_poll.c:(.rdata+0x20): first defined here
src/http_act.o:http_act.c:(.rdata+0x440): multiple definition of
`.weak.ist_lc.'
src/ev_poll.o:ev_poll.c:(.rdata+0x120): first defined here
src/raw_sock.o:raw_sock.c:(.rdata+0x0): multiple definition of
`.weak.ist_uc.'
src/ev_poll.o:ev_poll.c:(.rdata+0x20): first defined here
src/raw_sock.o:raw_sock.c:(.rdata+0x100): multiple definition of
`.weak.ist_lc.'
src/ev_poll.o:ev_poll.c:(.rdata+0x120): first defined here
src/proto_uxst.o:proto_uxst.c:(.rdata+0x560): multiple definition of
`.weak.ist_uc.___chkstk_ms'
src/mux_h2.o:mux_h2.c:(.rdata+0x800): first defined here
src/proto_uxst.o:proto_uxst.c:(.rdata+0x660): multiple definition of
`.weak.ist_lc.___chkstk_ms'
src/mux_h2.o:mux_h2.c:(.rdata+0x900): first defined here
src/ev_select.o:ev_select.c:(.rdata+0x20): multiple definition of
`.weak.ist_uc.'
src/ev_poll.o:ev_poll.c:(.rdata+0x20): first defined here
src/ev_select.o:ev_select.c:(.rdata+0x120): multiple definition of
`.weak.ist_lc.'
src/ev_poll.o:ev_poll.c:(.rdata+0x120): first defined here
src/http_conv.o:http_conv.c:(.rdata+0x60): multiple definition of
`.weak.ist_uc.'
src/ev_poll.o:ev_poll.c:(.rdata+0x20): first defined here
src/http_conv.o:http_conv.c:(.rdata+0x160): multiple definition of
`.weak.ist_lc.'
src/ev_poll.o:ev_poll.c:(.rdata+0x120): first defined here
src/http_acl.o:http_acl.c:(.rdata+0x280): multiple definition of
`.weak.ist_uc.'
src/ev_poll.o:ev_poll.c:(.rdata+0x20): first defined here
src/http_acl.o:http_acl.c:(.rdata+0x380): multiple definition of
`.weak.ist_lc.'
src/ev_poll.o:ev_poll.c:(.rdata+0x120): first defined here
collect2: error: ld returned 1 exit status
make: *** [Makefile:994: haproxy] Error 1

it feels like it's something trivial but I'm not versed enough to get a
hold on it. any help would be appreciated.

Regards,

Gil


Re: reg-tests are broken when running osx + openssl

2019-05-06 Thread Frederic Lecaille

On 5/6/19 9:54 AM, Илья Шипицин wrote:

there's some random failure

https://travis-ci.com/haproxy/haproxy/jobs/197824840

looks like test is not stable


According to the log, S13 and S37 syslog servers have the same port: 42464


Fred.



Re: [PATCH] MINOR: systemd: Make use of master socket in systemd unit

2019-05-06 Thread William Lallemand
On Sun, May 05, 2019 at 03:06:31PM +0200, Tim Düsterhus wrote:
> William,
> 
> Am 26.04.19 um 20:30 schrieb Tim Düsterhus:
> > William,
> > 
> > Am 26.04.19 um 14:56 schrieb William Lallemand:
> >> On Fri, Apr 26, 2019 at 12:15:37AM +0200, Tim Duesterhus wrote:
> >>>  [Service]
> >>> -Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
> >>> +Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid" 
> >>> "MASTER_SOCKET=/run/haproxy-master.sock"
> >>>  ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q
> >>> -ExecStart=@SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE
> >>> +ExecStart=@SBINDIR@/haproxy -Ws -S $MASTER_SOCKET -f $CONFIG -p $PIDFILE
> >>>  ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q
> >>
> >> In my opinion that's not a good idea to do it this way, because you can't
> >> disable the master CLI by unsetting the MASTER_SOCKET variable.
> >>
> >> It's probably better to have "-S /run/haproxy-master.sock" in an $OPTIONS
> >> variable at the end of the ExecStart line.
> >>
> > 
> > I'm not sure whether this is better. Yes you can disable the socket more
> > easily then, but you have to remember to add it back when editing the
> > 'OPTIONS' variable.
> > 
> > I believe it boils to to this question: Does the user usually want to
> > run with the socket enabled or not? My guess is that most users want to
> > have this socket (having is better than needing!), they might just want
> > to move it to a different location rather than outright disabling it.
> > During my tests it was only accessible to root, so there does not appear
> > a security issue in the default configuration either.
> > 
> > On an unrelated note: Debian patches the unit file to add in such a
> > variable, called 'EXTRAOPTS':
> > https://salsa.debian.org/haproxy-team/haproxy/blob/master/debian/patches/haproxy.service-use-environment-variables.patch
> > 
> > So if we want to go the 'OPTIONS' variable path we might want to re-use
> > that variable name. Any change will break their patch anyway so they
> > definitely notice.
> > 
> > Best regards
> > Tim Düsterhus
> > 
> 
> I'd like to bump my reply to you once more. As outlined previously I
> believe that the "default" expectation is having the socket, it can
> still be disabled by overriding the ExecStart= declaration.
> 
> Best regards
> Tim Düsterhus
> 

Hi Tim,

This socket gives full admin access to HAProxy without being in the
configuration, so it might surprise the user if it's there after an upgrade, it
should really be configurable. But I agree that it could be nice to have it
by default.

Regarding the overriding of ExecStart, I disagree. In my opinion this is a
confusing solution for the user, when doing that the user won't have the update
of the unit file in the package. Lots of people still prefer /etc/default/ and
/etc/sysconfig/ to add some options.

In this case we could indeed use the EXTRAOPTS variable, and add
"EXTRAOPTS=-S /run/haproxy-master.sock" at the end of the Environment line.

Regards,

-- 
William Lallemand



Re: reg-tests are broken when running osx + openssl

2019-05-06 Thread Илья Шипицин
surprisingly, LibreSSL builds are green

пн, 6 мая 2019 г. в 13:03, Willy Tarreau :

> On Mon, May 06, 2019 at 12:54:06PM +0500,  ??? wrote:
> > there's some random failure
> >
> > https://travis-ci.com/haproxy/haproxy/jobs/197824840
> >
> > looks like test is not stable
>
> isn't it the same at the other one you reported that looked related to
> the latest SSL fixes ?
>
> Willy
>


Re: reg-tests are broken when running osx + openssl

2019-05-06 Thread Willy Tarreau
On Mon, May 06, 2019 at 12:54:06PM +0500,  ??? wrote:
> there's some random failure
> 
> https://travis-ci.com/haproxy/haproxy/jobs/197824840
> 
> looks like test is not stable

isn't it the same at the other one you reported that looked related to
the latest SSL fixes ?

Willy



Re: reg-tests are broken when running osx + openssl

2019-05-06 Thread Илья Шипицин
there's some random failure

https://travis-ci.com/haproxy/haproxy/jobs/197824840

looks like test is not stable

пн, 6 мая 2019 г. в 11:11, Willy Tarreau :

> On Sun, May 05, 2019 at 10:11:38PM +0200, Frederic Lecaille wrote:
> > I had no access to any travis environment when it has been told in
> previous
> > mails that /tmp could not work, and /var/tmp could not either.
> > They were the first tested values.
> >
> > Now that I have setup a travis account, I have just double checked, and
> > contrary to what has been previously told, /tmp value for TMPDIR may make
> > the reg tests work on OS X with travis.
> >
> > So here is attached to this mail a better fix.
>
> Excellent, thank you Fred for testing and for the explanation! Patch
> merged now, we'll see if things improve.
>
> Willy
>


Re: [PR] BUILD: extend travis-ci matrix

2019-05-06 Thread Илья Шипицин
well, I hope travis-ci will be useful (or we will drop it).
as for PR, I meant that it should have been sent to list anyway (but it was
not sent for some reason).

I can send using "git send email" as well

пн, 6 мая 2019 г. в 11:05, Willy Tarreau :

> Hi Ilya,
>
> > I made another PR
> >
> > https://github.com/haproxy/haproxy/pull/92
>
> Thank you.
>
> > (I really like automatic PR to mailing list routing)
>
> Well, it was the only workable workaround we have when people send PRs.
> Sadly we can't block them. Apparently only mirror repositories can block
> PRs and github doesn't accept to create them anymore. But when there are
> multiple patches in the PR it's not usable as-is anymore, it only serves
> as a notification that someone sent something.
>
> For me, PRs tremendously increase the amount of manipulations, which is
> why I tend to postpone them until I think this will not divert me too
> much from what I am doing. I just timed the operations on this small
> batch and it went from ~15 seconds to review and merge your 4 patches
> if they were in mails (just pressing Enter in mutt to review them,
> possibly "e" to quickly edit if needed, and "A" to apply), then "g" to
> ack the merge, to around 3 minutes to :
>   - copy the link
>   - open a browser
>   - paste the link into the browser's URL bar
>   - locate the links to the commits in the page
>   - open each of them in a separate tab
>   - visit each tab in turn, and for each of them, press Ctrl-L and
> append ".patch" at the end of the URL, then press enter
>   - after the patches are loaded, visit all of them again, to look at the
> dates and figure in what order to apply them
>   - then for each of them :
>   - review the patch
>   - Ctrl-S to save the patch
>   - remember the file name (commit ID) and press Enter
>   - Alt-tab to switch to the xterm in the development directory
>   - git am ~/.patch
>   - rm -f ~/.patch
>   - Alt-tab to switch back to browser again
>   - Ctrl-W to close the tab
>   - close browser window
>   - back to mutt to ack merging and rant about my hate of this workflow
>
> => This is way longer and more error-prone than the first variant. And
>this doesn't even include commenting if I disagree on something, which
>additionally requires copy-pasting the relevant parts of the patch
>into the response message. There is a reason why most projects
>developed on github are so low quality with so horrible commits
>and zero maintenance : it encourages laziness and dirtiness. It
>costs so much effort to fix minor issues and encourage people to
>improve, compared to lazily click on "merge" that you simply think
>"bah..." and you merge it as-is, making it impossible to review them
>later when trying to make a maintenance version. So you end up with
>continuous development made of a pile of junk patches :-/
>
> Anyway now I've finally applied your patches. In the future, if you
> want to help with a smoother process, please either attach your patches
> or use git-send-email. The most efficient workflow is one patch per mail,
> like git-send-email does, as it eases reviews (which can also be done by
> multiple persons). If you attach them, make sure to use the file names
> created by git-format-patch so that there's no doubt when saving files
> for manual editing for example. Also it's important to indicate the
> intent with a patch or a patch series, i.e. "please merge this", "please
> don't merge this, it's just for discussing", "it should be mergeable once
> the makefile is fixed", "I think it's OK for merging but I'd prefer an
> extra review first", "how about we'd do this", etc. I'm all for being
> accommodating with submissions and slightly edit them if needed because
> I know that if the work has to be 100% on either side it can total more
> time than 90/10 or even 95/5 (but I never edit signed patches beyond
> fixing merging issues though). I'm just very careful about the time
> spent on my side because I know I'm already a bottleneck, so each extra
> minute added here delays everything else.
>
> Thanks!
> Willy
>


Re: Randomly high CPU usage

2019-05-06 Thread Marco Corte

Il 2019-04-18 18:33 Willy Tarreau ha scritto:

Hello Marco,

On Thu, Apr 18, 2019 at 05:27:26PM +0200, Marco Corte wrote:

Hello!

From time to time, about twice daily, and without any apparent reason,
haproxy jumps from using about 15% CPU usage to 100% (relative to the 
single

core it can use).
The situation becomes normal again after about 15-20 minutes.

(...)

Please keep this one until you can confirm the issue is gone, I may
have to ask it to you if it's still present.
(...)
Thanks!
Willy


Hello to the list!

I can confirm that version 1.9.7 does not show the behaviour any more.
In about one week I did not notice any CPU usage peak, and, more 
interestingly, the CPU usage appears lower on the average.


Thank you for the fix!

.marcoc



Re: v1.9.6 socket unresponsive with high cpu usage

2019-05-06 Thread William Dauchy
Hi Willy,

On Sun, May 05, 2019 at 07:07:21AM +0200, Willy Tarreau wrote:
> Hi William,
> > we got a similar issue with last v1.9.7+HEAD
> At first I thought you were again on a deadlock that I couldn't spot, due
> to the fact that nearly all threads were waiting on the LB lock, and I
> couldn't find how this could happen. But I didn't notice this one which
> is the most important :
> 
> > Thread 15 (Thread 0x7fe9b6631700 (LWP 2808)):
> > #0  0x56153d96d7a0 in __eb_insert_dup (new=0x56157f52f424, 
> > sub=0x56157f5640a4) at ebtree/ebtree.h:478
> > #1  eb_insert_dup (sub=, new=0x56157f52f424) at 
> > ebtree/ebtree.c:31
> > #2  0x56153d96df10 in __eb32_insert (new=new@entry=0x56157f52f424, 
> > root=, root@entry=0x56157deb4140) at ebtree/eb32tree.h:337
> > #3  eb32_insert (root=root@entry=0x56157deb4140, 
> > new=new@entry=0x56157f52f424) at ebtree/eb32tree.c:27
> > #4  0x56153d957fcb in fwrr_queue_srv (s=s@entry=0x56157f52f080) at 
> > src/lb_fwrr.c:371
> > #5  0x56153d9585e8 in fwrr_update_server_weight (srv=0x56157f52f080) at 
> > src/lb_fwrr.c:242
> > #6  0x56153d8ae8ac in srv_update_status (s=0x56157f52f080) at 
> > src/server.c:4923
> > #7  0x56153d8adfc2 in server_recalc_eweight 
> > (sv=sv@entry=0x56157f52f080, must_update=must_update@entry=1) at 
> > src/server.c:1310
> > #8  0x56153d8b6edd in server_warmup (t=0x5615899be8a0, 
> > context=0x56157f52f080, state=) at src/checks.c:1492
> > #9  0x56153d94d97a in process_runnable_tasks () at src/task.c:390
> > #10 0x56153d8c5c4f in run_poll_loop () at src/haproxy.c:2661
> > #11 run_thread_poll_loop (data=) at src/haproxy.c:2726
> > #12 0x7fe9bd5e7dd5 in start_thread () from /lib64/libpthread.so.0
> > #13 0x7fe9bc320ead in clone () from /lib64/libc.so.6
> 
> Thus I conclude that it crashed, and that all other threads just met at
> the same lock while the core was being dumped in this one. I figured what
> was missing, the server_warmup() function was missing a lock since 1.8.
> I've just fixed this and backported it to 1.9. I would be grateful if
> you could test it again, as I failed to reproduce the issue (it requires
> a high concurrency and bad luck, as often in such cases).

thank you very much for the patch. we are pushing it today.
http://git.haproxy.org/?p=haproxy-1.9.git;a=commit;h=207ba5a6bc1c03f2ba15ac3cd49bfa756fb760bb
for reference
I however don't know when I will be able to confirm as the issue was not
showing that often.

> Or maybe the tree got corrupted and __eb_insert_dup() entered an endless
> loop. If that's the case (I mean if it froze and didn't crash), I may
> have something to make this safer soon. I more or less managed to create
> a watchdog timer to detect lockups and abort the whole process with a
> trace when this happens. This will avoid keeping a faulty process in
> prod and may even allow a quicker restart. I don't intend to backport
> it to 1.9 though but depending on how effective and helpful it is, I
> could change my mind. In all cases I don't want to use such solutions
> to hide the dust under the carpet but instead to take detailed traces
> without requiring human intervention when this happens.

I like the idea :)
It would be nice for us to gather those info later!
-- 
William



Re: MEDIUM: Adding upstream socks4 proxy support

2019-05-06 Thread Willy Tarreau
Hi Alec,

On Tue, Apr 30, 2019 at 07:59:16PM +0800, Alec Liu wrote:
> > I'm seeing that you copied a doc retrieved from somewhere else that is
> > found at various places on the net. Have you checked the license for
> > this doc to be sure we can copy and distribute it like this ? It might
> > be perfectly fine but we need to be sure. In any case we'll have to
> > mention the license if it differs from the other ones.
> 
> I have the "SOCKS: A protocol for TCP proxy across firewalls" doc
> found at openssh's website
> under OpenSSH Sprcifications along with those RFCs.
> "https://www.openssh.com/specs.html;,
> "https://www.openssh.com/txt/socks4.protocol;
> And OpenSSH is using GPL, so I believe it should be fine.

OK. Please at least mention it in the commit message so that if anybody
complains, there is no doubt about the origin and we can even take
appropriate actions such as removing and replacing it with a link or
anything else. Similarly if this doc ever were updated, someone could
propose us some updates.

> > >  struct check_status {
> > > diff --git a/include/types/connection.h b/include/types/connection.h
> > > index 308be7d7..2a1b23b4 100644
> > > --- a/include/types/connection.h
> > > +++ b/include/types/connection.h
> > > @@ -47,6 +47,15 @@ struct server;
> > >  struct session;
> > >  struct pipe;
> > >
> > > +/* socks4 upstream proxy definitions */
> > > +struct socks4_request {
> > > + uint8_t version;/* SOCKS version number, 1 byte, must be 
> > > 0x04 for this version */
> > > + uint8_t command;/* 0x01 = establish a TCP/IP stream 
> > > connection */
> > > + uint16_t port;  /* port number, 2 bytes (in network byte 
> > > order) */
> > > + uint32_t ip;/* IP address, 4 bytes (in network byte 
> > > order) */
> > > + char user_id[8];/* the user ID string, variable length, 
> > > terminated with a null (0x00); Using "HAProxy\0" */
> > > +};
> > > +
> > >  /* Note: subscribing to these events is only valid after the caller has 
> > > really
> > >   * attempted to perform the operation, and failed to proceed or complete.
> > >   */
> > > @@ -156,7 +165,7 @@ enum {
> > >
> > >   CO_FL_EARLY_SSL_HS  = 0x4000,  /* We have early data pending, 
> > > don't start SSL handshake yet */
> > >   CO_FL_EARLY_DATA= 0x8000,  /* At least some of the data are 
> > > early data */
> > > - /* unused : 0x0001 */
> > > + CO_FL_SOCKS_HS  = 0x0001,  /* handshaking with upstream 
> > > SOCKS proxy */
> > >   /* unused : 0x0002 */
> >
> > From what I'm seeing later in the code, it's not exactly a handshake,
> > you have to send a header and to wait for the response header to arrive.
> > I'd rather have one flag per direction which save from having to deal
> > with state management.
> 
> Actually, I have the same idea as you, using two flags only. But ends up with
> this design later on. The reason I am doing it in this way, that's because:
> 1) I might want to add the socks5 protocol support onto it later on, which
> will become a little more complex, not just one request and response.
> We will get something like authentication method handshake, login,
> connection request, etc.

Then this is exactly what muxes are made for. They live their own life
and encapsulate application data in a way that is transparent for the
application layer. Ultimately we should be able to stack multiple such
transformations much easier than what it is today. The SSL layer is
currently being modified for this for example. I'd suggest that you
stick to two individual headers for SOCKS4 and that we see later how
SOCKS5 has to be done (if ever done by the way).

> 2) Looks like do not have much unused bits leftover within CO_FL_*.

That's not *your* problem but the one for the person having something
else to implement after you. Bits are like desserts in restaurants :
first come, first served.

> > >  /* possible connection error codes */
> > >  enum {
> > > @@ -451,6 +469,13 @@ struct connection {
> > >   struct {
> > >   struct sockaddr_storage from;   /* client address, or 
> > > address to spoof when connecting to the server */
> > >   struct sockaddr_storage to; /* address reached by the 
> > > client, or address to connect to */
> > > + struct {
> > > + unsigned char use;  /* socks4 
> > > proxy enabled  */
> > > + struct sockaddr_storage addr;   /* the 
> > > address of the socks4 proxy */
> > > + struct socks4_request req_line; /* the info 
> > > send to socks4 proxy */
> > > + unsigned int status;/* 
> > > CO_SOCKS_* */
> > > + signed short req_remain_len;/* the 
> > > length remain to sent */
> > > + } socks_proxy;
> >
> > It is extremely huge for the struct connection unfortunately. We're
> > 

Re: haproxy 2.0 docker images

2019-05-06 Thread Aleksandar Lazic

Hi.

Any answer to the questions below?

Regards
 Aleks

Sat Apr 27 12:47:17 GMT+02:00 2019 Aleksandar Lazic :

> Hi.
>
>
> I have now created some HAProxy 2.0 images ;-).
>
> The outputs below raises some questions to me.
>
> * Should in the OPTIONS output also be the EXTRA_OBJS ?
> * Should PCRE2 be used instead of PCRE ?
> * Should PRIVATE_CACHE be used in the default build?
> * Should SLZ be used in the default build?
> * Make NS sense in a container image?
> * Can DEVICEATLAS 51DEGREES WURFL be used together?
>  - From technically point of view
>  - From license point of view
>
> Images:
> https://hub.docker.com/r/me2digital/haproxy20-centos
> https://hub.docker.com/r/me2digital/haproxy20-boringssl
>
> Build logs:
> https://gitlab.com/aleks001/haproxy20-centos/-/jobs/203092688
> https://gitlab.com/aleks001/haproxy20-boringssl/-/jobs/203110753
>
> haproxy -vv outputs:
>
> ```
> HA-Proxy version 2.0-dev2-5e6a5b-228 2019/04/25 - https://haproxy.org/
> Build options :
>  TARGET = linux2628
>  CPU = generic
>  CC = gcc
>  CFLAGS = -O2 -g -fno-strict-aliasing -Wdeclaration-after-statement -fwrapv
> -Wno-unused-label -Wno-sign-compare -Wno-unused-parameter
> -Wno-old-style-declaration -Wno-ignored-qualifiers -Wno-clobbered
> -Wno-missing-field-initializers -Wtype-limits
>  OPTIONS = USE_PCRE=1 USE_PCRE_JIT=1 USE_THREAD=1 USE_PTHREAD_PSHARED=1
> USE_REGPARM=1 USE_LINUX_SPLICE=1 USE_GETADDRINFO=1 USE_OPENSSL=1 USE_LUA=1
> USE_ZLIB=1 USE_TFO=1
>
> Feature list : +EPOLL -KQUEUE -MY_EPOLL -MY_SPLICE +NETFILTER +PCRE +PCRE_JIT
> -PCRE2 -PCRE2_JIT +POLL -PRIVATE_CACHE +THREAD +PTHREAD_PSHARED +REGPARM
> -STATIC_PCRE -STATIC_PCRE2 +TPROXY +LINUX_TPROXY +LINUX_SPLICE +LIBCRYPT
> +CRYPT_H -VSYSCALL +GETADDRINFO +OPENSSL +LUA +FUTEX +ACCEPT4 -MY_ACCEPT4 
> +ZLIB
> -SLZ +CPU_AFFINITY +TFO -NS +DL +RT -DEVICEATLAS -51DEGREES -WURFL -SYSTEMD
> -OBSOLETE_LINKER +PRCTL
>
> Default settings :
>  bufsize = 16384, maxrewrite = 1024, maxpollevents = 200
>
> Built with multi-threading support (MAX_THREADS=64, default=1).
> Built with OpenSSL version : OpenSSL 1.1.1b 26 Feb 2019
> Running on OpenSSL version : OpenSSL 1.1.1b 26 Feb 2019
> OpenSSL library supports TLS extensions : yes
> OpenSSL library supports SNI : yes
> OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2 TLSv1.3
> Built with Lua version : Lua 5.3.5
> Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT
> IP_FREEBIND
> Built with zlib version : 1.2.7
> Running on zlib version : 1.2.7
> Compression algorithms supported : identity("identity"), deflate("deflate"),
> raw-deflate("deflate"), gzip("gzip")
> Built with PCRE version : 8.32 2012-11-30
> Running on PCRE version : 8.32 2012-11-30
> PCRE library supports JIT : yes
> Encrypted password support via crypt(3): yes
> Built with the Prometheus exporter as a service
>
> Available polling systems :
>  epoll : pref=300, test result OK
>  poll : pref=200, test result OK
>  select : pref=150, test result OK
> Total: 3 (3 usable), will use epoll.
>
> Available multiplexer protocols :
> (protocols marked as   cannot be specified using 'proto' keyword)
>  h2 : mode=HTX side=FE|BE
>  h2 : mode=HTTP side=FE
>: mode=HTX side=FE|BE
>: mode=TCP|HTTP side=FE|BE
>
> Available services :
>  prometheus-exporter
>
> Available filters :
>  [SPOE] spoe
>  [COMP] compression
>  [CACHE] cache
>  [TRACE] trace
> ```
>
> ```
> $ docker run --rm --entrypoint /usr/local/sbin/haproxy
> [MASKED]/haproxy20-boringssl -vv
> HA-Proxy version 2.0-dev2-5e6a5b-228 2019/04/25 - https://haproxy.org/
> Build options :
>  TARGET = linux2628
>  CPU = generic
>  CC = gcc
>  CFLAGS = -O2 -g -fno-strict-aliasing -Wdeclaration-after-statement -fwrapv
> -Wno-unused-label -Wno-sign-compare -Wno-unused-parameter
> -Wno-old-style-declaration -Wno-ignored-qualifiers -Wno-clobbered
> -Wno-missing-field-initializers -Wtype-limits -Wshift-negative-value
> -Wshift-overflow=2 -Wduplicated-cond -Wnull-dereference
>  OPTIONS = USE_PCRE2=1 USE_PCRE2_JIT=1 USE_THREAD=1 USE_PTHREAD_PSHARED=1
> USE_REGPARM=1 USE_LINUX_SPLICE=1 USE_GETADDRINFO=1 USE_OPENSSL=1 USE_LUA=1
> USE_ZLIB=1 USE_TFO=1
>
> Feature list : +EPOLL -KQUEUE -MY_EPOLL -MY_SPLICE +NETFILTER -PCRE -PCRE_JIT
> +PCRE2 +PCRE2_JIT +POLL -PRIVATE_CACHE +THREAD +PTHREAD_PSHARED +REGPARM
> -STATIC_PCRE -STATIC_PCRE2 +TPROXY +LINUX_TPROXY +LINUX_SPLICE +LIBCRYPT
> +CRYPT_H -VSYSCALL +GETADDRINFO +OPENSSL +LUA +FUTEX +ACCEPT4 -MY_ACCEPT4 
> +ZLIB
> -SLZ +CPU_AFFINITY +TFO -NS +DL +RT -DEVICEATLAS -51DEGREES -WURFL -SYSTEMD
> -OBSOLETE_LINKER +PRCTL
>
> Default settings :
>  bufsize = 16384, maxrewrite = 1024, maxpollevents = 200
>
> Built with multi-threading support (MAX_THREADS=64, default=1).
> Built with OpenSSL version : BoringSSL
> OpenSSL library supports TLS extensions : yes
> OpenSSL library supports SNI : yes
> OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2 TLSv1.3
> Built with Lua version : Lua 5.3.5
> Built with transparent proxy support using: IP_TRANSPARENT 

Re: [1.9.7] One of haproxy processes using 100% CPU

2019-05-06 Thread Willy Tarreau
Hi Maciej,

On Mon, May 06, 2019 at 06:49:26AM +0200, Maciej Zdeb wrote:
> Hi,
> 
> I confirm Willy patch fixed the problem! Thanks!

Great, thanks for confirming!
Willy



Re: reg-tests are broken when running osx + openssl

2019-05-06 Thread Willy Tarreau
On Sun, May 05, 2019 at 10:11:38PM +0200, Frederic Lecaille wrote:
> I had no access to any travis environment when it has been told in previous
> mails that /tmp could not work, and /var/tmp could not either.
> They were the first tested values.
> 
> Now that I have setup a travis account, I have just double checked, and
> contrary to what has been previously told, /tmp value for TMPDIR may make
> the reg tests work on OS X with travis.
> 
> So here is attached to this mail a better fix.

Excellent, thank you Fred for testing and for the explanation! Patch
merged now, we'll see if things improve.

Willy



Re: [PR] BUILD: extend travis-ci matrix

2019-05-06 Thread Willy Tarreau
Hi Ilya,

> I made another PR
> 
> https://github.com/haproxy/haproxy/pull/92

Thank you.

> (I really like automatic PR to mailing list routing)

Well, it was the only workable workaround we have when people send PRs.
Sadly we can't block them. Apparently only mirror repositories can block
PRs and github doesn't accept to create them anymore. But when there are
multiple patches in the PR it's not usable as-is anymore, it only serves
as a notification that someone sent something.

For me, PRs tremendously increase the amount of manipulations, which is
why I tend to postpone them until I think this will not divert me too
much from what I am doing. I just timed the operations on this small
batch and it went from ~15 seconds to review and merge your 4 patches
if they were in mails (just pressing Enter in mutt to review them,
possibly "e" to quickly edit if needed, and "A" to apply), then "g" to
ack the merge, to around 3 minutes to :
  - copy the link
  - open a browser
  - paste the link into the browser's URL bar
  - locate the links to the commits in the page
  - open each of them in a separate tab
  - visit each tab in turn, and for each of them, press Ctrl-L and
append ".patch" at the end of the URL, then press enter
  - after the patches are loaded, visit all of them again, to look at the
dates and figure in what order to apply them
  - then for each of them :
  - review the patch
  - Ctrl-S to save the patch
  - remember the file name (commit ID) and press Enter
  - Alt-tab to switch to the xterm in the development directory
  - git am ~/.patch
  - rm -f ~/.patch
  - Alt-tab to switch back to browser again
  - Ctrl-W to close the tab
  - close browser window
  - back to mutt to ack merging and rant about my hate of this workflow

=> This is way longer and more error-prone than the first variant. And
   this doesn't even include commenting if I disagree on something, which
   additionally requires copy-pasting the relevant parts of the patch
   into the response message. There is a reason why most projects
   developed on github are so low quality with so horrible commits
   and zero maintenance : it encourages laziness and dirtiness. It
   costs so much effort to fix minor issues and encourage people to
   improve, compared to lazily click on "merge" that you simply think
   "bah..." and you merge it as-is, making it impossible to review them
   later when trying to make a maintenance version. So you end up with
   continuous development made of a pile of junk patches :-/

Anyway now I've finally applied your patches. In the future, if you
want to help with a smoother process, please either attach your patches
or use git-send-email. The most efficient workflow is one patch per mail,
like git-send-email does, as it eases reviews (which can also be done by
multiple persons). If you attach them, make sure to use the file names
created by git-format-patch so that there's no doubt when saving files
for manual editing for example. Also it's important to indicate the
intent with a patch or a patch series, i.e. "please merge this", "please
don't merge this, it's just for discussing", "it should be mergeable once
the makefile is fixed", "I think it's OK for merging but I'd prefer an
extra review first", "how about we'd do this", etc. I'm all for being
accommodating with submissions and slightly edit them if needed because
I know that if the work has to be 100% on either side it can total more
time than 90/10 or even 95/5 (but I never edit signed patches beyond
fixing merging issues though). I'm just very careful about the time
spent on my side because I know I'm already a bottleneck, so each extra
minute added here delays everything else.

Thanks!
Willy