Re: Haproxy 1.6.5 listens on all IPv4 addresses

[Cyril Bonté]

Hi Vincent,

Le 14/05/2016 19:08, Vincent Bernat a écrit :

  ❦ 14 mai 2016 15:20 +0200, Cyril Bonté  :


What is the most important is to report this to the gcc maintainers so that
they can fix the bug. The fix will naturally flow into the distros.



I understand this and of course I could try to fill a bug on their side but
the gcc stuff is a bit out of my league.


After spending some time on this, I'm now able to produce a minimal
test case.

$ gcc-6 -O2 -o debug debug.c && ./debug
0
=> here we have an issue, as it shouldn't be 0

$ gcc-6 -O2 -DDEBUG -o debug debug.c && ./debug
7f01
7f01
=> Suddenly, everything works

And now, what happens with this debug code when -fno-tree-sra is used ?
$ gcc-6 -O2 -fno-tree-sra -DDEBUG -o debug debug.c && ./debug
7f01
0
=> It still doesn't work :-( So, disabling "tree-sra" doesn't
guarantee the right behaviour.

I also attach a debug2.c example, which is also disturbing. It is the
same code, but adds a local variable. Depending on the code order,
strange things happen (looks like a memory alignment issue).
For example :
$ gcc-6 -O2 -o debug2 debug2.c && ./debug2
0
fd7f

Note : my tests were made on a Debian SID with the gcc-6 package.


I think this is an aliasing problem. You cannot have two incompatible
variables pointing at the same memory spot. It seems that now
sockaddr_storage and sockaddr_in are not compatible anymore.

#v+
struct sockaddr_storage
   {
 __SOCKADDR_COMMON (ss_);/* Address family, etc.  */
 __ss_aligntype __ss_align;  /* Force desired alignment.  */
 char __ss_padding[_SS_PADSIZE];
   };

struct sockaddr_in
   {
 __SOCKADDR_COMMON (sin_);
 in_port_t sin_port; /* Port number.  */
 struct in_addr sin_addr;/* Internet address.  */

 /* Pad to size of `struct sockaddr'.  */
 unsigned char sin_zero[sizeof (struct sockaddr) -
__SOCKADDR_COMMON_SIZE -
sizeof (in_port_t) -
sizeof (struct in_addr)];
   };
#v-

If I introduce this type:

#v+
struct  sockaddr_storage_universal {
union {
struct sockaddr_storage sas;
struct sockaddr_in sai;
struct sockaddr_in6 sai6;
};
};
#v-

This works when used in place of "struct sockaddr_storage". I see the
glibc is using an union too instead of struct sockaddr or struct
sockaddr_storage. man 7 socket still says to use struct
sockaddr_storage.

Searching a bit, there is this old question on StackOverflow:
  
http://stackoverflow.com/questions/1429645/how-to-cast-sockaddr-storage-and-avoid-breaking-strict-aliasing-rules


Yes, that's where I stopped too after extracting the test case from the 
whole code.



--
Cyril Bonté

Re: Haproxy 1.6.5 listens on all IPv4 addresses

[Vincent Bernat]

 ❦ 14 mai 2016 15:20 +0200, Cyril Bonté  :

>>> What is the most important is to report this to the gcc maintainers so that
>>> they can fix the bug. The fix will naturally flow into the distros.
>>>
>>
>> I understand this and of course I could try to fill a bug on their side but
>> the gcc stuff is a bit out of my league.
>
> After spending some time on this, I'm now able to produce a minimal
> test case.
>
> $ gcc-6 -O2 -o debug debug.c && ./debug
> 0
> => here we have an issue, as it shouldn't be 0
>
> $ gcc-6 -O2 -DDEBUG -o debug debug.c && ./debug
> 7f01
> 7f01
> => Suddenly, everything works
>
> And now, what happens with this debug code when -fno-tree-sra is used ?
> $ gcc-6 -O2 -fno-tree-sra -DDEBUG -o debug debug.c && ./debug
> 7f01
> 0
> => It still doesn't work :-( So, disabling "tree-sra" doesn't
> guarantee the right behaviour.
>
> I also attach a debug2.c example, which is also disturbing. It is the
> same code, but adds a local variable. Depending on the code order,
> strange things happen (looks like a memory alignment issue).
> For example :
> $ gcc-6 -O2 -o debug2 debug2.c && ./debug2
> 0
> fd7f
>
> Note : my tests were made on a Debian SID with the gcc-6 package.

I think this is an aliasing problem. You cannot have two incompatible
variables pointing at the same memory spot. It seems that now
sockaddr_storage and sockaddr_in are not compatible anymore.

#v+
struct sockaddr_storage
  {
__SOCKADDR_COMMON (ss_);/* Address family, etc.  */
__ss_aligntype __ss_align;  /* Force desired alignment.  */
char __ss_padding[_SS_PADSIZE];
  };

