Extending statistics in the stick-table

[Marcus Schiesser]

Hi there,

using the stick-table (see
http://cbonte.github.io/haproxy-dconv/1.7/configuration.html#4-stick-table)
it is possible to
gather client-specific statistics, e.g. http_req_rate
Unfortunately in my case these statistics are just a subset of the
aggregated ones provided for the whole proxy (see
http://cbonte.github.io/haproxy-dconv/1.7/management.html#9).
I am interested in extending the statistics in the stick-table. Anyone else
too?

Best,
Marcus

Re: [ANNOUNCE] haproxy-1.7-dev5

[Cyril Bonté]

Le 26/10/2016 à 23:31, Willy Tarreau a écrit :

Hi Cyril,

On Wed, Oct 26, 2016 at 11:19:29PM +0200, Cyril Bonté wrote:

Hi all,

Le 25/10/2016 à 23:15, Willy Tarreau a écrit :

Hi,

HAProxy 1.7-dev5 was released on 2016/10/25. It added 65 new commits
after version 1.7-dev4.
[...]
Please find the usual URLs below :
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/


and I've regenerated the documentation for all versions this morning.
Unfortunately the process is not automatic anymore until I find some time to
fix my scripts ;-)
Occasionally, if you think the documentation is too old, don't hesitate to
send me an email to remind me I have some work to do :-) (as Lukas did some
days/weeks ago)


Well noted, thanks for taking care of this. Do you prefer to be CCed in
announces ?


It's fine as is, I only forgot to launch the job for 1.6.9, and it is 
supposed to be temporary (I already said that in july... and I really 
hope to fix this shortly). In the meantime, the 1.7 documentation can 
derivate quite quickly, this is where everybody can contact me to do the 
job ;-)



--
Cyril Bonté

Re: [ANNOUNCE] haproxy-1.7-dev5

[Willy Tarreau]

Hi Cyril,

On Wed, Oct 26, 2016 at 11:19:29PM +0200, Cyril Bonté wrote:
> Hi all,
> 
> Le 25/10/2016 à 23:15, Willy Tarreau a écrit :
> > Hi,
> > 
> > HAProxy 1.7-dev5 was released on 2016/10/25. It added 65 new commits
> > after version 1.7-dev4.
> > [...]
> > Please find the usual URLs below :
> >Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/
> 
> and I've regenerated the documentation for all versions this morning.
> Unfortunately the process is not automatic anymore until I find some time to
> fix my scripts ;-)
> Occasionally, if you think the documentation is too old, don't hesitate to
> send me an email to remind me I have some work to do :-) (as Lukas did some
> days/weeks ago)

Well noted, thanks for taking care of this. Do you prefer to be CCed in
announces ?

Willy

Re: [ANNOUNCE] haproxy-1.7-dev5

[Cyril Bonté]

Hi all,

Le 25/10/2016 à 23:15, Willy Tarreau a écrit :

Hi,

HAProxy 1.7-dev5 was released on 2016/10/25. It added 65 new commits
after version 1.7-dev4.
[...]
Please find the usual URLs below :
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/


and I've regenerated the documentation for all versions this morning. 
Unfortunately the process is not automatic anymore until I find some 
time to fix my scripts ;-)
Occasionally, if you think the documentation is too old, don't hesitate 
to send me an email to remind me I have some work to do :-) (as Lukas 
did some days/weeks ago)


--
Cyril Bonté

Re: stick table entries in use

[Willy Tarreau]

Hi,

