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