struct sockaddr_in
  {
__SOCKADDR_COMMON (sin_);
in_port_t sin_port; /* Port number.  */
struct in_addr sin_addr;/* Internet address.  */

/* Pad to size of `struct sockaddr'.  */
unsigned char sin_zero[sizeof (struct sockaddr) -
   __SOCKADDR_COMMON_SIZE -
   sizeof (in_port_t) -
   sizeof (struct in_addr)];
  };
#v-

If I introduce this type:

#v+
struct  sockaddr_storage_universal {
union {
struct sockaddr_storage sas;
struct sockaddr_in sai;
struct sockaddr_in6 sai6;
};
};
#v-

This works when used in place of "struct sockaddr_storage". I see the
glibc is using an union too instead of struct sockaddr or struct
sockaddr_storage. man 7 socket still says to use struct
sockaddr_storage.

Searching a bit, there is this old question on StackOverflow:
 
http://stackoverflow.com/questions/1429645/how-to-cast-sockaddr-storage-and-avoid-breaking-strict-aliasing-rules
-- 
I think we are in Rats' Alley where the dead men lost their bones.
-- T.S. Eliot

Re: Haproxy 1.6.5 listens on all IPv4 addresses

[Cyril Bonté]

Hi all,

Le 14/05/2016 14:06, Arthur Țițeică a écrit :

În ziua de sâmbătă, 14 mai 2016, la 13:57:35 EEST, Willy Tarreau a scris:

What is the most important is to report this to the gcc maintainers so that
they can fix the bug. The fix will naturally flow into the distros.



I understand this and of course I could try to fill a bug on their side but
the gcc stuff is a bit out of my league.


After spending some time on this, I'm now able to produce a minimal test 
case.


$ gcc-6 -O2 -o debug debug.c && ./debug
0
=> here we have an issue, as it shouldn't be 0

$ gcc-6 -O2 -DDEBUG -o debug debug.c && ./debug
7f01
7f01
=> Suddenly, everything works

And now, what happens with this debug code when -fno-tree-sra is used ?
$ gcc-6 -O2 -fno-tree-sra -DDEBUG -o debug debug.c && ./debug
7f01
0
=> It still doesn't work :-( So, disabling "tree-sra" doesn't guarantee 
the right behaviour.


I also attach a debug2.c example, which is also disturbing. It is the 
same code, but adds a local variable. Depending on the code order, 
strange things happen (looks like a memory alignment issue).

For example :
$ gcc-6 -O2 -o debug2 debug2.c && ./debug2
0
fd7f

Note : my tests were made on a Debian SID with the gcc-6 package.

--
Cyril Bonté
#include 
#include 
#include 
#include 
#include 

struct dbg_listener {
	struct sockaddr_storage addr;
};

int main(int argc, char **argv)
{
	struct dbg_listener *l;
	struct sockaddr_storage ss, *ss2, ss3;
	
	memset(, 0, sizeof(ss3));
	ss3.ss_family = AF_INET;
	((struct sockaddr_in *))->sin_addr.s_addr = inet_addr("127.0.0.1");

	ss2 = 

	ss = *ss2;

	l = calloc(1, sizeof(*l));
	l->addr = ss;

#ifdef DEBUG
	printf("%x\n", ntohl(((struct sockaddr_in *)())->sin_addr.s_addr));
#endif
	printf("%x\n", ntohl(((struct sockaddr_in *)(>addr))->sin_addr.s_addr));
	exit(0);
}
#include 
#include 
#include 
#include 
#include 

struct dbg_listener {
	struct sockaddr_storage addr;
};

int main(int argc, char **argv)
{
	struct dbg_listener *l;
	struct sockaddr_storage ss, *ss2, ss3;
	struct sockaddr_storage addr;
	
	memset(, 0, sizeof(ss3));
	ss3.ss_family = AF_INET;
	((struct sockaddr_in *))->sin_addr.s_addr = inet_addr("127.0.0.1");

	ss2 = 

	ss = *ss2;

addr = ss;

	l = calloc(1, sizeof(*l));
	l->addr = ss;

#ifdef DEBUG
	printf("%x\n", ntohl(((struct sockaddr_in *)())->sin_addr.s_addr));
#endif
	printf("%x\n", ntohl(((struct sockaddr_in *)(>addr))->sin_addr.s_addr));
	printf("%x\n", ntohl(((struct sockaddr_in *)())->sin_addr.s_addr));
	exit(0);
}

httpchk with: http-send-name-header & related...

[Mehdi Ahmadi]

When specifying:
```
option httpchk
```
As default or specific to a back-end - other properties are not passed or
set as part of the health check request.

For example:
- http-send-name-header
- forwardfor
Are not preserved or included in the health-check request.

At present it is possible to simply include required headers manually such
as:
```
backend TEST
 option httpchk HEAD / HTTP/1.1\r\nHost: fqnd.tld.org
 http-check expect status 200
```
However in the case of different / differing ID by server that'd correspond
to the expected Host value to be passed with each request - one would need
to result to multiple backends with varying checks or have multiple checks
per server in each back-end which may not be possible?

IMO it would be beneficial to include all related headers and options with
health checks by default with an added flag or property to disable this
behavior reverting back to what is the current behavior.

Is this a rational request & can it be anticipated in a future release?

