Re: [PATCH] BUG/MEDIUM: tools: fix build with static only toolchains

2020-07-24 Thread Willy Tarreau
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

2020-07-24 Thread Baruch Siach
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?

2020-07-24 Thread Willy Tarreau
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?

2020-07-24 Thread Ricardo Fraile

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

2020-07-24 Thread Christopher Faulet

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

2020-07-24 Thread Sander Klein

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

2020-07-24 Thread Willy Tarreau
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