Re: Travis Matrix (was: Re: [PATCH] wurfl device detection build fixes and dummy library)

[Willy Tarreau]

On Fri, Jun 14, 2019 at 01:34:34AM +0500,  ??? wrote:
> well, I am going to play with cron jobs say "after haproxy-2.0 release"

OK, that works for me!

> as for being good citizens, when I look at
> 
> https://travis-ci.org/openssl/openssl/builds
> 
> (25k builds 1hr each ), I'm sure we are good citizens. really :)

Indeed, it could be said we have some margin :-)

Willy

Re: Increase in sockets in TIME_WAIT with 1.9.x

[Willy Tarreau]

On Thu, Jun 13, 2019 at 05:31:09PM -0500, Dave Chiluk wrote:
> I was able to bisect this down to 53216e7 being the problematic commit,
> when using calls to setsockopt(... SO_LINGER ...) as the test metric.

Oh great, thank you for doing this!

> I used the number of calls to setsockopt with SO_LINGER in them using the
> following command.
> $ sudo timeout 60s strace -e setsockopt,close -p $(ps -lf -C haproxy | tail
> -n 1 | awk -e '{print $4}') 2>&1 | tee 1.9-${V} ; grep LINGER 1.9-${V} | wc
> -l
> 
> 53216e7 = 1
> 81a15af6b = 69
> 
> Interesting to note is that 1.8.17 only has roughly 17.  I'll see if I can
> do a bisection for that tomorrow.

This is interesting because there is no equivalent commit in 1.8 so it
may be possible that we've created a bug long ago that triggers more
easily in certain situations.

>  Hope that helps.

Of course it does :-)

Cheers,
Willy

Re: Increase in sockets in TIME_WAIT with 1.9.x

[Dave Chiluk]

I was able to bisect this down to 53216e7 being the problematic commit,
when using calls to setsockopt(... SO_LINGER ...) as the test metric.

I used the number of calls to setsockopt with SO_LINGER in them using the
following command.
$ sudo timeout 60s strace -e setsockopt,close -p $(ps -lf -C haproxy | tail
-n 1 | awk -e '{print $4}') 2>&1 | tee 1.9-${V} ; grep LINGER 1.9-${V} | wc
-l

53216e7 = 1
81a15af6b = 69

Interesting to note is that 1.8.17 only has roughly 17.  I'll see if I can
do a bisection for that tomorrow.  Hope that helps.

Dave.

On Thu, Jun 13, 2019 at 3:30 PM Willy Tarreau  wrote:

> On Thu, Jun 13, 2019 at 03:20:20PM -0500, Dave Chiluk wrote:
> > I've attached an haproxy.cfg that is as minimal as I felt comfortable.
> (...)
>
> many thanks for this, Dave, I truly appreciate it. I'll have a look at
> it hopefully tomorrow morning.
>
> Willy
>

Re: Travis Matrix (was: Re: [PATCH] wurfl device detection build fixes and dummy library)

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

пт, 14 июн. 2019 г. в 01:27, Willy Tarreau :

> On Thu, Jun 13, 2019 at 10:28:19PM +0500,  ??? wrote:
> > > > or we can drop boringssl build in favour of Manu :)
> > >
> > > I suggest to drop BoringSSL. It's not intended to be used outside of
> > > Google anyway. So if it breaks the user gets to keep both pieces and
> can
> > > report the issue in the tracker or on the list.
> > >
> >
> > recently, I've found that if you wish to specify custom cipher suites in
> > CloudFlare, you need to use BoringSSL syntax.
> > so there's some use of it. same for LibreSSL
>
> I'd say that I'm in favor of testing what doesn't add effort nor cost.
> At the moment BoringSSL represents a cost for the infrastructure and the
> build time, but if we can use a cron job with it it should be a good
> option. In this case we could possibly do the same with the least
> common combinations (e.g. linux+no option, older libressl builds) and
> be good citizens with Travis and save on build time.
>
> Just a suggestion anyway, I don't have strong feelings on this.
>

well, I am going to play with cron jobs say "after haproxy-2.0 release"

as for being good citizens, when I look at

https://travis-ci.org/openssl/openssl/builds

(25k builds 1hr each ), I'm sure we are good citizens. really :)



>
> Willy
>

Re: Increase in sockets in TIME_WAIT with 1.9.x

[Willy Tarreau]

On Thu, Jun 13, 2019 at 03:20:20PM -0500, Dave Chiluk wrote:
> I've attached an haproxy.cfg that is as minimal as I felt comfortable.
(...)

many thanks for this, Dave, I truly appreciate it. I'll have a look at
it hopefully tomorrow morning.

Willy

Re: Travis Matrix (was: Re: [PATCH] wurfl device detection build fixes and dummy library)

[Willy Tarreau]

On Thu, Jun 13, 2019 at 10:28:19PM +0500,  ??? wrote:
> > > or we can drop boringssl build in favour of Manu :)
> >
> > I suggest to drop BoringSSL. It's not intended to be used outside of
> > Google anyway. So if it breaks the user gets to keep both pieces and can
> > report the issue in the tracker or on the list.
> >
> 
> recently, I've found that if you wish to specify custom cipher suites in
> CloudFlare, you need to use BoringSSL syntax.
> so there's some use of it. same for LibreSSL

I'd say that I'm in favor of testing what doesn't add effort nor cost.
At the moment BoringSSL represents a cost for the infrastructure and the
build time, but if we can use a cron job with it it should be a good
option. In this case we could possibly do the same with the least
common combinations (e.g. linux+no option, older libressl builds) and
be good citizens with Travis and save on build time.

Just a suggestion anyway, I don't have strong feelings on this.

Willy

Re: Increase in sockets in TIME_WAIT with 1.9.x

[Dave Chiluk]

I've attached an haproxy.cfg that is as minimal as I felt comfortable.  We
are using admin sockets for dynamic configuration of backends so left the
server-templating in, but no other application was configured to
orchestrate haproxy at the time of testing.

I've also attached output from
$ sudo timeout 60s strace -e setsockopt,close -p $(ps -lf -C haproxy | tail
-n 1 | awk -e '{print $4}') 2>&1 | tee 1.8.17

Which shows the significant decrease in setting of SO_LINGER.  I guess I
lied earlier when I said there were none, but over 60s it looks like I
1.9.8 had 1/17 the number of SO_LINGER setsockopt calls vs 1.8.17.

Unfortunately the number of sockets sitting in TIME_WAIT fluctuates to the
point where there's not a great metric to use.  Looking at SO_LINGER
settings does appear to be consistent though.  I bet if I spawned 700
backend instances instead of 7 it would be more pronounced.

I got perf stack traces for setsockopt from both versions on our production
servers, but inlining made those traces mostly useless.

Let me know if there's anything else i can grab.

Dave.


On Thu, Jun 13, 2019 at 1:30 AM Willy Tarreau  wrote:

> On Wed, Jun 12, 2019 at 12:08:03PM -0500, Dave Chiluk wrote:
> > I did a bit more introspection on our TIME_WAIT connections.  The
> increase
> > in sockets in TIME_WAIT is definitely from old connections to our backend
> >  server instances.  Considering the fact that this server is doesn't
> > actually serve real traffic we can make a reasonable assumptions that
> this
> > is almost entirely due to increases in healthchecks.
>
> Great!
>
> > Doing an strace on haproxy 1.8.17 we see
> > 
> > sudo strace -e setsockopt,close -p 15743
> > strace: Process 15743 attached
> > setsockopt(17, SOL_TCP, TCP_NODELAY, [1], 4) = 0
> > setsockopt(17, SOL_SOCKET, SO_LINGER, {onoff=1, linger=0}, 8) = 0
> > close(17)   = 0
> > 
> >
> > Doing the same strace on 1.9.8 we see
> > 
> > sudo strace -e setsockopt,close -p 6670
> > strace: Process 6670 attached
> > setsockopt(4, SOL_TCP, TCP_NODELAY, [1], 4) = 0
> > close(4)= 0
> > 
> >
> > The calls to setsockopt(17, SOL_SOCKET, SO_LINGER, {onoff=1, linger=0},
> 8)
> > = 0
> > appear to be missing.
>
> Awesome, that's exactly the info I was missing. I suspected that for
> whatever reason the lingering was not disabled, at least now we have
> a proof of this! Now the trick is to figure why :-/
>
> > We are running centos 7 with kernel 3.10.0-957.1.3.el7.x86_64.
>
> OK, and with the setsockopt it should behave properly.
>
> > I'll keep digging into this, and see if I can get stack traces that
> result
> > in teh setsockopt calls on 1.8.17 so the stack can be more closely
> > inspected.
>
> Don't worry for this now, this is something we at least need to resolve
> before issuing 2.0 or it will cause some trouble. Then we'll backport the
> fix once the cause is figured out.
>
> However when I try here I don't have the problem, either in 1.9.8 or
> 2.0-dev7 :
>
> 08:27:30.212570 connect(14, {sa_family=AF_INET, sin_port=htons(9003),
> sin_addr=inet_addr("127.0.0.1")}, 16) = 0
> 08:27:30.212590 recvfrom(14, NULL, 2147483647,
> MSG_TRUNC|MSG_DONTWAIT|MSG_NOSIGNAL, NULL, NULL) = -1 EAGAIN (Resource
> temporarily unavailable)
> 08:27:30.212610 setsockopt(14, SOL_SOCKET, SO_LINGER, {l_onoff=1,
> l_linger=0}, 8) = 0
> 08:27:30.212630 close(14)   = 0
> 08:27:30.212659 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0,
> tv_nsec=6993282}) = 0
>
> So it must depend on the type of check. Could you please share a
> minimalistic
> config that reproduces this ?
>
> Thanks,
> Willy
>


1.8.17
Description: Binary data


domain_map
Description: Binary data


1.9.8
Description: Binary data


haproxy.cfg
Description: Binary data

Re: Travis Matrix (was: Re: [PATCH] wurfl device detection build fixes and dummy library)

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

чт, 13 июн. 2019 г. в 22:25, Tim Düsterhus :

> Ilya,
> Willy,
>
> Am 13.06.19 um 19:11 schrieb Илья Шипицин:
> > чт, 13 июн. 2019 г. в 22:07, Willy Tarreau :
> >
> >> On Thu, Jun 13, 2019 at 10:01:52PM +0500,  ??? wrote:
> >>> "everything enabled" is impossible. 51degrees may be enabled in two
> >>> mutually exclusive ways. it doubles number of build configurations.
> >>
> >> It's not big deal, the haproxy-specific code remains the same and only
> >> the 51D-specific one differs, so once Ben tells you "this one or that
> >> one is more relevant", we stick to it and that's fine.
>
> I agree with Willy here. The dummy library primarily exists to make sure
> the appropriate functiions exist to link against. Just select one (I
> guess the 'pattern' based one) and we should be good. The dummy library
> is an approximation anyway.
>
> >> Thinking about the boringssl mess which takes ages to rebuild, I don't
> >> know if it's possible to script some form of "once every X times" or
> >> alternatively "build only when random()%10==0". That could also help
> >> preserving travis resources while still reporting once in a while if
> >> we broke anything there. I'd also say that boringssl is constantly
> >> built by Manu, I wouldn't be surprised if he detected our breakage
> >> faster than travis :-)
> >>
> >
> > there are couple of options:
> >
> > 1) we can use ccache (and cache it between builds). however, ccache is
> > buggy itself, I turned it "off" already for several projects
> >
> > 2) we can setup special "cron" builds, for example it will build haproxy
> +
> > boringssl once a day.
> >
> > or we can drop boringssl build in favour of Manu :)
>
> I suggest to drop BoringSSL. It's not intended to be used outside of
> Google anyway. So if it breaks the user gets to keep both pieces and can
> report the issue in the tracker or on the list.
>

