Re: Bug when loading multiple configuration files

[Ben Cabot]

Hi Willy, Bryan,
Thanks for looking at this and getting it fixed quickly.

Thanks,
Ben

On 26 May 2016 at 17:01, Willy Tarreau  wrote:
> Hi Ben,
>
> On Wed, May 25, 2016 at 08:41:53AM +0100, Ben Cabot wrote:
>> Sorry I forgot include the build details. The configuration its self
>> does not seem to matter, you get the error if you if you load 2 empty
>> files or 2 with any listen or frontend / backend configurations. Its
>> just the fact you are loading 2 configuration files that causes the
>> problem.
>
> Thanks for reporting this. In fact it's interesting because this cleanup
> patch has uncovered a real bug. Look at readcfgfile() in cfgparse.c, the
> parsers are registered for each file. It just had the effect of wasting
> memory and slightly slowing down the config parser as the number of files
> increased, but now it fails. One more reason to keep it, and maybe even
> to backport it in the end.
>
> I've merged the attached patch to fix it.
>
> Thanks,
> Willy



-- 
LOADBALANCER.ORG LTD.
www.loadbalancer.org

Re: Bug when loading multiple configuration files

[Willy Tarreau]

Hi Ben,

On Wed, May 25, 2016 at 08:41:53AM +0100, Ben Cabot wrote:
> Sorry I forgot include the build details. The configuration its self
> does not seem to matter, you get the error if you if you load 2 empty
> files or 2 with any listen or frontend / backend configurations. Its
> just the fact you are loading 2 configuration files that causes the
> problem.

Thanks for reporting this. In fact it's interesting because this cleanup
patch has uncovered a real bug. Look at readcfgfile() in cfgparse.c, the
parsers are registered for each file. It just had the effect of wasting
memory and slightly slowing down the config parser as the number of files
increased, but now it fails. One more reason to keep it, and maybe even
to backport it in the end.

I've merged the attached patch to fix it.

Thanks,
Willy
>From 659fbf02300b721adef3de74a3c1a8e4d0851080 Mon Sep 17 00:00:00 2001
From: Willy Tarreau 
Date: Thu, 26 May 2016 17:55:28 +0200
Subject: BUG/MEDIUM: config: fix multiple declaration of section parsers

Ben Cabot reported that after commit 5e4261b ("CLEANUP: config:
detect double registration of a config section") recently introduced
in 1.7-dev, it's not possible anymore to load multiple configuration
files. Bryan Talbot provided a simple reproducer to exhibit the issue.

It turns out that function readcfgfile() registers new parsers for
section keywords for each new file. In addition to being useless, this
has the negative effect of wasting memory and slowing down the config
parser as the number of configuration files increases.

This fix only needs to be backported if/where the commit above is
backported.
---
 src/cfgparse.c | 29 -
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/src/cfgparse.c b/src/cfgparse.c
index 9b76465..fed5bd5 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -6968,19 +6968,6 @@ int readcfgfile(const char *file)
return -1;
}
 
-   /* Register internal sections */
-   if (!cfg_register_section("listen",   cfg_parse_listen) ||
-   !cfg_register_section("frontend", cfg_parse_listen) ||
-   !cfg_register_section("backend",  cfg_parse_listen) ||
-   !cfg_register_section("defaults", cfg_parse_listen) ||
-   !cfg_register_section("global",   cfg_parse_global) ||
-   !cfg_register_section("userlist", cfg_parse_users)  ||
-   !cfg_register_section("peers",cfg_parse_peers)  ||
-   !cfg_register_section("mailers",  cfg_parse_mailers) ||
-   !cfg_register_section("namespace_list",cfg_parse_netns) ||
-   !cfg_register_section("resolvers", cfg_parse_resolvers))
-   return -1;
-
if ((f=fopen(file,"r")) == NULL) {
free(thisline);
return -1;
@@ -9132,6 +9119,22 @@ void cfg_unregister_sections(void)
}
 }
 
