51d.c HTX patch
Hi Willy, I have attached a patch which fixes the bug raised in this GitHub issue: https://github.com/haproxy/haproxy/issues/450 The issue was reported in 2.0.12, so should be backported to 2.0 and 2.1 branches. If there is anything extra you need from me, do let me know as we'd like to get the fix out there as soon as we can. Thanks, Ben Shillito Developer [51Degrees]<https://51degrees.com/> O: +44 1183 287152 E: b...@51degrees.com<mailto:b...@51degrees.com?subject=Your%20Email> [https://51degrees.com/portals/0/images/twitterbird.png] @51Degrees<http://twitter.com/51Degreesmobi> [https://51degrees.com/portals/0/images/linkedinicon.png] 51Degrees<https://www.linkedin.com/company/2171864> [Find out More]<https://51degrees.com/emailsig.aspx> 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-BUG-MINOR-Fixed-bug-in-51d.c-when-HTX-is-enabled.patch Description: 0001-BUG-MINOR-Fixed-bug-in-51d.c-when-HTX-is-enabled.patch
RE: Possible optimization to 51d in multithread
Hi,
This is intentional, as the calling method would return early if the htx is
null (this is assuming that smp_prefetch_htx will not return a different result
the second time?):
htx = smp_prefetch_htx(smp, chn, 1);
if (!htx)
return 0;
Rather than doing the null check again, I will instead change the method to
take the htx which has already been checked. This way coverity should not have
an issue with it, we avoid having an extra null check, and we remove the second
call to smp_prefetch_htx which was unnecessary.
I will submit this as a separate patch to the other change.
Thanks,
Ben Shillito
Developer
[51Degrees]<https://51degrees.com/>
O: +44 1183 287152
E: b...@51degrees.com<mailto:b...@51degrees.com?subject=Your%20Email>
[https://51degrees.com/portals/0/images/twitterbird.png]
@51Degrees<http://twitter.com/51Degreesmobi>
[https://51degrees.com/portals/0/images/linkedinicon.png]
51Degrees<https://www.linkedin.com/company/2171864>
[Find out More]<https://51degrees.com/emailsig.aspx>
From: Илья Шипицин [mailto:chipits...@gmail.com]
Sent: 24 October 2019 08:04
To: Ben Shillito
Cc: Willy Tarreau ; HAProxy
Subject: Re: Possible optimization to 51d in multithread
Hello, Ben.
since you are going to touch 51d code, can you please review the following
coverity finding (it thinks there might be null pointer dereference) ?
239// No need to null check as this has already been carried out in the
240// calling method
2. returned_null: smp_prefetch_htx returns NULL (checked 35 out of 36 times).
[show
details<https://scan6.coverity.com/eventId=28590715-1=28590715-0=94351345=%2Fsrc%2Fhttp_fetch.c=172=294>]
3. var_assigned: Assigning: htx = NULL return value from smp_prefetch_htx.
241htx = smp_prefetch_htx(smp, chn, 1);
242
4. Condition i < global_51degrees.header_count, taking true branch.
243for (i = 0; i < global_51degrees.header_count; i++) {
244name.ptr = (global_51degrees.header_names + i)->area;
245name.len = (global_51degrees.header_names + i)->data;
246ctx.blk = NULL;
247
CID 1403847 (#1 of 1): Dereference null return value (NULL_RETURNS)5.
dereference: Dereferencing a pointer that might be NULL htx when calling
http_find_header. [show
details<https://scan6.coverity.com/eventId=28590715-16=28590715-1=94351414=%2Fsrc%2Fhttp_htx.c=54=129>]
248if (http_find_header(htx, name, , 1)) {
249
ws->importantHeaders[ws->importantHeadersCount].header =
ws->dataSet->httpHeaders + i;
250
ws->importantHeaders[ws->importantHeadersCount].headerValue = ctx.value.ptr;
251
ws->importantHeaders[ws->importantHeadersCount].headerValueLength =
ctx.value.len;
252ws->importantHeadersCount++;
253 }
254}
255}
256#endif
ср, 23 окт. 2019 г. в 14:40, Ben Shillito
mailto:b...@51degrees.com>>:
Hi Willy,
Thanks for letting me know. I will get the work for this scheduled in and get a
patch over to you.
I agree it looks like a fairly small change for what I'm sure is a considerable
performance increase for large systems.
Good to see this level of attention to detail when it comes to the performance
of HAProxy.
Thanks,
Ben Shillito
Developer
O: +44 1183 287152
E: b...@51degrees.com<mailto:b...@51degrees.com>
T: @51Degrees
-Original Message-
From: Willy Tarreau [mailto:w...@1wt.eu<mailto:w...@1wt.eu>]
Sent: 23 October 2019 07:04
To: Ben Shillito mailto:b...@51degrees.com>>
Cc: HAProxy mailto:haproxy@formilux.org>>
Subject: Possible optimization to 51d in multithread
Hi Ben,
after Brian reported the thread performance regression affecting the pattern
matchings in haproxy when relying on the LRU cache, I had a look at other users
of the LRU cache and found that 51d.c is using it with a lock as well and may
also suffer from a lack of linearity with threads.
You may want to have a look at this patch I just committed to make the pattern
LRU cache per thread :
403bfbb130 ("BUG/MEDIUM: pattern: make the pattern LRU cache thread-local and
lockless")
It's quite straightforward, and if you want to do the same on 51d.c, you just
have to make your lru_tree pointer thread_local and allocate it for each thread
in a small callback registered to be called after threads are initialized. Same
for the call to lru64_destroy(). Then you can remove the lru lock and gain a
lot of scalability.
I'll let you have a look because I'd rather not break somehing non- obvious in
your code and also because you know better than me how to benchmark the changes
using the real lib and database, but if you need some help, just let me know.
Given that it's quite small and simple a change, I'm fine with merging such a
patch for 2.1 even a bit late (but only t
RE: [PATCH] wurfl device detection build fixes and dummy library
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
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] wurfl device detection build fixes and dummy library
Thanks both, Ilya, I will take a look at that now. Ben Shillito Developer [51Degrees]<https://51degrees.com/> O: +44 1183 287152 E: b...@51degrees.com<mailto:b...@51degrees.com?subject=Your%20Email> [https://51degrees.com/portals/0/images/twitterbird.png] @51Degrees<http://twitter.com/51Degreesmobi> [https://51degrees.com/portals/0/images/linkedinicon.png] 51Degrees<https://www.linkedin.com/company/2171864> [Find out More]<https://51degrees.com/emailsig.aspx> 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
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
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]<https://51degrees.com/> O: +44 1183 287152 E: b...@51degrees.com<mailto:b...@51degrees.com?subject=Your%20Email> [https://51degrees.com/portals/0/images/twitterbird.png] @51Degrees<http://twitter.com/51Degreesmobi> [https://51degrees.com/portals/0/images/linkedinicon.png] 51Degrees<https://www.linkedin.com/company/2171864> [Find out More]<https://51degrees.com/emailsig.aspx> 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]<https://51degrees.com/> O: +44 1183 287152 E: b...@51degrees.com<mailto:b...@51degrees.com?subject=Your%20Email> [Image removed by sender.] @51Degrees<http://twitter.com/51Degreesmobi> [Image removed by sender.] 51Degrees<https://www.linkedin.com/company/2171864> [Image removed by sender. Find out More]<https://51degrees.com/emailsig.aspx> From: Илья Шипицин [mailto:chipits...@gmail.com<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]<https://51degrees.com/> O: +44 1183 287152 E: b...@51degree
RE: [PATCH] wurfl device detection build fixes and dummy library
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]<https://51degrees.com/> O: +44 1183 287152 E: b...@51degrees.com<mailto:b...@51degrees.com?subject=Your%20Email> [https://51degrees.com/portals/0/images/twitterbird.png] @51Degrees<http://twitter.com/51Degreesmobi> [https://51degrees.com/portals/0/images/linkedinicon.png] 51Degrees<https://www.linkedin.com/company/2171864> [Find out More]<https://51degrees.com/emailsig.aspx> 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]<https://51degrees.com/> O: +44 1183 287152 E: b...@51degrees.com<mailto:b...@51degrees.com?subject=Your%20Email> [Image removed by sender.] @51Degrees<http://twitter.com/51Degreesmobi> [Image removed by sender.] 51Degrees<https://www.linkedin.com/company/2171864> [Image removed by sender. Find out More]<https://51degrees.com/emailsig.aspx> From: Илья Шипицин [mailto:chipits...@gmail.com<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 ср, 1
RE: [PATCH] wurfl device detection build fixes and dummy library
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]<https://51degrees.com/> O: +44 1183 287152 E: b...@51degrees.com<mailto:b...@51degrees.com?subject=Your%20Email> [https://51degrees.com/portals/0/images/twitterbird.png] @51Degrees<http://twitter.com/51Degreesmobi> [https://51degrees.com/portals/0/images/linkedinicon.png] 51Degrees<https://www.linkedin.com/company/2171864> [Find out More]<https://51degrees.com/emailsig.aspx> 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<mailto:b...@51degrees.com> T: @51Degrees -Original Message- From: Willy Tarreau [mailto:w...@1wt.eu<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<mailto:i...@5
RE: [PATCH] wurfl device detection build fixes and dummy library
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 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: [PATCH] wurfl device detection build fixes and dummy library
Hi Willy, No worries, that's fine. 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'. Both patches are attached ready to be merged. If there is anything missing, let me know and can I'll try to make sure you have everything you need for a weekend release. 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 10:12 To: Ben Shillito Cc: Christopher Faulet ; HAProxy Subject: Re: [PATCH] wurfl device detection build fixes and dummy library On Wed, Jun 12, 2019 at 08:52:53AM +, Ben Shillito wrote: > Hi Willy, > > This unfortunately fell down our list of priorities in the last few weeks. Oh I certainly can understand, and I'm sorry I forgot about it so I didn't ping you earlier. > However, as this is a bit more urgent now with your weekend release, I > will get the change for HTX awareness to you either today or tomorrow > if that is ok with you? OK that's perfect, thank you! It will be less work on my side to merge your fix than to write the verification code to refuse to start! Thanks, 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-FEAT-51d-htx-The-_51d_fetch-method-and-the-methods-i.patch Description: 0001-FEAT-51d-htx-The-_51d_fetch-method-and-the-methods-i.patch 0002-MINOR-51d-Added-dummy-libraries-for-the-51Degrees-mo.patch Description: 0002-MINOR-51d-Added-dummy-libraries-for-the-51Degrees-mo.patch
RE: [PATCH] wurfl device detection build fixes and dummy library
Hi Willy, This unfortunately fell down our list of priorities in the last few weeks. However, as this is a bit more urgent now with your weekend release, I will get the change for HTX awareness to you either today or tomorrow if that is ok with you? 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 09:14 To: Ben Shillito Cc: Christopher Faulet ; HAProxy Subject: Re: [PATCH] wurfl device detection build fixes and dummy library Hi Ben, On Wed, Apr 24, 2019 at 06:20:16PM +0200, Christopher Faulet wrote: > Le 24/04/2019 à 17:56, Ben Shillito a écrit : > > Hi Willy, > > > > Thanks for the update. We will take a look and get a patch over to you. > > > > Hi, > > Ben, the function _51d_fetch() is not HTX aware. Take a look at other > HTTP sample fetches in src/http_fetch.c. I'm just realizing that we never got your patch for this. Do you have a patch handy or should we currently write a quick one to prevent one from starting with both 51d and HTX enabled ? HTX is on by default in 2.0 and the final release is expected around this week-end... Please just let me know, at least I don't want users to meet random crashes when using this code. Thanks, 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
Hi Willy, Thanks for the update. We will take a look and get a patch over to you. Thanks, Ben Shillito Developer O: +44 1183 287152 E: b...@51degrees.com T: @51Degrees -Original Message- From: Willy Tarreau [mailto:w...@1wt.eu] Sent: 23 April 2019 10:11 To: Paul Stephen Borile Cc: HAProxy Subject: Re: [PATCH] wurfl device detection build fixes and dummy library Hi Paul, On Fri, Apr 19, 2019 at 06:45:22PM +0200, Paul Stephen Borile wrote: > Hi Willy, > > fine for me, thanks for the adjustments and no problem backporting > this to 1.9. > I also confirm that the contact email address is working correctly. Fine thank you. I could finish the polishing (add USE_WURFL to the list of known and reported build options in the makefile) and I've reintegrated the code now. You probably don't see the value in having the dummy library, but for developers it's invaluable. I have now updated my build script to build with USE_WURFL=1 by default so that I'll now see if anything causes warnings or errors there if we touch structures that are used by your code. It would be really awesome if Device Atlas and 51Degrees could do the same, as the build coverage becomes much better with very little effort for everyone. David, Ben, if you read this, please have a look at contrib/wurfl to get an idea of what is sufficient to have your code always built by anyone. Patches welcome :-) Thanks, 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: Does anyone *really* use 51d or WURFL ?
Hi Willy, I have attached two patches. One is the threading change which maps the threading flag in 51Degrees to the one in HAProxy. There are also some changes in the 51d.c module code to make everything thread safe. The other is a minor bug in the multiple header matching when using the Hash Trie API. Thanks, Ben Shillito Developer O: +44 1183 287152 E: b...@51degrees.com T: @51Degrees -Original Message- From: Willy Tarreau [mailto:w...@1wt.eu] Sent: 21 January 2019 16:48 To: Ben Shillito Cc: haproxy@formilux.org Subject: Re: Does anyone *really* use 51d or WURFL ? On Mon, Jan 21, 2019 at 04:37:32PM +, Ben Shillito wrote: > Hi Willy, > > Ah yes, thanks, I missed the S first time reading it. > > There are actually a couple of things I'd like to check over a bit > more thoroughly like the caching used in 51d.c, so it will probably be > more like tomorrow. No problem. We'll probably issue another 1.9 on wednesday. If you can have something by then it could be backported into the next 1.9, otherwise it can still wait a bit. It's been waiting for more than one year, we're not in a hurry anymore :-) Thanks, 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-BUG-51d-In-Hash-Trie-multi-header-matching-was-affec.patch Description: 0001-BUG-51d-In-Hash-Trie-multi-header-matching-was-affec.patch 0002-FEAT-51d-Enabled-multi-threaded-operation-in-the-51D.patch Description: 0002-FEAT-51d-Enabled-multi-threaded-operation-in-the-51D.patch
RE: Does anyone *really* use 51d or WURFL ?
Hi Willy, Ah yes, thanks, I missed the S first time reading it. There are actually a couple of things I'd like to check over a bit more thoroughly like the caching used in 51d.c, so it will probably be more like tomorrow. Thanks, Ben Shillito Developer O: +44 1183 287152 E: b...@51degrees.com T: @51Degrees -Original Message- From: Willy Tarreau [mailto:w...@1wt.eu] Sent: 21 January 2019 16:06 To: Ben Shillito Cc: haproxy@formilux.org Subject: Re: Does anyone *really* use 51d or WURFL ? On Mon, Jan 21, 2019 at 04:00:13PM +, Ben Shillito wrote: > Hi Willy, > > I agree, setting the flag from the HAProxy USE_THREADS is probably the > neatest solution. Yep. Be careful, it's "USE_THREAD" (without trailing S). > I will get a patch over to you later on today. Fine, no emergency though. I prefer that you take the time to verify that it works before submitting :-) Thanks, 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: Does anyone *really* use 51d or WURFL ?
Hi Willy, I agree, setting the flag from the HAProxy USE_THREADS is probably the neatest solution. I will get a patch over to you later on today. Thanks, Ben Shillito Developer O: +44 1183 287152 E: b...@51degrees.com T: @51Degrees -Original Message- From: Willy Tarreau [mailto:w...@1wt.eu] Sent: 21 January 2019 15:44 To: Ben Shillito Cc: haproxy@formilux.org Subject: Re: Does anyone *really* use 51d or WURFL ? Hi Ben, First, thanks for your quick response. On Mon, Jan 21, 2019 at 03:05:08PM +, Ben Shillito wrote: > Hi Willy, > > I'd like to point out that the 51Degrees API does in fact support > multi-threaded operation by default. The HAProxy makefile however, > explicitly uses the FIFTYONEDEGREES_NO_THREADING compile option to > disable this when building > (https://github.com/haproxy/haproxy/blob/master/Makefile#L740). OK! > Multi-threaded operation in the API is tested under maximum load > before every release as can be seen here: > https://github.com/51Degrees/Device-Detection/blob/master/VisualStudio > /UnitTests/Performance/Base.cs#L89 > > Mutli-threaded operation is also stress tested in conjunction with > many threads detecting User-Agents, and another reloading the data > file repeatedly as seen here: > https://github.com/51Degrees/Device-Detection/blob/master/examples/Rel > oadFromFile.c Oh, rest assured that I have no doubt about the API's support itself, what I mean is that 51d.c in haproxy is not thread-compatible right now (at least due to the makefile entry above and to the test in 51d.c) while haproxy is built with threading on by default, and I find it suspicious that nobody noticed after more than one year. But it's very possible that for now nobody is interested in deploying it with threads. At the moment I'm concerned by the fact that in the whole project there remains exactly two files which are incompatible with thread support and as you can imagine it's always a pain to have to deal with exceptions like this. > If multi-threaded support is an issue, then it is a trivial change to > the makefile to align the HAProxy build to the 51Degrees default, and > give the option to disable threading for those who require that. Excellent! What I can suggest then is to set this define only when USE_THREAD is not set. This way you can make sure that 51d.c and the rest of haproxy are always aligned regarding thread support. Feel free to send a patch, I'll happily merge it and even backport it so that we don't have to worry anymore about the compatbility between threads and various options. Thanks! 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: Does anyone *really* use 51d or WURFL ?
Hi Willy, I'd like to point out that the 51Degrees API does in fact support multi-threaded operation by default. The HAProxy makefile however, explicitly uses the FIFTYONEDEGREES_NO_THREADING compile option to disable this when building (https://github.com/haproxy/haproxy/blob/master/Makefile#L740). Multi-threaded operation in the API is tested under maximum load before every release as can be seen here: https://github.com/51Degrees/Device-Detection/blob/master/VisualStudio/UnitTests/Performance/Base.cs#L89 Mutli-threaded operation is also stress tested in conjunction with many threads detecting User-Agents, and another reloading the data file repeatedly as seen here: https://github.com/51Degrees/Device-Detection/blob/master/examples/ReloadFromFile.c If multi-threaded support is an issue, then it is a trivial change to the makefile to align the HAProxy build to the 51Degrees default, and give the option to disable threading for those who require that. Regards, Ben Shillito Developer O: +44 1183 287152 E: b...@51degrees.com T: @51Degrees -Original Message- From: Willy Tarreau [mailto:w...@1wt.eu] Sent: 21 January 2019 14:36 To: haproxy@formilux.org Subject: Does anyone *really* use 51d or WURFL ? Hi all, recently it was figured that the buffer API changes caused some breakage to da.c and 51d.c (both fixed since), I don't know if wurfl builds at all by the way since the last update to the module is its introduction more than 2 years ago. But more importantly I'm realizing that neither 51d nor wurfl will start with threads enabled. This has been the case for about 15 months now, while we've had threads enabled on all relevant operating systems. Thus it leads me to the (possibly wrong) conclusion that absolutely nobody ever uses these options. Am I wrong ? I'm asking because each time we perform some API changes it's always a pain to go through these which don't build out of the box without external dependencies, and I suspect we're simply wasting our time and we should get rid of them if nobody uses them (or at the very least move them to contrib/). But if anyone is using them and disables threads for this, then it's fine, but in this case it would be nice if these two would check the thread safety of their code so that the thread restriction can be removed for the benefit of their users. Please advise. Thanks, 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] Buffer API changes for 51d.c
Hi Willy, Great, thanks for the quick turnaround. Regards, Ben Shillito Developer O: +44 1183 287152 E: b...@51degrees.com T: @51Degrees -Original Message- From: Willy Tarreau [mailto:w...@1wt.eu] Sent: 16 January 2019 16:27 To: Ben Shillito Cc: haproxy@formilux.org Subject: Re: [PATCH] Buffer API changes for 51d.c Hi Ben, On Wed, Jan 16, 2019 at 11:43:03AM +, Ben Shillito wrote: > Hi Willy, > > It appears that 51.d still uses some elements of the the now deprecated > buffer API, so I have attached a patch which updates the usage to the new > buffer API. > > This can also be backported to 1.9 where the new API was introduced. Great, now merged, and still in time for 1.9.2. Thank you! 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.
[PATCH] Buffer API changes for 51d.c
Hi Willy, It appears that 51.d still uses some elements of the the now deprecated buffer API, so I have attached a patch which updates the usage to the new buffer API. This can also be backported to 1.9 where the new API was introduced. Thanks, Ben Shillito Developer [51Degrees]<https://51degrees.com/> O: +44 1183 287152 E: b...@51degrees.com<mailto:b...@51degrees.com?subject=Your%20Email> [https://51degrees.com/portals/0/images/twitterbird.png] @51Degrees<http://twitter.com/51Degreesmobi> [https://51degrees.com/portals/0/images/linkedinicon.png] 51Degrees<https://www.linkedin.com/company/2171864> [Find out More]<https://51degrees.com/emailsig.aspx> 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-BUG-51d-Changes-to-the-buffer-API-in-1.9-were-not-ap.patch Description: 0001-BUG-51d-Changes-to-the-buffer-API-in-1.9-were-not-ap.patch
RE: [PATCHES] 51d: fix warning when building with 51Degrees release version 3.2.12.12
Hi Willy, Don't worry, it's an easy mistake. Yeah, you're absolutely right, my mistake. I assumed that both the URLs were "github.com/51degrees", however as the first one is "git.51degrees.com" which does indeed contain the deprecated Trie source, you can carry on using 3.2.10 on this URL. I have attached a patch on top of Dragan's which updates the branch name for 3.2.12 and adds instructions for obtaining a free Hash Trie data file. Whilst Trie is still available, I think it is important to make it clear to users that, as it uses a lot of memory and was not an optimal solution, the new hash based acyclic graph solution (Hash Trie) is the preferred solution. The new solution uses 20% of the memory and is 300% faster whilst offering tuning options such as difference and drift to avoid over fitting results to the training data. The source code folder continues to be named "trie" to maintain backwards compatibility in build processes, and source code is public interface compatible with prior versions (as implied by Dragan's statement in the readme). 51Degrees are suggesting the small number of Trie users migrate to the new version with HAProxy. Any feedback from the HAProxy community would be very useful to gauge demand for further updates to the prior Trie data files. Just to summarise, 3.2.10 on git.51degrees.com/device-detection should be used for the old Trie source, and 3.2.12 on github.com/51degrees/device-detection should be used for the new Hash Trie source. Apologies for any confusion. Regards, Ben Shillito Developer O: +44 1183 287152 E: b...@51degrees.com T: @51Degrees -Original Message- From: Willy TARREAU [mailto:wtarr...@haproxy.com] Sent: 05 October 2017 17:39 To: Ben Shillito <b...@51degrees.com> Cc: Dragan Dosen <ddo...@haproxy.com>; haproxy@formilux.org Subject: Re: [PATCHES] 51d: fix warning when building with 51Degrees release version 3.2.12.12 On Thu, Oct 05, 2017 at 04:09:09PM +, Ben Shillito wrote: > Hi Willy, > > I have pulled the latest master, but I only see two commits from today. I > assume I'm in the right place " http://git.haproxy.org/git/haproxy;? Argh! Sorry, my fault, I forgot to push :-( And as stupid as I am I did it to verify so now it's pushed (I could have made the changes in place otherwise). > Using 3.2.10 in 1.7 is fine for Pattern, my only concern is that the > old Trie source is not available to for users to compile in 3.2.10. I understood after the latest changes resulting in the public repository being erased that what was put back on the download site precisely was everything including the Trie that some people were using, but that only the database was not updated anymore for the Trie algo (ie: what was running in field would not prevent haproxy from being updated and rebuilt). And I thought we even got confirmations that it fixed the build issue for them. > All method signatures in 3.2.12 are unchanged from 3.2.10, so using > that in 1.7 will not be a problem. If you are more comfortable keeping > that locked to the fixed 3.2.10, then that is fine, The principle for our stable versions has always been the same for more than a decade : once a new version is released and put into stable state, we don't remove features so that users have exactly zero excuse for not updating their code within the branch they're using. So if they spent one month validating that haproxy 1.7 works and writing deployment scripts to keep it up to date, this process shall never fail during all the 1.7 maintenance cycle. This is why it's important to maintain the compatibility for something that was emitted. However : 1) it is not a guarantee that it will become better over time (ie in your case you provide a database that may or may not be updated, they're free to use an outdated one if that suits their needs), 2) we should always ensure to limit the number of upgrade cycles for our users, which means that new adopters who have not yet settled on a choice should always be encouraged to pick the most future-proof one. So in summary, deployments that used to work with 3.2.10 must continue to work with it when users simply upgrade their haproxy sources, but those not using it yet should prefer 3.2.12. > but the line of Dragan's statement should then read something like > "either use the proven stable but frozen 3.2.10 version which supports > the Pattern algorithm". That's the part that's confusing me, because based on our previous discussions that was started by Thierry's report of build breakage, I understood that what you've put in place ensured it was now OK for them but that they wouldn't benefit anymore from updates on the Trie database. Could you please double-check ? > Or, if it makes things easier for you, I can backport the > trie/51Degrees.c/h files to 3.2.10, meaning Trie can be used i
RE: [PATCHES] 51d: fix warning when building with 51Degrees release version 3.2.12.12
Hi Willy, I have pulled the latest master, but I only see two commits from today. I assume I'm in the right place " http://git.haproxy.org/git/haproxy;? Using 3.2.10 in 1.7 is fine for Pattern, my only concern is that the old Trie source is not available to for users to compile in 3.2.10. All method signatures in 3.2.12 are unchanged from 3.2.10, so using that in 1.7 will not be a problem. If you are more comfortable keeping that locked to the fixed 3.2.10, then that is fine, but the line of Dragan's statement should then read something like "either use the proven stable but frozen 3.2.10 version which supports the Pattern algorithm". Or, if it makes things easier for you, I can backport the trie/51Degrees.c/h files to 3.2.10, meaning Trie can be used in that version (the Hash Trie files we now distribute will be needed instead of the deprecated Trie). Thanks, Ben Shillito Developer O: +44 1183 287152 E: b...@51degrees.com T: @51Degrees -Original Message- From: Willy TARREAU [mailto:wtarr...@haproxy.com] Sent: 05 October 2017 16:40 To: Ben Shillito <b...@51degrees.com> Cc: Dragan Dosen <ddo...@haproxy.com>; haproxy@formilux.org Subject: Re: [PATCHES] 51d: fix warning when building with 51Degrees release version 3.2.12.12 On Thu, Oct 05, 2017 at 03:15:20PM +, Ben Shillito wrote: > Yes of course. I have attached a patch which has the correct branch and > updated instructions about where to get the free Hash Trie file now that it > is no longer part of the git repository. But this one is for the doc before Dragan's patch, please pull the master. By the way this makes me realize that we'll probably need to have a different doc for 1.7 and 1.8. I agree with you that 1.8 should only reference the up to date version. Build instructions for 3.2.10 should only appear in 1.7 and older for people who rely on an old signature, but there the download URL and instructions for 3.2.12 should be correct as well. So feel free to update your patch on top of the current master, removing references to 3.2.10 and Trie, and I'll take Dragan's patch + the part relevant to 3.2.12 for 1.7 to encourage users to upgrade without forcing them. Thanks! 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: [PATCHES] 51d: fix warning when building with 51Degrees release version 3.2.12.12
Hi Willy, Yes of course. I have attached a patch which has the correct branch and updated instructions about where to get the free Hash Trie file now that it is no longer part of the git repository. Regards, Ben Shillito Developer O: +44 1183 287152 E: b...@51degrees.com T: @51Degrees -Original Message- From: Willy TARREAU [mailto:wtarr...@haproxy.com] Sent: 05 October 2017 15:50 To: Ben Shillito <b...@51degrees.com> Cc: Dragan Dosen <ddo...@haproxy.com>; haproxy@formilux.org Subject: Re: [PATCHES] 51d: fix warning when building with 51Degrees release version 3.2.12.12 Hi Ben! On Thu, Oct 05, 2017 at 02:31:33PM +, Ben Shillito wrote: > Hi Dragan, > > The changes seem reasonable to me. A couple of things I should mention though. > > The branch name for 3.2.12.12 should be in the same format as 3.2.10 (i.e. > "v3.2.12"). The other thing is that the statement about 3.2.10 is not > correct. The Trie data files were large and the solution had become > unwieldly, so this has been removed from our repository. The Hash Trie > however, is completely interface compatible and is available from 3.2.12 > onwards. I suggest just stating "v3.2.12" and point out the new Hash Trie can > be obtained by signing up for free at > https://51degrees.com/products/store/on-premise-device-detection. Thanks for the heads up. Too bad I merged the patch this morning by lack of any comment. Would you care to provide an updated patch on top of the current master to reflect the best suggestions ? Thanks, 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-DOC-51d-Update-51Degrees-git-URL-to-feature-frozen-b.patch Description: 0001-DOC-51d-Update-51Degrees-git-URL-to-feature-frozen-b.patch
RE: [PATCHES] 51d: fix warning when building with 51Degrees release version 3.2.12.12
Hi Dragan, The changes seem reasonable to me. A couple of things I should mention though. The branch name for 3.2.12.12 should be in the same format as 3.2.10 (i.e. "v3.2.12"). The other thing is that the statement about 3.2.10 is not correct. The Trie data files were large and the solution had become unwieldly, so this has been removed from our repository. The Hash Trie however, is completely interface compatible and is available from 3.2.12 onwards. I suggest just stating "v3.2.12" and point out the new Hash Trie can be obtained by signing up for free at https://51degrees.com/products/store/on-premise-device-detection. Regards, Ben Shillito Developer O: +44 1183 287152 E: b...@51degrees.com T: @51Degrees -Original Message- From: Dragan Dosen [mailto:ddo...@haproxy.com] Sent: 02 October 2017 10:14 To: haproxy@formilux.org Cc: Willy Tarreau <wtarr...@haproxy.com>; Ben Shillito <b...@51degrees.com> Subject: [PATCHES] 51d: fix warning when building with 51Degrees release version 3.2.12.12 Hi all, I'm sending you a new set of patches -- fix for warning that appears when building with 51Degrees release that uses a new Hash Trie algorithm (release version 3.2.12.12): * BUILD/MINOR: 51d: fix warning when building with 51Degrees release version 3.2.12.12 * DOC: 51d: add 51Degrees git URL that points to release version 3.2.12.12 Patches can be backported in 1.7. Best regards, Dragan Dosen 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: Build error with 51degrees library
Hi Willy, Thanks for the additional change. And that's quite alright, if there is a problem with our API that is affecting users live builds like this, then it will always be our top priority. Regards, Ben Shillito Developer O: +44 1183 287152 E: b...@51degrees.com T: @51Degrees -Original Message- From: Willy Tarreau [mailto:w...@1wt.eu] Sent: 19 July 2017 19:56 To: Ben Shillito <b...@51degrees.com> Cc: Florian Tham <fgt...@gmail.com>; James Rosewell <ja...@51degrees.com>; Thierry FOURNIER <tfourn...@arpalert.org>; haproxy@formilux.org Subject: Re: Build error with 51degrees library Hi Ben, On Wed, Jul 19, 2017 at 05:49:44PM +, Ben Shillito wrote: > Hi Willy and HaProxy Forum, > > I appreciate this has caused users problems which we overlooked when > making these changes. Rest assured we are introducing fixed feature > branches as part of procedure to prevent this from happening again. > > We have done the following to remedy the current issues: > 1. branches v3.2.5 and v3.2.10 are now published as stable branches at > https://git.51degrees.com/device-detection.git. These are stable, feature > locked, branches and will only be updated with security fixes. > 2. I have submitted to Willy patch files for 1.6, 1.7 and main dev branches. > These change the URL which is referenced in the instructions, and 1.7/dev now > reference the 3.2.10 stable branch. Thank you. I'll fix the URL in your patches as there were some upper case characters causing "git clone" to fail, but the URL above works. And now with a version in the branch, I'm confident there should not be any similar issue anymore. Thanks for having dealt with this issue quickly. 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: Build error with 51degrees library
Hi Willy and HaProxy Forum, I appreciate this has caused users problems which we overlooked when making these changes. Rest assured we are introducing fixed feature branches as part of procedure to prevent this from happening again. We have done the following to remedy the current issues: 1. branches v3.2.5 and v3.2.10 are now published as stable branches at https://git.51degrees.com/device-detection.git. These are stable, feature locked, branches and will only be updated with security fixes. 2. I have submitted to Willy patch files for 1.6, 1.7 and main dev branches. These change the URL which is referenced in the instructions, and 1.7/dev now reference the 3.2.10 stable branch. Regards, Ben Shillito Developer O: +44 1183 287152 E: b...@51degrees.com T: @51Degrees -Original Message- From: Willy Tarreau [mailto:w...@1wt.eu] Sent: 19 July 2017 10:59 To: Ben Shillito <b...@51degrees.com> Cc: Florian Tham <fgt...@gmail.com>; James Rosewell <ja...@51degrees.com>; Thierry FOURNIER <tfourn...@arpalert.org>; haproxy@formilux.org Subject: Re: Build error with 51degrees library Hi Ben, On Wed, Jul 19, 2017 at 09:33:28AM +, Ben Shillito wrote: > Hi HaProxy Forum, > > In relation to the build error with 51Degrees library. The data files > associated with Trie are very large and were destroying the > performance of the Git repository. As we believe a very small > percentage of users were actually using the files they are now being > distributed separately to GitHub. Very small or not, the reality is that the documented build procedure for STABLE releases has been broken again without even a prior warning. > See this blog post for full details. > > https://51degrees.com/blog/51degrees-github-repository-housekeeping-im > portant-update Hardly an acceptable explanation for anyone trying to upgrade their haproxy to benefit from the latest security fixes, I'm sorry. Also if it's just a matter of database size, you could imagine distributing the smallest version and providing a conversion utility to turn it into the most appropriate format in field. > If you'd like to continue to use Trie please can you contact us via > the web site at https://51degrees.com/contact-us. We can then advise you > further. Then why not try to address the problem by placing the code somewhere online instead of asking every user to have to contact you ? > There are no plans to alter the distribution of the Pattern algorithm > as the files are substantially smaller and work well with Git. All > HAProxy builds should continue to work with the Pattern option. Just > like HAProxy the open source business model is at the core of what we do. So in short what you're saying is that the Trie algorithm that used to be available in opensource and promoted in the documentation as the fastest one is no longer opensource, screwing all its users and forcing them to choose between falling back to the slower Pattern method or not updating haproxy and stay with securiy issues in production. > The v3.2.5 branch has now been republished, this was missed when the > repository was trimmed, so apologies for any backporting problems this > has caused. Thanks, so this will fix version 1.6 but not version 1.7 since last year you provided a patch to replace the API before 1.7.0 release. Can you please also publish the last branch needed to build 1.7 and provide an update for the doc to explain how to build it now ? You have no idea of the pain you cause to everyone by constantly breaking stable versions, really! And this throws a discredit on haproxy as a stable product! Please use branches and stop referencing your development branch as the official one! > Willy; we're unsure what you mean by "broke the stable series *AGAIN*" > and "betrayed". Very simple : you did this already one year ago in the middle of haproxy 1.6 when the public code API was suddenly replaced in an incompatible way, causing quite some pain by then already. Users pick a version, they deploy it, prepare their update processes and don't have to think about it anymore, trusting all of us to deliver timely updates that they can deploy quickly. In your case, once in a while they discover that some code broke, they have to revisit their build process to adapt to a different branch, after waiting for some time for the previous code to be made available again, etc. This happened twice in haproxy's existence, and the two times it came from your software. It's not serious. > Could you provide further information so we can see if there's a > harmonious way forward if we've made an innocent mistake? The proper way to deliver a library is to version it. While you can encourage your users to test the development branch at their own risk, any user should be able to rely on a stable branch which only provides fixes. Thus for
RE: Build error with 51degrees library
Hi HaProxy Forum, In relation to the build error with 51Degrees library. The data files associated with Trie are very large and were destroying the performance of the Git repository. As we believe a very small percentage of users were actually using the files they are now being distributed separately to GitHub. See this blog post for full details. https://51degrees.com/blog/51degrees-github-repository-housekeeping-important-update If you'd like to continue to use Trie please can you contact us via the web site at https://51degrees.com/contact-us. We can then advise you further. There are no plans to alter the distribution of the Pattern algorithm as the files are substantially smaller and work well with Git. All HAProxy builds should continue to work with the Pattern option. Just like HAProxy the open source business model is at the core of what we do. The v3.2.5 branch has now been republished, this was missed when the repository was trimmed, so apologies for any backporting problems this has caused. Willy; we’re unsure what you mean by “broke the stable series *AGAIN*” and “betrayed”. Could you provide further information so we can see if there’s a harmonious way forward if we’ve made an innocent mistake? Thanks, Ben Ben Shillito Developer O: +44 1183 287152 E: b...@51degrees.com T: @51Degrees -Original Message- From: Florian Tham [mailto:fgt...@gmail.com] Sent: 19 July 2017 09:20 To: Willy Tarreau <w...@1wt.eu> Cc: James Rosewell <ja...@51degrees.com>; Ben Shillito <b...@51degrees.com>; m...@51degrees.com; Thierry FOURNIER <tfourn...@arpalert.org>; haproxy@formilux.org Subject: Re: Build error with 51degrees library On Wed, Jul 19, 2017 at 9:27 AM, Willy Tarreau <w...@1wt.eu> wrote: > I just found a fork of the github repo here which I think could > possibly work, it even contains the v3.2.5 branch : > > https://github.com/aerendil/device-detection-nginx-fix > > It would be a good idea to clone it before it disappears. Thank you, this works fine with haproxy-1.7 and buys me the time I need to look into device detection alternatives. One would also need to hold on to the 51Degrees-LiteV3.2.trie data file. Currently, 51degrees only provides .dat files for use with the pattern algorithm. Best regards, Florian 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.
[PATCH] 51Degrees Docs
I have attached a patch which adds definitions for the 51Degrees converter and fetch functions to docs/configuration.txt so the they will appear in the online documentation. This should also be backported. Thanks, Ben. Ben Shillito Developer [51Degrees]<https://51degrees.com/> O: +44 1183 287152 E: b...@51degrees.com<mailto:b...@51degrees.com?subject=Your%20Email> [https://51degrees.com/portals/0/images/twitterbird.png] @51Degrees<http://twitter.com/51Degreesmobi> [https://51degrees.com/portals/0/images/linkedinicon.png] 51Degrees<https://www.linkedin.com/company/2171864> [Find out More]<https://51degrees.com/emailsig.aspx> 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-DOC-Added-51Degrees-conv-and-fetch-functions-to-docu.patch Description: 0001-DOC-Added-51Degrees-conv-and-fetch-functions-to-docu.patch
RE: 51Degrees Trie Update Patch
Hi Willy,
Yes I agree, this is a problem.
I have attached a patch with a change to the readme which explains that that
the correct version must be pulled if using <=1.6.
Regards,
Ben.
Ben Shillito
Developer
E: b...@51degrees.com
T: @51Degrees
-Original Message-
From: Willy Tarreau [mailto:w...@1wt.eu]
Sent: 04 August 2016 16:43
To: Ben Shillito <b...@51degrees.com>
Cc: haproxy@formilux.org
Subject: Re: 51Degrees Trie Update Patch
Hi Ben,
sorry I missed this one.
On Wed, Jul 06, 2016 at 11:25:17AM +0000, Ben Shillito wrote:
> Attached is a patch submission which makes changes to the 51Degrees
> Trie implementation to work with recent changes to
> github.com/51Degrees/Device-Detection/src/trie/51Degrees.c.
Thanks, but that makes me realize something really bad :
--- a/src/51d.c
+++ b/src/51d.c
@@ -235,7 +235,7 @@ static void _51d_set_device_offsets(struct sample *smp)
(global._51degrees.header_names + index)->len,
msg->chn->buf->p, idx, ) == 1) {
(offsets->firstOffset + offsets->size)->httpHeaderOffset =
*(global._51degrees.header_offsets + index);
-(offsets->firstOffset + offsets->size)->deviceOffset =
fiftyoneDegreesGetDeviceOffset(ctx.line + ctx.val);
+(offsets->firstOffset + offsets->size)->deviceOffset =
+fiftyoneDegreesGetDeviceOffset(_51degrees.data_set, ctx.line +
+ctx.val);
offsets->size++;
}
As you can see the API was changed in the master branch which is also the one
which was documented in the stable version as being the one to pull from. This
is really not cool because stable users who apply what is written in the readme
will get a build failure. Could you please at least provide an update for the
build procedure in the readme that we can backport to 1.6 to reference a branch
that continues to work for these users ? While it will not fix the problem for
existing users, at least they can find a hint in newer releases about how to
fix their issues.
Thanks,
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-DOC-Updated-51Degrees-section-of-ReadMe-to-reflect-c.patch
Description: 0001-DOC-Updated-51Degrees-section-of-ReadMe-to-reflect-c.patch
51Degrees Trie Update Patch
Attached is a patch submission which makes changes to the 51Degrees Trie implementation to work with recent changes to github.com/51Degrees/Device-Detection/src/trie/51Degrees.c. Ben Shillito Developer [51Degrees]<https://51degrees.com/> O: +44 1183 287152 E: b...@51degrees.com<mailto:b...@51degrees.com?subject=Your%20Email> [https://51degrees.com/portals/0/images/twitterbird.png] @51Degrees<http://twitter.com/51Degreesmobi> [https://51degrees.com/portals/0/images/linkedinicon.png] 51Degrees<https://www.linkedin.com/company/2171864> [Find out More]<https://51degrees.com/emailsig.aspx> 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-MAJOR-updated-51d-Trie-implementation-to-incor.patch Description: 0001-BUILD-MAJOR-updated-51d-Trie-implementation-to-incor.patch
DOC: Edited 51Degrees section of README.
Hi, Attached is a patch with an edit to the information in the README regarding 51Degrees installation and configuration. Ben. 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-DOC-Edited-51Degrees-section-of-README.patch Description: 0001-DOC-Edited-51Degrees-section-of-README.patch