recently, I've found that if you wish to specify custom cipher suites in
CloudFlare, you need to use BoringSSL syntax.
so there's some use of it. same for LibreSSL



>
> Best regards
> Tim Düsterhus
>

Re: Travis Matrix (was: Re: [PATCH] wurfl device detection build fixes and dummy library)

[Tim Düsterhus]

Ilya,
Willy,

Am 13.06.19 um 19:11 schrieb Илья Шипицин:
> чт, 13 июн. 2019 г. в 22:07, Willy Tarreau :
> 
>> On Thu, Jun 13, 2019 at 10:01:52PM +0500,  ??? wrote:
>>> "everything enabled" is impossible. 51degrees may be enabled in two
>>> mutually exclusive ways. it doubles number of build configurations.
>>
>> It's not big deal, the haproxy-specific code remains the same and only
>> the 51D-specific one differs, so once Ben tells you "this one or that
>> one is more relevant", we stick to it and that's fine.

I agree with Willy here. The dummy library primarily exists to make sure
the appropriate functiions exist to link against. Just select one (I
guess the 'pattern' based one) and we should be good. The dummy library
is an approximation anyway.

>> Thinking about the boringssl mess which takes ages to rebuild, I don't
>> know if it's possible to script some form of "once every X times" or
>> alternatively "build only when random()%10==0". That could also help
>> preserving travis resources while still reporting once in a while if
>> we broke anything there. I'd also say that boringssl is constantly
>> built by Manu, I wouldn't be surprised if he detected our breakage
>> faster than travis :-)
>>
> 
> there are couple of options:
> 
> 1) we can use ccache (and cache it between builds). however, ccache is
> buggy itself, I turned it "off" already for several projects
> 
> 2) we can setup special "cron" builds, for example it will build haproxy +
> boringssl once a day.
> 
> or we can drop boringssl build in favour of Manu :)

I suggest to drop BoringSSL. It's not intended to be used outside of
Google anyway. So if it breaks the user gets to keep both pieces and can
report the issue in the tracker or on the list.

Best regards
Tim Düsterhus

Re: Travis Matrix (was: Re: [PATCH] wurfl device detection build fixes and dummy library)

[Willy Tarreau]

On Thu, Jun 13, 2019 at 10:01:52PM +0500,  ??? wrote:
> "everything enabled" is impossible. 51degrees may be enabled in two
> mutually exclusive ways. it doubles number of build configurations.

It's not big deal, the haproxy-specific code remains the same and only
the 51D-specific one differs, so once Ben tells you "this one or that
one is more relevant", we stick to it and that's fine.

Thinking about the boringssl mess which takes ages to rebuild, I don't
know if it's possible to script some form of "once every X times" or
alternatively "build only when random()%10==0". That could also help
preserving travis resources while still reporting once in a while if
we broke anything there. I'd also say that boringssl is constantly
built by Manu, I wouldn't be surprised if he detected our breakage
faster than travis :-)

Willy

Re: Travis Matrix (was: Re: [PATCH] wurfl device detection build fixes and dummy library)

[Willy Tarreau]

On Thu, Jun 13, 2019 at 06:28:09PM +0200, Tim Düsterhus wrote:
> I'm unhappy with that patch:
> 
> a) It makes unrelated changes to the OpenSSL version (that should be a
> separate patch).

I'm also at fault here because I saw this and didn't bother that much,
being busy on other stuff :-/

> b) It removed the "gcc without any flags" build and instead replaced it
> with "gcc with all flags, but a separate 51d from all others".

I didn't notice this one.

> I consider the "X without any flags" very useful to detect missing
> preprocessor guards and code accidentally relying on a build flag.

I agree, and in "without any flags" I'd even disable threads as we
broke the thread-less builds quite a few times in the past.

> I suggest to draft a real plan on how the tests should proceed, taking
> into account to not abuse Travis' free service, before making any more
> changes to the configuration.
> 
> I suggest:
> 
> - Linux + gcc   with everything disabled.
> - Linux + clang with everything disabled.
> - Linux + gcc   with everything enabled (incl. device detection and
> prometheus).
> - Linux + clang with everything enabled (incl. device detection and
> prometheus).
> - Linux + gcc   with ONLY SSL enabled for each of the various SSL libraries.
> - Mac + clang   with everything from core HAProxy enabled (no device
> detection and prometheus).
> - Windows   with everything from core HAProxy enabled (no device
> detection and prometheus).
> 
> That way the edge cases (everything enabled and everything disabled) are
> systematically tested. The less important platforms (Windows / Mac;
> everything that is not the default OpenSSL) get some basic exposure
> without skyrocketing build times, because things are redundantly tested
> (e.g. no need to test compression with each SSL library, that's why only
> SSL should be enabled for those).

I tend to think that this proposal is quite reasonable.

Willy

Re: Travis Matrix (was: Re: [PATCH] wurfl device detection build fixes and dummy library)

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

чт, 13 июн. 2019 г. в 21:28, Tim Düsterhus :

> Ilya,
>
> (removed Ben and Christopher from Cc, as this no longer about 51d)
>
> Am 13.06.19 um 17:01 schrieb Илья Шипицин:
> > please find "travis-ci + 51degree" patch attached.
>
> I'm unhappy with that patch:
>
> a) It makes unrelated changes to the OpenSSL version (that should be a
> separate patch).
> b) It removed the "gcc without any flags" build and instead replaced it
> with "gcc with all flags, but a separate 51d from all others".
>
> I consider the "X without any flags" very useful to detect missing
> preprocessor guards and code accidentally relying on a build flag.
>
> I'm aware that there still is "clang without any flags", but there
> really should be "clang without any flags" and "gcc without any flags"
> as history has shown that they sometimes do things differently.
>
> All in all the Travis configuration is pretty unorganized by now. It
> "randomly" mashes together various options so that everything is somehow
> tested at least once, but there is no real logic behind it. As an
> example: Why is the Prometheus test added to LibreSSL 2.7.5? Why not
> BoringSSL?
>
> I suggest to draft a real plan on how the tests should proceed, taking
> into account to not abuse Travis' free service, before making any more
> changes to the configuration.
>
> I suggest:
>
> - Linux + gcc   with everything disabled.
> - Linux + clang with everything disabled.
> - Linux + gcc   with everything enabled (incl. device detection and
> prometheus).
>


"everything enabled" is impossible. 51degrees may be enabled in two
mutually exclusive ways. it doubles number of build configurations.



> - Linux + clang with everything enabled (incl. device detection and
> prometheus).
> - Linux + gcc   with ONLY SSL enabled for each of the various SSL
> libraries.
> - Mac + clang   with everything from core HAProxy enabled (no device
> detection and prometheus).
> - Windows   with everything from core HAProxy enabled (no device
> detection and prometheus).
>
> That way the edge cases (everything enabled and everything disabled) are
> systematically tested. The less important platforms (Windows / Mac;
> everything that is not the default OpenSSL) get some basic exposure
> without skyrocketing build times, because things are redundantly tested
> (e.g. no need to test compression with each SSL library, that's why only
> SSL should be enabled for those).
>
> Best regards
> Tim Düsterhus
>

Re: Travis Matrix (was: Re: [PATCH] wurfl device detection build fixes and dummy library)

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

чт, 13 июн. 2019 г. в 21:28, Tim Düsterhus :

> Ilya,
>
> (removed Ben and Christopher from Cc, as this no longer about 51d)
>
> Am 13.06.19 um 17:01 schrieb Илья Шипицин:
> > please find "travis-ci + 51degree" patch attached.
>
> I'm unhappy with that patch:
>
> a) It makes unrelated changes to the OpenSSL version (that should be a
> separate patch).
> b) It removed the "gcc without any flags" build and instead replaced it
> with "gcc with all flags, but a separate 51d from all others".
>
> I consider the "X without any flags" very useful to detect missing
> preprocessor guards and code accidentally relying on a build flag.
>
> I'm aware that there still is "clang without any flags", but there
> really should be "clang without any flags" and "gcc without any flags"
> as history has shown that they sometimes do things differently.
>
> All in all the Travis configuration is pretty unorganized by now. It
> "randomly" mashes together various options so that everything is somehow
> tested at least once, but there is no real logic behind it. As an
> example: Why is the Prometheus test added to LibreSSL 2.7.5? Why not
> BoringSSL?
>

I'd like to add PCRE2 and SLZ to future builds
also, I'd like to add more clang sanitizers (currently we use only ASAN and
it is extremely useful)

also, I'm not sure how many LibreSSL versions do we actually need (I taken
three of them)

as for gcc, I'm not happy with it's sanitizers, but maybe I'll add some
later. that's why we have much
more clang builds.

also, I'm not very happy with current BoringSSL caching (it caches only
downloads, and it seems
that S3 cache is almost the same as clean checkout)



>
> I suggest to draft a real plan on how the tests should proceed, taking
> into account to not abuse Travis' free service, before making any more
> changes to the configuration.
>


sure. I do not mind


>
> I suggest:
>
> - Linux + gcc   with everything disabled.
> - Linux + clang with everything disabled.
> - Linux + gcc   with everything enabled (incl. device detection and
> prometheus).
> - Linux + clang with everything enabled (incl. device detection and
> prometheus).
> - Linux + gcc   with ONLY SSL enabled for each of the various SSL
> libraries.
> - Mac + clang   with everything from core HAProxy enabled (no device
> detection and prometheus).
> - Windows   with everything from core HAProxy enabled (no device
> detection and prometheus).
>
> That way the edge cases (everything enabled and everything disabled) are
> systematically tested. The less important platforms (Windows / Mac;
> everything that is not the default OpenSSL) get some basic exposure
> without skyrocketing build times, because things are redundantly tested
> (e.g. no need to test compression with each SSL library, that's why only
> SSL should be enabled for those).
>
> Best regards
> Tim Düsterhus
>

Travis Matrix (was: Re: [PATCH] wurfl device detection build fixes and dummy library)

[Tim Düsterhus]

Ilya,

(removed Ben and Christopher from Cc, as this no longer about 51d)

Am 13.06.19 um 17:01 schrieb Илья Шипицин:
> please find "travis-ci + 51degree" patch attached.

I'm unhappy with that patch:

a) It makes unrelated changes to the OpenSSL version (that should be a
separate patch).
b) It removed the "gcc without any flags" build and instead replaced it
with "gcc with all flags, but a separate 51d from all others".

I consider the "X without any flags" very useful to detect missing
preprocessor guards and code accidentally relying on a build flag.

I'm aware that there still is "clang without any flags", but there
really should be "clang without any flags" and "gcc without any flags"
as history has shown that they sometimes do things differently.

All in all the Travis configuration is pretty unorganized by now. It
"randomly" mashes together various options so that everything is somehow
tested at least once, but there is no real logic behind it. As an
example: Why is the Prometheus test added to LibreSSL 2.7.5? Why not
BoringSSL?

I suggest to draft a real plan on how the tests should proceed, taking
into account to not abuse Travis' free service, before making any more
changes to the configuration.

I suggest:

- Linux + gcc   with everything disabled.
- Linux + clang with everything disabled.
- Linux + gcc   with everything enabled (incl. device detection and
prometheus).
- Linux + clang with everything enabled (incl. device detection and
prometheus).
- Linux + gcc   with ONLY SSL enabled for each of the various SSL libraries.
- Mac + clang   with everything from core HAProxy enabled (no device
detection and prometheus).
- Windows   with everything from core HAProxy enabled (no device
detection and prometheus).

That way the edge cases (everything enabled and everything disabled) are
systematically tested. The less important platforms (Windows / Mac;
everything that is not the default OpenSSL) get some basic exposure
without skyrocketing build times, because things are redundantly tested
(e.g. no need to test compression with each SSL library, that's why only
SSL should be enabled for those).