On Wed, Oct 26, 2016 at 11:11:23AM +, Gernot Pörner wrote:
> Hi,
> 
> we are running haproxy 1.5.18 on Ubuntu 14. 
> 
> Recently we had some issues with misuse of our api by botnets coming from 
> many different IPs 
> and implemented a mitigation based on haproxys stick-table feature which 
> works pretty good for us.
> 
> The relevant part of our config looks like this:
> 
> acl document_requestpath_beg -i /api/v1/sessions
> acl too_many_tries  sc0_gpc0_rate() gt 5
> acl mark_seen   sc0_inc_gpc0 gt 0
> stick-table type string size 100k expire 5m store gpc0_rate(5s)
> tcp-request content track-sc0 hdr(True-Client-IP) if 
> METH_POST document_request
> use_backend l2b_slow_down   if mark_seen too_many_tries
> backend l2b_slow_down
> mode http
> timeout tarpit 20s
> http-request tarpit
> 
> Now, when some IP (IPv4 or IPv6 doesn't matter since we use string here) 
> sends more than
> 5 POST requests to our api resource in 5 seconds, it will end up in the table 
> and all 
> subsequent requests are delayed and answered with an error 500.
> 
> After 5 minutes the table entry should expire. This usually works but 
> sometimes it
> doesn't. We see entries in the stick table whose expiry is down to 0 but they 
> don't 
> get removed since the "use" counter is something greater than zero
> 
> It then looks like this:
> 
> echo "show table l1f_loadbalancer" | sudo socat stdio /var/run/haproxy.stat
> 
> # table: l1f_loadbalancer, type: string, size:102400, used:7
> 0x429af6c: key=37.191.169.199 use=1 exp=0 gpc0_rate(5000)=0
> 0x2f1d54c: key=37.191.204.19 use=1 exp=0 gpc0_rate(5000)=0
> 0x3c105bc: key=50.115.248.18 use=1 exp=0 gpc0_rate(5000)=0
> 0x3293ebc: key=65.214.65.27 use=1 exp=0 gpc0_rate(5000)=0
> 0x30b0acc: key=68.180.24.112 use=2 exp=0 gpc0_rate(5000)=0
> 0x53571bc: key=68.180.57.63 use=1 exp=0 gpc0_rate(5000)=0
> 0x300f93c: key=68.187.196.250 use=1 exp=0 gpc0_rate(5000)=0
> 
> I also cannot delete them while they are "in use":
> 
> echo "clear table l1f_loadbalancer key 50.115.248.18" | sudo socat stdio 
> /var/run/haproxy.stat
> Entry currently in use, cannot remove
> 
> clear table without a key only removes entries which are at use=0

That's expected. "use>0" means that some sessions still track this entry,
so it cannot be removed. The value represents the number of trackers
still on it. It is possible that you're having some persistent HTTP
connections bound to it and that you may have to wait for the idle
timeout to expire. If you're seeing the same ones last much longer
than the request timeout, there might be another issue though. Then
issuing "show sess all" on the stats socket to get a dump before
restarting may possibly help.

By the way if you want to use stick-tables for such usages, I strongly
recommend trying 1.6 which is richer there. You can for example track in
http-request rules, and you can exchange the stick counters between peers.

Willy

Re: [PR] Update haproxy.spec URLs to haproxy.org

[Willy Tarreau]

On Wed, Oct 26, 2016 at 04:02:00PM +0200, PR Bot wrote:
> Dear list!
> 
> Author: Grant 
> Number of patches: 1
> 
> This is an automated relay of the Github pull request:
>Update haproxy.spec URLs to haproxy.org
> 
> Patch title(s): 
>Update haproxy.spec URLs to haproxy.org
> 
> Link:
>https://github.com/haproxy/haproxy/pull/23

Applied, thanks.
willy

Re: High CPU usage when exists CLOSE_WAIT connections

[Willy Tarreau]

Hi,

On Wed, Oct 26, 2016 at 04:33:15PM +, Robson Roberto Souza Peixoto wrote:
> Can anyone help-me with it, please? It's look like a bug:
> http://discourse.haproxy.org/t/high-cpu-usage-when-exists-close-wait-connections/754

You forgot to mention the exact version (haproxy -vv). What OS is
this ? I'm seeing poll() being used while on modern OSes we use
epoll or kqueue. This is not important but it can help spot the
problem.

Do the source or destination ports represent something special in your
case, such as peers, stats page or whatever ?

Willy

Re: [PR] Minor spelling correction

[Willy Tarreau]

On Wed, Oct 26, 2016 at 04:01:58PM +0200, PR Bot wrote:
> Dear list!
> 
> Author: Ian Miell 
> Number of patches: 2
> 
> This is an automated relay of the Github pull request:
>Minor spelling correction
> 
> Patch title(s): 
>Very minor spelling correction
>Merge pull request #1 from ianmiell/ianmiell-patch-1

Applied, thanks.

Willy

High CPU usage when exists CLOSE_WAIT connections

[Robson Roberto Souza Peixoto]

Can anyone help-me with it, please? It's look like a bug:
http://discourse.haproxy.org/t/high-cpu-usage-when-exists-close-wait-connections/754
-- 
Robson Roberto Souza Peixoto
Robinho
Master in Computer Science, University of Campinas
IRC: robsonpeixoto
Twitter: http://twitter.com/robinhopeixoto
github: https://github.com/robsonpeixoto

[PR] Update haproxy.spec URLs to haproxy.org

[PR Bot]

Dear list!

Author: Grant 
Number of patches: 1

This is an automated relay of the Github pull request:
   Update haproxy.spec URLs to haproxy.org

Patch title(s): 
   Update haproxy.spec URLs to haproxy.org

Link:
   https://github.com/haproxy/haproxy/pull/23

Edit locally:
   wget https://github.com/haproxy/haproxy/pull/23.patch && vi 23.patch

Apply locally:
   curl https://github.com/haproxy/haproxy/pull/23.patch | git am -

Description:


Instructions:
   This github pull request will be closed automatically; patch should be
   reviewed on the haproxy mailing list (haproxy@formilux.org). Everyone is
   invited to comment, even the patch's author. Please keep the author and
   list CCed in replies. Please note that in absence of any response this
   pull request will be lost.

[PR] Minor spelling correction

[PR Bot]

Dear list!

Author: Ian Miell 
Number of patches: 2

This is an automated relay of the Github pull request:
   Minor spelling correction

Patch title(s): 
   Very minor spelling correction
   Merge pull request #1 from ianmiell/ianmiell-patch-1

Link:
   https://github.com/haproxy/haproxy/pull/33

Edit locally:
   wget https://github.com/haproxy/haproxy/pull/33.patch && vi 33.patch

Apply locally:
   curl https://github.com/haproxy/haproxy/pull/33.patch | git am -

Description:
   'optionnally' -> 'optionally'

Instructions:
   This github pull request will be closed automatically; patch should be
   reviewed on the haproxy mailing list (haproxy@formilux.org). Everyone is
   invited to comment, even the patch's author. Please keep the author and
   list CCed in replies. Please note that in absence of any response this
   pull request will be lost.

Re: [PR] Cleanup

[Willy Tarreau]

On Wed, Oct 26, 2016 at 03:16:30PM +0200, Lukas Tribus wrote:
> > Link:
> > https://github.com/haproxy/haproxy/pull/42
> 
> This pull-request is from October 2015, I believe at least some of those
> patches have been applied, others may not be relevant anymore; but I didn't
> want to discard the whole series because I didn't carefully verify the
> patches.

Confirmed, I handled it with Erwan's help a few months ago.

Thanks,
willy

Re: [PR] Cleanup

[Lukas Tribus]

Hello,


Am 26.10.2016 um 15:06 schrieb PR Bot No-Reply:

Dear list!

Author: Erwan Velu 
Number of patches: 8

This is an automated relay of the Github pull request:
Cleanup

Patch title(s):
CLEANUP: don't ignore debian/ directory if present
cfgparse: Protect free on NULL pointer
proto_http: Removing useless variable assignation
proto_http: Removing useless variable
proto_http: Removing useless delta assignation
dumpstats: Removing useless variables allocation
payload: Removing useless pointer arithmetic
dns: Removing usless variable & assignation

Link:
https://github.com/haproxy/haproxy/pull/42


This pull-request is from October 2015, I believe at least some of those 
patches have been applied, others may not be relevant anymore; but I 
didn't want to discard the whole series because I didn't carefully 
verify the patches.



Thanks,

Lukas

[PR] Cleanup

[PR Bot No-Reply]

Dear list!

Author: Erwan Velu 
Number of patches: 8

This is an automated relay of the Github pull request:
   Cleanup

Patch title(s): 
   CLEANUP: don't ignore debian/ directory if present
   cfgparse: Protect free on NULL pointer
   proto_http: Removing useless variable assignation
   proto_http: Removing useless variable
   proto_http: Removing useless delta assignation
   dumpstats: Removing useless variables allocation
   payload: Removing useless pointer arithmetic
   dns: Removing usless variable & assignation

Link:
   https://github.com/haproxy/haproxy/pull/42

Edit locally:
   wget https://github.com/haproxy/haproxy/pull/42.patch && vi 42.patch

Apply locally:
   curl https://github.com/haproxy/haproxy/pull/42.patch | git am -

Description:
   Please find in this PR my with contribution to the project.
   It
   does contains a few commits targeting a static analysis of possible
   mistakes or useless code.
   My changes looks like safe but as I'm
   new to that code, I could be wrong. Feel free to pick only some
   commits if some looks like too picky or useless for the project.
   That was my 2 cents applying to the current master tree.
   Cheers,
   Erwan

Instructions:
   This github pull request will be closed automatically; patch should be
   reviewed on the haproxy mailing list (haproxy@formilux.org). Everyone is
   invited to comment, even the patch's author. Please keep the author and
   list CCed in replies. Please note that in absence of any response this
   pull request will be lost.

Re: S FTP + HA PROXY confutation facing one serious issue.

[mal reddy]

Hi Ha proxy Team,

Any updates.

*MyObjective*:Client upload file via HaProxy and it will upload in sftp
servers

*example*:
expected this type of configuration in Ha Proxy

a.kk.com
 192.168.0.1(sftp 1 server ip)
b.kk.com
  192.168.0.2(sftp 2 server ip)
default(if the config didn't found any things then it will
redirect to default sftp server)
192.168.0.1(sftp 1 server ip)



   I checked HA-Proxy document and other website but most of solution that
found was http.

using ha proxy configuration document, i successfully upload file for
one client.

 but problem when i have to configuration more then one clients that
time

it will redirect all clients request to one sftp server instead of
different sftp server.



Thanks,
Malreddy Tugu.



On Tue, Oct 25, 2016 at 11:38 AM, Hemang Rami  wrote:

> Hello HaProxy Team,
>
>
> *MyObjective*:Client upload file via HaProxy and it will upload in sftp
> servers
>
> *example*:
> expected this type of configuration in Ha Proxy
>
> a.kk.com
>  192.168.0.1(sftp 1 server ip)
> b.kk.com
>   192.168.0.2(sftp 2 server ip)
> default(if the config didn't found any things then it will
> redirect to default sftp server)
> 192.168.0.1(sftp 1 server ip)
>
>
>
>I checked HA-Proxy document and other website but most of solution that
> found was http.
>
> using ha proxy configuration document, i successfully upload file for
> one client.
>
>  but problem when i have to configuration more then one clients that
> time
>
> it will redirect all clients request to one sftp server instead of
> different sftp server.
>
>
> i upload my architecture diagram for more clarity.
>
>
>
>
> --
>
>
> Thank You
> Regards,
> Hemang Rami
> (09427062026)
>
>
>
>
>
> --
>
>
> Thank You
> Regards,
> Hemang Rami
> (09427062026)
>
>
>

Re: haproxy-systemd-wrapper exit code problem

[Gabriele Cerami]

On 26 Oct, Willy Tarreau wrote:
> to think of it. Are you sure your version doesn't contain certain patches
> affecting the error return path, maybe just some local debugging patches
> that were forgotten ?


I'm only seeing this applied before creating the package
https://git.centos.org/blob/rpms!haproxy.git/7463da74923be7492605bf742540ca6abe44d86e/SOURCES!haproxy-tcp-user-timeout.patch

I've not checked, but this kind of patches are usually only backports 
of already existing patches.


Thanks for the quick reply.

stick table entries in use

[Gernot Pörner]

Hi,

we are running haproxy 1.5.18 on Ubuntu 14. 

Recently we had some issues with misuse of our api by botnets coming from many 
different IPs 
and implemented a mitigation based on haproxys stick-table feature which works 
pretty good for us.

The relevant part of our config looks like this:

acl document_requestpath_beg -i /api/v1/sessions
acl too_many_tries  sc0_gpc0_rate() gt 5
acl mark_seen   sc0_inc_gpc0 gt 0
stick-table type string size 100k expire 5m store gpc0_rate(5s)
tcp-request content track-sc0 hdr(True-Client-IP) if 
METH_POST document_request
use_backend l2b_slow_down   if mark_seen too_many_tries
backend l2b_slow_down
mode http
timeout tarpit 20s
http-request tarpit

Now, when some IP (IPv4 or IPv6 doesn't matter since we use string here) sends 
more than
5 POST requests to our api resource in 5 seconds, it will end up in the table 
and all 
subsequent requests are delayed and answered with an error 500.

After 5 minutes the table entry should expire. This usually works but sometimes 
it
doesn't. We see entries in the stick table whose expiry is down to 0 but they 
don't 
get removed since the "use" counter is something greater than zero

It then looks like this:

echo "show table l1f_loadbalancer" | sudo socat stdio /var/run/haproxy.stat

# table: l1f_loadbalancer, type: string, size:102400, used:7
0x429af6c: key=37.191.169.199 use=1 exp=0 gpc0_rate(5000)=0
0x2f1d54c: key=37.191.204.19 use=1 exp=0 gpc0_rate(5000)=0
0x3c105bc: key=50.115.248.18 use=1 exp=0 gpc0_rate(5000)=0
0x3293ebc: key=65.214.65.27 use=1 exp=0 gpc0_rate(5000)=0
0x30b0acc: key=68.180.24.112 use=2 exp=0 gpc0_rate(5000)=0
0x53571bc: key=68.180.57.63 use=1 exp=0 gpc0_rate(5000)=0
0x300f93c: key=68.187.196.250 use=1 exp=0 gpc0_rate(5000)=0

I also cannot delete them while they are "in use":

echo "clear table l1f_loadbalancer key 50.115.248.18" | sudo socat stdio 
/var/run/haproxy.stat
Entry currently in use, cannot remove

clear table without a key only removes entries which are at use=0

The only way to get rid of them is a complete restart of haproxy

Can anybody tell me why this is happening? The documentation, while being great 
in general
is a bit short on the stick-table output so I am not really sure what "use=x" 
exactly stands
for if it is > 0.

Thanks

Gernot

[PR] Added dist tag to to release field in .spec file

[Lukas Tribus]

Dear list!

Author: Sasa Tekovic 
Number of patches: 1

This is an automated relay of the Github pull request:
   Added dist tag to to release field in .spec file

Patch title(s): 
   Added dist tag to to release field in .spec file

Link:
   https://github.com/haproxy/haproxy/pull/60

Edit locally:
   wget https://github.com/haproxy/haproxy/pull/60.patch && vi 60.patch

Apply locally:
   curl https://github.com/haproxy/haproxy/pull/60.patch | git am -

Description:
   Added dist tag to to release field in order to differentiate RPMs
   built for different releases of RHEL/CentOS and similar distributions.

Instructions:
   This github pull request will be closed automatically; patch should be
   reviewed on the haproxy mailing list (haproxy@formilux.org). Everyone is
   invited to comment, even the patch's author. Please keep the author and
   list CCed in replies. Please note that in absence of any response this
   pull request will be lost.

Re: haproxy-systemd-wrapper exit code problem

[Willy Tarreau]

On Wed, Oct 26, 2016 at 11:53:08AM +0200, Gabriele Cerami wrote:
> On 22 Oct, Willy Tarreau wrote:
> > Hi Gabriele,
> > 
> > On Tue, Oct 18, 2016 at 09:40:14PM +0200, Gabriele Cerami wrote:
> > > Hi,
> > > 
> > > We're having a problem with version 1.5.14 of haproxy, packaged for
> > > CentOS 7, but it seems even the code in master is affected.
> > > 
> > > In situations where bind is not possible (in our case, the address was
> > > already in use) tcp_connect_server returns with a status of 256
> > > (ERR_ALERT). This value is then passed down as exit code for
> > > haproxy-systemd-wrapper.
> > 
> > Huh ? First, tcp_connect_server() is not involved here, it's used to
> > connect to a server, so it never provides an exit code. And even if
> > instead you meant a bind issue instead, all error codes are limited
> > to 5 bits so the higher value you can have is 0x1F = 31 if all flags
> 
> Yeah, sorry about that hastily made root cause analysis.
> I'll stick to the facts.
> This is what we're seeing
> 
> http://paste.openstack.org/show/586174/

OK so it's the bind for the listener which fails, that makes more sense.
I'm amazed to see that some warnings and errors are reported as coming
from the wrapper while they're emitted by haproxy. Is it another funny
trick of systemd to make your troubleshooting more complicated ?

I don't understand why yet, but I'm not seeing the haproxy message
which should say "[haproxy.main()] cannot bind socket.", instead we're
seeing a different syntax and it is reported from the wrapper. Also
the exit path here clearly does "exit(1)". So I don't know much what
to think of it. Are you sure your version doesn't contain certain patches
affecting the error return path, maybe just some local debugging patches
that were forgotten ?

> The complete output is somewhat difficult to get, it's available here
> http://logs.openstack.org/periodic/periodic-tripleo-ci-centos-7-ovb-nonha/9697c38/logs/undercloud/var/log/undercloud_install.txt.gz
> 
> but you'd have to download it, ungzip it, then open the journal log with
> journalctl --file
> var/log/journal/d327fbd51a9f4c4997bdeba685dc22d2/system.journal  | less
> -AXFR

Unfortunately you just made me realize another good reason for staying
away from systemd-based distros, which is that your logs can only be
processed by people using the same stuff, but I personally can't do
anything with these files :-/

> As you can see, the thread spawned by the wrapper is returning 256 as
> status code, and this is interpreded as 0/success by systemd, even if
> it's actually failing to bind to a port, and thus exiting.

That's normal because exit codes are supposed to be in the lower 8 (well
7) bits. So in practice systemd really receives zero here. However I'm
seeing something now. We're directly returning status here which is wrong
as it contains some higher bits, we should use WEXITSTATUS() there if
WIFEXITED() is true. In fact, now I'm pretty sure that this 256 simply
is exit code 1 shifted 8 bits left. Please note that the wrapper in 1.5.14
still has lots of defects that were fixed later, specifically in the
return path. But this one was not fixed yet and definitely needs to be
addressed.

I'll try to propose a patch (but you'll have to adjust it by hand if you
want to put it into 1.5.14).

