Re: [PATCH] BUG/MEDIUM: tools: fix build with static only toolchains
Hi Baruch, On Fri, Jul 24, 2020 at 01:05:54PM +0300, Baruch Siach wrote: > I can confirm that the patch below also fixes the same build issue. Great, thank you! I've updated your first patch to use this one instead, adjusted the commit message accordingly, and marked it for backport to 2.1. Thanks! Willy
Re: [PATCH] BUG/MEDIUM: tools: fix build with static only toolchains
Hi Willy, On Fri, Jul 24 2020, Willy Tarreau wrote: > On Fri, Jul 24, 2020 at 07:40:01AM +0200, Willy Tarreau wrote: >> On Fri, Jul 24, 2020 at 07:52:20AM +0300, Baruch Siach wrote: >> > uClibc toolchains built with no dynamic library support don't provide >> > the dlfcn.h header. That leads to build failure: >> > >> > CC src/tools.o >> > src/tools.c:15:10: fatal error: dlfcn.h: No such file or directory >> > #include >> > ^ >> > Enable dladdr only when USE_DL is defined. >> >> Oh thank you, I've encountered it as well a few days ago while working >> on something else and forgot to note it on my todo list! However, I'm >> looking at the original commit which introduced this and it's almost a >> revert. We used to rely on USE_DL and turned it into __ELF__ because >> some platforms don't need USE_DL. >> >> I wanted to turn to sets of WANT/NEED/HAVE instead of the generic USE, >> and I think this one will be one of the first benefiting from this. I'll >> have a look at this. > > In order to make something less intrusive that may be backported to 2.2 > and 2.1, what do you think about this: > > -#ifdef __ELF__ > +#if (defined(__ELF__) && !defined(__linux__)) || defined(USE_DL) > > => __ELF__ is enough on all non-linux platforms >USE_DL is required on linux > > This would not change much from your patch and limit the scope of changes > for stable versions. If you can confirm that it works for you, I can adapt > your patch to do that and commit it (unless you prefer to send a v2, of > course). I can confirm that the patch below also fixes the same build issue. Thanks, baruch diff --git a/src/tools.c b/src/tools.c index 1c664852ad73..21bb1efbeeb0 100644 --- a/src/tools.c +++ b/src/tools.c @@ -10,7 +10,7 @@ * */ -#ifdef __ELF__ +#if (defined(__ELF__) && !defined(__linux__)) || defined(USE_DL) #define _GNU_SOURCE #include #include @@ -4410,7 +4410,7 @@ const char *get_exec_path() return ret; } -#ifdef __ELF__ +#if (defined(__ELF__) && !defined(__linux__)) || defined(USE_DL) /* calls dladdr() or dladdr1() on and . If dladdr1 is available, * also returns the symbol size in , otherwise returns 0 there. */ @@ -,7 +,7 @@ static int dladdr_and_size(const void *addr, Dl_info *dli, size_t *size) * The file name (lib or executable) is limited to what lies between the last * '/' and the first following '.'. An optional prefix is prepended before * the output if not null. The file is not dumped when it's the same as the one - * that contains the "main" symbol, or when __ELF__ is not set. + * that contains the "main" symbol, or when __ELF__ && USE_DL are not set. * * The symbol's base address is returned, or NULL when unresolved, in order to * allow the caller to match it against known ones. @@ -4472,7 +4472,7 @@ const void *resolve_sym_name(struct buffer *buf, const char *pfx, void *addr) #endif }; -#ifdef __ELF__ +#if (defined(__ELF__) && !defined(__linux__)) || defined(USE_DL) Dl_info dli, dli_main; size_t size; const char *fname, *p; @@ -4489,7 +4489,7 @@ const void *resolve_sym_name(struct buffer *buf, const char *pfx, void *addr) } } -#ifdef __ELF__ +#if (defined(__ELF__) && !defined(__linux__)) || defined(USE_DL) /* Now let's try to be smarter */ if (!dladdr_and_size(addr, , )) goto unknown; @@ -4529,7 +4529,7 @@ const void *resolve_sym_name(struct buffer *buf, const char *pfx, void *addr) chunk_appendf(buf, "+%#lx", (long)(addr - dli.dli_fbase)); return NULL; } -#endif /* __ELF__ */ +#endif /* __ELF__ && USE_DL */ unknown: /* unresolved symbol from the main file, report relative offset to main */ if ((void*)addr < (void*)main) -- ~. .~ Tk Open Systems =}ooO--U--Ooo{= - bar...@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
Re: How to debug matching ACLs?
Hello Ricardo, On Fri, Jul 24, 2020 at 10:36:59AM +0200, Ricardo Fraile wrote: (...) > I think that the "set-var" way is more flexible, as you can combine it with > any other variable that helps to identify the client request. The proposed > "here" converter can helps more. OK, I'll try to think about it more. > As an idea, it would be nice if these debug modifications done over the ACLs > can be logged based on a condition, for example only with a particular > client pattern, something like: > >acl client_debug hdr_sub(User-Agent) client1-user-agent >log-format %ci,%[var(txn.last_expr)] if client_debug You cannot condition a log format versus another one, but what you can do is use an intermediary variable: acl client_debug hdr_sub(User-Agent) client1-user-agent http-request ser-var(txn.debug_expr) var(txn.last_expr) if client_debug log-format %ci,%[var(txn.debug_expr)] => your debug_expr variable will only be set if client_debug is true, otherwise it will log an empty string. Willy
Re: How to debug matching ACLs?
Hello Willy, Following your suggestions, I've been testing the "debug" solution (in a 2.2 HAproxy) with this sample conf: http-request use-service prometheus-exporter if { path,debug(buf0) -m beg /metrics } seeing from the socket the entries registered on buf0: # echo "show events buf0" | socat stdio /var/run/haproxy.sock <0>2020-07-24T09:24:19.598250+02:00 [debug] buf0: type=str <0>2020-07-24T09:24:26.981110+02:00 [debug] buf0: type=str <0>2020-07-24T09:24:34.598446+02:00 [debug] buf0: type=str Later on, the same http condition, but now with "set-var": http-request use-service prometheus-exporter if { path,set-var(txn.last_expr) -m beg /metrics } log-format %ci,%[var(txn.last_expr)] and seeing the lines on the logs: Jul 24 09:33:49 server1 haproxy[10291]: 192.168.1.17,/metrics Jul 24 09:34:04 server1 haproxy[10291]: 192.168.1.17,/metrics I think that the "set-var" way is more flexible, as you can combine it with any other variable that helps to identify the client request. The proposed "here" converter can helps more. As an idea, it would be nice if these debug modifications done over the ACLs can be logged based on a condition, for example only with a particular client pattern, something like: acl client_debug hdr_sub(User-Agent) client1-user-agent log-format %ci,%[var(txn.last_expr)] if client_debug Thanks for your detailed explanations,
Re: Haproxy 2.2.0 segfault
Le 24/07/2020 à 10:34, Sander Klein a écrit : On 2020-07-20 21:41, Sander Klein wrote: On 2020-07-20 19:16, Christopher Faulet wrote: Le 20/07/2020 à 17:22, Sander Klein a écrit : On 2020-07-20 16:38, Christopher Faulet wrote: Could you retry with the latest 2.2 snapshot (http://www.haproxy.org/download/2.2/src/snapshot/haproxy-ss-LATEST.tar.gz) ? Yes, I just did. Still a segfault. Just in case the new core is below. Ok. Thanks to have tested. Could you share your configuration please ? Don't forget to sanitize it if necessary. I will sent if off list. FYI, I've just tested with HAProxy 2.2.1 and I cannot reproduce it anymore. Nice (I guess :) ! I was busy and I forgot to ask you to test the 2.2.1. Thanks for the feedback. -- Christopher Faulet
Re: Haproxy 2.2.0 segfault
On 2020-07-20 21:41, Sander Klein wrote: On 2020-07-20 19:16, Christopher Faulet wrote: Le 20/07/2020 à 17:22, Sander Klein a écrit : On 2020-07-20 16:38, Christopher Faulet wrote: Could you retry with the latest 2.2 snapshot (http://www.haproxy.org/download/2.2/src/snapshot/haproxy-ss-LATEST.tar.gz) ? Yes, I just did. Still a segfault. Just in case the new core is below. Ok. Thanks to have tested. Could you share your configuration please ? Don't forget to sanitize it if necessary. I will sent if off list. FYI, I've just tested with HAProxy 2.2.1 and I cannot reproduce it anymore. Greets, Sander 0x2E78FBE8.asc Description: application/pgp-keys signature.asc Description: OpenPGP digital signature
Re: [PATCH] BUG/MEDIUM: tools: fix build with static only toolchains
On Fri, Jul 24, 2020 at 07:40:01AM +0200, Willy Tarreau wrote: > On Fri, Jul 24, 2020 at 07:52:20AM +0300, Baruch Siach wrote: > > uClibc toolchains built with no dynamic library support don't provide > > the dlfcn.h header. That leads to build failure: > > > > CC src/tools.o > > src/tools.c:15:10: fatal error: dlfcn.h: No such file or directory > > #include > > ^ > > Enable dladdr only when USE_DL is defined. > > Oh thank you, I've encountered it as well a few days ago while working > on something else and forgot to note it on my todo list! However, I'm > looking at the original commit which introduced this and it's almost a > revert. We used to rely on USE_DL and turned it into __ELF__ because > some platforms don't need USE_DL. > > I wanted to turn to sets of WANT/NEED/HAVE instead of the generic USE, > and I think this one will be one of the first benefiting from this. I'll > have a look at this. In order to make something less intrusive that may be backported to 2.2 and 2.1, what do you think about this: -#ifdef __ELF__ +#if (defined(__ELF__) && !defined(__linux__)) || defined(USE_DL) => __ELF__ is enough on all non-linux platforms USE_DL is required on linux This would not change much from your patch and limit the scope of changes for stable versions. If you can confirm that it works for you, I can adapt your patch to do that and commit it (unless you prefer to send a v2, of course). Thanks, Willy