Welcome! Log In Create A New Profile

Advanced

[PATCH] dns: Handle SRV record weights correctly

Posted by Olivier Houchard 
Olivier Houchard
[PATCH] dns: Handle SRV record weights correctly
January 08, 2018 04:40PM
Hi,

The attached patch attempts to map SRV record weight to haproxy weight
correctly,
SRV weight goes from 0 to 65536 while haproxy uses 0 to 256, so we have to
divide it by 256, and a SRV weight of 0 doesn't mean the server shouldn't be
used, so we use a minimum weight of 1.

Regards,

Olivier
From 8e8ab23223274ac75fdf1cfe2847337133fd59d2 Mon Sep 17 00:00:00 2001
From: Olivier Houchard <[email protected]>
Date: Mon, 8 Jan 2018 16:28:57 +0100
Subject: [PATCH] MINOR: Handle SRV record weight correctly.

A SRV record weight can range from 0 to 65535, while haproxy weight goes
from 0 to 255, so we have to divide it by 256 before handing it to haproxy.
Also, a SRV record with a weight of 0 doesn't mean the server shouldn't be
used, so use a minimum weight of 1.

This should probably be backported to 1.8.
---
include/types/dns.h | 2 +-
src/dns.c | 21 ++++++++++++++++++---
2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/include/types/dns.h b/include/types/dns.h
index b1f068a61..9b1d08df7 100644
--- a/include/types/dns.h
+++ b/include/types/dns.h
@@ -143,7 +143,7 @@ struct dns_answer_item {
int16_t class; /* query class */
int32_t ttl; /* response TTL */
int16_t priority; /* SRV type priority */
- int16_t weight; /* SRV type weight */
+ uint16_t weight; /* SRV type weight */
int16_t port; /* SRV type port */
int16_t data_len; /* number of bytes in target below */
struct sockaddr address; /* IPv4 or IPv6, network format */
diff --git a/src/dns.c b/src/dns.c
index fceef2e48..22af18dc9 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -522,10 +522,17 @@ static void dns_check_dns_response(struct dns_resolution *res)
if (srv->srvrq == srvrq && srv->svc_port == item->port &&
item->data_len == srv->hostname_dn_len &&
!memcmp(srv->hostname_dn, item->target, item->data_len)) {
- if (srv->uweight != item->weight) {
+ int ha_weight;
+
+ /* We still want to use a 0 weight server */
+ if (item->weight < 256)
+ ha_weight = 1;
+ else
+ ha_weight = item->weight / 256;
+ if (srv->uweight != ha_weight) {
char weight[9];

- snprintf(weight, sizeof(weight), "%d", item->weight);
+ snprintf(weight, sizeof(weight), "%d", ha_weight);
server_parse_weight_change_request(srv, weight);
}
HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
@@ -547,6 +554,7 @@ static void dns_check_dns_response(struct dns_resolution *res)
if (srv) {
const char *msg = NULL;
char weight[9];
+ int ha_weight;
char hostname[DNS_MAX_NAME_SIZE];

if (dns_dn_label_to_str(item->target, item->data_len+1,
@@ -563,7 +571,14 @@ static void dns_check_dns_response(struct dns_resolution *res)
if ((srv->check.state & CHK_ST_CONFIGURED) &&
!(srv->flags & SRV_F_CHECKPORT))
srv->check.port = item->port;
- snprintf(weight, sizeof(weight), "%d", item->weight);
+
+ /* We still want to use a 0 weight server */
+ if (item->weight < 256)
+ ha_weight = 1;
+ else
+ ha_weight = item->weight / 256;
+
+ snprintf(weight, sizeof(weight), "%d", ha_weight);
server_parse_weight_change_request(srv, weight);
HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
}
--
2.14.3
Willy Tarreau
Re: [PATCH] dns: Handle SRV record weights correctly
January 09, 2018 03:30PM
Hi Olivier,

On Mon, Jan 08, 2018 at 04:35:35PM +0100, Olivier Houchard wrote:
> Hi,
>
> The attached patch attempts to map SRV record weight to haproxy weight
> correctly,
> SRV weight goes from 0 to 65536 while haproxy uses 0 to 256, so we have to
> divide it by 256, and a SRV weight of 0 doesn't mean the server shouldn't be
> used, so we use a minimum weight of 1.

From what I'm seeing in the code, it's 0..65535 for the SRV record. And
that allows us to simplify it and use the full range of the weight like
this :

hap_weight = srv_weight / 256 + 1;

=> 0..255 return 1
1..511 return 2
...
65280..65535 return 256

What do you think ?

Willy
Olivier Houchard
Re: [PATCH] dns: Handle SRV record weights correctly
January 09, 2018 03:40PM
Hi Willy,

On Tue, Jan 09, 2018 at 03:17:24PM +0100, Willy Tarreau wrote:
> Hi Olivier,
>
> On Mon, Jan 08, 2018 at 04:35:35PM +0100, Olivier Houchard wrote:
> > Hi,
> >
> > The attached patch attempts to map SRV record weight to haproxy weight
> > correctly,
> > SRV weight goes from 0 to 65536 while haproxy uses 0 to 256, so we have to
> > divide it by 256, and a SRV weight of 0 doesn't mean the server shouldn't be
> > used, so we use a minimum weight of 1.
>
> From what I'm seeing in the code, it's 0..65535 for the SRV record. And
> that allows us to simplify it and use the full range of the weight like
> this :
>
> hap_weight = srv_weight / 256 + 1;
>
> => 0..255 return 1
> 1..511 return 2
> ...
> 65280..65535 return 256
>
> What do you think ?
>
> Willy
>

Sure, sounds good, for some reason I thought the max was 255, but it's
actually 256, one day I'll learn how to read C.

Regards,

Olivier
Willy Tarreau
Re: [PATCH] dns: Handle SRV record weights correctly
January 09, 2018 03:50PM
On Tue, Jan 09, 2018 at 03:28:22PM +0100, Olivier Houchard wrote:
> Hi Willy,
>
> On Tue, Jan 09, 2018 at 03:17:24PM +0100, Willy Tarreau wrote:
> > Hi Olivier,
> >
> > On Mon, Jan 08, 2018 at 04:35:35PM +0100, Olivier Houchard wrote:
> > > Hi,
> > >
> > > The attached patch attempts to map SRV record weight to haproxy weight
> > > correctly,
> > > SRV weight goes from 0 to 65536 while haproxy uses 0 to 256, so we have to
> > > divide it by 256, and a SRV weight of 0 doesn't mean the server shouldn't be
> > > used, so we use a minimum weight of 1.
> >
> > From what I'm seeing in the code, it's 0..65535 for the SRV record. And
> > that allows us to simplify it and use the full range of the weight like
> > this :
> >
> > hap_weight = srv_weight / 256 + 1;
> >
> > => 0..255 return 1
> > 1..511 return 2
> > ...
> > 65280..65535 return 256
> >
> > What do you think ?
> >
> > Willy
> >
>
> Sure, sounds good, for some reason I thought the max was 255, but it's
> actually 256, one day I'll learn how to read C.

I can only recommend you this one, which I hope will also help me write
less bugs in haproxy once I finish it :

http://www.c-for-dummies.com/

Willy
Olivier Houchard
Re: [PATCH] dns: Handle SRV record weights correctly
January 09, 2018 03:50PM
Hi,

On Tue, Jan 09, 2018 at 03:28:22PM +0100, Olivier Houchard wrote:
> Hi Willy,
>
> On Tue, Jan 09, 2018 at 03:17:24PM +0100, Willy Tarreau wrote:
> > Hi Olivier,
> >
> > On Mon, Jan 08, 2018 at 04:35:35PM +0100, Olivier Houchard wrote:
> > > Hi,
> > >
> > > The attached patch attempts to map SRV record weight to haproxy weight
> > > correctly,
> > > SRV weight goes from 0 to 65536 while haproxy uses 0 to 256, so we have to
> > > divide it by 256, and a SRV weight of 0 doesn't mean the server shouldn't be
> > > used, so we use a minimum weight of 1.
> >
> > From what I'm seeing in the code, it's 0..65535 for the SRV record. And
> > that allows us to simplify it and use the full range of the weight like
> > this :
> >
> > hap_weight = srv_weight / 256 + 1;
> >
> > => 0..255 return 1
> > 1..511 return 2
> > ...
> > 65280..65535 return 256
> >
> > What do you think ?
> >
> > Willy
> >
>
> Sure, sounds good, for some reason I thought the max was 255, but it's
> actually 256, one day I'll learn how to read C.
>

Updated patch attached.

Regards,

Olivier
From 7043d8da312605b2876286c95a2c0c53d6bd43e5 Mon Sep 17 00:00:00 2001
From: Olivier Houchard <[email protected]>
Date: Mon, 8 Jan 2018 16:28:57 +0100
Subject: [PATCH] MINOR: dns: Handle SRV record weight correctly.

A SRV record weight can range from 0 to 65535, while haproxy weight goes
from 0 to 255, so we have to divide it by 256 before handing it to haproxy.
Also, a SRV record with a weight of 0 doesn't mean the server shouldn't be
used, so use a minimum weight of 1.

This should probably be backported to 1.8.
---
include/types/dns.h | 2 +-
src/dns.c | 19 ++++++++++++++++---
2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/include/types/dns.h b/include/types/dns.h
index b1f068a61..9b1d08df7 100644
--- a/include/types/dns.h
+++ b/include/types/dns.h
@@ -143,7 +143,7 @@ struct dns_answer_item {
int16_t class; /* query class */
int32_t ttl; /* response TTL */
int16_t priority; /* SRV type priority */
- int16_t weight; /* SRV type weight */
+ uint16_t weight; /* SRV type weight */
int16_t port; /* SRV type port */
int16_t data_len; /* number of bytes in target below */
struct sockaddr address; /* IPv4 or IPv6, network format */
diff --git a/src/dns.c b/src/dns.c
index fceef2e48..a957710ed 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -522,10 +522,16 @@ static void dns_check_dns_response(struct dns_resolution *res)
if (srv->srvrq == srvrq && srv->svc_port == item->port &&
item->data_len == srv->hostname_dn_len &&
!memcmp(srv->hostname_dn, item->target, item->data_len)) {
- if (srv->uweight != item->weight) {
+ int ha_weight;
+
+ /* Make sure weight is at least 1, so
+ * that the server will be used.
+ */
+ ha_weight = item->weight / 256 + 1;
+ if (srv->uweight != ha_weight) {
char weight[9];

- snprintf(weight, sizeof(weight), "%d", item->weight);
+ snprintf(weight, sizeof(weight), "%d", ha_weight);
server_parse_weight_change_request(srv, weight);
}
HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
@@ -547,6 +553,7 @@ static void dns_check_dns_response(struct dns_resolution *res)
if (srv) {
const char *msg = NULL;
char weight[9];
+ int ha_weight;
char hostname[DNS_MAX_NAME_SIZE];

if (dns_dn_label_to_str(item->target, item->data_len+1,
@@ -563,7 +570,13 @@ static void dns_check_dns_response(struct dns_resolution *res)
if ((srv->check.state & CHK_ST_CONFIGURED) &&
!(srv->flags & SRV_F_CHECKPORT))
srv->check.port = item->port;
- snprintf(weight, sizeof(weight), "%d", item->weight);
+
+ /* Make sure weight is at least 1, so
+ * that the server will be used.
+ */
+ ha_weight = item->weight / 256 + 1;
+
+ snprintf(weight, sizeof(weight), "%d", ha_weight);
server_parse_weight_change_request(srv, weight);
HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
}
--
2.14.3
Willy Tarreau
Re: [PATCH] dns: Handle SRV record weights correctly
January 09, 2018 03:50PM
On Tue, Jan 09, 2018 at 03:39:29PM +0100, Olivier Houchard wrote:
> Updated patch attached.

cool, now applied, thanks!
Willy
Sorry, only registered users may post in this forum.

Click here to login