Willy

Re: haproxy-systemd-wrapper exit code problem

[Gabriele Cerami]

On 22 Oct, Willy Tarreau wrote:
> Hi Gabriele,
> 
> On Tue, Oct 18, 2016 at 09:40:14PM +0200, Gabriele Cerami wrote:
> > Hi,
> > 
> > We're having a problem with version 1.5.14 of haproxy, packaged for
> > CentOS 7, but it seems even the code in master is affected.
> > 
> > In situations where bind is not possible (in our case, the address was
> > already in use) tcp_connect_server returns with a status of 256
> > (ERR_ALERT). This value is then passed down as exit code for
> > haproxy-systemd-wrapper.
> 
> Huh ? First, tcp_connect_server() is not involved here, it's used to
> connect to a server, so it never provides an exit code. And even if
> instead you meant a bind issue instead, all error codes are limited
> to 5 bits so the higher value you can have is 0x1F = 31 if all flags

Yeah, sorry about that hastily made root cause analysis.
I'll stick to the facts.
This is what we're seeing

http://paste.openstack.org/show/586174/

The complete output is somewhat difficult to get, it's available here
http://logs.openstack.org/periodic/periodic-tripleo-ci-centos-7-ovb-nonha/9697c38/logs/undercloud/var/log/undercloud_install.txt.gz