Best regards
Tim Düsterhus

RE: [PATCH] wurfl device detection build fixes and dummy library

[Ben Shillito]

Hi Willy,

Great, thanks.

Yeah that makes total sense. Don't want warnings that can't be solved.

Regards,

Ben Shillito
Developer
O: +44 1183 287152
E: b...@51degrees.com
T: @51Degrees

-Original Message-
From: Willy Tarreau [mailto:w...@1wt.eu]
Sent: 13 June 2019 17:06
To: Ben Shillito 
Cc:  ??? ; Christopher Faulet 
; HAProxy 
Subject: Re: [PATCH] wurfl device detection build fixes and dummy library

On Thu, Jun 13, 2019 at 03:57:04PM +, Ben Shillito wrote:
> Also, after thinking earlier about making it more obvious that it is a
> dummy library, I have attached a patch which adds "(dummy library)" to
> the output of REGISTER_BUILD_OPTS macro.

Ah perfect, I also wanted to do it but considered it of lower importance than 
the remaining stuff so it could be postponed. I've merged it now, thank you.

> I thought about pushing a warning when the dummy library is used just
> to be super obvious. Don't know what you think of that?

No it's not a good idea. We're trying to make sure that *any* single warning 
must be addressable by the end user, otherwise people get used to seeing them 
and to ignore them.

If people build with the dummy lib and complain, they'll send their haproxy 
-vv, you see "dummy" there it becomes "you don't need to run with this, please 
prove the problem still happens without", end of the story.

I asked about these libs for convenience for *developers* not for users.
But if it causes warnings in all our builds, it will end up being disabled 
which is counter productive.

Cheers,
Willy
This email and any attachments are confidential and may also be privileged. If 
you are not the named recipient, please notify the sender immediately and do 
not disclose, use, store or copy the information contained herein. This is an 
email from 51Degrees.mobi Limited, 5 Charlotte Close, Reading. RG47BY. T: +44 
118 328 7152; E: i...@51degrees.com; 51Degrees.mobi Limited t/as 51Degrees.

Re: [PATCH] wurfl device detection build fixes and dummy library

[Willy Tarreau]

On Thu, Jun 13, 2019 at 03:57:04PM +, Ben Shillito wrote:
> Also, after thinking earlier about making it more obvious that it is a dummy
> library, I have attached a patch which adds "(dummy library)" to the output
> of REGISTER_BUILD_OPTS macro.

Ah perfect, I also wanted to do it but considered it of lower importance
than the remaining stuff so it could be postponed. I've merged it now, thank
you.

> I thought about pushing a warning when the dummy library is used just to be
> super obvious. Don't know what you think of that?

No it's not a good idea. We're trying to make sure that *any* single
warning must be addressable by the end user, otherwise people get used
to seeing them and to ignore them.

If people build with the dummy lib and complain, they'll send their
haproxy -vv, you see "dummy" there it becomes "you don't need to run
with this, please prove the problem still happens without", end of the
story.

I asked about these libs for convenience for *developers* not for users.
But if it causes warnings in all our builds, it will end up being disabled
which is counter productive.

Cheers,
Willy

RE: [PATCH] wurfl device detection build fixes and dummy library

[Ben Shillito]

Same here. I can't see anything related to 51Degrees in there.

I will leave this for now, but let me know if either of you notice anything I 
need to look into.

Also, after thinking earlier about making it more obvious that it is a dummy 
library, I have attached a patch which adds "(dummy library)" to the output of 
REGISTER_BUILD_OPTS macro.

I thought about pushing a warning when the dummy library is used just to be 
super obvious. Don't know what you think of that?

Thanks,

Ben Shillito
Developer
O: +44 1183 287152
E: b...@51degrees.com
T: @51Degrees

-Original Message-
From: Willy Tarreau [mailto:w...@1wt.eu]
Sent: 13 June 2019 16:30
To: Ben Shillito 
Cc:  ??? ; Christopher Faulet 
; HAProxy 
Subject: Re: [PATCH] wurfl device detection build fixes and dummy library

On Thu, Jun 13, 2019 at 03:21:36PM +, Ben Shillito wrote:
> Thanks both,
>
> Ilya, I will take a look at that now.

I suspect it's totally unrelated, the failure was on this reg test and may be 
just a race :

## Test case: reg-tests/checks/4be_1srv_health_checks.vtc ## ## test 
results in: "/tmp/haregtests-2019-06-13_15-05-08.R3rhNr/vtc.8210.558cb932"
 h11.5 CLI expect failed ~ "# be_id be_name srv_id srv_name srv_addr 
srv_op_state srv_admin_state srv_uweight srv_iweight srv_time_since_last_change 
srv_check_status srv_check_result srv_check_health srv_check_state 
srv_agent_state bk_f_forced_id srv_f_forced_id srv_fqdn srv_port srvrecord

The last push seems to be running fine for now.

Willy

This email and any attachments are confidential and may also be privileged. If 
you are not the named recipient, please notify the sender immediately and do 
not disclose, use, store or copy the information contained herein. This is an 
email from 51Degrees.mobi Limited, 5 Charlotte Close, Reading. RG47BY. T: +44 
118 328 7152; E: i...@51degrees.com; 51Degrees.mobi Limited t/as 51Degrees.


0001-BUILD-MINOR-51d-Updated-build-registration-output-to.patch
Description:  0001-BUILD-MINOR-51d-Updated-build-registration-output-to.patch

Re: [PATCH 0/1] compression: Set Vary: Accept-Encoding

[Tim Düsterhus]

Willy,

Am 13.06.19 um 17:26 schrieb Willy Tarreau:
> On Wed, Jun 12, 2019 at 10:36:51PM +0200, Tim Duesterhus wrote:
>> Willy,
>>
>> First things first:
>>
>> Yes, I'm aware that this is shortly before 2.0. Either put this into 2.0
>> or put this into `next` if you consider it too risky (or just wait until
>> 2.0 release before merging).
> 
> I had a quick look at your description. I'm shocked because I would have
> bet we already do emit Vary (and would have lost). The first approach is

Yes, it's missing. And it's not even documented.

> probably better as the second one can have a big impact on existing
> deployments by totally disabling any front-level cache when they are
> simple enough not to cache when there's a Vary header in the response.

Yes, I agree. The first one was my first patch for that reason. After I
realized that it requires careful ordering in *_set_comp_reshdr I
created the second, more simple one. No one benefits from a Vary header
that sometimes is incorrect. A completely missing Vary header is obvious
at least. In any case the reg-tests I included should prevent most
surprises.

> Could you please create an issue on github about it, and possibly link
> to this thread with your patches so that we don't forget it ?

Sure: https://github.com/haproxy/haproxy/issues/121

Best regards
Tim Düsterhus

Re: [PATCH] wurfl device detection build fixes and dummy library

[Willy Tarreau]

On Thu, Jun 13, 2019 at 03:21:36PM +, Ben Shillito wrote:
> Thanks both,
> 
> Ilya, I will take a look at that now.

I suspect it's totally unrelated, the failure was on this reg test and
may be just a race :

## Test case: reg-tests/checks/4be_1srv_health_checks.vtc ##
## test results in: 
"/tmp/haregtests-2019-06-13_15-05-08.R3rhNr/vtc.8210.558cb932"
 h11.5 CLI expect failed ~ "# be_id be_name srv_id srv_name srv_addr 
srv_op_state srv_admin_state srv_uweight srv_iweight srv_time_since_last_change 
srv_check_status srv_check_result srv_check_health srv_check_state 
srv_agent_state bk_f_forced_id srv_f_forced_id srv_fqdn srv_port srvrecord

The last push seems to be running fine for now.

Willy

Re: [PATCH 0/1] compression: Set Vary: Accept-Encoding

[Willy Tarreau]

Hi Tim,

On Wed, Jun 12, 2019 at 10:36:51PM +0200, Tim Duesterhus wrote:
> Willy,
> 
> First things first:
> 
> Yes, I'm aware that this is shortly before 2.0. Either put this into 2.0
> or put this into `next` if you consider it too risky (or just wait until
> 2.0 release before merging).

I had a quick look at your description. I'm shocked because I would have
bet we already do emit Vary (and would have lost). The first approach is
probably better as the second one can have a big impact on existing
deployments by totally disabling any front-level cache when they are
simple enough not to cache when there's a Vary header in the response.
But the first one requires more head scratching to figure what possible
impacts it could have. Given it has existed since the beginning, or at
least 5 years, I think we can take our time to figure the best fix for
it after the release.

Could you please create an issue on github about it, and possibly link
to this thread with your patches so that we don't forget it ?

Thanks!
Willy

RE: [PATCH] wurfl device detection build fixes and dummy library

[Ben Shillito]

Thanks both,

Ilya, I will take a look at that now.