+__attribute__((constructor))
+static void cfgparse_init(void)
+{
+   /* Register internal sections */
+   cfg_register_section("listen", cfg_parse_listen);
+   cfg_register_section("frontend",   cfg_parse_listen);
+   cfg_register_section("backend",cfg_parse_listen);
+   cfg_register_section("defaults",   cfg_parse_listen);
+   cfg_register_section("global", cfg_parse_global);
+   cfg_register_section("userlist",   cfg_parse_users);
+   cfg_register_section("peers",  cfg_parse_peers);
+   cfg_register_section("mailers",cfg_parse_mailers);
+   cfg_register_section("namespace_list", cfg_parse_netns);
+   cfg_register_section("resolvers",  cfg_parse_resolvers);
+}
+
 /*
  * Local variables:
  *  c-indent-level: 8
-- 
1.7.12.1

Re: Bug when loading multiple configuration files

[Ben Cabot]

Sorry I forgot include the build details. The configuration its self
does not seem to matter, you get the error if you if you load 2 empty
files or 2 with any listen or frontend / backend configurations. Its
just the fact you are loading 2 configuration files that causes the
problem.

HA-Proxy version 1.7-dev3-1416746-24 2016/05/20
Copyright 2000-2016 Willy Tarreau 

Build options :
  TARGET  = linux2628
  CPU = generic
  CC  = gcc
  CFLAGS  = -m64 -march=x86-64 -O2 -g -fno-strict-aliasing
-Wdeclaration-after-statement
  OPTIONS = USE_ZLIB=1 USE_REGPARM=1 USE_OPENSSL=1 USE_STATIC_PCRE=1

Default settings :
  maxconn = 2000, bufsize = 16384, maxrewrite = 1024, maxpollevents = 200

Encrypted password support via crypt(3): yes
Built with zlib version : 1.2.3
Compression algorithms supported : identity("identity"),
deflate("deflate"), raw-deflate("deflate"), gzip("gzip")
Built with OpenSSL version : OpenSSL 1.0.1t  3 May 2016
Running on OpenSSL version : OpenSSL 1.0.1t  3 May 2016
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports prefer-server-ciphers : yes
Built with PCRE version : 7.8 2008-09-05
PCRE library supports JIT : no (USE_PCRE_JIT not set)
Built without Lua support
Built with transparent proxy support using: IP_TRANSPARENT
IPV6_TRANSPARENT IP_FREEBIND

Available polling systems :
  epoll : pref=300,  test result OK
   poll : pref=200,  test result OK
 select : pref=150,  test result OK
Total: 3 (3 usable), will use epoll.

Available filters :
[TRACE] trace
[COMP] compression


Ben

On 24 May 2016 at 23:59, Bryan Talbot  wrote:
> The OP didn’t provide many details, but I am able to reproduce this too using 
> 1.7-dev and the config files shown below. Git bisect shows the break at the 
> commit mentioned.
>
>
> $> cat haproxy.cfg haproxy2.cfg
> global
>
> defaults
> timeout client 5s
> timeout server 5s
> timeout connect 5s
> mode http
>
> listen www
> bind :8000
>
>
> listen www2
> bind :8001
>
>
> $> cat git-bisect-run.sh
> #!/bin/bash -e
> make clean
> make TARGET=generic USE_OPENSSL=1 ADDLIB=-lcrypto 
> SSL_INC=/usr/local/opt/openssl/include SSL_LIB=/usr/local/opt/openssl/lib 
> USE_ZLIB=1 USE_PCRE=1 -j4
> ./haproxy -c -f ./haproxy.cfg -f ./haproxy2.cfg || exit 1
> ./haproxy -vv
>
>
>
>
>
>> On May 24, 2016, at May 24, 4:50 AM, Ben Cabot  wrote:
>>
>> Hi all,
>> I think we have found an issue when using multiple configuration
>> files. The config parser tries to register the listen section twice
>> causing the error below.
>>
>> [root@lbmaster haproxy]# /usr/local/sbin/haproxy -f
>> /etc/haproxy/haproxy.cfg -f /etc/haproxy/haproxy_manual.cfg
>> [ALERT] 144/113841 (10937) : register section 'listen': already registered.
>> [ALERT] 144/113841 (10937) : Could not open configuration file
>> /etc/haproxy/haproxy_manual.cfg : Success
>>
>>
>> It looks to be introduced in 5e4261b0 but I'm unsure how to fix it.
>> Please can someone take a look.
>>
>> Thanks,
>>
>> Ben
>>
>



-- 
LOADBALANCER.ORG LTD.
www.loadbalancer.org

Re: Bug when loading multiple configuration files

[Bryan Talbot]

The OP didn’t provide many details, but I am able to reproduce this too using 
1.7-dev and the config files shown below. Git bisect shows the break at the 
commit mentioned.


$> cat haproxy.cfg haproxy2.cfg
global

defaults
timeout client 5s
timeout server 5s
timeout connect 5s
mode http

listen www
bind :8000


listen www2
bind :8001


$> cat git-bisect-run.sh
#!/bin/bash -e
make clean
make TARGET=generic USE_OPENSSL=1 ADDLIB=-lcrypto 
SSL_INC=/usr/local/opt/openssl/include SSL_LIB=/usr/local/opt/openssl/lib 
USE_ZLIB=1 USE_PCRE=1 -j4
./haproxy -c -f ./haproxy.cfg -f ./haproxy2.cfg || exit 1
./haproxy -vv





> On May 24, 2016, at May 24, 4:50 AM, Ben Cabot  wrote:
> 
> Hi all,
> I think we have found an issue when using multiple configuration
> files. The config parser tries to register the listen section twice
> causing the error below.
> 
> [root@lbmaster haproxy]# /usr/local/sbin/haproxy -f
> /etc/haproxy/haproxy.cfg -f /etc/haproxy/haproxy_manual.cfg
> [ALERT] 144/113841 (10937) : register section 'listen': already registered.
> [ALERT] 144/113841 (10937) : Could not open configuration file
> /etc/haproxy/haproxy_manual.cfg : Success
> 
> 
> It looks to be introduced in 5e4261b0 but I'm unsure how to fix it.
> Please can someone take a look.
> 
> Thanks,
> 
> Ben
>