but you'd have to download it, ungzip it, then open the journal log with
journalctl --file
var/log/journal/d327fbd51a9f4c4997bdeba685dc22d2/system.journal  | less
-AXFR

As you can see, the thread spawned by the wrapper is returning 256 as
status code, and this is interpreded as 0/success by systemd, even if
it's actually failing to bind to a port, and thus exiting.

thanks for any help

Re: OpenSSL 1.1.0 support

[Willy Tarreau]

Hi Dirkjan,

sorry for the delay.

On Sat, Sep 17, 2016 at 10:56:13PM +0200, Dirkjan Bussink wrote:
> I've gone for a somewhat different approach in this patch with a
> compatibility header file. It defines some inline functions from 1.1.0 when
> they are not available. Right now it implements the minimal things that
> HAProxy needs, so they aren't full replacements of what OpenSSL 1.1.0 would
> provide.

I definitely like this approach, and to be clear I want it merged before
1.7 release.

> It's in a separate header file, was not sure if this was something that
> should go in include/proto/ssl_sock.h then. Also brings up the question what
> the minimum version of OpenSSL is that is supported? A lot of the functions
> used are already available in 1.0.0 (which is already EOL), so not sure if
> the compatibility code is needed for 0.9.8 then?

Yes I'd rather have it work with 0.9.8 as it's still supported and used by
some LTS distros. For example, RHEL5's regular support is due till March 2017
and extended support till november 2020. It's not very likely that such users
will decide to upgrade to 1.7 now especially since some of the SSL-related
features only work with newer versions. But if we can make it work as it used
to with limited effort it's better. Is your current patch capable of supporting
0.9.8 ?

