Re: Minor - patch 1.6.x - Fix some warnings in Connection.c

[Willy Tarreau]

Hi Jonathan,

On Tue, May 24, 2016 at 05:04:16AM -0500, Jonathan Fisher wrote:
> What's the style you prefer?

This one without the double negation :-)

-   if (!memcmp(line, "TCP4 ", 5) != 0) {
+   if (memcmp(line, "TCP4 ", 5) == 0) {

I've just backported it now.

Cheers,
Willy

unsubscribe

[Nathan Williams]

Re: [PATCH] BUG/MEDIUM: dns: unbreak DNS resolver after header fix

[Willy Tarreau]

Hi Vincent,

On Wed, May 25, 2016 at 10:54:14PM +0200, Vincent Bernat wrote:
>  ??? 25 mai 2016 22:15 +0200, Lukas Tribus  :
> 
> > DNS requests (using the internal resolver) are corrupted since commit
> > e2f84977165a ("BUG/MINOR: dns: fix DNS header definition").
> >
> > Fix it by defining the struct in network byte order, while complying
> > with RFC 2535, section 6.1.
> 
> So sorry about that. It's quite odd since I tested the DNS resolution,
> but maybe not in the right way.
> 
> The RFC says:
> 
>1  1  1  1  1  1
>  0  1  2  3  4  5  6  7  8  9  0  1  2  3  4  5
> +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
> |  ID   |
> +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
> |QR|   Opcode  |AA|TC|RD|RA| Z|AD|CD|   RCODE   |
> +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
> |QDCOUNT|
> +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
> |ANCOUNT|
> +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
> |NSCOUNT|
> +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
> |ARCOUNT|
> +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
> 
> So, it's not about network byte order or not, this is just about bit
> numbering and I was so bad at it. I agree with your patch.

Great, thanks for the quicky verification!

I'm taking it as-is then.

Cheers,
Willy

Re: [PATCH] BUG/MEDIUM: dns: unbreak DNS resolver after header fix

[Willy Tarreau]

Hi Lukas,

On Wed, May 25, 2016 at 10:15:11PM +0200, Lukas Tribus wrote:
> DNS requests (using the internal resolver) are corrupted since commit
> e2f84977165a ("BUG/MINOR: dns: fix DNS header definition").
> 
> Fix it by defining the struct in network byte order, while complying
> with RFC 2535, section 6.1.

So if I apply it and diff with the code prior to the commit above, I get
this :

diff --git a/include/types/dns.h b/include/types/dns.h
index 757eaaf..50636fd 100644
--- a/include/types/dns.h
+++ b/include/types/dns.h
@@ -69,9 +69,9 @@ struct dns_header {
unsigned char   opcode :4;  /* operation code */
unsigned char   qr :1;  /* query/response 0: query, 1: response 
*/
unsigned char   rcode :4;   /* response code */
-   unsigned char   z :1;   /* no used */
-   unsigned char   ad :1;  /* authentic data */
unsigned char   cd :1;  /* checking disabled */
+   unsigned char   ad :1;  /* authentic data */
+   unsigned char   z :1;   /* not used */
unsigned char   ra :1;  /* recursion available 0: no, 1: yes */
unsigned short  qdcount :16;/* question count */
unsigned short  ancount :16;/* answer count */

Does everyone agree that it's indeed the expected fix for the original
issue ? I prefer to ask since this patch almost undoes Vincent's initial
patch, and that both commit messages cite the RFC to justify being right
but neither says exactly what field are causing problem.

Thanks guys,
Willy

[PATCH] BUG/MEDIUM: dns: unbreak DNS resolver after header fix

[Lukas Tribus]

DNS requests (using the internal resolver) are corrupted since commit
e2f84977165a ("BUG/MINOR: dns: fix DNS header definition").

Fix it by defining the struct in network byte order, while complying
with RFC 2535, section 6.1.

First reported by Eduard Vopicka on discourse.

This must be backported to 1.6 (1.6.5 is affected).
---
 include/types/dns.h | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/types/dns.h b/include/types/dns.h
index 98adc98..50636fd 100644
--- a/include/types/dns.h
+++ b/include/types/dns.h
@@ -63,16 +63,16 @@
 /* DNS request or response header structure */
 struct dns_header {
unsigned short  id:16;  /* identifier */
-   unsigned char   qr :1;  /* query/response 0: query, 1: response 
*/
-   unsigned char   opcode :4;  /* operation code */
-   unsigned char   aa :1;  /* authoritative answer 0: no, 1: yes */
-   unsigned char   tc :1;  /* truncation 0:no, 1: yes */
unsigned char   rd :1;  /* recursion desired 0: no, 1: yes */
-   unsigned char   ra :1;  /* recursion available 0: no, 1: yes */
-   unsigned char   z :1;   /* not used */
-   unsigned char   ad :1;  /* authentic data */
-   unsigned char   cd :1;  /* checking disabled */
+   unsigned char   tc :1;  /* truncation 0:no, 1: yes */
+   unsigned char   aa :1;  /* authoritative answer 0: no, 1: yes */
+   unsigned char   opcode :4;  /* operation code */
+   unsigned char   qr :1;  /* query/response 0: query, 1: response 
*/
unsigned char   rcode :4;   /* response code */
+   unsigned char   cd :1;  /* checking disabled */
+   unsigned char   ad :1;  /* authentic data */
+   unsigned char   z :1;   /* not used */
+   unsigned char   ra :1;  /* recursion available 0: no, 1: yes */
unsigned short  qdcount :16;/* question count */
unsigned short  ancount :16;/* answer count */
unsigned short  nscount :16;/* authority count */
-- 
1.9.1

Re: Bug when loading multiple configuration files

[Ben Cabot]

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

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

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

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

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

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

Available filters :
[TRACE] trace
[COMP] compression


Ben

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



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