Thanks to everyone for their continued input & contributions.

Cheers,.
Mehdi -

Re: Haproxy 1.6.5 listens on all IPv4 addresses

[Arthur Țițeică]

În ziua de sâmbătă, 14 mai 2016, la 13:57:35 EEST, Willy Tarreau a scris:
> On Sat, May 14, 2016 at 02:36:39PM +0300, Arthur ??i??eic?? wrote:
> > În ziua de vineri, 13 mai 2016, la 23:47:07 EEST, Willy Tarreau a scris:
> > > On Fri, May 13, 2016 at 11:25:28PM +0200, Cyril Bonté wrote:
> > > > > In the mean time, there may be a -fsomething option to disable a
> > > > > bogus optimization which causes this, but that's not easy to spot
> > > > > as it will depend on any side effect of other code :-/
> > > > 
> > > > I was trying to identify one, and indeed, once I add "-fno-tree-sra",
> > > > it
> > > > works as expected.
> > > 
> > > This one already exists in gcc 4 and 5, so its implementation might be
> > > bogus> > 
> > > in 6. The doc says :
> > >-ftree-sra
> > >
> > >Perform scalar replacement of aggregates.  This pass replaces
> > >structure references with scalars to prevent committing
> > > 
> > > structures to memory too early.  This flag is enabled by default at -O
> > > and
> > > higher.
> > > 
> > > Thus I suspect that by delaying memory references a bit too much they
> > > end
> > > up completely forgetting to commit the changes :-/
> > 
> > Thank you everyone for the effort to diagnose this.
> > 
> > I've also managed to build 1.6.5 with "-fno-tree-sra" and I confirm that
> > all seems to be working as expected.
> > 
> > I'll pass the info in this thread to the Arch Linux maintainers.
> > I don't think they'll go back to gcc 5 but so far the additional flag
> > seems
> > like the better option.
> 
> What is the most important is to report this to the gcc maintainers so that
> they can fix the bug. The fix will naturally flow into the distros.
> 

I understand this and of course I could try to fill a bug on their side but 
the gcc stuff is a bit out of my league.

Re: Haproxy 1.6.5 listens on all IPv4 addresses

[Willy Tarreau]

On Sat, May 14, 2016 at 02:36:39PM +0300, Arthur ??i??eic?? wrote:
> În ziua de vineri, 13 mai 2016, la 23:47:07 EEST, Willy Tarreau a scris:
> > On Fri, May 13, 2016 at 11:25:28PM +0200, Cyril Bonté wrote:
> > > > In the mean time, there may be a -fsomething option to disable a
> > > > bogus optimization which causes this, but that's not easy to spot
> > > > as it will depend on any side effect of other code :-/
> > > 
> > > I was trying to identify one, and indeed, once I add "-fno-tree-sra", it
> > > works as expected.
> > 
> > This one already exists in gcc 4 and 5, so its implementation might be bogus
> > in 6. The doc says :
> > 
> >-ftree-sra
> >Perform scalar replacement of aggregates.  This pass replaces
> >structure references with scalars to prevent committing
> > structures to memory too early.  This flag is enabled by default at -O and
> > higher.
> > 
> > Thus I suspect that by delaying memory references a bit too much they end
> > up completely forgetting to commit the changes :-/
> > 
> 
> Thank you everyone for the effort to diagnose this.
> 
> I've also managed to build 1.6.5 with "-fno-tree-sra" and I confirm that all 
> seems to be working as expected.
> 
> I'll pass the info in this thread to the Arch Linux maintainers.
> I don't think they'll go back to gcc 5 but so far the additional flag seems 
> like the better option.

What is the most important is to report this to the gcc maintainers so that
they can fix the bug. The fix will naturally flow into the distros.

Willy

Re: Haproxy 1.6.5 listens on all IPv4 addresses

[Arthur Țițeică]

În ziua de vineri, 13 mai 2016, la 23:47:07 EEST, Willy Tarreau a scris:
> On Fri, May 13, 2016 at 11:25:28PM +0200, Cyril Bonté wrote:
> > > In the mean time, there may be a -fsomething option to disable a
> > > bogus optimization which causes this, but that's not easy to spot
> > > as it will depend on any side effect of other code :-/
> > 
> > I was trying to identify one, and indeed, once I add "-fno-tree-sra", it
> > works as expected.
> 
> This one already exists in gcc 4 and 5, so its implementation might be bogus
> in 6. The doc says :
> 
>-ftree-sra
>Perform scalar replacement of aggregates.  This pass replaces
>structure references with scalars to prevent committing
> structures to memory too early.  This flag is enabled by default at -O and
> higher.
> 
> Thus I suspect that by delaying memory references a bit too much they end
> up completely forgetting to commit the changes :-/
> 

Thank you everyone for the effort to diagnose this.

I've also managed to build 1.6.5 with "-fno-tree-sra" and I confirm that all 
seems to be working as expected.

I'll pass the info in this thread to the Arch Linux maintainers.
I don't think they'll go back to gcc 5 but so far the additional flag seems 
like the better option.

Regards