> Is this more the approach you were thinking about?

Yes definitely. Also your new patch is much more readable and will make it
easier to drop older versions in the future, I like it. I'll ping Emeric
again since we got no response (ie I'll yell in the office) :-)

Thanks!
Willy

Re: clang static analysis of haproxy

[Willy Tarreau]

On Wed, Oct 26, 2016 at 01:33:24PM +0500,  ?? wrote:
> properly tuned travis-ci moves the pressure from maintainer to contributor,
> exactly like you told above :-)
> 
> https://travis-ci.org/openresty/lua-nginx-module/pull_requests
> 
> PR are being automatically tested (build matrix might be "gcc, clang" x
> "linux, osx", even more wide if you use docker here)

Most of the problems I have with contributions are actually not in the
code, as code can be fixed, but the scarcity of commit messages. People
send a patch and disappear. Then all of us have to deal with that patch
for 15 years, regularly scratching our head every time it is suspected
of participating to a bug, trying to find whether or not what was done
there was intended or not. And even for backports the details are very
important. In general when commit messages are of good quality I merge
them almost without reading the code, because I know that anyone will be
able to amend it later if needed. I'm not sure travis-ci will take care
of this.

Willy

Re: SSL patches

[Thierry Fournier]

On Mon, 24 Oct 2016 19:16:03 +0200
Willy Tarreau  wrote:

