Re: Recommendations for deleting headers by regexp in 2.x?
Le 23/01/2020 à 19:59, James Brown a écrit : I spent a couple of minutes and made the attached (pretty bad) patch to add a del-header-by-prefix. Just an idea. Instead of adding a new action, it could be cleaner to extend the del-header action adding some keywords. Something like: http-request del-header begin-with http-request del-header end-with http-request del-header match It could be also extended to replace-header and replace-value actions. Just my 2 cents, -- Christopher Faulet
[PATCH] switch to clang-9 in Linux/travis-ci builds
Hello,
let us use clang-9 instead of default clang-7 for linux builds.
Cheers,
Ilya Shipitsin
From e1f7c5d551e6ab0c593f633846bf338c0fed64f1 Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin
Date: Fri, 24 Jan 2020 11:36:41 +0500
Subject: [PATCH] BUILD: CI: use clang-9 for travis-ci builds
---
.travis.yml | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index bf4b82aa9..bba7d74a2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -20,7 +20,7 @@ env:
addons:
apt:
update: true
-packages: [ liblua5.3-dev, libsystemd-dev, libpcre2-dev, socat ]
+packages: [ liblua5.3-dev, libsystemd-dev, libpcre2-dev, clang-9, socat ]
homebrew:
update: true
packages: [ socat ]
@@ -41,12 +41,12 @@ matrix:
arch: amd64
if: type != cron
compiler: clang
-env: TARGET=linux-glibc OPENSSL_VERSION=1.1.1d
+env: TARGET=linux-glibc OPENSSL_VERSION=1.1.1d CC=clang-9
- os: linux
arch: arm64
if: type != cron
compiler: clang
-env: TARGET=linux-glibc OPENSSL_VERSION=1.1.1d
+env: TARGET=linux-glibc OPENSSL_VERSION=1.1.1d CC=clang-9
- os: linux
if: type == cron
compiler: clang
@@ -60,27 +60,27 @@ matrix:
- os: linux
if: type == cron
compiler: clang
-env: TARGET=linux-glibc OPENSSL_VERSION=1.1.0l FIFTYONEDEGREES_SRC="contrib/51d/src/trie"
+env: TARGET=linux-glibc OPENSSL_VERSION=1.1.0l FIFTYONEDEGREES_SRC="contrib/51d/src/trie" CC=clang-9
- os: linux
if: type != cron
compiler: clang
-env: TARGET=linux-glibc LIBRESSL_VERSION=3.0.2
+env: TARGET=linux-glibc LIBRESSL_VERSION=3.0.2 CC=clang-9
- os: linux
if: type == cron
compiler: clang
-env: TARGET=linux-glibc LIBRESSL_VERSION=2.9.2
+env: TARGET=linux-glibc LIBRESSL_VERSION=2.9.2 CC=clang-9
- os: linux
if: type == cron
compiler: clang
-env: TARGET=linux-glibc LIBRESSL_VERSION=2.8.3 EXTRA_OBJS="contrib/prometheus-exporter/service-prometheus.o"
+env: TARGET=linux-glibc LIBRESSL_VERSION=2.8.3 EXTRA_OBJS="contrib/prometheus-exporter/service-prometheus.o" CC=clang-9
- os: linux
if: type == cron
compiler: clang
-env: TARGET=linux-glibc BORINGSSL=yes
+env: TARGET=linux-glibc BORINGSSL=yes CC=clang-9
- os: linux
if: type != cron
compiler: clang
-env: TARGET=linux-glibc FLAGS=
+env: TARGET=linux-glibc FLAGS= CC=clang-9
- os: osx
if: type != cron
compiler: clang
@@ -88,7 +88,7 @@ matrix:
- os: linux
if: type == cron
compiler: clang
-env: TARGET=linux-glibc FLAGS="USE_SLZ=1 USE_PCRE2=1 USE_PCRE2_JIT=1 USE_LUA=1 USE_OPENSSL=1 USE_SYSTEMD=1 USE_WURFL=1 WURFL_INC=contrib/wurfl WURFL_LIB=contrib/wurfl USE_51DEGREES=1"
+env: TARGET=linux-glibc FLAGS="USE_SLZ=1 USE_PCRE2=1 USE_PCRE2_JIT=1 USE_LUA=1 USE_OPENSSL=1 USE_SYSTEMD=1 USE_WURFL=1 WURFL_INC=contrib/wurfl WURFL_LIB=contrib/wurfl USE_51DEGREES=1" CC=clang-9
before_script:
- git clone http://git.1wt.eu/git/libslz.git/
- cd libslz && make && make PREFIX=${HOME}/opt install && cd ..
--
2.24.1
Re: [PATCH[ re-enable cygwin CI builds (Github Actions)
чт, 23 янв. 2020 г. в 15:20, Willy Tarreau : > On Wed, Jan 22, 2020 at 11:10:05PM +0100, Tim Düsterhus wrote: > > > both travis and github actions do offer 4 parallel builds, while > cirrus and > > > app veyor offer 1 parallel build. > > > > Parallel Builds just improve test speed. I don't consider that an > > important selling point for us. The development process is fairly > > asynchronous anyway and the important thing is that there are results > > for more obscure configurations, not that there results within 1 minute. > > Sometimes it's nice to get a low-latency feedback on recent changes. But > lately travis has become amazingly slow, turning from tens of seconds to > several minutes, and regtests occasionally failing on timeouts, so it's > clear that this low-latency argument is not true anymore. I think they're > victim of their success and the spawned VMs are simply overloaded by too > many users doing lots of builds there. We do try to be good citizens by > limiting our combinations but it's possibly not the case for everyone. > > I still think we should not be too demanding on these tools. They do > help us, 2.0 and 2.1's quality at the release date was far above > anything we've done in the past, so I'm fine if we continue to use a > diversity of tools depending on their capabilities. For sure it's less > work to always look at a single place but that's no big deal overall. > we can simplify things a bit. if we turn README into README.md , so we can add badges and links to builds. it saves time, badges are either green or red. if it is green, ok, no need to look deeper (amd mo need to remember links to CI) like that [image: Screenshot from 2020-01-24 10-59-18.png] > > Willy >
Re: arm64 builds?
On Fri, Jan 24, 2020 at 07:12:49AM +0500, ??? wrote: > ??, 24 ???. 2020 ?. ? 01:04, ??? : > > > > > > > ??, 24 ???. 2020 ?. ? 00:54, Willy Tarreau : > > > >> On Fri, Jan 24, 2020 at 12:46:12AM +0500, ??? wrote: > >> > > diff --git a/Makefile b/Makefile > >> > > index 8399f6ca35..4757bc77e6 100644 > >> > > --- a/Makefile > >> > > +++ b/Makefile > >> > > @@ -199,6 +199,7 @@ SPEC_CFLAGS += $(call > >> cc-opt,-Wshift-negative-value) > >> > > SPEC_CFLAGS += $(call cc-opt,-Wshift-overflow=2) > >> > > SPEC_CFLAGS += $(call cc-opt,-Wduplicated-cond) > >> > > SPEC_CFLAGS += $(call cc-opt,-Wnull-dereference) > >> > > +SPEC_CFLAGS += $(call cc-opt,-Walloc-size-larger-than=-1) > >> > > > >> > > >> > > >> > CC src/cfgparse.o > >> > src/cfgparse.c: In function 'check_config_validity': > >> > src/cfgparse.c:3642:33: warning: product '2147483648 * 8' of arguments 1 > >> > and 2 exceeds 'SIZE_MAX' [-Walloc-size-larger-than=] > >> > >> Pfff The only good news is that it takes -1 as SIZE_MAX. > >> > >> > newsrv->idle_orphan_conns = calloc((unsigned int)global.nbthread, > >> > sizeof(*newsrv->idle_orphan_conns)); > >> > > >> > > >> ^ > >> > src/cfgparse.c:3642:33: note: argument 1 in the range [2147483648, > >> > 4294967295] > >> (...) > >> > why is it complaining about "product '2147483648 * 8" ? > >> > >> because calloc multiplies the two fields and gcc decided that the largest > >> value we could possibly pass to the first one if we were as stupid as it > >> is is 2147483648. Interestingly it took the largest negative value turned > >> to positive and ignored the positive ones that can be turned to the second > >> half that are negative if nbthread was negative. > >> > >> I really think this test is totally bogus and that there is no way to > >> express it correctly. I mean, gcc only lets us use 8, 16, 32 or 64 bits. > >> If you need to calloc a few megabytes, you'll be forced to apply a mask > >> to the value just to shut it up, and *create* the overflow problem > >> yourself > >> when it didn't exist. > >> > >> Let's give up on this one if it doesn't cause too much trouble to you. > >> Otherwise we might cheat doing this : > >> > >> calloc((unsigned short)global.nbthread, ...) > >> > >> But I really despise this given that we have to make the code wrong just > >> to please this shitty compiler. > >> > > > > > > it was ubuntu 18.04 + gcc8, I'll try 19.10 + gcc9 > > > > gcc9 produces the same warning OK thanks. Willy
Re: arm64 builds?
пт, 24 янв. 2020 г. в 01:04, Илья Шипицин : > > > пт, 24 янв. 2020 г. в 00:54, Willy Tarreau : > >> On Fri, Jan 24, 2020 at 12:46:12AM +0500, ??? wrote: >> > > diff --git a/Makefile b/Makefile >> > > index 8399f6ca35..4757bc77e6 100644 >> > > --- a/Makefile >> > > +++ b/Makefile >> > > @@ -199,6 +199,7 @@ SPEC_CFLAGS += $(call >> cc-opt,-Wshift-negative-value) >> > > SPEC_CFLAGS += $(call cc-opt,-Wshift-overflow=2) >> > > SPEC_CFLAGS += $(call cc-opt,-Wduplicated-cond) >> > > SPEC_CFLAGS += $(call cc-opt,-Wnull-dereference) >> > > +SPEC_CFLAGS += $(call cc-opt,-Walloc-size-larger-than=-1) >> > > >> > >> > >> > CC src/cfgparse.o >> > src/cfgparse.c: In function 'check_config_validity': >> > src/cfgparse.c:3642:33: warning: product '2147483648 * 8' of arguments 1 >> > and 2 exceeds 'SIZE_MAX' [-Walloc-size-larger-than=] >> >> Pfff The only good news is that it takes -1 as SIZE_MAX. >> >> > newsrv->idle_orphan_conns = calloc((unsigned int)global.nbthread, >> > sizeof(*newsrv->idle_orphan_conns)); >> > >> > >> ^ >> > src/cfgparse.c:3642:33: note: argument 1 in the range [2147483648, >> > 4294967295] >> (...) >> > why is it complaining about "product '2147483648 * 8" ? >> >> because calloc multiplies the two fields and gcc decided that the largest >> value we could possibly pass to the first one if we were as stupid as it >> is is 2147483648. Interestingly it took the largest negative value turned >> to positive and ignored the positive ones that can be turned to the second >> half that are negative if nbthread was negative. >> >> I really think this test is totally bogus and that there is no way to >> express it correctly. I mean, gcc only lets us use 8, 16, 32 or 64 bits. >> If you need to calloc a few megabytes, you'll be forced to apply a mask >> to the value just to shut it up, and *create* the overflow problem >> yourself >> when it didn't exist. >> >> Let's give up on this one if it doesn't cause too much trouble to you. >> Otherwise we might cheat doing this : >> >> calloc((unsigned short)global.nbthread, ...) >> >> But I really despise this given that we have to make the code wrong just >> to please this shitty compiler. >> > > > it was ubuntu 18.04 + gcc8, I'll try 19.10 + gcc9 > gcc9 produces the same warning > > >> >> Willy >> >
Re: Recommendations for deleting headers by regexp in 2.x?
Willy, Am 23.01.20 um 21:40 schrieb Willy Tarreau: > On Thu, Jan 23, 2020 at 12:05:55PM -0800, James Brown wrote: >> Glad to do any other debugging you'd like. Just running `make >> TARGET=linux-glibc USE_NS=` or `make TARGET=osx`; nothing fancy. > > Ah now I have it. I think that it happens when we skip the code > paths that get ifdef'd out when openssl is not built in. We'll > have a look tomorrow now, thanks to you and Tim for the info! > Travis seems to confirm this. Only the build without SSL fails. And it fails all tests. https://github.com/haproxy/haproxy/runs/405627155 Best regards Tim Düsterhus
Re: Recommendations for deleting headers by regexp in 2.x?
On Thu, Jan 23, 2020 at 12:05:55PM -0800, James Brown wrote: > Glad to do any other debugging you'd like. Just running `make > TARGET=linux-glibc USE_NS=` or `make TARGET=osx`; nothing fancy. Ah now I have it. I think that it happens when we skip the code paths that get ifdef'd out when openssl is not built in. We'll have a look tomorrow now, thanks to you and Tim for the info! Willy
Re: Recommendations for deleting headers by regexp in 2.x?
Oh, and the crash is introduced in
8af03b396a6025437b675f9ecaa5db321ec4918c;
56dd354b3c55876c4e693fe5eee919e85b2bad53 is the last version that doesn't
crash for me.
On Thu, Jan 23, 2020 at 12:06 PM Tim Düsterhus wrote:
> Willy,
> James,
>
> Am 23.01.20 um 21:00 schrieb Willy Tarreau:
> > I'm impressed, I'm unable to reproduce it!
>
> FWIW, I can reproduce it:
>
> > [timwolla@/s/haproxy ((f22758d1…))]./haproxy -vv
> > HA-Proxy version 2.2-dev1-f22758-30 2020/01/23 - https://haproxy.org/
> > Status: development branch - not safe for use in production.
> > Known bugs: https://github.com/haproxy/haproxy/issues?q=is:issue+is:open
> > Build options :
> > TARGET = linux-glibc
> > CPU = generic
> > CC = gcc
> > CFLAGS = -O2 -g -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
> > OPTIONS =
> >
> > 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
> >
> > Default settings :
> > bufsize = 16384, maxrewrite = 1024, maxpollevents = 200
> >
> > Built with multi-threading support (MAX_THREADS=64, default=4).
> > Built with network namespace support.
> > Built with transparent proxy support using: IP_TRANSPARENT
> IPV6_TRANSPARENT IP_FREEBIND
> > Built without PCRE or PCRE2 support (using libc's regex instead)
> > Encrypted password support via crypt(3): yes
> > Built without compression support (neither USE_ZLIB nor USE_SLZ are set).
> > Compression algorithms supported : identity("identity")
> >
> > 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 multiplexer protocols :
> > (protocols marked as cannot be specified using 'proto' keyword)
> > h2 : mode=HTTP side=FE|BE mux=H2
> > fcgi : mode=HTTP side=BEmux=FCGI
> > : mode=HTTP side=FE|BE mux=H1
> > : mode=TCPside=FE|BE mux=PASS
> >
> > Available services : none
> >
> > Available filters :
> > [SPOE] spoe
> > [CACHE] cache
> > [FCGI] fcgi-app
> > [TRACE] trace
> > [COMP] compression
> >
> > [timwolla@/s/haproxy ((f22758d1…))]./haproxy -d -f ./crasher.cfg
> > Available polling systems :
> > epoll : pref=300, test result OK
> >poll : pref=200, test result OK
> > select : pref=150, test result FAILED
> > Total: 3 (2 usable), will use epoll.
> >
> > Available filters :
> > [SPOE] spoe
> > [CACHE] cache
> > [FCGI] fcgi-app
> > [TRACE] trace
> > [COMP] compression
> > Using epoll() as the polling mechanism.
> > :test_fe.accept(0004)=0011 from [:::127.0.0.1:48030]
> ALPN=
> > :test_fe.clireq[0011:]: GET / HTTP/1.1
> > :test_fe.clihdr[0011:]: host: localhost:
> > :test_fe.clihdr[0011:]: user-agent: curl/7.47.0
> > :test_fe.clihdr[0011:]: accept: */*
> > 0001:test_fe.accept(0004)=0011 from [:::127.0.0.1:48030]
> ALPN=
> > 0001:test_fe.clicls[0010:]
> > 0001:test_fe.closed[0010:]
> > fish: “./haproxy -d -f ./crasher.cfg” terminated by signal SIGSEGV
> (Address boundary error)
>
> And in another Terminal:
>
> > $ curl localhost:
> > curl: (52) Empty reply from server
>
> With valgrind:
>
> > ==19765== Memcheck, a memory error detector
> > ==19765== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
> > ==19765== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright
> info
> > ==19765== Command: ./haproxy -d -f ./crasher.cfg
> > ==19765==
> > Available polling systems :
> > epoll : pref=300, test result OK
> >poll : pref=200, test result OK
> > select : pref=150, test result FAILED
> > Total: 3 (2 usable), will use epoll.
> >
> > Available filters :
> > [SPOE] spoe
> > [CACHE] cache
> > [FCGI] fcgi-app
> > [TRACE] trace
> > [COMP] compression
> > Using epoll() as the polling mechanism.
> > [WARNING] 022/210543 (19765) : [./haproxy.main()] Cannot raise FD limit
> to 2071, limit is 1024. This will fail in >= v2.3
> > [ALERT] 022/210543 (19765) : [./haproxy.main()] FD limit (1024) too low
> for maxconn=1024/maxsock=2071. Please raise 'ulimit-n' to 2071 or more to
> avoid any trouble.This will fail in >= v2.3
> > ==19765== Thread 2:
> > ==19765== Syscall param timer_create(evp.sige
Re: Recommendations for deleting headers by regexp in 2.x?
Willy,
James,
Am 23.01.20 um 21:00 schrieb Willy Tarreau:
> I'm impressed, I'm unable to reproduce it!
FWIW, I can reproduce it:
> [timwolla@/s/haproxy ((f22758d1…))]./haproxy -vv
> HA-Proxy version 2.2-dev1-f22758-30 2020/01/23 - https://haproxy.org/
> Status: development branch - not safe for use in production.
> Known bugs: https://github.com/haproxy/haproxy/issues?q=is:issue+is:open
> Build options :
> TARGET = linux-glibc
> CPU = generic
> CC = gcc
> CFLAGS = -O2 -g -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
> OPTIONS =
>
> 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
>
> Default settings :
> bufsize = 16384, maxrewrite = 1024, maxpollevents = 200
>
> Built with multi-threading support (MAX_THREADS=64, default=4).
> Built with network namespace support.
> Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT
> IP_FREEBIND
> Built without PCRE or PCRE2 support (using libc's regex instead)
> Encrypted password support via crypt(3): yes
> Built without compression support (neither USE_ZLIB nor USE_SLZ are set).
> Compression algorithms supported : identity("identity")
>
> 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 multiplexer protocols :
> (protocols marked as cannot be specified using 'proto' keyword)
> h2 : mode=HTTP side=FE|BE mux=H2
> fcgi : mode=HTTP side=BEmux=FCGI
> : mode=HTTP side=FE|BE mux=H1
> : mode=TCPside=FE|BE mux=PASS
>
> Available services : none
>
> Available filters :
> [SPOE] spoe
> [CACHE] cache
> [FCGI] fcgi-app
> [TRACE] trace
> [COMP] compression
>
> [timwolla@/s/haproxy ((f22758d1…))]./haproxy -d -f ./crasher.cfg
> Available polling systems :
> epoll : pref=300, test result OK
>poll : pref=200, test result OK
> select : pref=150, test result FAILED
> Total: 3 (2 usable), will use epoll.
>
> Available filters :
> [SPOE] spoe
> [CACHE] cache
> [FCGI] fcgi-app
> [TRACE] trace
> [COMP] compression
> Using epoll() as the polling mechanism.
> :test_fe.accept(0004)=0011 from [:::127.0.0.1:48030] ALPN=
> :test_fe.clireq[0011:]: GET / HTTP/1.1
> :test_fe.clihdr[0011:]: host: localhost:
> :test_fe.clihdr[0011:]: user-agent: curl/7.47.0
> :test_fe.clihdr[0011:]: accept: */*
> 0001:test_fe.accept(0004)=0011 from [:::127.0.0.1:48030] ALPN=
> 0001:test_fe.clicls[0010:]
> 0001:test_fe.closed[0010:]
> fish: “./haproxy -d -f ./crasher.cfg” terminated by signal SIGSEGV (Address
> boundary error)
And in another Terminal:
> $ curl localhost:
> curl: (52) Empty reply from server
With valgrind:
> ==19765== Memcheck, a memory error detector
> ==19765== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
> ==19765== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
> ==19765== Command: ./haproxy -d -f ./crasher.cfg
> ==19765==
> Available polling systems :
> epoll : pref=300, test result OK
>poll : pref=200, test result OK
> select : pref=150, test result FAILED
> Total: 3 (2 usable), will use epoll.
>
> Available filters :
> [SPOE] spoe
> [CACHE] cache
> [FCGI] fcgi-app
> [TRACE] trace
> [COMP] compression
> Using epoll() as the polling mechanism.
> [WARNING] 022/210543 (19765) : [./haproxy.main()] Cannot raise FD limit to
> 2071, limit is 1024. This will fail in >= v2.3
> [ALERT] 022/210543 (19765) : [./haproxy.main()] FD limit (1024) too low for
> maxconn=1024/maxsock=2071. Please raise 'ulimit-n' to 2071 or more to avoid
> any trouble.This will fail in >= v2.3
> ==19765== Thread 2:
> ==19765== Syscall param timer_create(evp.sigev_value) points to uninitialised
> byte(s)
> ==19765==at 0x5292FE0: timer_create@@GLIBC_2.3.3 (timer_create.c:78)
> ==19765==by 0x53824D: init_wdt_per_thread (wdt.c:146)
> ==19765==by 0x4B1D84: run_thread_poll_loop (haproxy.c:2723)
> ==19765==by 0x50796B9: start_thread (pthread_create.c:333)
> ==19765==by 0x559E41C: clone (clone.S:109)
> ==19765== Address 0x643ea64 is on thread 2's sta
Re: Recommendations for deleting headers by regexp in 2.x?
Glad to do any other debugging you'd like. Just running `make
TARGET=linux-glibc USE_NS=` or `make TARGET=osx`; nothing fancy.
On Thu, Jan 23, 2020 at 12:00 PM Willy Tarreau wrote:
> On Thu, Jan 23, 2020 at 11:54:17AM -0800, James Brown wrote:
> > Where master == f22758d12af5e9f3919f24bf913b883a62df7d93, the following
> > config fails on both linux-glibc and osx:
> >
> > global
> > maxconn 1024
> >
> > defaults
> > timeout client 9s
> > timeout server 9s
> > timeout connect 1s
> >
> > frontend test_fe
> > mode http
> > bind :::
> > use_backend test_be
> >
> > backend test_be
> > mode http
> > server localhost 127.0.0.1:1
> >
> > Every request crashes immediately before connecting to the backend.
>
> I'm impressed, I'm unable to reproduce it!
>
> $ telnet 0
> Trying 0.0.0.0...
> Connected to 0.
> Escape character is '^]'.
> GET / HTTP/1.1
>
> HTTP/1.1 200 OK
> connection: close
>
> > Backtrace:
> >
> > Program received signal SIGSEGV, Segmentation fault.
> > back_handle_st_con (s=0x94abd0) at src/backend.c:1937
> > 1937 if (!conn->mux && !(conn->flags & CO_FL_WAIT_XPRT)) {
> > (gdb) bt
> > #0 back_handle_st_con (s=0x94abd0) at src/backend.c:1937
> > #1 0x0042ae75 in process_stream (t=0x94b020, context=0x94abd0,
> > state=) at src/stream.c:1662
> > #2 0x005083c2 in process_runnable_tasks () at src/task.c:461
> > #3 0x004bb36b in run_poll_loop (data=) at
> > src/haproxy.c:2630
> > #4 run_thread_poll_loop (data=) at
> src/haproxy.c:2783
> > #5 0x004bdba5 in main (argc=, argv= > optimized out>) at src/haproxy.c:3483
> >
> > Segfault is on the same line on OS X and Linux.
>
> I'm pretty sure the connection is null (or almost null as derived from
> the CS) though that should not happen at this place. I'll have another
> look at this one tomorrow. Additionally this "if" block will be entirely
> removed :-) But I really want to understand how we manage to enter there
> with an invalid connection.
>
> Thank you!
> Willy
>
--
James Brown
Engineer
Re: arm64 builds?
пт, 24 янв. 2020 г. в 00:54, Willy Tarreau : > On Fri, Jan 24, 2020 at 12:46:12AM +0500, ??? wrote: > > > diff --git a/Makefile b/Makefile > > > index 8399f6ca35..4757bc77e6 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -199,6 +199,7 @@ SPEC_CFLAGS += $(call > cc-opt,-Wshift-negative-value) > > > SPEC_CFLAGS += $(call cc-opt,-Wshift-overflow=2) > > > SPEC_CFLAGS += $(call cc-opt,-Wduplicated-cond) > > > SPEC_CFLAGS += $(call cc-opt,-Wnull-dereference) > > > +SPEC_CFLAGS += $(call cc-opt,-Walloc-size-larger-than=-1) > > > > > > > > > CC src/cfgparse.o > > src/cfgparse.c: In function 'check_config_validity': > > src/cfgparse.c:3642:33: warning: product '2147483648 * 8' of arguments 1 > > and 2 exceeds 'SIZE_MAX' [-Walloc-size-larger-than=] > > Pfff The only good news is that it takes -1 as SIZE_MAX. > > > newsrv->idle_orphan_conns = calloc((unsigned int)global.nbthread, > > sizeof(*newsrv->idle_orphan_conns)); > > > > > ^ > > src/cfgparse.c:3642:33: note: argument 1 in the range [2147483648, > > 4294967295] > (...) > > why is it complaining about "product '2147483648 * 8" ? > > because calloc multiplies the two fields and gcc decided that the largest > value we could possibly pass to the first one if we were as stupid as it > is is 2147483648. Interestingly it took the largest negative value turned > to positive and ignored the positive ones that can be turned to the second > half that are negative if nbthread was negative. > > I really think this test is totally bogus and that there is no way to > express it correctly. I mean, gcc only lets us use 8, 16, 32 or 64 bits. > If you need to calloc a few megabytes, you'll be forced to apply a mask > to the value just to shut it up, and *create* the overflow problem yourself > when it didn't exist. > > Let's give up on this one if it doesn't cause too much trouble to you. > Otherwise we might cheat doing this : > > calloc((unsigned short)global.nbthread, ...) > > But I really despise this given that we have to make the code wrong just > to please this shitty compiler. > it was ubuntu 18.04 + gcc8, I'll try 19.10 + gcc9 > > Willy >
Re: Recommendations for deleting headers by regexp in 2.x?
On Thu, Jan 23, 2020 at 11:54:17AM -0800, James Brown wrote:
> Where master == f22758d12af5e9f3919f24bf913b883a62df7d93, the following
> config fails on both linux-glibc and osx:
>
> global
> maxconn 1024
>
> defaults
> timeout client 9s
> timeout server 9s
> timeout connect 1s
>
> frontend test_fe
> mode http
> bind :::
> use_backend test_be
>
> backend test_be
> mode http
> server localhost 127.0.0.1:1
>
> Every request crashes immediately before connecting to the backend.
I'm impressed, I'm unable to reproduce it!
$ telnet 0
Trying 0.0.0.0...
Connected to 0.
Escape character is '^]'.
GET / HTTP/1.1
HTTP/1.1 200 OK
connection: close
> Backtrace:
>
> Program received signal SIGSEGV, Segmentation fault.
> back_handle_st_con (s=0x94abd0) at src/backend.c:1937
> 1937 if (!conn->mux && !(conn->flags & CO_FL_WAIT_XPRT)) {
> (gdb) bt
> #0 back_handle_st_con (s=0x94abd0) at src/backend.c:1937
> #1 0x0042ae75 in process_stream (t=0x94b020, context=0x94abd0,
> state=) at src/stream.c:1662
> #2 0x005083c2 in process_runnable_tasks () at src/task.c:461
> #3 0x004bb36b in run_poll_loop (data=) at
> src/haproxy.c:2630
> #4 run_thread_poll_loop (data=) at src/haproxy.c:2783
> #5 0x004bdba5 in main (argc=, argv= optimized out>) at src/haproxy.c:3483
>
> Segfault is on the same line on OS X and Linux.
I'm pretty sure the connection is null (or almost null as derived from
the CS) though that should not happen at this place. I'll have another
look at this one tomorrow. Additionally this "if" block will be entirely
removed :-) But I really want to understand how we manage to enter there
with an invalid connection.
Thank you!
Willy
Re: arm64 builds?
On Fri, Jan 24, 2020 at 12:46:12AM +0500, ??? wrote: > > diff --git a/Makefile b/Makefile > > index 8399f6ca35..4757bc77e6 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -199,6 +199,7 @@ SPEC_CFLAGS += $(call cc-opt,-Wshift-negative-value) > > SPEC_CFLAGS += $(call cc-opt,-Wshift-overflow=2) > > SPEC_CFLAGS += $(call cc-opt,-Wduplicated-cond) > > SPEC_CFLAGS += $(call cc-opt,-Wnull-dereference) > > +SPEC_CFLAGS += $(call cc-opt,-Walloc-size-larger-than=-1) > > > > > CC src/cfgparse.o > src/cfgparse.c: In function 'check_config_validity': > src/cfgparse.c:3642:33: warning: product '2147483648 * 8' of arguments 1 > and 2 exceeds 'SIZE_MAX' [-Walloc-size-larger-than=] Pfff The only good news is that it takes -1 as SIZE_MAX. > newsrv->idle_orphan_conns = calloc((unsigned int)global.nbthread, > sizeof(*newsrv->idle_orphan_conns)); > > ^ > src/cfgparse.c:3642:33: note: argument 1 in the range [2147483648, > 4294967295] (...) > why is it complaining about "product '2147483648 * 8" ? because calloc multiplies the two fields and gcc decided that the largest value we could possibly pass to the first one if we were as stupid as it is is 2147483648. Interestingly it took the largest negative value turned to positive and ignored the positive ones that can be turned to the second half that are negative if nbthread was negative. I really think this test is totally bogus and that there is no way to express it correctly. I mean, gcc only lets us use 8, 16, 32 or 64 bits. If you need to calloc a few megabytes, you'll be forced to apply a mask to the value just to shut it up, and *create* the overflow problem yourself when it didn't exist. Let's give up on this one if it doesn't cause too much trouble to you. Otherwise we might cheat doing this : calloc((unsigned short)global.nbthread, ...) But I really despise this given that we have to make the code wrong just to please this shitty compiler. Willy
Re: Recommendations for deleting headers by regexp in 2.x?
Where master == f22758d12af5e9f3919f24bf913b883a62df7d93, the following
config fails on both linux-glibc and osx:
global
maxconn 1024
defaults
timeout client 9s
timeout server 9s
timeout connect 1s
frontend test_fe
mode http
bind :::
use_backend test_be
backend test_be
mode http
server localhost 127.0.0.1:1
Every request crashes immediately before connecting to the backend.
Backtrace:
Program received signal SIGSEGV, Segmentation fault.
back_handle_st_con (s=0x94abd0) at src/backend.c:1937
1937 if (!conn->mux && !(conn->flags & CO_FL_WAIT_XPRT)) {
(gdb) bt
#0 back_handle_st_con (s=0x94abd0) at src/backend.c:1937
#1 0x0042ae75 in process_stream (t=0x94b020, context=0x94abd0,
state=) at src/stream.c:1662
#2 0x005083c2 in process_runnable_tasks () at src/task.c:461
#3 0x004bb36b in run_poll_loop (data=) at
src/haproxy.c:2630
#4 run_thread_poll_loop (data=) at src/haproxy.c:2783
#5 0x004bdba5 in main (argc=, argv=) at src/haproxy.c:3483
Segfault is on the same line on OS X and Linux.
On Thu, Jan 23, 2020 at 11:49 AM Willy Tarreau wrote:
> On Thu, Jan 23, 2020 at 11:05:57AM -0800, James Brown wrote:
> > Update: I rebased back to the last non-segfaulting commit and this
> patch's
> > functionality appears to work in very limited testing.
>
> Cool, thanks for doing it. I've quickly read it and I'm also convinced it
> must work. I'll take more time tomorrow doing a more in-depth review and
> suggesting some parts to split into different patches.
>
> I'm also interested in knowing more about this segfault in master, because
> for me all reg-tests were OK before I pushed last update, thus if you have
> a reproducer I'm very interested :-)
>
> Thanks,
> Willy
>
--
James Brown
Engineer
Re: Recommendations for deleting headers by regexp in 2.x?
On Thu, Jan 23, 2020 at 11:05:57AM -0800, James Brown wrote: > Update: I rebased back to the last non-segfaulting commit and this patch's > functionality appears to work in very limited testing. Cool, thanks for doing it. I've quickly read it and I'm also convinced it must work. I'll take more time tomorrow doing a more in-depth review and suggesting some parts to split into different patches. I'm also interested in knowing more about this segfault in master, because for me all reg-tests were OK before I pushed last update, thus if you have a reproducer I'm very interested :-) Thanks, Willy
Re: arm64 builds?
чт, 23 янв. 2020 г. в 23:17, Willy Tarreau : > On Thu, Jan 23, 2020 at 10:59:00PM +0500, ??? wrote: > > > Could you please try to use (size_t) instead of (unsigned int) ? If > it's > > > enough to shut it up, I'm fine with doing that change. Otherwise we'll > > > probably get rid of that stupid warning. > > > > > > > CC src/server.o > > CC src/cfgparse.o > > src/cfgparse.c: In function 'check_config_validity': > > src/cfgparse.c:3642:33: warning: argument 1 range [2147483648, > 4294967295] > > exceeds maximum object size 2147483647 [-Walloc-size-larger-than=] > > newsrv->idle_orphan_conns = calloc((size_t)global.nbthread, > > sizeof(*newsrv->idle_orphan_conns)); > > > > ^~~ > > In file included from src/cfgparse.c:24: > > /usr/arm-linux-gnueabihf/include/stdlib.h:541:14: note: in a call to > > allocation function 'calloc' declared here > > extern void *calloc (size_t __nmemb, size_t __size) > > ^~ > > CC src/checks.o > > ^CMakefile:846: recipe for target 'src/checks.o' failed > > make: *** [src/checks.o] Interrupt > > Thanks for the test! So basically this clearly proves we respect the > calling convention but the compiler still complains. OK I'm seeing in > the mad that it's for functions "decorated" with the "alloc_size" > attribute. Thus in short they enforce constraints that cannot be > expressed with available types. This is becoming totally ridiculous. > > We're getting a huge collection of stupid warnings to shut up. I > suspect that the sum of all the code written to shut them is larger > than what haproxy 1.0 used to be :-/ > > The man page says we can disable this crap using > -Walloc-size-larger-than=SIZE_MAX but on my compiler (8.2) it simply > fails to build. However when passing explicit values not even that > large, I don't get any such warning, so I'm starting to think that > it's a real bug of GCC 9.2, because quite frankly, aside calling > calloc() with a char or short in argument, there's almost no way out > of this absurdity. > > For me, calling it with -Walloc-size-larger-than=-1 makes it stay > silent. is it also the case for you ? You can patch your Makefile this > way: > > diff --git a/Makefile b/Makefile > index 8399f6ca35..4757bc77e6 100644 > --- a/Makefile > +++ b/Makefile > @@ -199,6 +199,7 @@ SPEC_CFLAGS += $(call cc-opt,-Wshift-negative-value) > SPEC_CFLAGS += $(call cc-opt,-Wshift-overflow=2) > SPEC_CFLAGS += $(call cc-opt,-Wduplicated-cond) > SPEC_CFLAGS += $(call cc-opt,-Wnull-dereference) > +SPEC_CFLAGS += $(call cc-opt,-Walloc-size-larger-than=-1) > CC src/cfgparse.o src/cfgparse.c: In function ‘check_config_validity’: src/cfgparse.c:3642:33: warning: product ‘2147483648 * 8’ of arguments 1 and 2 exceeds ‘SIZE_MAX’ [-Walloc-size-larger-than=] newsrv->idle_orphan_conns = calloc((unsigned int)global.nbthread, sizeof(*newsrv->idle_orphan_conns)); ^ src/cfgparse.c:3642:33: note: argument 1 in the range [2147483648, 4294967295] In file included from src/cfgparse.c:24: /usr/arm-linux-gnueabihf/include/stdlib.h:541:14: note: in a call to allocation function ‘calloc’ declared here extern void *calloc (size_t __nmemb, size_t __size) ^~ CC src/checks.o CC src/backend.o why is it complaining about "product ‘2147483648 * 8" ? > > Memory usage tuning > # If small memory footprint is required, you can reduce the buffer size. > There > > If it still fails, we'll have to ignore it and wait for this monstrosity > to be fixed by their authors :-/ > > Willy >
Re: Recommendations for deleting headers by regexp in 2.x?
Update: I rebased back to the last non-segfaulting commit and this patch's functionality appears to work in very limited testing. On Thu, Jan 23, 2020 at 10:59 AM James Brown wrote: > I spent a couple of minutes and made the attached (pretty bad) patch to > add a del-header-by-prefix. > > Unfortunately, I made it off of master before noticing that master > segfaults on every request, so I haven't tested it yet. At least it > compiles... Feel free to use it, throw it away, or whatever else suits your > fancy. > > > > On Thu, Jan 23, 2020 at 9:26 AM James Brown wrote: > >> Yes, they’re all identified by a prefix. >> >> On Thu, Jan 23, 2020 at 02:03 Willy Tarreau wrote: >> >>> Hi James, >>> >>> On Wed, Jan 22, 2020 at 04:19:41PM -0800, James Brown wrote: >>> > We're upgrading from 1.8 to 2.x and one of the things I've noticed is >>> that >>> > reqidel and rspidel seem to be totally gone in 2.1... What's the new >>> > recommendation to delete headers from request/response based on a >>> regular >>> > expression? Do I have to write a Lua action to do this now? I read >>> through >>> > the documentation for http-request and http-response and there doesn't >>> seem >>> > to be an `http-request del-header-by-regex`... >>> > >>> > Our use case is that we have dozens of different internal headers >>> behind a >>> > prefix, and we promise that we'll strip them all for incoming requests >>> and >>> > outgoing responses at the edge load balancer. That is harder to do if >>> we >>> > can't delete all headers matching a certain regex... >>> >>> That's an intereting use case, which I find totally legitimate and that >>> we need to figure how to address. In 2.0 you can still rely on rspdel >>> but we then need to have a solution for 2.2. Probably that in the short >>> term using Lua will be the easiest solution. And maybe we'd need to add >>> a new action such as "del-headers" which would take a regex or a prefix. >>> By the way, are all your headers identified by the same prefix ? I'm >>> asking because if that's the case, maybe we could append an optional >>> argument to del-header to mention that we want to delete all those >>> starting with this prefix and not just this exact one. >>> >>> Willy >>> >> -- >> James Brown >> Engineer >> > > > -- > James Brown > Engineer > -- James Brown Engineer
Re: Recommendations for deleting headers by regexp in 2.x?
I spent a couple of minutes and made the attached (pretty bad) patch to add a del-header-by-prefix. Unfortunately, I made it off of master before noticing that master segfaults on every request, so I haven't tested it yet. At least it compiles... Feel free to use it, throw it away, or whatever else suits your fancy. On Thu, Jan 23, 2020 at 9:26 AM James Brown wrote: > Yes, they’re all identified by a prefix. > > On Thu, Jan 23, 2020 at 02:03 Willy Tarreau wrote: > >> Hi James, >> >> On Wed, Jan 22, 2020 at 04:19:41PM -0800, James Brown wrote: >> > We're upgrading from 1.8 to 2.x and one of the things I've noticed is >> that >> > reqidel and rspidel seem to be totally gone in 2.1... What's the new >> > recommendation to delete headers from request/response based on a >> regular >> > expression? Do I have to write a Lua action to do this now? I read >> through >> > the documentation for http-request and http-response and there doesn't >> seem >> > to be an `http-request del-header-by-regex`... >> > >> > Our use case is that we have dozens of different internal headers >> behind a >> > prefix, and we promise that we'll strip them all for incoming requests >> and >> > outgoing responses at the edge load balancer. That is harder to do if we >> > can't delete all headers matching a certain regex... >> >> That's an intereting use case, which I find totally legitimate and that >> we need to figure how to address. In 2.0 you can still rely on rspdel >> but we then need to have a solution for 2.2. Probably that in the short >> term using Lua will be the easiest solution. And maybe we'd need to add >> a new action such as "del-headers" which would take a regex or a prefix. >> By the way, are all your headers identified by the same prefix ? I'm >> asking because if that's the case, maybe we could append an optional >> argument to del-header to mention that we want to delete all those >> starting with this prefix and not just this exact one. >> >> Willy >> > -- > James Brown > Engineer > -- James Brown Engineer 0001-add-http-request-del-header-prefix-and-http-response.patch Description: Binary data
Re: arm64 builds?
On Thu, Jan 23, 2020 at 07:41:11PM +0100, Tim Düsterhus wrote: > Willy, > > Am 23.01.20 um 19:17 schrieb Willy Tarreau: > > Thanks for the test! So basically this clearly proves we respect the > > calling convention but the compiler still complains. OK I'm seeing in > > the mad that it's for functions "decorated" with the "alloc_size" > > attribute. Thus in short they enforce constraints that cannot be > > expressed with available types. This is becoming totally ridiculous. > > Googling for that error message says something about the *result* > needing to fit into ptrdiff_t and also something about negative numbers. > > Maybe it is sufficient to change `nbthread` from `int` to `unsigned > int`? That's essentially what the cast did. Also we don't do it because I think I remember that we used to use value -1 to mean "unspecified" during the parsing. > Otherwise the compiler assumes that a negative nbthread casted to > an unsigned value becomes too large. No, the reality is only about the possible extent of the value. By turning it to unsigned it it would still be allowed to cover 0..2^32-1. It turns out the compiler knows nothing about this value and just says "if you did something wrong, it could theorically do that". That's exactly the problem with this type of warnings, they're utter shit because they assume the compiler knows better than the developer what the possible range of his numbers are but the compiler doesn't even look where the values are assigned and knows nothing, so it only relies on types. Anyway it's obvious that stupid gcc warnings started to appear after gcc developers stopped using it for themselves by rewriting it in C++, so they don't care if they piss off their users. Possibly some of them don't even read C anymore since they don't need to :-/ Willy
Re: arm64 builds?
Willy, Am 23.01.20 um 19:17 schrieb Willy Tarreau: > Thanks for the test! So basically this clearly proves we respect the > calling convention but the compiler still complains. OK I'm seeing in > the mad that it's for functions "decorated" with the "alloc_size" > attribute. Thus in short they enforce constraints that cannot be > expressed with available types. This is becoming totally ridiculous. Googling for that error message says something about the *result* needing to fit into ptrdiff_t and also something about negative numbers. Maybe it is sufficient to change `nbthread` from `int` to `unsigned int`? Otherwise the compiler assumes that a negative nbthread casted to an unsigned value becomes too large. I didn't check whether the code assumes that nbthread can be negative anywhere. Best regards Tim Düsterhus
Re: arm64 builds?
On Thu, Jan 23, 2020 at 10:59:00PM +0500, ??? wrote: > > Could you please try to use (size_t) instead of (unsigned int) ? If it's > > enough to shut it up, I'm fine with doing that change. Otherwise we'll > > probably get rid of that stupid warning. > > > > CC src/server.o > CC src/cfgparse.o > src/cfgparse.c: In function 'check_config_validity': > src/cfgparse.c:3642:33: warning: argument 1 range [2147483648, 4294967295] > exceeds maximum object size 2147483647 [-Walloc-size-larger-than=] > newsrv->idle_orphan_conns = calloc((size_t)global.nbthread, > sizeof(*newsrv->idle_orphan_conns)); > > ^~~ > In file included from src/cfgparse.c:24: > /usr/arm-linux-gnueabihf/include/stdlib.h:541:14: note: in a call to > allocation function 'calloc' declared here > extern void *calloc (size_t __nmemb, size_t __size) > ^~ > CC src/checks.o > ^CMakefile:846: recipe for target 'src/checks.o' failed > make: *** [src/checks.o] Interrupt Thanks for the test! So basically this clearly proves we respect the calling convention but the compiler still complains. OK I'm seeing in the mad that it's for functions "decorated" with the "alloc_size" attribute. Thus in short they enforce constraints that cannot be expressed with available types. This is becoming totally ridiculous. We're getting a huge collection of stupid warnings to shut up. I suspect that the sum of all the code written to shut them is larger than what haproxy 1.0 used to be :-/ The man page says we can disable this crap using -Walloc-size-larger-than=SIZE_MAX but on my compiler (8.2) it simply fails to build. However when passing explicit values not even that large, I don't get any such warning, so I'm starting to think that it's a real bug of GCC 9.2, because quite frankly, aside calling calloc() with a char or short in argument, there's almost no way out of this absurdity. For me, calling it with -Walloc-size-larger-than=-1 makes it stay silent. is it also the case for you ? You can patch your Makefile this way: diff --git a/Makefile b/Makefile index 8399f6ca35..4757bc77e6 100644 --- a/Makefile +++ b/Makefile @@ -199,6 +199,7 @@ SPEC_CFLAGS += $(call cc-opt,-Wshift-negative-value) SPEC_CFLAGS += $(call cc-opt,-Wshift-overflow=2) SPEC_CFLAGS += $(call cc-opt,-Wduplicated-cond) SPEC_CFLAGS += $(call cc-opt,-Wnull-dereference) +SPEC_CFLAGS += $(call cc-opt,-Walloc-size-larger-than=-1) Memory usage tuning # If small memory footprint is required, you can reduce the buffer size. There If it still fails, we'll have to ignore it and wait for this monstrosity to be fixed by their authors :-/ Willy
Re: arm64 builds?
чт, 23 янв. 2020 г. в 15:14, Willy Tarreau : > On Thu, Jan 23, 2020 at 01:09:22PM +0500, ??? wrote: > > hello, > > > > I tried to build using cross compiler (arm32 on amd64). sorry for > > screenshot. > > Willy, do you mean errors like that ? > > So for those not seeing the screenshot, it says: > warning: argument 1 range [2147483648, 4294967295] exceeds maximum object > size 2147483647 : > > new->idle_orphan_conns = calloc((unsigned int)global.nbthread, > sizeof(*new->idle_orphan_conns)); > > Thus the cast to unsigned int was likely placed there to shut it up > once. Looking at the man page, it says: > >void *calloc(size_t nmemb, size_t size); > > size_t is unsigned (unless you're using an older gcc < 2.4 on a unusual > OS). I don't see how it can be said to have a maximum size of 2147483647. > > This is exactly the type of stupid warnings that causes us to add casts > that hide real bugs :-/ > > Could you please try to use (size_t) instead of (unsigned int) ? If it's > enough to shut it up, I'm fine with doing that change. Otherwise we'll > probably get rid of that stupid warning. > CC src/server.o CC src/cfgparse.o src/cfgparse.c: In function ‘check_config_validity’: src/cfgparse.c:3642:33: warning: argument 1 range [2147483648, 4294967295] exceeds maximum object size 2147483647 [-Walloc-size-larger-than=] newsrv->idle_orphan_conns = calloc((size_t)global.nbthread, sizeof(*newsrv->idle_orphan_conns)); ^~~ In file included from src/cfgparse.c:24: /usr/arm-linux-gnueabihf/include/stdlib.h:541:14: note: in a call to allocation function ‘calloc’ declared here extern void *calloc (size_t __nmemb, size_t __size) ^~ CC src/checks.o ^CMakefile:846: recipe for target 'src/checks.o' failed make: *** [src/checks.o] Interrupt > > Thanks! > Willy >
Re: Recommendations for deleting headers by regexp in 2.x?
Yes, they’re all identified by a prefix. On Thu, Jan 23, 2020 at 02:03 Willy Tarreau wrote: > Hi James, > > On Wed, Jan 22, 2020 at 04:19:41PM -0800, James Brown wrote: > > We're upgrading from 1.8 to 2.x and one of the things I've noticed is > that > > reqidel and rspidel seem to be totally gone in 2.1... What's the new > > recommendation to delete headers from request/response based on a regular > > expression? Do I have to write a Lua action to do this now? I read > through > > the documentation for http-request and http-response and there doesn't > seem > > to be an `http-request del-header-by-regex`... > > > > Our use case is that we have dozens of different internal headers behind > a > > prefix, and we promise that we'll strip them all for incoming requests > and > > outgoing responses at the edge load balancer. That is harder to do if we > > can't delete all headers matching a certain regex... > > That's an intereting use case, which I find totally legitimate and that > we need to figure how to address. In 2.0 you can still rely on rspdel > but we then need to have a solution for 2.2. Probably that in the short > term using Lua will be the easiest solution. And maybe we'd need to add > a new action such as "del-headers" which would take a regex or a prefix. > By the way, are all your headers identified by the same prefix ? I'm > asking because if that's the case, maybe we could append an optional > argument to del-header to mention that we want to delete all those > starting with this prefix and not just this exact one. > > Willy > -- James Brown Engineer
Re: [PATCH] ssl certificates load speedup and dedup (pem/ctx)
Manu, Am 21.01.20 um 12:42 schrieb Emmanuel Hocdet: > Patches updated, depend on "[PATCH] BUG/MINOR: ssl: > ssl_sock_load_pem_into_ckch is not consistent" Out of curiosity: > +issuer-path > + Assigns a directory to load certificate chain for issuer completion. All > + files must be in PEM format. For certificates loaded with "crt" or > "crt-list", > + if certificate chain is not included in PEM (also commonly known as > intermediate > + certificate), haproxy will complete chain if issuer match the first > certificate > + of the chain loaded with "issuer-path". "issuer-path" directive can be set > + several times. Will HAProxy complete the chain if multiple intermediate certificates are required? Consider this: Root CA -> Intermediate CA -> Intermediate CB -> End Certificate I configure `issuer-path` to a directory that contains the following certificates: - Root CA - Intermediate CA - Intermediate CB Then I configure a `crt` pointing to a file containing only the End Certificate. What will HAProxy send to the client? Best regards Tim Düsterhus
Re: [PATCH[ re-enable cygwin CI builds (Github Actions)
чт, 23 янв. 2020 г. в 20:54, Willy Tarreau : > On Thu, Jan 23, 2020 at 08:40:19PM +0500, ??? wrote: > > those timeouts are not related to travis itself, I beleive they are > mostly > > related to either real failures or tests instability (race condition). > > These tests are racy by nature and some rely on short delays (i.e. health > checks). If tests are run on an overloaded machine I don't find it > surprising that a few will fail. Often I click restart and they succeed. > we started to observe asan failures *** h1 debug|AddressSanitizer:DEADLYSIGNAL *** h1 debug|= *** h1 debug|==10330==ERROR: AddressSanitizer: SEGV on unknown address 0x0018 (pc 0x006e27b1 bp 0x0003 sp 0x7ffe0d8ee080 T0) *** h1 debug|==10330==The signal is caused by a READ memory access. *** h1 debug|==10330==Hint: address points to the zero page. dT 0.015 *** h1 debug|==10330==WARNING: failed to fork (errno 11) *** h1 debug|==10330==WARNING: failed to fork (errno 11) *** h1 debug|==10330==WARNING: failed to fork (errno 11) dT 0.016 *** h1 debug|==10330==WARNING: failed to fork (errno 11) *** h1 debug|==10330==WARNING: failed to fork (errno 11) *** h1 debug|==10330==WARNING: Failed to use and restart external symbolizer! *** h1 debug|#0 0x6e27b0 (/home/travis/build/haproxy/haproxy/haproxy+0x6e27b0) *** h1 debug|#1 0x551799 (/home/travis/build/haproxy/haproxy/haproxy+0x551799) *** h1 debug|#2 0x8218ed (/home/travis/build/haproxy/haproxy/haproxy+0x8218ed) *** h1 debug|#3 0x735000 (/home/travis/build/haproxy/haproxy/haproxy+0x735000) *** h1 debug|#4 0x7328c8 (/home/travis/build/haproxy/haproxy/haproxy+0x7328c8) *** h1 debug|#5 0x7ff91cd88b96 (/lib/x86_64-linux-gnu/libc.so.6+0x21b96) *** h1 debug|#6 0x41bd09 (/home/travis/build/haproxy/haproxy/haproxy+0x41bd09) *** h1 debug| *** h1 debug|AddressSanitizer can not provide additional info. *** h1 debug|SUMMARY: AddressSanitizer: SEGV (/home/travis/build/haproxy/haproxy/haproxy+0x6e27b0) *** h1 debug|==10330==ABORTING dT 0.017 c1 fd=9 EOF, as expected I will have a look. Maybe we will switch asan off for good. > These ones often fail there and only there, so the environment definitely > has an impact. And seeing the execution time which has become 10-30 times > what it is on simple laptop really makes me feel like the VMs are seriously > stressed. > > > the bigger is number of tests, the more we depend on those timeouts. > > > > however, we can try to run tests in parallel :) > > It would be worse! > > Willy >
Re: [PATCH[ re-enable cygwin CI builds (Github Actions)
On Thu, Jan 23, 2020 at 08:40:19PM +0500, ??? wrote: > those timeouts are not related to travis itself, I beleive they are mostly > related to either real failures or tests instability (race condition). These tests are racy by nature and some rely on short delays (i.e. health checks). If tests are run on an overloaded machine I don't find it surprising that a few will fail. Often I click restart and they succeed. These ones often fail there and only there, so the environment definitely has an impact. And seeing the execution time which has become 10-30 times what it is on simple laptop really makes me feel like the VMs are seriously stressed. > the bigger is number of tests, the more we depend on those timeouts. > > however, we can try to run tests in parallel :) It would be worse! Willy
Re: [PATCH[ re-enable cygwin CI builds (Github Actions)
чт, 23 янв. 2020 г. в 15:20, Willy Tarreau : > On Wed, Jan 22, 2020 at 11:10:05PM +0100, Tim Düsterhus wrote: > > > both travis and github actions do offer 4 parallel builds, while > cirrus and > > > app veyor offer 1 parallel build. > > > > Parallel Builds just improve test speed. I don't consider that an > > important selling point for us. The development process is fairly > > asynchronous anyway and the important thing is that there are results > > for more obscure configurations, not that there results within 1 minute. > > Sometimes it's nice to get a low-latency feedback on recent changes. But > lately travis has become amazingly slow, turning from tens of seconds to > several minutes, and regtests occasionally failing on timeouts, so it's > clear that this low-latency argument is not true anymore. I think they're > victim of their success and the spawned VMs are simply overloaded by too > many users doing lots of builds there. We do try to be good citizens by > limiting our combinations but it's possibly not the case for everyone. > those timeouts are not related to travis itself, I beleive they are mostly related to either real failures or tests instability (race condition). the bigger is number of tests, the more we depend on those timeouts. however, we can try to run tests in parallel :) > > I still think we should not be too demanding on these tools. They do > help us, 2.0 and 2.1's quality at the release date was far above > anything we've done in the past, so I'm fine if we continue to use a > diversity of tools depending on their capabilities. For sure it's less > work to always look at a single place but that's no big deal overall. > > Willy >
Re: [PATCH] ssl certificates load speedup and dedup (pem/ctx)
On Tue, Jan 21, 2020 at 12:42:04PM +0100, Emmanuel Hocdet wrote:
> Hi,
>
> Patches updated, depend on "[PATCH] BUG/MINOR: ssl:
> ssl_sock_load_pem_into_ckch is not consistent"
>
> ++
> Manu
>
Hello,
It could be great to share more of the certificates in memory, but some points
are
confusing in my opinion:
> +issuer-path
> + Assigns a directory to load certificate chain for issuer completion. All
> + files must be in PEM format. For certificates loaded with "crt" or
> "crt-list",
> + if certificate chain is not included in PEM (also commonly known as
> intermediate
> + certificate), haproxy will complete chain if issuer match the first
> certificate
> + of the chain loaded with "issuer-path". "issuer-path" directive can be set
> + several times.
> +
That's not a good idea to be able to add a new path to the list each time this
keyword is found, this is not how the configuration works in the global
section, a new line with the keyword should remove the previous one.
> - PEM files into one (e.g. cat cert.pem key.pem > combined.pem). If your CA
> + PEM files into one (e.g. cat key.pem cert.pem > combined.pem). If your CA
I'm not sure to understand why this was changed, maybe by mistake? If not could
you elaborate in the commit message, or split this part in another commit if
it's not related to the feature.
> + /* Find Certificate Chain in global */
> + if (chain == NULL) {
> + AUTHORITY_KEYID *akid;
> + akid = X509_get_ext_d2i(cert, NID_authority_key_identifier,
> NULL, NULL);
> + if (akid) {
> + struct issuer_chain *issuer;
> + struct eb64_node *node;
> + u64 hk;
> + hk = XXH64(ASN1_STRING_get0_data(akid->keyid),
> ASN1_STRING_length(akid->keyid), 0);
> + for (node = eb64_lookup(&global_ssl.cert_issuer_tree,
> hk); node; node = eb64_next(node)) {
> + issuer = container_of(node, typeof(*issuer),
> node);
> + if
> (X509_check_issued(sk_X509_value(issuer->chain, 0), cert) == X509_V_OK) {
> + chain =
> X509_chain_up_ref(issuer->chain);
> + break;
> + }
> + }
> + AUTHORITY_KEYID_free(akid);
> + }
> + }
I'm not sure about this, what's bothering me is that the certificate is not
found using a filename, but only found by looking for the SKID. So it's kind of
magic because it's not named in the configuration. It can be bothering if you
don't want to complete the chain for some of the certificates, or if you have a
multi-tenant configuration.
Maybe we should use a keyword on the bind line to load it instead of this, like
`ssl crt foo.pem issuer bar.pem` and this keyword could be applied to a lot
of certificates by using the default-server keyword.
Also I want to clarify some confusion with the issuer / chain:
- the keyword "issuer" is used for the ocsp response, it's used as the issuer
of the ocsp response, but we could probably use it as a certificate to
complete the chain and split the PEM right?
- it looks like your code is only filling the chain with one certificate,
shouldn't it try to resolve the whole chain ?
> Subject: [PATCH 2/2] MINOR: ssl/cli: 'set ssl issuer' load an issuer
> certificate from the CLI
>
> $ echo -e "set ssl issuer <<\n$(cat chain.pem)\n" | \
> socat stdio /var/run/haproxy.stat
> Issuer loaded
>
> The operation update/load issuer certificate in memory. It's the CLI
> version from "issuer-path" directive. It's usefull with "set ssl cert"
> to update certificate without chain certificates. The chain certificates
> will be shared between certificates with same issuer.
I think there is a consistency problem there, because it does not recreate
the ckchs. What we are trying to do with the CLI, is to have the same effect as
a reload of the process. So it should be able to find which certificate is
using it, and update it.
Regards,
--
William Lallemand
Re: [PATCH v2 0/3] MINOR: lua: Add lua-prepend-path configuration option
Hi Tim, On Thu, Jan 23, 2020 at 03:56:29PM +0100, Tim Düsterhus wrote: > > Are you happy with my patch series as is or would you like to see > > changes? Should I update the config syntax? > > This might have slipped through the cracks. Can you please take a look? > If you don't have the time to look at it right now, just let me know. Sorry, Thierry responded to me he was OK with either solution yesterday but immediately after I got diverted by lots of other stuff and forgot again :-( I'm adding it in my todo list, maybe for tomorrow. Thanks, Willy
Re: [PATCH v2 0/3] MINOR: lua: Add lua-prepend-path configuration option
Thierry, Willy, Am 17.01.20 um 13:58 schrieb Tim Düsterhus: Idea 1: lua-prepend-path path /etc/haproxy/lua-modules/?.lua lua-prepend-path cpath /etc/haproxy/lua-cmodules/?.lua Idea 2: lua-prepend-path /etc/haproxy/lua-modules/?.lua lua-prepend-path cpath /etc/haproxy/lua-cmodules/?.lua Idea 3: lua-prepend-path /etc/haproxy/lua-modules/?.lua lua-prepend-cpath /etc/haproxy/lua-cmodules/?.lua >>> >>> I guess the third one is better, at least for a reason which is that >>> it causes less confusion when asking a bug reporter "what's in your >>> lua-prepend-path ?". We've seen sufficient confusion from the maxconn >>> word being used for different tihngs depending on where it's set, so >>> we'd rather keep this clear. >> >> Let me give my reasoning for my choice: >> >> - I wanted to avoid two completely distinct configuration options for >> what is essentially the same thing. It would require adding both to the >> documentation. >> - I made the type optional, because I expect the majority of modules >> loaded via this option to be pure Lua modules. The path is meant to be >> the home of HAProxy specific modules. I consider it unlikely for an user >> to develop a C based Lua module that is HAProxy specific when they can >> simply use a regular C based HAProxy module without corkscrewing it >> through Lua. Stuff such as cjson would be put into the system wide Lua >> path and thus be available to every Lua script including HAProxy, >> because it sits in the default path that is compiled into the Lua VM. >> - I put the type last, because I consider optional parameters that are >> in the middle to be unintuitive and it complicates parsing, because a >> single index might either be the type or the path if the type is not given. >> >> However I don't particularly care for any of this. If you feel like we >> should to lua-prepend-path and lua-prepend-cpath instead then I'm happy >> to adjust the patch (or you do it). > > Are you happy with my patch series as is or would you like to see > changes? Should I update the config syntax? This might have slipped through the cracks. Can you please take a look? If you don't have the time to look at it right now, just let me know. Best regards Tim Düsterhus
[no subject]
Hi ! I have a strange behaviour trying to use swagger generated SDK for the dataplane API (golang SDK). When I want to get the current configuration on `/services/haproxy/configuration/raw` calling `GetHAProxyConfiguration()` I have this error: ``` Configuration-Version in header must be of type int64: "" ``` --- Rémi Collignon-Ducret - OVHcloud
Re: PATCH] BUG/MINOR: ssl: ocsp_issuer must be set in the right way
On Thu, Jan 23, 2020 at 01:10:59PM +0100, Emmanuel Hocdet wrote: > > Following discussion from "[PATCH] BUG/MINOR: ssl: > ssl_sock_load_pem_into_ckch is not consistent ». > Thanks, merged. -- William Lallemand
Re: Haproxy loadbalancing out going mail to Antispam servers
Hello, On Wed, 22 Jan 2020 at 16:18, Brent Clark wrote: > > Good day Guys > > We have a project where we are trying to load balance to our outbound > Spamexperts Antispam relays / servers. > > We hit a snag where our clients servers are getting 'Too many concurrent > SMTP connections from this IP address'. As a result the mail queue is > building up on the servers. > > After reverting our change, the problem went away. > > Our setup is: > (CLIENT SERVERS INDC) ---> 587 (HAPROXY) ---> (ANTISPAM) ---> (INTERNET) > > While I am performance tuning and repoking under the hood etc, could I > ask if someone could please peer review my config / setup. > > https://pastebin.com/raw/3D8frtzw > > If someone from the community can help, it would be appreciated. Your antispam relays see thousands of connections from a single IP address - the haproxy server, which is why it blocks the IP address of haproxy. You should implement the PROXY protocol to let your antispam relays know what the real client IP is, otherwise you will be in a world of hurt *especially* since AntiSPAM is so reliant on correct source IP addresses for IP reputation and blacklists, etc. Lukas
Re: [PATCH] MEDIUM: cli: Allow multiple filter entries for show table
On Thu, Jan 23, 2020 at 12:24:19PM +0100, Tim Düsterhus wrote:
> Willy,
>
> Am 23.01.20 um 10:56 schrieb Willy Tarreau:
> >> I've using clang during the testing, and apparently -Wextra doesn't enable
> >> it on clang (I'm using v9.0), only -pedantic helps:
> >>
> >> clang -Wall -Wextra -Wpedantic test.c
> >>
> >> test.c:6:12: warning: ordered comparison between pointer and zero ('char *'
> >> and 'int') is an extension [-Wpedantic]
> >
> > Ah that's interesting, because from a semantic perspective there is no
> > reason to compare an int and a pointer, given that an int is a distance
> > between two pointers (divided by the element's size), so being able to
>
> Not to disgress too much, but isn't `ptrdiff_t` from `stddef.h` the
> distance between two pointers? :-)
Yes, it's just an alias for whatever integer applies to your platform
(i.e. int or long basically). Unless you're on Windows which is LLP64,
long is almost always OK.
Willy
PATCH] BUG/MINOR: ssl: ocsp_issuer must be set in the right way
Following discussion from "[PATCH] BUG/MINOR: ssl: ssl_sock_load_pem_into_ckch is not consistent ». 0001-BUG-MINOR-ssl-ocsp_issuer-must-be-set-in-the-right-w.patch Description: Binary data
Re: [PATCH] BUG/MINOR: ssl: ssl_sock_load_pem_into_ckch is not consistent
> Le 23 janv. 2020 à 11:19, William Lallemand a écrit : > > On Wed, Jan 22, 2020 at 05:22:51PM +0100, Emmanuel Hocdet wrote: >> >>> Le 22 janv. 2020 à 15:56, William Lallemand a >>> écrit : >>> >> Indeed, and the case of ckch->ocsp_issuer is also problematic. >> > > Right. > > I fixed this, thanks. > See, i think it would be cleaner to use ssl_sock_free_cert_ket_and_chain_contents. My concern for ckch->ocsp_issuer is also to set it correctly (patch is coming). Manu
Re: [PATCH] MEDIUM: cli: Allow multiple filter entries for show table
Willy,
Am 23.01.20 um 10:56 schrieb Willy Tarreau:
>> I've using clang during the testing, and apparently -Wextra doesn't enable
>> it on clang (I'm using v9.0), only -pedantic helps:
>>
>> clang -Wall -Wextra -Wpedantic test.c
>>
>> test.c:6:12: warning: ordered comparison between pointer and zero ('char *'
>> and 'int') is an extension [-Wpedantic]
>
> Ah that's interesting, because from a semantic perspective there is no
> reason to compare an int and a pointer, given that an int is a distance
> between two pointers (divided by the element's size), so being able to
Not to disgress too much, but isn't `ptrdiff_t` from `stddef.h` the
distance between two pointers? :-)
Of course comparing a pointer to a `ptrdiff_t` still doesn't make sense.
> do that implies that it is possible to add two pointers, which makes no
> sense:
>
> let i := p1 - p2
> for (i < p3) to be true, this would mean:
> p1 - p2 < p3
> p1 < p2 + p3:-)
>
> And going further we might even multiply pointers to create areas or
> volumes maybe! Compilers will never cease to amaze me! At least this
> explains the difference between what you saw and what I saw.
>
Best regards
Tim Düsterhus
Re: mcli vtest broken by commit.?. MEDIUM: connections: Get ride of the xprt_done callback.
Hi again,
On Wed, Jan 22, 2020 at 11:07:47PM +0100, Willy Tarreau wrote:
> Hi Pieter,
>
> On Wed, Jan 22, 2020 at 09:57:31PM +0100, PiBa-NL wrote:
> > Hi Olivier,
> >
> > Just to let you know, seems this commit has broken a few regtests:
> > http://git.haproxy.org/?p=haproxy.git;a=commit;h=477902bd2e8c1e978ad43d22dba1f28525bb797a
> >
> > https://api.cirrus-ci.com/v1/task/5885732300521472/logs/main.log
> > Testing with haproxy version: 2.2-dev1
> > #top TEST reg-tests/mcli/mcli_show_info.vtc TIMED OUT (kill -9)
> > #top TEST reg-tests/mcli/mcli_show_info.vtc FAILED (10.044) signal=9
> > #top TEST reg-tests/mcli/mcli_start_progs.vtc TIMED OUT (kill -9)
> > #top TEST reg-tests/mcli/mcli_start_progs.vtc FAILED (10.019) signal=9
> >
> > Can reproduce it on my own FreeBSD machine as well, the testcase just sits
> > and waits.. until the vtest-timeout strikes.
> >
> > Do you need more info? If so what can i provide.?
Just a heads up that I have a fix that addresses this. I'll just wait
for Olivier's approval to be sure I'm not breaking any assumption but
I think the solution is OK. The commit above indeed outlined that we're
still having dirty stuff. We're scratching the paint and discovering
some rust under it. I prefer to remove the rust than add new paint :-)
For those who need the fix now because it impacts their development,
I'm attaching it.
Cheers,
Willy
>From 0ae472722910d6ecc63666f950ac57b6cf102c6d Mon Sep 17 00:00:00 2001
From: Willy Tarreau
Date: Thu, 23 Jan 2020 09:11:58 +0100
Subject: BUG/MEDIUM: connection: remove CO_FL_CONNECTED and only rely on
CO_FL_WAIT_*
Commit 477902bd2e ("MEDIUM: connections: Get ride of the xprt_done
callback.") broke the master CLI for a very obscure reason. It happens
that short requests immediately terminated by a shutdown are properly
received, CS_FL_EOS is correctly set, but in si_cs_recv(), we refrain
from setting CF_SHUTR on the channel because CO_FL_CONNECTED is not
yet set on the connection since we've not passed again through
conn_fd_handler(). While dropping this test does fix the issue, the root
cause is deeper and actually comes down to the fact that CO_FL_CONNECTED
is lazily set at various check points in the code but not every time we
drop one wait bit. It is not the first time we face this situation.
Originally this flag was used to detect the transition between WAIT_*
and CONNECTED in order to call ->wake() from the FD handler. But since
at least 1.8-dev1 with commit 7bf3fa3c23 ("BUG/MAJOR: connection: update
CO_FL_CONNECTED before calling the data layer"), CO_FL_CONNECTED is
always synchronized against the two others before being checked. Moreover,
with the I/Os moved to tasklets, the decision to call the ->wake() function
is performed after the I/Os in si_cs_process() and equivalent, which don't
care about this transition either.
So in essence, checking for CO_FL_CONNECTED has become a lazy wait to
check for (CO_FL_WAIT_L4_CONN | CO_FL_WAIT_L6_CONN), but that always
relies on someone else having synchronized it.
This patch addresses it once for all by killing this flag and only checking
the two others (for which a composite mask CO_FL_WAIT_L4L6 was added). This
revealed a number of inconsistencies that were purposely not addressed here:
- most places do check both L4+L6 and HANDSHAKE at the same time, which
makes sense
- some places like assign_server() or back_handle_st_con() and a few
sample fetches looking for proxy protocol do check for L4+L6 but
don't care about HANDSHAKE ; these ones will probably fail on TCP
request session rules if the handshake is not complete.
- some handshake handlers do validate that a connection is established
at L4 but didn't clear CO_FL_WAIT_L4_CONN
- the ->ctl method of mux_fcgi, mux_pt and mux_h1 only checks for L4+L6
before declaring the mux ready while the snd_buf function also checks
for the handshake's completion. Likely the former should validate the
handshake as well and we should get rid of these extra tests in snd_buf.
- raw_sock_from_buf() would directly set CO_FL_CONNECTED and would only
later clear CO_FL_WAIT_L4_CONN.
- xprt_handshake would set CO_FL_CONNECTED itself without actually
clearing CO_FL_WAIT_L4_CONN, which could apparently happen only if
waiting for a pure Rx handshake.
- most places in ssl_sock that were checking CO_FL_CONNECTED don't need
to include the L4 check as an L6 check is enough to decide whether to
wait for more info or not.
It also becomes obvious when reading the test in si_cs_recv() that caused
the failure mentioned above that once converted it doesn't make any sense
anymore: having CS_FL_EOS set while still waiting for L4 and L6 to complete
cannot happen since for CS_FL_EOS to be set, the other ones must have been
validated.
Some of these parts will still deserve further cleanup, and some of the
observations above may induce some backports of potential bug fixes once
totally a
Re: arm64 builds?
On Thu, Jan 23, 2020 at 03:21:40PM +0500, ??? wrote: > also, can we treat warnings as errors for CI builds ? it would save a bunch > of time, instead of > looking at build log, we can watch for build status. Yes definitely. You just have to add "ERR=1" to your "make" command line and it will append -Werror. I do this on my builds as well. And I agree it will save some time because quite frankly, analysing why a regtest failed when we already have a build warning is pointless! Willy
Re: arm64 builds?
чт, 23 янв. 2020 г. в 15:14, Willy Tarreau : > On Thu, Jan 23, 2020 at 01:09:22PM +0500, ??? wrote: > > hello, > > > > I tried to build using cross compiler (arm32 on amd64). sorry for > > screenshot. > > Willy, do you mean errors like that ? > > So for those not seeing the screenshot, it says: > warning: argument 1 range [2147483648, 4294967295] exceeds maximum object > size 2147483647 : > > new->idle_orphan_conns = calloc((unsigned int)global.nbthread, > sizeof(*new->idle_orphan_conns)); > > Thus the cast to unsigned int was likely placed there to shut it up > once. Looking at the man page, it says: > >void *calloc(size_t nmemb, size_t size); > > size_t is unsigned (unless you're using an older gcc < 2.4 on a unusual > OS). I don't see how it can be said to have a maximum size of 2147483647. > > This is exactly the type of stupid warnings that causes us to add casts > that hide real bugs :-/ > > Could you please try to use (size_t) instead of (unsigned int) ? If it's > enough to shut it up, I'm fine with doing that change. Otherwise we'll > probably get rid of that stupid warning. > yep. I'll play with it a liitle bit, and hopefully we will add cross builds to Github Actions. also, can we treat warnings as errors for CI builds ? it would save a bunch of time, instead of looking at build log, we can watch for build status. > > Thanks! > Willy >
Re: [PATCH[ re-enable cygwin CI builds (Github Actions)
On Wed, Jan 22, 2020 at 11:10:05PM +0100, Tim Düsterhus wrote: > > both travis and github actions do offer 4 parallel builds, while cirrus and > > app veyor offer 1 parallel build. > > Parallel Builds just improve test speed. I don't consider that an > important selling point for us. The development process is fairly > asynchronous anyway and the important thing is that there are results > for more obscure configurations, not that there results within 1 minute. Sometimes it's nice to get a low-latency feedback on recent changes. But lately travis has become amazingly slow, turning from tens of seconds to several minutes, and regtests occasionally failing on timeouts, so it's clear that this low-latency argument is not true anymore. I think they're victim of their success and the spawned VMs are simply overloaded by too many users doing lots of builds there. We do try to be good citizens by limiting our combinations but it's possibly not the case for everyone. I still think we should not be too demanding on these tools. They do help us, 2.0 and 2.1's quality at the release date was far above anything we've done in the past, so I'm fine if we continue to use a diversity of tools depending on their capabilities. For sure it's less work to always look at a single place but that's no big deal overall. Willy
Re: [PATCH] BUG/MINOR: ssl: ssl_sock_load_pem_into_ckch is not consistent
On Wed, Jan 22, 2020 at 05:22:51PM +0100, Emmanuel Hocdet wrote: > > > Le 22 janv. 2020 à 15:56, William Lallemand a > > écrit : > > > Indeed, and the case of ckch->ocsp_issuer is also problematic. > Right. I fixed this, thanks. -- William Lallemand
Re: arm64 builds?
On Thu, Jan 23, 2020 at 01:09:22PM +0500, ??? wrote: > hello, > > I tried to build using cross compiler (arm32 on amd64). sorry for > screenshot. > Willy, do you mean errors like that ? So for those not seeing the screenshot, it says: warning: argument 1 range [2147483648, 4294967295] exceeds maximum object size 2147483647 : new->idle_orphan_conns = calloc((unsigned int)global.nbthread, sizeof(*new->idle_orphan_conns)); Thus the cast to unsigned int was likely placed there to shut it up once. Looking at the man page, it says: void *calloc(size_t nmemb, size_t size); size_t is unsigned (unless you're using an older gcc < 2.4 on a unusual OS). I don't see how it can be said to have a maximum size of 2147483647. This is exactly the type of stupid warnings that causes us to add casts that hide real bugs :-/ Could you please try to use (size_t) instead of (unsigned int) ? If it's enough to shut it up, I'm fine with doing that change. Otherwise we'll probably get rid of that stupid warning. Thanks! Willy
Re: Recommendations for deleting headers by regexp in 2.x?
Hi James, On Wed, Jan 22, 2020 at 04:19:41PM -0800, James Brown wrote: > We're upgrading from 1.8 to 2.x and one of the things I've noticed is that > reqidel and rspidel seem to be totally gone in 2.1... What's the new > recommendation to delete headers from request/response based on a regular > expression? Do I have to write a Lua action to do this now? I read through > the documentation for http-request and http-response and there doesn't seem > to be an `http-request del-header-by-regex`... > > Our use case is that we have dozens of different internal headers behind a > prefix, and we promise that we'll strip them all for incoming requests and > outgoing responses at the edge load balancer. That is harder to do if we > can't delete all headers matching a certain regex... That's an intereting use case, which I find totally legitimate and that we need to figure how to address. In 2.0 you can still rely on rspdel but we then need to have a solution for 2.2. Probably that in the short term using Lua will be the easiest solution. And maybe we'd need to add a new action such as "del-headers" which would take a regex or a prefix. By the way, are all your headers identified by the same prefix ? I'm asking because if that's the case, maybe we could append an optional argument to del-header to mention that we want to delete all those starting with this prefix and not just this exact one. Willy
Re: Haproxy loadbalancing out going mail to Antispam servers
Hi, On Wed, Jan 22, Brent Clark wrote: > We have a project where we are trying to load balance to our outbound > Spamexperts Antispam relays / servers. > > We hit a snag where our clients servers are getting 'Too many concurrent > SMTP connections from this IP address'. As a result the mail queue is > building up on the servers. What generates this error (the antispam servers ?) Your antispam servers probably see all connections coming from haproxy ip-address (and not the clients address). > After reverting our change, the problem went away. > > Our setup is: > (CLIENT SERVERS INDC) ---> 587 (HAPROXY) ---> (ANTISPAM) ---> (INTERNET) Do you control the antispam servers and do the antispam servers support for example proxy-protocol (postfix, exim etc) ? (https://cbonte.github.io/haproxy-dconv/2.0/configuration.html#5.2-send-proxy) -Jarno -- Jarno Huuskonen
Re: [PATCH] MEDIUM: cli: Allow multiple filter entries for show table
Hi Adis,
On Thu, Jan 23, 2020 at 10:40:12AM +0100, Adis Nezirovic wrote:
> On 1/22/20 6:17 PM, Willy Tarreau wrote:
> > Strangely, while I was certain I had build-tested the original one,
> > apparently
> > I failed as it didn't build for two errors, which I fixed in a subsequent
> > patch.
> > I'm seeing that your patches still seem to rely on this bug (data_op < 0)
> > which
> > normally cannot compile so I find this surprising.
>
> I've using clang during the testing, and apparently -Wextra doesn't enable
> it on clang (I'm using v9.0), only -pedantic helps:
>
> clang -Wall -Wextra -Wpedantic test.c
>
> test.c:6:12: warning: ordered comparison between pointer and zero ('char *'
> and 'int') is an extension [-Wpedantic]
Ah that's interesting, because from a semantic perspective there is no
reason to compare an int and a pointer, given that an int is a distance
between two pointers (divided by the element's size), so being able to
do that implies that it is possible to add two pointers, which makes no
sense:
let i := p1 - p2
for (i < p3) to be true, this would mean:
p1 - p2 < p3
p1 < p2 + p3:-)
And going further we might even multiply pointers to create areas or
volumes maybe! Compilers will never cease to amaze me! At least this
explains the difference between what you saw and what I saw.
> Sorry about that, after being away from C for a while, it's hard to switch
> on some brain, instead of letting the compiler to catch stupid bugs like
> this one.
Hey don't be sorry, we all do bugs and the compiler cannot catch them all
(and I'd actually like that it tries less so that it doesn't report false
positives all the time that require to add dangerous code to shut it up).
My comment was really about pulling the alarm trigger in case there was
something wrong in your patchset that you didn't notice.
> > I still merged the first one of these two because I think it's OK, however,
> > could you please add a bit of commit message to the second one ? As a rule
> > of thumb, the commit message should be used to "sell" me your patch and to
> > sell it to whoever would be hesitating in backporting it or not. Thus I
> > think
> > that here the benefits and/or impacts are not obvious from this one-liner :
>
> Here is me doing my best, selling you the good stuff, and the reason to
> backport the whole feature (so we can detect the multi-filter from CLI, and
> not rely on haproxy version or similar), see the attached patch.
:-)
Pretty nice, now merged.
Thank you!
Willy
Re: SameSite attribute for persistent session cookie
Le 22/01/2020 à 09:45, mickael.br...@orange.com a écrit : Anyway thank you for the commit in the development version. Do you think you will also backport in 1.X version? I don't know how HAProxy releases are planned. Any idea of a planning for releases (2.X and 1.X if relevant)? Hi, I backported the feature as far as 1.8. And for now, there is no release planned. Regards, -- Christopher Faulet
Re: [PATCH] MEDIUM: cli: Allow multiple filter entries for show table
On 1/22/20 6:17 PM, Willy Tarreau wrote:
Strangely, while I was certain I had build-tested the original one, apparently
I failed as it didn't build for two errors, which I fixed in a subsequent patch.
I'm seeing that your patches still seem to rely on this bug (data_op < 0) which
normally cannot compile so I find this surprising.
I've using clang during the testing, and apparently -Wextra doesn't
enable it on clang (I'm using v9.0), only -pedantic helps:
clang -Wall -Wextra -Wpedantic test.c
test.c:6:12: warning: ordered comparison between pointer and zero ('char
*' and 'int') is an extension [-Wpedantic]
Sorry about that, after being away from C for a while, it's hard to
switch on some brain, instead of letting the compiler to catch stupid
bugs like this one.
I still merged the first one of these two because I think it's OK, however,
could you please add a bit of commit message to the second one ? As a rule
of thumb, the commit message should be used to "sell" me your patch and to
sell it to whoever would be hesitating in backporting it or not. Thus I think
that here the benefits and/or impacts are not obvious from this one-liner :
Here is me doing my best, selling you the good stuff, and the reason to
backport the whole feature (so we can detect the multi-filter from CLI,
and not rely on haproxy version or similar), see the attached patch.
Best regards,
Adis
--
Adis Nezirovic
Software Engineer
HAProxy Technologies - Powering your uptime!
375 Totten Pond Road, Suite 302 | Waltham, MA 02451, US
+1 (844) 222-4340 | https://www.haproxy.com
>From 8437fd4ac1a9cc5a9d88daad3a23e0d2a2eaeb4f Mon Sep 17 00:00:00 2001
From: Adis Nezirovic
Date: Wed, 22 Jan 2020 16:50:27 +0100
Subject: [PATCH] MINOR: cli: Report location of errors or any extra data for
"show table"
When using multiple filters with "show table", it can be useful to
report which filter entry failed
> show table MY_TABLE data.gpc0 gt 0 data.gpc0a lt 1000
Filter entry #2: Unknown data type
> show table MY_TABLE data.gpc0 gt 0 data.gpc0 lt 1000a
Filter entry #2: Require a valid integer value to compare against
We now also catch garbage data after the filter
> show table MY_TABLE data.gpc0 gt 0 data.gpc0 lt 1000 data.gpc0 gt 1\
data.gpc0 gt 10 a
Detected extra data in filter, 16th word of input, after '10'
Even before multi-filter feature we've also silently accepted garbage
after the input, hiding potential bugs
> show table MY_TABLE data.gpc0 gt 0 data.gpc0
or
> show table MY_TABLE data.gpc0 gt 0 a
In both cases, only first filter entry would be used, silently ignoring
extra filter entry or garbage data.
Last, but not the least, it is now possible to detect multi-filter
feature from cli with something like the following:
> show table MY_TABLE data.blah
Filter entry #1: Unknown data type
---
src/stick_table.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/stick_table.c b/src/stick_table.c
index 1b397e59e..1e7d4f3a8 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -3601,6 +3601,7 @@ static int table_process_entry_per_key(struct appctx *appctx, char **args)
static int table_prepare_data_request(struct appctx *appctx, char **args)
{
int i;
+ char *err = NULL;
if (appctx->ctx.table.action != STK_CLI_ACT_SHOW && appctx->ctx.table.action != STK_CLI_ACT_CLR)
return cli_err(appctx, "content-based lookup is only supported with the \"show\" and \"clear\" actions\n");
@@ -3611,17 +3612,21 @@ static int table_prepare_data_request(struct appctx *appctx, char **args)
/* condition on stored data value */
appctx->ctx.table.data_type[i] = stktable_get_data_type(args[3+3*i] + 5);
if (appctx->ctx.table.data_type[i] < 0)
- return cli_err(appctx, "Unknown data type\n");
+ return cli_dynerr(appctx, memprintf(&err, "Filter entry #%i: Unknown data type\n", i + 1));
if (!((struct stktable *)appctx->ctx.table.target)->data_ofs[appctx->ctx.table.data_type[i]])
- return cli_err(appctx, "Data type not stored in this table\n");
+ return cli_dynerr(appctx, memprintf(&err, "Filter entry #%i: Data type not stored in this table\n", i + 1));
appctx->ctx.table.data_op[i] = get_std_op(args[4+3*i]);
if (appctx->ctx.table.data_op[i] < 0)
- return cli_err(appctx, "Require and operator among \"eq\", \"ne\", \"le\", \"ge\", \"lt\", \"gt\"\n");
+ return cli_dynerr(appctx, memprintf(&err, "Filter entry #%i: Require and operator among \"eq\", \"ne\", \"le\", \"ge\", \"lt\", \"gt\"\n", i + 1));
if (!*args[5+3*i] || strl2llrc(args[5+3*i], strlen(args[5+3*i]), &appctx->ctx.table.value[i]) != 0)
- return cli_err(appctx, "Require a valid integer value to compare against\n");
+ return cli_dynerr(appctx, memprintf(&err, "Filter entry #%i: Require a valid integer value to compare against\n", i + 1));
+ }
+
+ if (*args[3+3*i]) {
+ return cli_dynerr(appctx, memprintf(&err, "Detected extra data in filter, %ith word of input, after '%s'\n", 3+3*i + 1
Re: arm64 builds?
hello, I tried to build using cross compiler (arm32 on amd64). sorry for screenshot. Willy, do you mean errors like that ? [image: Screenshot from 2020-01-23 13-03-49.png] ср, 6 нояб. 2019 г. в 12:26, Willy Tarreau : > Hi Ilya, > > On Tue, Nov 05, 2019 at 07:20:43PM +0500, ??? wrote: > > only arm64 are available. > > we can try arm using cross build, for example. > > Then don't bother with this, it's likely then that they will not > have all the environment available. Maybe even their hardware does > not support arm32 at all. It was just a suggestion to try to optimize > the solution but even arm64 is already nice to have. > > > is it common way to use cross build for building haproxy ? > > Yes it is. Actually I don't think I've built it natively for a very > long time now, as even on my laptop I'm used to stick to the cross- > compilers which are distributed on the distcc build farm running on > the lab load generators :-) > > In practice just pass "CC=/path/to/gcc" and let it do its job. It will > automatically use LD=$(CC) if you don't override it. You may have to > pass PCRE_INC/PCRE_LIB, SSL_INC/SSL_LIB, LUA_INC/LUA_LIB but that's > about all. > > Just for reference here's the command line I'm using when building > (and a variant with -O0 which builds in 3.5 seconds). It may look > large because I force all debugging options but it's in a script so > I don't have to type it :-) > >PATH=/f/tc/x86_64-gcc47_glibc218-linux-gnu/bin:$PATH make -j 120 > TMPDIR=/dev/shm DISTCC_FALLBACK=0 DISTCC_SKIP_LOCAL_RETRY=0 > DISTCC_BACKOFF_PERIOD=0 DISTCC_PAUSE_TIME_MSEC=50 > DISTCC_HOSTS="--localslots_cpp=120 10.0.0.235/120,lzo" > CC=/g/public/linux/master/x86_64-gcc47_glibc218-linux-gnu-gcc > TARGET=linux-glibc DEP= USE_PCRE=1 PCREDIR= > DEFINE="-DDEBUG_DONT_SHARE_POOLS -DDEBUG_MEMORY_POOLS -DDEBUG_UAF > -DDEBUG_EXPR -DDEBUG_STRICT -DDEBUG_DEV" USE_OPENSSL=1 USE_ZLIB=1 USE_LUA=1 > LUA_LIB_NAME=lua EXTRA= USE_SLZ=1 SLZ_INC=/g/public/slz/src > SLZ_LIB=/g/public/slz USE_ZLIB= USE_DEVICEATLAS=1 > DEVICEATLAS_SRC=contrib/deviceatlas USE_51DEGREES=1 > 51DEGREES_SRC=contrib/51d/src/trie USE_WURFL=1 WURFL_INC=contrib/wurfl > WURFL_LIB=contrib/wurfl CPU_CFLAGS="-O2" "$@" > > When testing with various local openssl branches I do have another variant > of this which uses the local, native pre-processor and linkers, and remote > compilers. It's a bit ugly since they're not on the same version but in > practice it works fine. > > Cheers, > Willy >
