Re: Bug when loading multiple configuration files
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
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
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
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 >