> Hi Thierry,
> 
> On Mon, Oct 24, 2016 at 06:32:29PM +0200, Thierry Fournier wrote:
> > Hi, thank you for the tests. It is exactly the last test. The patches
> > seems to be good.
> > 
> > The patches can be merged, if it is possible.
> 
> OK I've merged them now, thanks Thierry for doing this, and thanks
> Beluc for testing.


Thanks Willy and Beluc.

Thierry

Re: clang static analysis of haproxy

[Илья Шипицин]

2016-10-26 13:26 GMT+05:00 Willy Tarreau :

> On Wed, Oct 26, 2016 at 01:15:43PM +0500,  ?? wrote:
> > as time goes on ... I see, haproxy moved to github ?
>
> No, it's another fork dedicated to travis that causes some confusion
> and that we're trying to smoothly move to fix this recurrent problem.
>
> > I like github, especially for PR and travis.
>
> Ah that's bad because PRs are exactly the reason I don't like github :
> they move the burden from contributors to maintainers in that it is
> really painful to request good quality patches and even more to fix
> the minor issues they have. And more importantly, it doesn't show
> people how to contribute good code nor does it incite people to start
> to participate. We all started by commenting a patch received in a
> mail where we spotted an obvious bug or wanted to give an opinion. It
> it important to keep this. We're in fact even setting up something to
> automatically forward the PRs to the list so that everyone can
> participate, and automatically close them since github doesn't offer
> the option to disable them.
>
> > should I open a PR on those (not very critial) issues ?
>
> Don't do it yet please, Lukas is currently trying to get rid of all
> of them in order to move the current repository.
>
> > and what do you think of travis-ci ? I have some experience in using it,
> > very nice.
>
> No opinion, I don't know it at all, but that's exactly the purpose of
> the current "haproxy" project on github, it contains some patches for
> travis maintained by Jeff. That's all I know about it. Maybe if we
> integrate this cleanly Jeff will not even need to maintain this repo
> anymore.
>