Ben Shillito
Developer
[51Degrees]
O: +44 1183 287152
E: b...@51degrees.com
[https://51degrees.com/portals/0/images/twitterbird.png] 
@51Degrees
[https://51degrees.com/portals/0/images/linkedinicon.png] 
51Degrees
[Find out More]

From: Илья Шипицин [mailto:chipits...@gmail.com]
Sent: 13 June 2019 16:19
To: Willy Tarreau 
Cc: Ben Shillito ; Christopher Faulet 
; HAProxy 
Subject: Re: [PATCH] wurfl device detection build fixes and dummy library

Ben,

I enabled "trie" on one build configuration and ... it failed

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

it never failed before. can you have a look ?
(also, it did NOT fail in my own fork)

чт, 13 июн. 2019 г. в 20:03, Willy Tarreau mailto:w...@1wt.eu>>:
On Thu, Jun 13, 2019 at 08:01:14PM +0500,  ??? wrote:
> please find "travis-ci + 51degree" patch attached.

applied, thanks Ilya.
willy
This email and any attachments are confidential and may also be privileged. If 
you are not the named recipient, please notify the sender immediately and do 
not disclose, use, store or copy the information contained herein. This is an 
email from 51Degrees.mobi Limited, 5 Charlotte Close, Reading. RG47BY. T: +44 
118 328 7152; E: i...@51degrees.com; 51Degrees.mobi Limited t/as 51Degrees.

Re: [PATCH] wurfl device detection build fixes and dummy library

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

Ben,

I enabled "trie" on one build configuration and ... it failed

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

it never failed before. can you have a look ?
(also, it did NOT fail in my own fork)

чт, 13 июн. 2019 г. в 20:03, Willy Tarreau :

> On Thu, Jun 13, 2019 at 08:01:14PM +0500,  ??? wrote:
> > please find "travis-ci + 51degree" patch attached.
>
> applied, thanks Ilya.
> willy
>

Re: [PATCH] wurfl device detection build fixes and dummy library

[Willy Tarreau]

On Thu, Jun 13, 2019 at 08:01:14PM +0500,  ??? wrote:
> please find "travis-ci + 51degree" patch attached.

applied, thanks Ilya.
willy

Re: [PATCH] wurfl device detection build fixes and dummy library

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

please find "travis-ci + 51degree" patch attached.

чт, 13 июн. 2019 г. в 19:06, Willy Tarreau :

> On Thu, Jun 13, 2019 at 02:02:33PM +, Ben Shillito wrote:
> > Hi Willy,
> >
> > Yes, I agree the paths in the dummy library should match that of the
> actual library. And yes, that patch is good with me.
>
> Thanks for the fast response, now merged.
>
> Willy
>
From d2d22f75c1c45e84d3622e0cf45e48fc784b6ab8 Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Thu, 13 Jun 2019 20:00:05 +0500
Subject: [PATCH] BUILD: travis-ci: add 51Degree device detection, update
 openssl to 1.1.1c

---
 .travis.yml | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 032ee13c..cfa68a93 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -5,10 +5,11 @@ language: c
 
 env:
   global:
-- FLAGS="USE_ZLIB=1 USE_PCRE=1 USE_PCRE_JIT=1 USE_LUA=1 USE_OPENSSL=1 USE_GETADDRINFO=1 USE_TFO=1 USE_SYSTEMD=1 USE_WURFL=1 WURFL_INC=contrib/wurfl WURFL_LIB=contrib/wurfl USE_DEVICEATLAS=1 DEVICEATLAS_SRC=contrib/deviceatlas"
+- FLAGS="USE_ZLIB=1 USE_PCRE=1 USE_PCRE_JIT=1 USE_LUA=1 USE_OPENSSL=1 USE_GETADDRINFO=1 USE_TFO=1 USE_SYSTEMD=1 USE_WURFL=1 WURFL_INC=contrib/wurfl WURFL_LIB=contrib/wurfl USE_DEVICEATLAS=1 DEVICEATLAS_SRC=contrib/deviceatlas USE_51DEGREES=1"
 - SSL_LIB=${HOME}/opt/lib
 - SSL_INC=${HOME}/opt/include
 - TMPDIR=/tmp
+- FIFTYONEDEGREES_SRC="contrib/51d/src/pattern"
 
 addons:
   apt:
@@ -23,13 +24,13 @@ matrix:
   include:
   - os: linux
 compiler: gcc
-env: TARGET=linux2628 FLAGS=
+env: TARGET=linux2628 FIFTYONEDEGREES_SRC="contrib/51d/src/trie"
   - os: linux-ppc64le
 compiler: gcc
-env: TARGET=linux2628 OPENSSL_VERSION=1.1.1b LABEL="linux-ppc64le"
+env: TARGET=linux2628 OPENSSL_VERSION=1.1.1c LABEL="linux-ppc64le"
   - os: linux
 compiler: clang
-env: TARGET=linux2628 OPENSSL_VERSION=1.1.1b
+env: TARGET=linux2628 OPENSSL_VERSION=1.1.1c
   - os: linux
 compiler: clang
 env: TARGET=linux2628 OPENSSL_VERSION=1.1.0j
@@ -53,7 +54,7 @@ matrix:
 env: TARGET=linux2628 FLAGS=
   - os: osx
 compiler: clang
-env: TARGET=osx FLAGS="USE_OPENSSL=1" OPENSSL_VERSION=1.1.1b
+env: TARGET=osx FLAGS="USE_OPENSSL=1" OPENSSL_VERSION=1.1.1c
   - os: windows
 install:
   - choco install bash make libssl-devel cygwin-devel gcc-core libgcc1 binutils lua-devel libpcre-devel zlib-devel --source cygwin
@@ -69,7 +70,7 @@ install:
 script:
   - if [ "${CC}"  = "clang" ]; then export FLAGS="$FLAGS USE_OBSOLETE_LINKER=1" DEBUG_CFLAGS="-g -fsanitize=address" LDFLAGS="-fsanitize=address"; fi
   - make -C contrib/wurfl
-  - make -j3 CC=$CC V=1 TARGET=$TARGET $FLAGS DEBUG_CFLAGS="$DEBUG_CFLAGS" LDFLAGS="$LDFLAGS" EXTRA_OBJS="$EXTRA_OBJS"
+  - make -j3 CC=$CC V=1 TARGET=$TARGET $FLAGS DEBUG_CFLAGS="$DEBUG_CFLAGS" LDFLAGS="$LDFLAGS" 51DEGREES_SRC="$FIFTYONEDEGREES_SRC" EXTRA_OBJS="$EXTRA_OBJS"
   - if [ "${TRAVIS_OS_NAME}" = "linux" ]; then export LD_LIBRARY_PATH="${HOME}/opt/lib:${LD_LIBRARY_PATH:-}"; fi
   - if [ "${TRAVIS_OS_NAME}" = "osx" ]; then export DYLD_LIBRARY_PATH="${HOME}/opt/lib:${DYLD_LIBRARY_PATH:-}"; fi
   - ./haproxy -vv
-- 
2.20.1

Re: [PATCH] server state: cleanup and load global file in a tree

[Baptiste]

Last mail, this is not backportable. HAProxy 2.0+ only.

On Thu, Jun 13, 2019 at 4:12 PM Baptiste  wrote:

> these patches replace to 2 previous ones. I fixed a compilation warning
> about possible used of uninitialized variable in the second patch.
> I also ran the reg-tests successfully.
>
> Cheers
>
>>

Re: [PATCH] server state: cleanup and load global file in a tree

[Baptiste]

these patches replace to 2 previous ones. I fixed a compilation warning
about possible used of uninitialized variable in the second patch.
I also ran the reg-tests successfully.

Cheers

>
From 0c5b17976ec703b12040d813bdd6ac975af7b4d7 Mon Sep 17 00:00:00 2001
From: Baptiste Assmann 
Date: Thu, 13 Jun 2019 13:24:29 +0200
Subject: [PATCH 2/2] MEDIUM: server: server-state global file stored in a tree

Server states can be recovered from either a "global" file (all backends)
or a "local" file (per backend).

The way the algorithm to parse the state file was first implemented was good
enough for a low number of backends and servers per backend.
Basically, for each backend the state file (global or local) is opened,
parsed entirely and for each line we check if it contains data related to
a server from the backend we're currently processing.
We must read the file entirely, just in case some lines for the current
backend are stored at the end of the file.
This does not scale at all!

This patch changes the behavior above for the "global" file only. Now,
the global file is read and parsed once and all lines it contains are
stored in a tree, for faster discovery.
This result in way much less fopen, fgets, and strcmp calls, which make
loading of very big state files very quick now.
---
 include/types/server.h |  11 ++
 src/server.c   | 412 -
 2 files changed, 294 insertions(+), 129 deletions(-)

diff --git a/include/types/server.h b/include/types/server.h
index 4a0772685..568528976 100644
--- a/include/types/server.h
+++ b/include/types/server.h
@@ -345,6 +345,17 @@ struct server {
 	struct sockaddr_storage socks4_addr;	/* the address of the SOCKS4 Proxy, including the port */
 };
 
+
+/* Storage structure to load server-state lines from a flat file into
+ * an ebtree, for faster processing
+ */
+struct state_line {
+	char *line;
+	struct ebmb_node name_name;
+	/* WARNING don't put anything after name_name, it's used by the key */
+};
+
+
 /* Descriptor for a "server" keyword. The ->parse() function returns 0 in case of
  * success, or a combination of ERR_* flags if an error is encountered. The
  * function pointer can be NULL if not implemented. The function also has an
diff --git a/src/server.c b/src/server.c
index 66fba992d..1a9dd1617 100644
--- a/src/server.c
+++ b/src/server.c
@@ -47,10 +47,14 @@
 #include 
 #include 
 
+#include 
+
 static void srv_update_status(struct server *s);
 static void srv_update_state(struct server *srv, int version, char **params);
 static int srv_apply_lastaddr(struct server *srv, int *err_code);
 static int srv_set_fqdn(struct server *srv, const char *fqdn, int dns_locked);
+static void srv_state_parse_line(char *buf, const int version, char **params, char **srv_params);
+static int srv_state_get_version(FILE *f);
 
 /* List head of all known server keywords */
 static struct srv_kw_list srv_keywords = {
@@ -69,6 +73,9 @@ struct dict server_name_dict = {
 	.values = EB_ROOT_UNIQUE,
 };
 
+/* tree where global state_file is loaded */
+struct eb_root state_file = EB_ROOT;
+
 int srv_downtime(const struct server *s)
 {
 	if ((s->cur_state != SRV_ST_STOPPED) && s->last_change < now.tv_sec)		// ignore negative time
@@ -3363,6 +3370,130 @@ static void srv_update_state(struct server *srv, int version, char **params)
 	}
 }
 
+
+/*
+ * read next line from file  and return the server state version if one found.
+ * If no version is found, then 0 is returned
+ * Note that this should be the first read on 
+ */
+static int srv_state_get_version(FILE *f) {
+	char buf[2];
+	int ret;
+
+	/* first character of first line of the file must contain the version of the export */
+	if (fgets(buf, 2, f) == NULL) {
+		return 0;
+	}
+
+	ret = atoi(buf);
+	if ((ret < SRV_STATE_FILE_VERSION_MIN) ||
+	(ret > SRV_STATE_FILE_VERSION_MAX))
+		return 0;
+
+	return ret;
+}
+
+
+/*
+ * parses server state line stored in  and supposedly in version .
+ * Set  and  accordingly.
+ * In case of error, params[0] is set to NULL.
+ */
+static void srv_state_parse_line(char *buf, const int version, char **params, char **srv_params)
+{
+	int buflen, arg, srv_arg;
+	char *cur, *end;
+
+	buflen = strlen(buf);
+	cur = buf;
+	end = cur + buflen;
+
+	/* we need at least one character */
+	if (buflen == 0) {
+		params[0] = NULL;
+		return;
+	}
+
+	/* ignore blank characters at the beginning of the line */
+	while (isspace(*cur))
+		++cur;
+
+	/* Ignore empty or commented lines */
+	if (cur == end || *cur == '#') {
+		params[0] = NULL;
+		return;
+	}
+
+	/* truncated lines */
+	if (buf[buflen - 1] != '\n') {
+		//ha_warning("server-state file '%s': truncated line\n", filepath);
+		params[0] = NULL;
+		return;
+	}
+
+	/* Removes trailing '\n' */
+	buf[buflen - 1] = '\0';
+
+	/* we're now ready to move the line into *srv_params[] */
+	params[0] = cur;
+	arg = 1;
+	srv_arg = 0;
+	while (*cur && arg < SRV_STATE_FILE_MAX_FIELDS) {
+		if (isspace(*cur)) {
+			*cur = '\0';
+			++cur;

Re: [PATCH] wurfl device detection build fixes and dummy library

[Willy Tarreau]

On Thu, Jun 13, 2019 at 02:02:33PM +, Ben Shillito wrote:
> Hi Willy,
> 
> Yes, I agree the paths in the dummy library should match that of the actual 
> library. And yes, that patch is good with me.

Thanks for the fast response, now merged.

Willy

RE: [PATCH] wurfl device detection build fixes and dummy library

[Ben Shillito]

Hi Willy,

Yes, I agree the paths in the dummy library should match that of the actual 
library. And yes, that patch is good with me.

Thanks,

Ben Shillito
Developer
O: +44 1183 287152
E: b...@51degrees.com
T: @51Degrees

-Original Message-
From: Willy Tarreau [mailto:w...@1wt.eu]
Sent: 13 June 2019 14:59
To: Ben Shillito 
Cc:  ??? ; Christopher Faulet 
; HAProxy 
Subject: Re: [PATCH] wurfl device detection build fixes and dummy library

Ben,

what do you think of this one ?

Willy
This email and any attachments are confidential and may also be privileged. If 
you are not the named recipient, please notify the sender immediately and do 
not disclose, use, store or copy the information contained herein. This is an 
email from 51Degrees.mobi Limited, 5 Charlotte Close, Reading. RG47BY. T: +44 
118 328 7152; E: i...@51degrees.com; 51Degrees.mobi Limited t/as 51Degrees.

Re: [PATCH] wurfl device detection build fixes and dummy library

[Willy Tarreau]

Ben,

what do you think of this one ?

Willy
commit 5e4c5003c5cd5709e599aedbfdd5ef223dd3dc79
Author: Willy Tarreau 
Date:   Thu Jun 13 15:56:10 2019 +0200

CLEANUP: 51d: move the 51d dummy lib to contrib/51d/src to match the real 
lib

This way the directory structure remains the same as with the real lib and
one can apply the same build options regardless of where the lib is stored,
removing any possible confusion.

diff --git a/contrib/51d/cityhash/city.c b/contrib/51d/src/cityhash/city.c
similarity index 100%
rename from contrib/51d/cityhash/city.c
rename to contrib/51d/src/cityhash/city.c
diff --git a/contrib/51d/pattern/51Degrees.c 
b/contrib/51d/src/pattern/51Degrees.c
similarity index 100%
rename from contrib/51d/pattern/51Degrees.c
rename to contrib/51d/src/pattern/51Degrees.c
diff --git a/contrib/51d/pattern/51Degrees.h 
b/contrib/51d/src/pattern/51Degrees.h
similarity index 100%
rename from contrib/51d/pattern/51Degrees.h
rename to contrib/51d/src/pattern/51Degrees.h
diff --git a/contrib/51d/threading.c b/contrib/51d/src/threading.c
similarity index 100%
rename from contrib/51d/threading.c
rename to contrib/51d/src/threading.c
diff --git a/contrib/51d/trie/51Degrees.c b/contrib/51d/src/trie/51Degrees.c
similarity index 100%
rename from contrib/51d/trie/51Degrees.c
rename to contrib/51d/src/trie/51Degrees.c
diff --git a/contrib/51d/trie/51Degrees.h b/contrib/51d/src/trie/51Degrees.h
similarity index 100%
rename from contrib/51d/trie/51Degrees.h
rename to contrib/51d/src/trie/51Degrees.h
diff --git a/doc/51Degrees-device-detection.txt 
b/doc/51Degrees-device-detection.txt
index 2d4679d35..f0349abaa 100644
--- a/doc/51Degrees-device-detection.txt
+++ b/doc/51Degrees-device-detection.txt
@@ -58,9 +58,11 @@ For HAProxy developers who need to verify that their changes 
didn't affect the
 directory. This does not function, but implements the API such that the
 51Degrees module can be used (but not return any meaningful information). To
 test either Pattern or Hash Trie, build with:
-$ make TARGET= USE_51DEGREES=1 51DEGREES_SRC=contrib/51d/pattern
+
+$ make TARGET= USE_51DEGREES=1 
51DEGREES_SRC=contrib/51d/src/pattern
 or
-$ make TARGET= USE_51DEGREES=1 51DEGREES_SRC=contrib/51d/trie
+$ make TARGET= USE_51DEGREES=1 51DEGREES_SRC=contrib/51d/src/trie
+
 respectively.
 
 The configuration file needs to set the following parameters:

[PATCH] server state: cleanup and load global file in a tree

[Baptiste]

Hi all,

Please find enclosed to this email a couple of patches:
0001: cleans up the server state code to match on server names only
(since 7da71293e431b5ebb3d6289a55b0102331788ee6, server name is a reliable
information)

0002: loads global server state file in a tree for fast processing.
As an example, I set up a config file with 1000 backends of random number
of servers (from 1 to 1000+), resulting in a state file of 240K lines...
Loading this file without server state enabled takes 5.2s on my laptop and
5.6s with server state enabled.
As a measure of comparison, HAProxy 1.9.x takes around 1m35s to load the
same file (no tree involved)...

Baptiste
From f8ed4d51f8aadd61baec4094caec2e1e11a957ab Mon Sep 17 00:00:00 2001
From: Baptiste Assmann 
Date: Thu, 13 Jun 2019 13:24:29 +0200
Subject: [PATCH 2/2] MEDIUM: server: server-state global file stored in a tree

Server states can be recovered from either a "global" file (all backends)
 or a "local" file (per backend).

The way the algorithm to parse the state file was first implemented was good
enough for a low number of backends and servers per backend.
Basically, for each backend the state file (global or local) is opened,
parsed entirely and for each line we check if it contains data related to
a server from the backend we're currently processing.
We must read the file entirely, just in case some lines for the current
backend are stored at the end of the file.
This does not scale at all!

This patch changes the behavior above for the "global" file only. Now,
the global file is read and parsed once and all lines it contains are
stored in a tree, for faster discovery.
This result in way much less fopen, fgets, and strcmp calls, which make
loading of very big state files very quick now.
---
 include/types/server.h |  11 ++
 src/server.c   | 411 -
 2 files changed, 293 insertions(+), 129 deletions(-)

diff --git a/include/types/server.h b/include/types/server.h
index 4a0772685..568528976 100644
--- a/include/types/server.h
+++ b/include/types/server.h
@@ -345,6 +345,17 @@ struct server {
 	struct sockaddr_storage socks4_addr;	/* the address of the SOCKS4 Proxy, including the port */
 };
 
+
+/* Storage structure to load server-state lines from a flat file into
+ * an ebtree, for faster processing
+ */
+struct state_line {
+	char *line;
+	struct ebmb_node name_name;
+	/* WARNING don't put anything after name_name, it's used by the key */
+};
+
+
 /* Descriptor for a "server" keyword. The ->parse() function returns 0 in case of
  * success, or a combination of ERR_* flags if an error is encountered. The
  * function pointer can be NULL if not implemented. The function also has an
diff --git a/src/server.c b/src/server.c
index 66fba992d..777d0d0dc 100644
--- a/src/server.c
+++ b/src/server.c
@@ -47,10 +47,14 @@
 #include 
 #include 
 
+#include 
+
 static void srv_update_status(struct server *s);
 static void srv_update_state(struct server *srv, int version, char **params);
 static int srv_apply_lastaddr(struct server *srv, int *err_code);
 static int srv_set_fqdn(struct server *srv, const char *fqdn, int dns_locked);
+static void srv_state_parse_line(char *buf, const int version, char **params, char **srv_params);
+static int srv_state_get_version(FILE *f);
 
 /* List head of all known server keywords */
 static struct srv_kw_list srv_keywords = {
@@ -69,6 +73,9 @@ struct dict server_name_dict = {
 	.values = EB_ROOT_UNIQUE,
 };
 
+/* tree where global state_file is loaded */
+struct eb_root state_file = EB_ROOT;
+
 int srv_downtime(const struct server *s)
 {
 	if ((s->cur_state != SRV_ST_STOPPED) && s->last_change < now.tv_sec)		// ignore negative time
@@ -3363,6 +3370,130 @@ static void srv_update_state(struct server *srv, int version, char **params)
 	}
 }
 
+
+/*
+ * read next line from file  and return the server state version if one found.
+ * If no version is found, then 0 is returned
+ * Note that this should be the first read on 
+ */
+static int srv_state_get_version(FILE *f) {
+	char buf[2];
+	int ret;
+
+	/* first character of first line of the file must contain the version of the export */
+	if (fgets(buf, 2, f) == NULL) {
+		return 0;
+	}
+
+	ret = atoi(buf);
+	if ((ret < SRV_STATE_FILE_VERSION_MIN) ||
+	(ret > SRV_STATE_FILE_VERSION_MAX))
+		return 0;
+
+	return ret;
+}
+
+
+/*
+ * parses server state line stored in  and supposedly in version .
+ * Set  and  accordingly.
+ * In case of error, params[0] is set to NULL.
+ */
+static void srv_state_parse_line(char *buf, const int version, char **params, char **srv_params)
+{
+	int buflen, arg, srv_arg;
+	char *cur, *end;
+
+	buflen = strlen(buf);
+	cur = buf;
+	end = cur + buflen;
+
+	/* we need at least one character */
+	if (buflen == 0) {
+		params[0] = NULL;
+		return;
+	}
+
+	/* ignore blank characters at the beginning of the line */
+	while (isspace(*cur))
+		++cur;
+
+	/* Ignore empty or commented lines */
+	if (cur == end || *cur == '#') {

Re: [PATCH] BUILD: Silence gcc warning about unused return value

[Willy Tarreau]

Hi guys,

On Wed, Jun 12, 2019 at 10:31:37PM +0200, Vincent Bernat wrote:
>  ? 12 juin 2019 20:47 +02, Tim Duesterhus :
> 
> > -   write(2, trash.area, trash.data);
> > +   shut_your_big_mouth_gcc(write(2, trash.area, trash.data));

Now merged, thanks Tim!

> An alternative not discussed in 89efaed6b67b (which introduced this
> function) is to use "(void)!":
> 
>  (void)!write(2, trash.area, trash.data);
> 
> For an entertaining discussion, see:
> 
>  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425
> 
> Of course, like the "if (foo()) /* do nothing */;", this can be broken
> in the future.

Oh that's a very interesting lecture. And indeed I wouldn't trust it
for a long time, as it can become quite easy for future compilers to
detect that the transformed variable is useless. There's actually a
trick as well consisting in using two variables each assigned to the
other one. The compiler cannot detect anymore that any of them is not
used :

   static inline void unused(int ret)
   {
   int a, b;
   // mark all variables as read and written
   a = ret;
   b = a;
   a = b;
   }

Yes that's ugly as well, I strongly prefer this in practice :


   static inline void unused(int ret)
   {
   asm("" ::r(ret));
   }

At least I'm certain it doesn't emit any code.

Willy

Re: [PATCH] wurfl device detection build fixes and dummy library

[Willy Tarreau]

On Thu, Jun 13, 2019 at 12:59:33PM +, Ben Shillito wrote:
> Thanks for the link, that's useful. Looks like the path to the source is the 
> issue.
> 
> It should be "51DEGREES_SRC=contrib/51d/pattern" rather than 
> "51DEGREES_SRC=contrib/51d/src/pattern".

That's what I just noticed as well, since it's the dummy lib it's important
to use the build syntax related to the dummy lib, which doesn't contain the
"src" subcomponent. This just makes me think that we should probably create
this "src" subcomponent in contrib/51d so that the directory structure
matches what you have in the official lib, it will then simplify the build
process and documentation.

Willy

RE: [PATCH] wurfl device detection build fixes and dummy library

[Ben Shillito]

Thanks for the link, that’s useful. Looks like the path to the source is the 
issue.

It should be “51DEGREES_SRC=contrib/51d/pattern” rather than 
“51DEGREES_SRC=contrib/51d/src/pattern”.

On your performance point, the Pattern algorithm offers the match metrics and 
is a bit more thorough in looking for “closest” matches if the User-Agent is 
unknown. It is more than fast enough for the majority of users, however use 
cases like ad tech can receive billions of requests per day. For that reason, 
it is only the high volume users which choose to forgo the extra analysis that 
Pattern offers in order to make use of the efficiency of Trie.

Regards,

Ben Shillito
Developer
[51Degrees]
O: +44 1183 287152
E: b...@51degrees.com
[https://51degrees.com/portals/0/images/twitterbird.png] 
@51Degrees
[https://51degrees.com/portals/0/images/linkedinicon.png] 
51Degrees
[Find out More]

From: Илья Шипицин [mailto:chipits...@gmail.com]
Sent: 13 June 2019 13:27
To: Ben Shillito 
Cc: Willy Tarreau ; Christopher Faulet ; 
HAProxy 
Subject: Re: [PATCH] wurfl device detection build fixes and dummy library



чт, 13 июн. 2019 г. в 17:09, Ben Shillito 
mailto:b...@51degrees.com>>:
In Travis the contrib version would be fine. But if you are testing something 
which will later be deployed to production, I would suggest using cloning the 
actual GitHub repo instead to be safe.


this is how I enable it on travis-ci

https://github.com/chipitsine/haproxy/blob/master/.travis.yml#L8

(you may see USE_51DEGREES=1 51DEGREES_SRC=contrib/51d/src/pattern )

here's build

https://travis-ci.com/chipitsine/haproxy/jobs/207722666#L306-L310




In answer to your other question, yes, Pattern and Trie in HAProxy are mutually 
exclusive. Which one you choose depends on your needs. Trie provides the 
maximum performance, and Pattern takes slightly more time in order to give you 
better metrics indicating the quality of match. Unless you are handling upwards 
of a million requests per second, Pattern will be enough.

For more details on the differences between Pattern and Trie you can take a 
look at our “how device detection works” page here: 
https://51degrees.com/support/documentation/how-device-detection-works

ok, we'll test both

not clear, why one might need "maximum performance" algorithm and ... "not so 
fast another algorithm"
from my point of view, I would use the fastest algorithm all the time



And for the performance figures you can expect to see from both in HAProxy, see 
our benchmarks page here: 
https://51degrees.com/Support/Documentation/APIs/C-V32/Benchmarks

If you need any more information, or help setting up, do let me know.

Regards,

Ben Shillito
Developer
[Image removed by sender. 51Degrees]
O: +44 1183 287152
E: b...@51degrees.com
[Image removed by sender.] @51Degrees
[Image removed by sender.] 51Degrees
[Image removed by sender. Find out More]

From: Илья Шипицин [mailto:chipits...@gmail.com]
Sent: 13 June 2019 11:31
To: Ben Shillito mailto:b...@51degrees.com>>
Cc: Willy Tarreau mailto:w...@1wt.eu>>; Christopher Faulet 
mailto:cfau...@haproxy.com>>; HAProxy 
mailto:haproxy@formilux.org>>
Subject: Re: [PATCH] wurfl device detection build fixes and dummy library



чт, 13 июн. 2019 г. в 15:25, Ben Shillito 
mailto:b...@51degrees.com>>:
Hi,

The docs are correct. However, the 51Degrees source in the “contrib” directory 
should only be used for testing changes to the HAProxy source. The code 
contained in here does not contain any 51Degrees functionality, just method 
stubs to enable compilation and testing.

it's what I meant actually.
I added "USE_51DEGREES=1 51DEGREES_SRC=contrib/51d/src/pattern" to travis-ci 
job in order to see whether it will build or not.

or do I need to clone github repo anyway ?


another question is "what should be added to travis-ci?" I see "src/pattern" 
and "src/trie" alternatives. Are they mutually exclusiive ? So, should we test 
either of them ?


You should use the 51Degrees source located at 
https://github.com/51degrees/device-detection.git. Then point 51DEGREES_SRC to 
either device-detection/src/pattern or device-detection/src/trie.

Perhaps if the documentation is not clear enough we should modify it, or even 
add a warning to the compilation.

Let me know if this gets you up and running.

Regards,

Ben Shillito
Developer
[Image removed by sender. 51Degrees]
O: +44 1183 287152
E: b...@51degrees.com
[Image removed by sender.] @51Degrees
[Image removed by sender.] 

Re: [PATCH] wurfl device detection build fixes and dummy library

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

чт, 13 июн. 2019 г. в 17:09, Ben Shillito :

> In Travis the contrib version would be fine. But if you are testing
> something which will later be deployed to production, I would suggest using
> cloning the actual GitHub repo instead to be safe.
>


this is how I enable it on travis-ci

https://github.com/chipitsine/haproxy/blob/master/.travis.yml#L8

(you may see USE_51DEGREES=1 51DEGREES_SRC=contrib/51d/src/pattern )

here's build

https://travis-ci.com/chipitsine/haproxy/jobs/207722666#L306-L310




>
>
> In answer to your other question, yes, Pattern and Trie in HAProxy are
> mutually exclusive. Which one you choose depends on your needs. Trie
> provides the maximum performance, and Pattern takes slightly more time in
> order to give you better metrics indicating the quality of match. Unless
> you are handling upwards of a million requests per second, Pattern will be
> enough.
>
>
>
> For more details on the differences between Pattern and Trie you can take
> a look at our “how device detection works” page here:
> https://51degrees.com/support/documentation/how-device-detection-works
>

ok, we'll test both

not clear, why one might need "maximum performance" algorithm and ... "not
so fast another algorithm"
from my point of view, I would use the fastest algorithm all the time



>
>
> And for the performance figures you can expect to see from both in
> HAProxy, see our benchmarks page here:
> https://51degrees.com/Support/Documentation/APIs/C-V32/Benchmarks
>
>
>
> If you need any more information, or help setting up, do let me know.
>
>
>
> Regards,
>
>
>
> Ben Shillito
> Developer
>
> [image: 51Degrees] 
>
> O: +44 1183 287152 
> E: b...@51degrees.com 
>  @51Degrees 
>  51Degrees 
>
> [image: Find out More] 
>
>
>
> *From:* Илья Шипицин [mailto:chipits...@gmail.com]
> *Sent:* 13 June 2019 11:31
> *To:* Ben Shillito 
> *Cc:* Willy Tarreau ; Christopher Faulet ;
> HAProxy 
> *Subject:* Re: [PATCH] wurfl device detection build fixes and dummy
> library
>
>
>
>
>
>
>
> чт, 13 июн. 2019 г. в 15:25, Ben Shillito :
>
> Hi,
>
>
>
> The docs are correct. However, the 51Degrees source in the “contrib”
> directory should only be used for testing changes to the HAProxy source.
> The code contained in here does not contain any 51Degrees functionality,
> just method stubs to enable compilation and testing.
>
>
>
> it's what I meant actually.
>
> I added "USE_51DEGREES=1 51DEGREES_SRC=contrib/51d/src/pattern" to
> travis-ci job in order to see whether it will build or not.
>
>
>
> or do I need to clone github repo anyway ?
>
>
>
>
>
> another question is "what should be added to travis-ci?" I see
> "src/pattern" and "src/trie" alternatives. Are they mutually exclusiive ?
> So, should we test either of them ?
>
>
>
>
>
> You should use the 51Degrees source located at
> https://github.com/51degrees/device-detection.git. Then point
> 51DEGREES_SRC to either device-detection/src/pattern or
> device-detection/src/trie.
>
>
>
> Perhaps if the documentation is not clear enough we should modify it, or
> even add a warning to the compilation.
>
>
>
> Let me know if this gets you up and running.
>
>
>
> Regards,
>
>
>
> Ben Shillito
> Developer
>
> [image: Image removed by sender. 51Degrees] 
>
> O: +44 1183 287152 
> E: b...@51degrees.com 
> [image: Image removed by sender.] @51Degrees
> 
> [image: Image removed by sender.] 51Degrees
> 
>
> [image: Image removed by sender. Find out More]
> 
>
>
>
> *From:* Илья Шипицин [mailto:chipits...@gmail.com]
> *Sent:* 13 June 2019 11:07
> *To:* Ben Shillito 
> *Cc:* Willy Tarreau ; Christopher Faulet ;
> HAProxy 
> *Subject:* Re: [PATCH] wurfl device detection build fixes and dummy
> library
>
>
>
> Ben,
>
>
>
> what is the proper way of building 51degree ?
>
>
>
> I added "USE_51DEGREES=1 51DEGREES_SRC=contrib/51d/src/pattern" (as I seen
> in documentation)
>
>
>
> gcc -Iinclude -Iebtree -Wall -Wextra  -O2  -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   
> -DUSE_EPOLL-DUSE_NETFILTER -DUSE_PCRE -DUSE_PCRE_JIT   -DUSE_POLL  
> -DUSE_THREAD -DUSE_TPROXY -DUSE_LINUX_TPROXY -DUSE_LINUX_SPLICE 
> -DUSE_LIBCRYPT -DUSE_CRYPT_H  -DUSE_GETADDRINFO -DUSE_OPENSSL -DUSE_LUA 
> -DUSE_FUTEX -DUSE_ACCEPT4  -DUSE_ZLIB  -DUSE_CPU_AFFINITY -DUSE_TFO  -DUSE_DL 
> -DUSE_RT -DUSE_DEVICEATLAS -DUSE_51DEGREES -DUSE_WURFL -DUSE_SYSTEMD  
> -DUSE_PRCTL -DUSE_THREAD_DUMP   -I/home/travis/opt/include 
> -I/usr/include/lua5.3 -I/usr/include/lua5.3 -Icontrib/deviceatlas 
> -Icontrib/src/trie -Icontrib/wurfl -DUSE_PCRE -I/usr/include 

RE: [PATCH] wurfl device detection build fixes and dummy library

[Ben Shillito]

In Travis the contrib version would be fine. But if you are testing something 
which will later be deployed to production, I would suggest using cloning the 
actual GitHub repo instead to be safe.

In answer to your other question, yes, Pattern and Trie in HAProxy are mutually 
exclusive. Which one you choose depends on your needs. Trie provides the 
maximum performance, and Pattern takes slightly more time in order to give you 
better metrics indicating the quality of match. Unless you are handling upwards 
of a million requests per second, Pattern will be enough.

For more details on the differences between Pattern and Trie you can take a 
look at our “how device detection works” page here: 
https://51degrees.com/support/documentation/how-device-detection-works

And for the performance figures you can expect to see from both in HAProxy, see 
our benchmarks page here: 
https://51degrees.com/Support/Documentation/APIs/C-V32/Benchmarks

If you need any more information, or help setting up, do let me know.

Regards,

Ben Shillito
Developer
[51Degrees]
O: +44 1183 287152
E: b...@51degrees.com
[https://51degrees.com/portals/0/images/twitterbird.png] 
@51Degrees
[https://51degrees.com/portals/0/images/linkedinicon.png] 
51Degrees
[Find out More]

From: Илья Шипицин [mailto:chipits...@gmail.com]
Sent: 13 June 2019 11:31
To: Ben Shillito 
Cc: Willy Tarreau ; Christopher Faulet ; 
HAProxy 
Subject: Re: [PATCH] wurfl device detection build fixes and dummy library



чт, 13 июн. 2019 г. в 15:25, Ben Shillito 
mailto:b...@51degrees.com>>:
Hi,

The docs are correct. However, the 51Degrees source in the “contrib” directory 
should only be used for testing changes to the HAProxy source. The code 
contained in here does not contain any 51Degrees functionality, just method 
stubs to enable compilation and testing.

it's what I meant actually.
I added "USE_51DEGREES=1 51DEGREES_SRC=contrib/51d/src/pattern" to travis-ci 
job in order to see whether it will build or not.

or do I need to clone github repo anyway ?


another question is "what should be added to travis-ci?" I see "src/pattern" 
and "src/trie" alternatives. Are they mutually exclusiive ? So, should we test 
either of them ?


You should use the 51Degrees source located at 
https://github.com/51degrees/device-detection.git. Then point 51DEGREES_SRC to 
either device-detection/src/pattern or device-detection/src/trie.

Perhaps if the documentation is not clear enough we should modify it, or even 
add a warning to the compilation.

Let me know if this gets you up and running.

Regards,

Ben Shillito
Developer
[Image removed by sender. 51Degrees]
O: +44 1183 287152
E: b...@51degrees.com
[Image removed by sender.] @51Degrees
[Image removed by sender.] 51Degrees
[Image removed by sender. Find out More]

From: Илья Шипицин [mailto:chipits...@gmail.com]
Sent: 13 June 2019 11:07
To: Ben Shillito mailto:b...@51degrees.com>>
Cc: Willy Tarreau mailto:w...@1wt.eu>>; Christopher Faulet 
mailto:cfau...@haproxy.com>>; HAProxy 
mailto:haproxy@formilux.org>>
Subject: Re: [PATCH] wurfl device detection build fixes and dummy library

Ben,

what is the proper way of building 51degree ?

I added "USE_51DEGREES=1 51DEGREES_SRC=contrib/51d/src/pattern" (as I seen in 
documentation)


gcc -Iinclude -Iebtree -Wall -Wextra  -O2  -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   
-DUSE_EPOLL-DUSE_NETFILTER -DUSE_PCRE -DUSE_PCRE_JIT   -DUSE_POLL  
-DUSE_THREAD -DUSE_TPROXY -DUSE_LINUX_TPROXY -DUSE_LINUX_SPLICE 
-DUSE_LIBCRYPT -DUSE_CRYPT_H  -DUSE_GETADDRINFO -DUSE_OPENSSL -DUSE_LUA 
-DUSE_FUTEX -DUSE_ACCEPT4  -DUSE_ZLIB  -DUSE_CPU_AFFINITY -DUSE_TFO  -DUSE_DL 
-DUSE_RT -DUSE_DEVICEATLAS -DUSE_51DEGREES -DUSE_WURFL -DUSE_SYSTEMD  
-DUSE_PRCTL -DUSE_THREAD_DUMP   -I/home/travis/opt/include 
-I/usr/include/lua5.3 -I/usr/include/lua5.3 -Icontrib/deviceatlas 
-Icontrib/src/trie -Icontrib/wurfl -DUSE_PCRE -I/usr/include  
-DCONFIG_HAPROXY_VERSION=\"2.0-dev7\" -DCONFIG_HAPROXY_DATE=\"2019/06/11\" -c 
-o src/da.o src/da.c

make: *** No rule to make target 'contrib/src/trie/../cityhash/city.o', needed 
by 'haproxy'.  Stop.

make: *** Waiting for unfinished jobs

is documentation correct ? or I should use some different way to build

ср, 12 июн. 2019 г. в 22:01, Ben Shillito 
mailto:b...@51degrees.com>>:
Hi Willy,

Great, thanks for those changes, and good spot.

I agree that this is a significant step 

Re: Haproxy 1.9.8 comsumes 100% CPU

[Aleksandar Lazic]

Am 13.06.2019 um 13:47 schrieb Akom II:
> Hello,
>    Ubuntu 18.04  and haproxy 1.9.8 with latest patch on 2019/06/08
>    I'm currently encounter haproxy consume all CPU core, at the begging I
> curious might relate  to peers function which is our app rely heavily on 
> those ,
> it happen quite fast after restart service for couple minutes then it start to
> consume all CPU core , and there is an impact on service then consider disable
> peers function and keep monitoring it still happen but duration is longer 
> around
> 10 minutes btw those settings are on production and serve avg 500 req/s on
> websocket + SNI

Please can you give us more informations like.

haproxy -vv
Config

show sess
show stat
show errors
show fd
show info
show profiling

> Cheers,
> CK

BR
Aleks

Haproxy 1.9.8 comsumes 100% CPU

[Akom II]

Hello,
   Ubuntu 18.04  and haproxy 1.9.8 with latest patch on  2019/06/08
   I'm currently encounter haproxy consume all CPU core, at the begging I
curious might relate  to peers function which is our app rely heavily on
those , it happen quite fast after restart service for couple minutes then
it start to consume all CPU core , and there is an impact on service then
consider disable peers function and keep monitoring it still happen but
duration is longer around 10 minutes btw those settings are on production
and serve avg 500 req/s on websocket + SNI

Cheers,
CK

Re: Idea + question regarding the build targets

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

as for popular distro, for example fedora, I'd spent some time on packaging
rpm in fedora copr (rather than telling people proper Makefile options).
if there's an interest in "official" package distro, I'd take part with
fedora copr and maybe few others

ср, 12 июн. 2019 г. в 10:41, Willy Tarreau :

> Hi all,
>
> I'm currently re-reading my notes for the upcoming release and found
> something I noted not that long ago regarding the TARGET variable of
> the makefile. The list of operating systems hasn't changed in a while
> and in parallel we've added a bunch of build options that do not solely
> depend on the kernel anymore, but more on features related to the whole
> environment (pcre, openssl, lua, systemd, zlib, and so on).
>
> I thought that now that the targets definition is very simple in the
> makefile, we could simply add distro names and versions and adjust the
> set of default USE_xxx variables based on this. We could thus have
> rhel{8,7,6,5}, debian{9,8,7,6}, ubuntu{18,16}, fedora{31,30,29} and so
> on, just like we already have for solaris/openbsd/freebsd for example,
> except that we generally only reference one version there. It would
> become easier to add new variants even to stable distros to ease the
> build for new users, who would just have to issue "make TARGET=debian9"
> for example, without having to figure if PCRE2 has to be specified or
> OPENSSL or any such thing.
>
> This this is fairly minor and basically just requires a few lines to
> be added to the makefile and to the doc, so I'm fine with including
> this to the final release.
>
> So my questions are :
>   - does anybody think it's a bad idea ?
>   - and among those who support the idea (if any), do you use
> particular options for a given distro (pcre2, lua version, systemd)
>   - should we consider that using these pre-set distros enables more
> features by default given that packages are readily available ?
> (openssl seems obvious, zlib and pcre{1,2} probably, lua maybe)
>
> Note that it is also something we can add as backports after the release.
> It's just that if we want to teach people to start using a distro name
> over a kernel version, we'd rather propose a few entries first.
>
> Thanks for any hint,
> Willy
>
>

Re: [PATCH] wurfl device detection build fixes and dummy library

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

чт, 13 июн. 2019 г. в 15:25, Ben Shillito :

> Hi,
>
>
>
> The docs are correct. However, the 51Degrees source in the “contrib”
> directory should only be used for testing changes to the HAProxy source.
> The code contained in here does not contain any 51Degrees functionality,
> just method stubs to enable compilation and testing.
>

it's what I meant actually.
I added "USE_51DEGREES=1 51DEGREES_SRC=contrib/51d/src/pattern" to
travis-ci job in order to see whether it will build or not.

or do I need to clone github repo anyway ?


another question is "what should be added to travis-ci?" I see
"src/pattern" and "src/trie" alternatives. Are they mutually exclusiive ?
So, should we test either of them ?


>
>
> You should use the 51Degrees source located at
> https://github.com/51degrees/device-detection.git. Then point
> 51DEGREES_SRC to either device-detection/src/pattern or
> device-detection/src/trie.
>
>
>
> Perhaps if the documentation is not clear enough we should modify it, or
> even add a warning to the compilation.
>
>
>
> Let me know if this gets you up and running.
>
>
>
> Regards,
>
>
>
> Ben Shillito
> Developer
>
> [image: 51Degrees] 
>
> O: +44 1183 287152 
> E: b...@51degrees.com 
>  @51Degrees 
>  51Degrees 
>
> [image: Find out More] 
>
>
>
> *From:* Илья Шипицин [mailto:chipits...@gmail.com]
> *Sent:* 13 June 2019 11:07
> *To:* Ben Shillito 
> *Cc:* Willy Tarreau ; Christopher Faulet ;
> HAProxy 
> *Subject:* Re: [PATCH] wurfl device detection build fixes and dummy
> library
>
>
>
> Ben,
>
>
>
> what is the proper way of building 51degree ?
>
>
>
> I added "USE_51DEGREES=1 51DEGREES_SRC=contrib/51d/src/pattern" (as I seen
> in documentation)
>
>
>
> gcc -Iinclude -Iebtree -Wall -Wextra  -O2  -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   
> -DUSE_EPOLL-DUSE_NETFILTER -DUSE_PCRE -DUSE_PCRE_JIT   -DUSE_POLL  
> -DUSE_THREAD -DUSE_TPROXY -DUSE_LINUX_TPROXY -DUSE_LINUX_SPLICE 
> -DUSE_LIBCRYPT -DUSE_CRYPT_H  -DUSE_GETADDRINFO -DUSE_OPENSSL -DUSE_LUA 
> -DUSE_FUTEX -DUSE_ACCEPT4  -DUSE_ZLIB  -DUSE_CPU_AFFINITY -DUSE_TFO  -DUSE_DL 
> -DUSE_RT -DUSE_DEVICEATLAS -DUSE_51DEGREES -DUSE_WURFL -DUSE_SYSTEMD  
> -DUSE_PRCTL -DUSE_THREAD_DUMP   -I/home/travis/opt/include 
> -I/usr/include/lua5.3 -I/usr/include/lua5.3 -Icontrib/deviceatlas 
> -Icontrib/src/trie -Icontrib/wurfl -DUSE_PCRE -I/usr/include  
> -DCONFIG_HAPROXY_VERSION=\"2.0-dev7\" -DCONFIG_HAPROXY_DATE=\"2019/06/11\" -c 
> -o src/da.o src/da.c
>
> make: *** No rule to make target 'contrib/src/trie/../cityhash/city.o', 
> needed by 'haproxy'.  Stop.
>
> make: *** Waiting for unfinished jobs
>
> is documentation correct ? or I should use some different way to build
>
>
>
> ср, 12 июн. 2019 г. в 22:01, Ben Shillito :
>
> Hi Willy,
>
> Great, thanks for those changes, and good spot.
>
> I agree that this is a significant step forward, and having the entire
> codebase testable in CI will certainly make everything that bit smoother.
>
> Thanks,
>
> Ben Shillito
> Developer
> O: +44 1183 287152
> E: b...@51degrees.com
> T: @51Degrees
>
> -Original Message-
> From: Willy Tarreau [mailto:w...@1wt.eu]
> Sent: 12 June 2019 17:07
> To: Ben Shillito 
> Cc: Christopher Faulet ; HAProxy <
> haproxy@formilux.org>
> Subject: Re: [PATCH] wurfl device detection build fixes and dummy library
>
> On Wed, Jun 12, 2019 at 02:49:37PM +, Ben Shillito wrote:
> > While I was working on the HTX changes, I thought it was probably a
> > good time to also implement the dummy library as I had my brain in
> 'HAProxy mode'.
>
> Ah, excellent, thank you :
>
>   $./haproxy -vv|grep -i 51d
>   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 +THREAD_DUMP -EVPORTS
>   Built with 51Degrees support.
>
> :-)
>
> Ilya will likely be happy to see that we can now build 100% of our
> codebase in the CI, this is a significant step forward!
>
> I performed very minor changes to your patches, for the first one I marked
> it "BUG/MINOR" so that we backport it to 1.9 (since it's still broken
> there) and for the second I fixed the doc where you accidently dropped
> "51d" after "contrib/" in the build command :-)
>
> Thanks a lot for your responsiveness,
> Willy
> This email and any attachments are confidential and may also be
> privileged. If you are not the named 

RE: [PATCH] wurfl device detection build fixes and dummy library

[Ben Shillito]

Hi,

The docs are correct. However, the 51Degrees source in the “contrib” directory 
should only be used for testing changes to the HAProxy source. The code 
contained in here does not contain any 51Degrees functionality, just method 
stubs to enable compilation and testing.

You should use the 51Degrees source located at 
https://github.com/51degrees/device-detection.git. Then point 51DEGREES_SRC to 
either device-detection/src/pattern or device-detection/src/trie.

Perhaps if the documentation is not clear enough we should modify it, or even 
add a warning to the compilation.

Let me know if this gets you up and running.

Regards,

Ben Shillito
Developer
[51Degrees]
O: +44 1183 287152
E: b...@51degrees.com
[https://51degrees.com/portals/0/images/twitterbird.png] 
@51Degrees
[https://51degrees.com/portals/0/images/linkedinicon.png] 
51Degrees
[Find out More]

From: Илья Шипицин [mailto:chipits...@gmail.com]
Sent: 13 June 2019 11:07
To: Ben Shillito 
Cc: Willy Tarreau ; Christopher Faulet ; 
HAProxy 
Subject: Re: [PATCH] wurfl device detection build fixes and dummy library

Ben,

what is the proper way of building 51degree ?

I added "USE_51DEGREES=1 51DEGREES_SRC=contrib/51d/src/pattern" (as I seen in 
documentation)


gcc -Iinclude -Iebtree -Wall -Wextra  -O2  -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   
-DUSE_EPOLL-DUSE_NETFILTER -DUSE_PCRE -DUSE_PCRE_JIT   -DUSE_POLL  
-DUSE_THREAD -DUSE_TPROXY -DUSE_LINUX_TPROXY -DUSE_LINUX_SPLICE 
-DUSE_LIBCRYPT -DUSE_CRYPT_H  -DUSE_GETADDRINFO -DUSE_OPENSSL -DUSE_LUA 
-DUSE_FUTEX -DUSE_ACCEPT4  -DUSE_ZLIB  -DUSE_CPU_AFFINITY -DUSE_TFO  -DUSE_DL 
-DUSE_RT -DUSE_DEVICEATLAS -DUSE_51DEGREES -DUSE_WURFL -DUSE_SYSTEMD  
-DUSE_PRCTL -DUSE_THREAD_DUMP   -I/home/travis/opt/include 
-I/usr/include/lua5.3 -I/usr/include/lua5.3 -Icontrib/deviceatlas 
-Icontrib/src/trie -Icontrib/wurfl -DUSE_PCRE -I/usr/include  
-DCONFIG_HAPROXY_VERSION=\"2.0-dev7\" -DCONFIG_HAPROXY_DATE=\"2019/06/11\" -c 
-o src/da.o src/da.c

make: *** No rule to make target 'contrib/src/trie/../cityhash/city.o', needed 
by 'haproxy'.  Stop.

make: *** Waiting for unfinished jobs


is documentation correct ? or I should use some different way to build

ср, 12 июн. 2019 г. в 22:01, Ben Shillito 
mailto:b...@51degrees.com>>:
Hi Willy,

Great, thanks for those changes, and good spot.

I agree that this is a significant step forward, and having the entire codebase 
testable in CI will certainly make everything that bit smoother.

Thanks,

Ben Shillito
Developer
O: +44 1183 287152
E: b...@51degrees.com
T: @51Degrees

-Original Message-
From: Willy Tarreau [mailto:w...@1wt.eu]
Sent: 12 June 2019 17:07
To: Ben Shillito mailto:b...@51degrees.com>>
Cc: Christopher Faulet mailto:cfau...@haproxy.com>>; 
HAProxy mailto:haproxy@formilux.org>>
Subject: Re: [PATCH] wurfl device detection build fixes and dummy library

On Wed, Jun 12, 2019 at 02:49:37PM +, Ben Shillito wrote:
> While I was working on the HTX changes, I thought it was probably a
> good time to also implement the dummy library as I had my brain in 'HAProxy 
> mode'.

Ah, excellent, thank you :

  $./haproxy -vv|grep -i 51d
  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 +THREAD_DUMP -EVPORTS
  Built with 51Degrees support.

:-)

Ilya will likely be happy to see that we can now build 100% of our codebase in 
the CI, this is a significant step forward!

I performed very minor changes to your patches, for the first one I marked it 
"BUG/MINOR" so that we backport it to 1.9 (since it's still broken there) and 
for the second I fixed the doc where you accidently dropped "51d" after 
"contrib/" in the build command :-)

Thanks a lot for your responsiveness,
Willy
This email and any attachments are confidential and may also be privileged. If 
you are not the named recipient, please notify the sender immediately and do 
not disclose, use, store or copy the information contained herein. This is an 
email from 51Degrees.mobi Limited, 5 Charlotte Close, Reading. RG47BY. T: +44 
118 328 7152; E: i...@51degrees.com; 51Degrees.mobi 
Limited t/as 51Degrees.
This email and any attachments are confidential and may also be privileged. If 
you 

Re: [PATCH] wurfl device detection build fixes and dummy library

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

Ben,

what is the proper way of building 51degree ?

I added "USE_51DEGREES=1 51DEGREES_SRC=contrib/51d/src/pattern" (as I seen
in documentation)

gcc -Iinclude -Iebtree -Wall -Wextra  -O2  -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   -DUSE_EPOLL-DUSE_NETFILTER -DUSE_PCRE
-DUSE_PCRE_JIT   -DUSE_POLL  -DUSE_THREAD -DUSE_TPROXY
-DUSE_LINUX_TPROXY -DUSE_LINUX_SPLICE -DUSE_LIBCRYPT -DUSE_CRYPT_H
-DUSE_GETADDRINFO -DUSE_OPENSSL -DUSE_LUA -DUSE_FUTEX -DUSE_ACCEPT4
-DUSE_ZLIB  -DUSE_CPU_AFFINITY -DUSE_TFO  -DUSE_DL -DUSE_RT
-DUSE_DEVICEATLAS -DUSE_51DEGREES -DUSE_WURFL -DUSE_SYSTEMD
-DUSE_PRCTL -DUSE_THREAD_DUMP   -I/home/travis/opt/include
-I/usr/include/lua5.3 -I/usr/include/lua5.3 -Icontrib/deviceatlas
-Icontrib/src/trie -Icontrib/wurfl -DUSE_PCRE -I/usr/include
-DCONFIG_HAPROXY_VERSION=\"2.0-dev7\"
-DCONFIG_HAPROXY_DATE=\"2019/06/11\" -c -o src/da.o src/da.c
make: *** No rule to make target
'contrib/src/trie/../cityhash/city.o', needed by 'haproxy'.  Stop.
make: *** Waiting for unfinished jobs


is documentation correct ? or I should use some different way to build


ср, 12 июн. 2019 г. в 22:01, Ben Shillito :

> Hi Willy,
>
> Great, thanks for those changes, and good spot.
>
> I agree that this is a significant step forward, and having the entire
> codebase testable in CI will certainly make everything that bit smoother.
>
> Thanks,
>
> Ben Shillito
> Developer
> O: +44 1183 287152
> E: b...@51degrees.com
> T: @51Degrees
>
> -Original Message-
> From: Willy Tarreau [mailto:w...@1wt.eu]
> Sent: 12 June 2019 17:07
> To: Ben Shillito 
> Cc: Christopher Faulet ; HAProxy <
> haproxy@formilux.org>
> Subject: Re: [PATCH] wurfl device detection build fixes and dummy library
>
> On Wed, Jun 12, 2019 at 02:49:37PM +, Ben Shillito wrote:
> > While I was working on the HTX changes, I thought it was probably a
> > good time to also implement the dummy library as I had my brain in
> 'HAProxy mode'.
>
> Ah, excellent, thank you :
>
>   $./haproxy -vv|grep -i 51d
>   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 +THREAD_DUMP -EVPORTS
>   Built with 51Degrees support.
>
> :-)
>
> Ilya will likely be happy to see that we can now build 100% of our
> codebase in the CI, this is a significant step forward!
>
> I performed very minor changes to your patches, for the first one I marked
> it "BUG/MINOR" so that we backport it to 1.9 (since it's still broken
> there) and for the second I fixed the doc where you accidently dropped
> "51d" after "contrib/" in the build command :-)
>
> Thanks a lot for your responsiveness,
> Willy
> This email and any attachments are confidential and may also be
> privileged. If you are not the named recipient, please notify the sender
> immediately and do not disclose, use, store or copy the information
> contained herein. This is an email from 51Degrees.mobi Limited, 5 Charlotte
> Close, Reading. RG47BY. T: +44 118 328 7152; E: i...@51degrees.com;
> 51Degrees.mobi Limited t/as 51Degrees.
>
>

Re: [PR] MINOR: doc: Remove -Ds option in man page

[William Lallemand]

On Thu, Jun 13, 2019 at 09:23:04AM +, PR Bot wrote:
> Description:
>`-Ds` option should be removed from man page. It is no longer
>supported as this commit shows: https://github.com/haproxy/haproxy/com
>mit/095ba4c2428ec8bcccb134b3d24f07de2aabbdcd
>
>I noticed that
>the option has already gone when upgrading my haproxy from 1.7 to
>2.0-dev7. I have had the old and new haproxy running as a pod on my
>Kubernetes cluster, therefore, it is important for me to know what
>option is provided to keep the process in the foreground.
>After upgrading, I was able to make my haproxy running successfully by
>removing `-Ds` option and `daemon` global parameter in the config.
>Thank you.
> 

Good catch.

However this option was never meant to be used this way, it was supposed to be
used with the systemd wrapper binary, that's why it was removed when the
systemd wrapper was replaced by the master worker mode.

Depending on your usage, you probably want to use the master worker mode (-W),
which will allow multi-process mode in foreground.

Patch applied, thanks.

-- 
William Lallemand

[PR] MINOR: doc: Remove -Ds option in man page

[PR Bot]

Dear list!

Author: Kazuo Yagi 
Number of patches: 1

This is an automated relay of the Github pull request:
   MINOR: doc: Remove -Ds option in man page

Patch title(s): 
   MINOR: doc: Remove -Ds option in man page

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

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

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

Description:
   `-Ds` option should be removed from man page. It is no longer
   supported as this commit shows: https://github.com/haproxy/haproxy/com
   mit/095ba4c2428ec8bcccb134b3d24f07de2aabbdcd
   
   I noticed that
   the option has already gone when upgrading my haproxy from 1.7 to
   2.0-dev7. I have had the old and new haproxy running as a pod on my
   Kubernetes cluster, therefore, it is important for me to know what
   option is provided to keep the process in the foreground.
   After upgrading, I was able to make my haproxy running successfully by
   removing `-Ds` option and `daemon` global parameter in the config.
   Thank you.

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: Increase in sockets in TIME_WAIT with 1.9.x

[Willy Tarreau]

On Wed, Jun 12, 2019 at 12:08:03PM -0500, Dave Chiluk wrote:
> I did a bit more introspection on our TIME_WAIT connections.  The increase
> in sockets in TIME_WAIT is definitely from old connections to our backend
>  server instances.  Considering the fact that this server is doesn't
> actually serve real traffic we can make a reasonable assumptions that this
> is almost entirely due to increases in healthchecks.

Great!

> Doing an strace on haproxy 1.8.17 we see
> 
> sudo strace -e setsockopt,close -p 15743
> strace: Process 15743 attached
> setsockopt(17, SOL_TCP, TCP_NODELAY, [1], 4) = 0
> setsockopt(17, SOL_SOCKET, SO_LINGER, {onoff=1, linger=0}, 8) = 0
> close(17)   = 0
> 
> 
> Doing the same strace on 1.9.8 we see
> 
> sudo strace -e setsockopt,close -p 6670
> strace: Process 6670 attached
> setsockopt(4, SOL_TCP, TCP_NODELAY, [1], 4) = 0
> close(4)= 0
> 
> 
> The calls to setsockopt(17, SOL_SOCKET, SO_LINGER, {onoff=1, linger=0}, 8)
> = 0
> appear to be missing.

Awesome, that's exactly the info I was missing. I suspected that for
whatever reason the lingering was not disabled, at least now we have
a proof of this! Now the trick is to figure why :-/

> We are running centos 7 with kernel 3.10.0-957.1.3.el7.x86_64.

OK, and with the setsockopt it should behave properly.

> I'll keep digging into this, and see if I can get stack traces that result
> in teh setsockopt calls on 1.8.17 so the stack can be more closely
> inspected.

Don't worry for this now, this is something we at least need to resolve
before issuing 2.0 or it will cause some trouble. Then we'll backport the
fix once the cause is figured out.

However when I try here I don't have the problem, either in 1.9.8 or 2.0-dev7 :

08:27:30.212570 connect(14, {sa_family=AF_INET, sin_port=htons(9003), 
sin_addr=inet_addr("127.0.0.1")}, 16) = 0
08:27:30.212590 recvfrom(14, NULL, 2147483647, 
MSG_TRUNC|MSG_DONTWAIT|MSG_NOSIGNAL, NULL, NULL) = -1 EAGAIN (Resource 
temporarily unavailable)
08:27:30.212610 setsockopt(14, SOL_SOCKET, SO_LINGER, {l_onoff=1, l_linger=0}, 
8) = 0
08:27:30.212630 close(14)   = 0
08:27:30.212659 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, 
tv_nsec=6993282}) = 0

So it must depend on the type of check. Could you please share a minimalistic
config that reproduces this ?

Thanks,
Willy