Re: Haproxy 1.7 and Ipv6-only hosts
Hi Maxim, On Tue, Jan 10, 2017 at 03:42:44PM +0300, ?? wrote: > Hi Willy, Baptiste! > > Patch is working for me. Thank you very much for help! Excellent, thanks to you for the feedback. I'll merge it and backport it and I should be able to emit 1.7.2 soon now. cheers, Willy
Re: Haproxy 1.7 and Ipv6-only hosts
Hi Willy, Baptiste! Patch is working for me. Thank you very much for help! 2017-01-06 21:53 GMT+03:00 Willy Tarreau: > Hi Baptiste, Maxim, > > On Wed, Dec 28, 2016 at 02:04:44PM +0100, Baptiste wrote: > > On Fri, Dec 23, 2016 at 5:21 PM, Willy Tarreau wrote: > > > Regarding this issue, I think that in fact we should decide to split > the > > > server port apart from the address. After all, we're manipulating the > port > > > and the address separately everywhere, we even have some extra > settings in > > > other places (eg the fact that the ports are relative). I have not yet > > > analysed the impact of all of this but I think that's definitely > something > > > we need to consider for the mid term. It will also remove most of the > > > "switch (family)" needed to retrieve/manipulate ports. > (...) > > I tend to fully agree if you mean having a "service_port" parameter in > the > > "struct server" (or some kind of) > > Each time we have to manipulate addr and/or port in the struct > > sockaddr_storage, it's a nightmare and we have to think about all the > > corner cases... > > OK so I managed to do it and I think I got it working fine now. It took > some time because in order not to miss any place I renamed the struct > member so that I was sure to spot all places. > > I found a small bug in the current DNS resolution implementation when the > family is set to AF_UNSPEC, it immediately marks it as invalid and tries > again, so my DNS server got flooded during my tests :-) But that's fixed > now. > > Thus now str2sa_range() doesn't change the family if the address doesn't > resolved. It however continues to *also* copy the port into the address > in the case where it resolves so that we don't have to touch all other > call places (listeners, peers, source, etc). I could get this config to > resolve all addresses as expected : > > resolvers mydns > nameserver dns1 8.8.8.8:53 > resolve_retries 3 > timeout retry 1s > hold valid 1s > > defaults > option httplog > log 127.0.0.1:5514 local0 > modehttp > timeout connect 5s > timeout client 60s > timeout server 90s > > frontend f > bind *: > #bind ::: > > backend b > # default-server resolvers mydns ## doesn't work > server s1 127.0.0.1:8000 check > server s2 127.0.0.2:8000 check disabled > server s3 wtap.haproxy.local:8000 init-addr none check > resolvers mydns > server s4 haproxy.ipv6.1wt.eu:80 init-addr none check > resolvers mydns > server s5 www6.1wt.eu:80 > server s6 i...@www6.1wt.eu:80 > server s7 i...@www6.1wt.eu:80 init-addr libc > server s8 1wt.eu:80 resolve-prefer ipv6 check init-addr none > resolvers mydns > server s9 1wt.eu:80 resolve-prefer ipv4 check > > By the way I found that "resolvers" doesn't work in default-server AND > doesn't emit any warning so I had to specify it on each line. We need to > fix this before issuing 1.7.2. > > I'd like you guys to take a look at the attached patches (rebased on 1.7). > There's also an all-in-one patch that you can apply to latest 1.7 snapshot > Maxim if you're interested. > > For me it passes all tests and seems to be OK now. > > Barring any objection, I'll merge all of them into 1.7 and will make a note > for distro maintainers to properly pick them or they'll get annoying bug > reports. > > Thanks, > Willy >
Re: Haproxy 1.7 and Ipv6-only hosts
Hi Baptiste, Maxim, On Wed, Dec 28, 2016 at 02:04:44PM +0100, Baptiste wrote: > On Fri, Dec 23, 2016 at 5:21 PM, Willy Tarreauwrote: > > Regarding this issue, I think that in fact we should decide to split the > > server port apart from the address. After all, we're manipulating the port > > and the address separately everywhere, we even have some extra settings in > > other places (eg the fact that the ports are relative). I have not yet > > analysed the impact of all of this but I think that's definitely something > > we need to consider for the mid term. It will also remove most of the > > "switch (family)" needed to retrieve/manipulate ports. (...) > I tend to fully agree if you mean having a "service_port" parameter in the > "struct server" (or some kind of) > Each time we have to manipulate addr and/or port in the struct > sockaddr_storage, it's a nightmare and we have to think about all the > corner cases... OK so I managed to do it and I think I got it working fine now. It took some time because in order not to miss any place I renamed the struct member so that I was sure to spot all places. I found a small bug in the current DNS resolution implementation when the family is set to AF_UNSPEC, it immediately marks it as invalid and tries again, so my DNS server got flooded during my tests :-) But that's fixed now. Thus now str2sa_range() doesn't change the family if the address doesn't resolved. It however continues to *also* copy the port into the address in the case where it resolves so that we don't have to touch all other call places (listeners, peers, source, etc). I could get this config to resolve all addresses as expected : resolvers mydns nameserver dns1 8.8.8.8:53 resolve_retries 3 timeout retry 1s hold valid 1s defaults option httplog log 127.0.0.1:5514 local0 modehttp timeout connect 5s timeout client 60s timeout server 90s frontend f bind *: #bind ::: backend b # default-server resolvers mydns ## doesn't work server s1 127.0.0.1:8000 check server s2 127.0.0.2:8000 check disabled server s3 wtap.haproxy.local:8000 init-addr none check resolvers mydns server s4 haproxy.ipv6.1wt.eu:80 init-addr none check resolvers mydns server s5 www6.1wt.eu:80 server s6 i...@www6.1wt.eu:80 server s7 i...@www6.1wt.eu:80 init-addr libc server s8 1wt.eu:80 resolve-prefer ipv6 check init-addr none resolvers mydns server s9 1wt.eu:80 resolve-prefer ipv4 check By the way I found that "resolvers" doesn't work in default-server AND doesn't emit any warning so I had to specify it on each line. We need to fix this before issuing 1.7.2. I'd like you guys to take a look at the attached patches (rebased on 1.7). There's also an all-in-one patch that you can apply to latest 1.7 snapshot Maxim if you're interested. For me it passes all tests and seems to be OK now. Barring any objection, I'll merge all of them into 1.7 and will make a note for distro maintainers to properly pick them or they'll get annoying bug reports. Thanks, Willy >From c5ffce6f998945c1c477387da9908d408b464622 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Fri, 6 Jan 2017 17:41:29 +0100 Subject: MEDIUM: server: split the address and the port into two different fields X-Bogosity: Ham, tests=bogofilter, spamicity=0.00, version=1.2.4 Keeping the address and the port in the same field causes a lot of problems, specifically on the DNS part where we're forced to cheat on the family to be able to keep the port. This causes some issues such as some families not being resolvable anymore. This patch first moves the service port to a new field "svc_port" so that the port field is never used anymore in the "addr" field (struct sockaddr_storage). All call places were adapted (there aren't that many). --- include/types/server.h | 3 ++- src/backend.c | 21 + src/checks.c | 14 +++--- src/server.c | 5 +++-- src/stats.c| 4 ++-- 5 files changed, 31 insertions(+), 16 deletions(-) diff --git a/include/types/server.h b/include/types/server.h index 20c314b..4678934 100644 --- a/include/types/server.h +++ b/include/types/server.h @@ -230,7 +230,8 @@ struct server { const struct netns_entry *netns;/* contains network namespace name or NULL. Network namespace comes from configuration */ /* warning, these structs are huge, keep them at the bottom */ - struct sockaddr_storage addr; /* the address to connect to */ + struct sockaddr_storage addr; /* the address to connect to, doesn't include the port */ + unsigned int svc_port; /* the port to connect to (for relevant families) */ struct xprt_ops *xprt; /* transport-layer operations */
Re: Haproxy 1.7 and Ipv6-only hosts
On Fri, Dec 23, 2016 at 5:21 PM, Willy Tarreauwrote: > Hi Baptiste, > > > On Fri, Dec 23, 2016 at 04:57:36PM +0100, Willy Tarreau wrote: > (...) > > The problem is that in order not > > to lose the port which was already parsed, we temporarily set the family > to > > AF_INET and store the port in the sockaddr. The problem is that by doing > so > > we lose the initial value of this family and we believe it was forced as > if > > you had had "i...@www6.1wt.eu", so the error is valid in str2ip2. If we > think > > about it for a moment, we need to store the port only for : > > > > AF_INET (no change needed) > > AF_INET6 (no change needed) > > AF_UNSPEC > > > > It's the last case which causes trouble in fact. We only need to be able > to > > store the information that the family is unknown and that we already > have a > > port. We could even hijack an existing family for this but that's not > clean. > > We cannot leave AF_UNSPEC here as it's used when we pass an fd. I'll > have to > > think about it a bit :-/ > > Regarding this issue, I think that in fact we should decide to split the > server port apart from the address. After all, we're manipulating the port > and the address separately everywhere, we even have some extra settings in > other places (eg the fact that the ports are relative). I have not yet > analysed the impact of all of this but I think that's definitely something > we need to consider for the mid term. It will also remove most of the > "switch (family)" needed to retrieve/manipulate ports. > > What do you think ? > > Thanks, > Willy > Hi Willy, I tend to fully agree if you mean having a "service_port" parameter in the "struct server" (or some kind of) Each time we have to manipulate addr and/or port in the struct sockaddr_storage, it's a nightmare and we have to think about all the corner cases... Baptiste
Re: Haproxy 1.7 and Ipv6-only hosts
On Fri, Dec 23, 2016 at 05:21:57PM +0100, Willy Tarreau wrote: > Hi Baptiste, > > > On Fri, Dec 23, 2016 at 04:57:36PM +0100, Willy Tarreau wrote: > (...) > > The problem is that in order not > > to lose the port which was already parsed, we temporarily set the family to > > AF_INET and store the port in the sockaddr. The problem is that by doing so > > we lose the initial value of this family and we believe it was forced as if > > you had had "i...@www6.1wt.eu", so the error is valid in str2ip2. If we > > think > > about it for a moment, we need to store the port only for : > > > > AF_INET (no change needed) > > AF_INET6 (no change needed) > > AF_UNSPEC > > > > It's the last case which causes trouble in fact. We only need to be able to > > store the information that the family is unknown and that we already have a > > port. We could even hijack an existing family for this but that's not clean. > > We cannot leave AF_UNSPEC here as it's used when we pass an fd. I'll have to > > think about it a bit :-/ > > Regarding this issue, I think that in fact we should decide to split the > server port apart from the address. After all, we're manipulating the port > and the address separately everywhere, we even have some extra settings in > other places (eg the fact that the ports are relative). I have not yet > analysed the impact of all of this but I think that's definitely something > we need to consider for the mid term. It will also remove most of the > "switch (family)" needed to retrieve/manipulate ports. > > What do you think ? Also, I think that for the stable backport we can cheat. AF_UNSPEC is not used on the server side as we (obviously) don't support connecting to a file descriptor. So we should be able to return AF_UNSPEC with the port arbitrarily set in the IPv4 part. str2ip2() doesn't use the port, it only preserves it. We "just" need to be extra careful never to pass AF_UNSPEC with anything but zero in the port to str2ip2(). That still sounds tricky in the end :-/ Willy
Re: Haproxy 1.7 and Ipv6-only hosts
Hi Baptiste, On Fri, Dec 23, 2016 at 04:57:36PM +0100, Willy Tarreau wrote: (...) > The problem is that in order not > to lose the port which was already parsed, we temporarily set the family to > AF_INET and store the port in the sockaddr. The problem is that by doing so > we lose the initial value of this family and we believe it was forced as if > you had had "i...@www6.1wt.eu", so the error is valid in str2ip2. If we think > about it for a moment, we need to store the port only for : > > AF_INET (no change needed) > AF_INET6 (no change needed) > AF_UNSPEC > > It's the last case which causes trouble in fact. We only need to be able to > store the information that the family is unknown and that we already have a > port. We could even hijack an existing family for this but that's not clean. > We cannot leave AF_UNSPEC here as it's used when we pass an fd. I'll have to > think about it a bit :-/ Regarding this issue, I think that in fact we should decide to split the server port apart from the address. After all, we're manipulating the port and the address separately everywhere, we even have some extra settings in other places (eg the fact that the ports are relative). I have not yet analysed the impact of all of this but I think that's definitely something we need to consider for the mid term. It will also remove most of the "switch (family)" needed to retrieve/manipulate ports. What do you think ? Thanks, Willy
Re: Haproxy 1.7 and Ipv6-only hosts
Hi Maksim, On Fri, Dec 23, 2016 at 12:59:05PM +0300, ?? wrote: > Hi! > > Since I've installed 1.7.1 version of haproxy over 1.6.10 ??? it stopped > working with ipv6-only backends (no A-record in DNS at all, only ), > even with USE_GETADDRINFO=1 set. Haproxy says, that it 'could not resolve > address' and exits on a parsing phase. > > The problem is in fuction str2sa_range, and a quick and ugly hack, which > made me happy (all my backends are dual-stack, or ipv6-only): > --- haproxy-1.7.1.orig/src/standard.c > +++ haproxy-1.7.1/src/standard.c > @@ -963,7 +963,7 @@ struct sockaddr_storage *str2sa_range(co > * force it to IPv4 for now. > */ > memset(, 0, sizeof(ss)); > - ss.ss_family = AF_INET; > + ss.ss_family = AF_INET6; > } > else if ((!resolve && !fqdn) || > (resolve && str2ip2(str2, , 1) == NULL)) { > > Please, fix it. I could reproduce it with the following config, which I guess matches your use case : backend b1 timeout connect 5s timeout server 10s server s1 www6.1wt.eu:80 Indeed it fails to resolve. Now I see why. The problem is that in order not to lose the port which was already parsed, we temporarily set the family to AF_INET and store the port in the sockaddr. The problem is that by doing so we lose the initial value of this family and we believe it was forced as if you had had "i...@www6.1wt.eu", so the error is valid in str2ip2. If we think about it for a moment, we need to store the port only for : AF_INET (no change needed) AF_INET6 (no change needed) AF_UNSPEC It's the last case which causes trouble in fact. We only need to be able to store the information that the family is unknown and that we already have a port. We could even hijack an existing family for this but that's not clean. We cannot leave AF_UNSPEC here as it's used when we pass an fd. I'll have to think about it a bit :-/ In the mean time you can apply the following patch which will let you force "ipv4@" or "ipv6@" to work around the issue. It's probably part of the final fix in that we don't want to lose the family. diff --git a/src/standard.c b/src/standard.c index 079aef8..db26e0d 100644 --- a/src/standard.c +++ b/src/standard.c @@ -962,8 +962,11 @@ struct sockaddr_storage *str2sa_range(const char *str, int *low, int *high, char /* we'll still want to store the port, so let's * force it to IPv4 for now. */ + sa_family_t family = ss.ss_family; memset(, 0, sizeof(ss)); - ss.ss_family = AF_INET; + ss.ss_family = family; + if (ss.ss_family != AF_INET && ss.ss_family != AF_INET6) + ss.ss_family = AF_INET; } else if ((!resolve && !fqdn) || (resolve && str2ip2(str2, , 1) == NULL)) { Thanks, Willy