properly tuned travis-ci moves the pressure from maintainer to contributor,
exactly like you told above :-)

https://travis-ci.org/openresty/lua-nginx-module/pull_requests

PR are being automatically tested (build matrix might be "gcc, clang" x
"linux, osx", even more wide if you use docker here)


>
> Regards,
> Willt
>

Re: clang static analysis of haproxy

[Willy Tarreau]

On Wed, Oct 26, 2016 at 01:15:43PM +0500,  ?? wrote:
> as time goes on ... I see, haproxy moved to github ?

No, it's another fork dedicated to travis that causes some confusion
and that we're trying to smoothly move to fix this recurrent problem.

> I like github, especially for PR and travis.

Ah that's bad because PRs are exactly the reason I don't like github :
they move the burden from contributors to maintainers in that it is
really painful to request good quality patches and even more to fix
the minor issues they have. And more importantly, it doesn't show
people how to contribute good code nor does it incite people to start
to participate. We all started by commenting a patch received in a
mail where we spotted an obvious bug or wanted to give an opinion. It
it important to keep this. We're in fact even setting up something to
automatically forward the PRs to the list so that everyone can
participate, and automatically close them since github doesn't offer
the option to disable them.

> should I open a PR on those (not very critial) issues ?

Don't do it yet please, Lukas is currently trying to get rid of all
of them in order to move the current repository.

> and what do you think of travis-ci ? I have some experience in using it,
> very nice.

No opinion, I don't know it at all, but that's exactly the purpose of
the current "haproxy" project on github, it contains some patches for
travis maintained by Jeff. That's all I know about it. Maybe if we
integrate this cleanly Jeff will not even need to maintain this repo
anymore.

Regards,
Willt

Re: clang static analysis of haproxy

[Илья Шипицин]

as time goes on ... I see, haproxy moved to github ?
I like github, especially for PR and travis.

should I open a PR on those (not very critial) issues ?

and what do you think of travis-ci ? I have some experience in using it,
very nice.

2016-06-07 15:20 GMT+05:00 Willy Tarreau :

> Hi,
>
> On Mon, Jun 06, 2016 at 09:41:48PM +0500,  ?? wrote:
> > Hello,
> >
> > I run scan-build (which is clang static analysis tool) on haproxy git
> source
> >
> > here's result
> >
> > http://chipitsine.github.io/haproxy-1.7-dev/
> >
> >
> > there some null pointer dereferences and memory leaks, I think worth
> > looking at them.
>
> Thanks for doing this. As usual with such tools, a lot are false alarms
> but when looking at the logic error ones, I'm seeing this one which is
> totally valid :
>
>http://chipitsine.github.io/haproxy-1.7-dev/report-bf7c44.html#EndPath
>
> (the parenthesis is misplaced, it should be removed).
>
> This one is wrong :
>http://chipitsine.github.io/haproxy-1.7-dev/report-562959.html#EndPath
>(s is never NULL)
>
> This one as well :
>http://chipitsine.github.io/haproxy-1.7-dev/report-562959.html#EndPath
>(LIST_NEXT is valid if LIST_ISEMPTY is false)
>
> This one is right (but split in multiple reports) :
>http://chipitsine.github.io/haproxy-1.7-dev/report-562959.html#EndPath
>http://chipitsine.github.io/haproxy-1.7-dev/report-38458e.html#EndPath
>(*err=*merr may exhibit the same issue). It will not happend from all
>the call places anyway, but I guess there's something wrong with the
>whole code path, as _merr seems to be sort of a fallback for err and
>is not used as such.
>
> This one is wrong :
>http://chipitsine.github.io/haproxy-1.7-dev/report-a9cda9.html#EndPath
>(stktable_data_cast() cannot return NULL for a valid ptr, which is known
> to be valid because the data type is stored in the table according to
> some earlier tests).
>
> This one is wrong :
>http://chipitsine.github.io/haproxy-1.7-dev/report-ece278.html#EndPath
>(the loop must have at least one iteration since the server list was
> checked to have at least one server).
>
> This one is wrong :
>http://chipitsine.github.io/haproxy-1.7-dev/report-ba43f3.html#EndPath
>(a server always belongs to a proxy).
>
> This one is wrong :
>http://chipitsine.github.io/haproxy-1.7-dev/report-aafd4e.html#EndPath
>http://chipitsine.github.io/haproxy-1.7-dev/report-859fe6.html#EndPath
>(curmailers is global and is not null here).
>
> Same here with curr_resolvers :
>http://chipitsine.github.io/haproxy-1.7-dev/report-7d9a8b.html#EndPath
>http://chipitsine.github.io/haproxy-1.7-dev/report-678199.html#EndPath
>http://chipitsine.github.io/haproxy-1.7-dev/report-3c1789.html#EndPath
>http://chipitsine.github.io/haproxy-1.7-dev/report-c929f2.html#EndPath
>
> And here with curpeers :
>http://chipitsine.github.io/haproxy-1.7-dev/report-de79db.html#EndPath
>http://chipitsine.github.io/haproxy-1.7-dev/report-403a2e.html#EndPath
>http://chipitsine.github.io/haproxy-1.7-dev/report-ffb215.html#EndPath
>
> This one is wrong because tmplog is always assigned non-null values :
>http://chipitsine.github.io/haproxy-1.7-dev/report-9872e8.html#EndPath
>
> This is the one for which you proposed a patch :
>http://chipitsine.github.io/haproxy-1.7-dev/report-0bb120.html#EndPath
>By the way an error message is missing in your patch, to mention that
>either "request" or "response" is expected here.
>
> This one is right :
>http://chipitsine.github.io/haproxy-1.7-dev/report-df4c7e.html#EndPath
>(like the previous one, it's not important since we're aborting the
> startup, but better fix it).
>
> This one is interesting :
>http://chipitsine.github.io/haproxy-1.7-dev/report-93fb8d.html#EndPath
>It is not possible because a pattern taking type SMP_T_BOOL will not
>have the pat_parse_dotted_ver() parser, but the analyser was quite good
>at finding its way through all these conditions and picking that
>combination. We can definitely initialize minor to 0 just for the sake
>of easier debugging (it may later cause an extra report of unused store
>but we don't care, it's annoying to read garbage in gdb, better stay on
>zero).
>
> Overall I find that the reports are quick to read, quick to eliminate or
> make one wonder the good questions. Thanks for reporting this. You're
> welcome to provide patches for the good ones above if you want :-)
>
> Willy
>

Re: Github PR: Fix typo in description of `-st` parameter

[Willy Tarreau]

Hi Jorrit,

On Tue, Oct 25, 2016 at 10:58:54PM +, Lukas Tribus wrote:
> Author: Jorrit Schippers 
> Number of patches: 1
> Patch titles: 
> Fix typo in description of `-st` parameter
> 
> Link: https://github.com/haproxy/haproxy/pull/65

Patch adjusted and merged. Please in the future check CONTRIBUTING and
put your description in the commit message itself.

Thanks!
Willy