From ca8e355ae9a8ccabe50dd30e0cd8fb9810a484eb Mon Sep 17 00:00:00 2001 From: Emeric Brun Date: Fri, 11 Jun 2021 10:08:05 +0200 Subject: [PATCH] MEDIUM: resolvers: add a ref on server to the used A/AAAA answer item This patch adds a head list into answer items on servers which use this record to set their IPs. It makes lookup on duplicated ip faster and allow to check immediatly if an item is still valid renewing the IP. This results in better performances on A/AAAA resolutions. This is an optimization but it could avoid to trigger the haproxy's internal wathdog in some circumstances. And for this reason it should be backported as far we can (2.0 ?) (cherry picked from commit bd78c912fd73b8506a39c53b6f0b00330f998aca) Signed-off-by: Willy Tarreau (cherry picked from commit f9ca5d8bc12441c3c21a66b92ea401803a102f2b) [cf: Changes applied in src/dns.c instead of src/resolvers.c] Signed-off-by: Christopher Faulet --- include/haproxy/dns-t.h | 1 + include/haproxy/dns.h | 2 +- include/haproxy/server-t.h | 1 + src/dns.c | 110 +++++++++++++++++++++++++++++++------------- src/server.c | 2 + 5 files changed, 82 insertions(+), 34 deletions(-) diff --git a/include/haproxy/dns-t.h b/include/haproxy/dns-t.h index 95d4097..f44240b 100644 --- a/include/haproxy/dns-t.h +++ b/include/haproxy/dns-t.h @@ -149,6 +149,7 @@ struct dns_answer_item { char target[DNS_MAX_NAME_SIZE+1]; /* Response data: SRV or CNAME type target */ unsigned int last_seen; /* When was the answer was last seen */ struct dns_answer_item *ar_item; /* pointer to a RRset from the additional section, if exists */ + struct list attached_servers; /* attached server head */ struct list list; }; diff --git a/include/haproxy/dns.h b/include/haproxy/dns.h index a47c256..d5d76ac 100644 --- a/include/haproxy/dns.h +++ b/include/haproxy/dns.h @@ -41,7 +41,7 @@ int dns_get_ip_from_response(struct dns_response_packet *dns_p, struct dns_options *dns_opts, void *currentip, short currentip_sin_family, void **newip, short *newip_sin_family, - void *owner); + struct server *owner); void dns_purge_resolution_answer_records(struct dns_resolution *resolution); int dns_link_resolution(void *requester, int requester_type, int requester_locked); diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h index de4a2d5..e401744 100644 --- a/include/haproxy/server-t.h +++ b/include/haproxy/server-t.h @@ -330,6 +330,7 @@ struct server { } ssl_ctx; #endif struct dns_srvrq *srvrq; /* Pointer representing the DNS SRV requeest, if any */ + struct list ip_rec_item; /* to attach server to a A or AAAA record item */ struct { const char *file; /* file where the section appears */ struct eb32_node id; /* place in the tree of used IDs */ diff --git a/src/dns.c b/src/dns.c index 4ef2749..fe1f9ea 100644 --- a/src/dns.c +++ b/src/dns.c @@ -631,7 +631,7 @@ static void dns_check_dns_response(struct dns_resolution *res) struct dns_resolvers *resolvers = res->resolvers; struct dns_requester *req, *reqback; struct dns_answer_item *item, *itemback; - struct server *srv; + struct server *srv, *srvback; struct dns_srvrq *srvrq; list_for_each_entry_safe(item, itemback, &res->response.answer_list, list) { @@ -648,34 +648,38 @@ static void dns_check_dns_response(struct dns_resolution *res) /* Remove obsolete items */ if (tick_is_lt(tick_add(item->last_seen, resolvers->hold.obsolete), now_ms)) { - if (item->type != DNS_RTYPE_SRV) - goto rm_obselete_item; - - list_for_each_entry_safe(req, reqback, &res->requesters, list) { - if ((srvrq = objt_dns_srvrq(req->owner)) == NULL) - continue; - + if (item->type == DNS_RTYPE_A || item->type == DNS_RTYPE_AAAA) { /* Remove any associated server */ - for (srv = srvrq->proxy->srv; srv != NULL; srv = srv->next) { - HA_SPIN_LOCK(SERVER_LOCK, &srv->lock); - if (srv->srvrq == srvrq && srv->svc_port == item->port && - item->data_len == srv->hostname_dn_len && - !dns_hostname_cmp(srv->hostname_dn, item->target, item->data_len)) { - dns_unlink_resolution(srv->dns_requester, 0); - srvrq_update_srv_status(srv, 1); - free(srv->hostname); - free(srv->hostname_dn); - srv->hostname = NULL; - srv->hostname_dn = NULL; - srv->hostname_dn_len = 0; - memset(&srv->addr, 0, sizeof(srv->addr)); - srv->svc_port = 0; - } - HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock); + list_for_each_entry_safe(srv, srvback, &item->attached_servers, ip_rec_item) { + LIST_DEL_INIT(&srv->ip_rec_item); + } + } + else if (item->type == DNS_RTYPE_SRV) { + list_for_each_entry(req, &res->requesters, list) { + if ((srvrq = objt_dns_srvrq(req->owner)) == NULL) + continue; + + /* Remove any associated server */ + for (srv = srvrq->proxy->srv; srv != NULL; srv = srv->next) { + HA_SPIN_LOCK(SERVER_LOCK, &srv->lock); + if (srv->srvrq == srvrq && srv->svc_port == item->port && + item->data_len == srv->hostname_dn_len && + !dns_hostname_cmp(srv->hostname_dn, item->target, item->data_len)) { + dns_unlink_resolution(srv->dns_requester, 0); + srvrq_update_srv_status(srv, 1); + free(srv->hostname); + free(srv->hostname_dn); + srv->hostname = NULL; + srv->hostname_dn = NULL; + srv->hostname_dn_len = 0; + memset(&srv->addr, 0, sizeof(srv->addr)); + srv->svc_port = 0; + } + HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock); + } } } - rm_obselete_item: LIST_DEL(&item->list); if (item->ar_item) { pool_free(dns_answer_item_pool, item->ar_item); @@ -963,6 +967,7 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend, /* initialization */ dns_answer_record->ar_item = NULL; dns_answer_record->last_seen = TICK_ETERNITY; + LIST_INIT(&dns_answer_record->attached_servers); offset = 0; len = dns_read_name(resp, bufend, reader, tmpname, DNS_MAX_NAME_SIZE, &offset, 0); @@ -1214,6 +1219,7 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend, if (dns_answer_record == NULL) goto invalid_resp; dns_answer_record->last_seen = TICK_ETERNITY; + LIST_INIT(&dns_answer_record->attached_servers); offset = 0; len = dns_read_name(resp, bufend, reader, tmpname, DNS_MAX_NAME_SIZE, &offset, 0); @@ -1394,9 +1400,9 @@ int dns_get_ip_from_response(struct dns_response_packet *dns_p, struct dns_options *dns_opts, void *currentip, short currentip_sin_family, void **newip, short *newip_sin_family, - void *owner) + struct server *owner) { - struct dns_answer_item *record; + struct dns_answer_item *record, *found_record = NULL; int family_priority; int currentip_found; unsigned char *newip4, *newip6; @@ -1405,6 +1411,10 @@ int dns_get_ip_from_response(struct dns_response_packet *dns_p, int score, max_score; int allowed_duplicated_ip; + /* srv is linked to an alive ip record */ + if (owner && LIST_ADDED(&owner->ip_rec_item)) + return DNS_UPD_NO; + family_priority = dns_opts->family_prio; allowed_duplicated_ip = dns_opts->accept_duplicate_ip; *newip = newip4 = newip6 = NULL; @@ -1469,10 +1479,26 @@ int dns_get_ip_from_response(struct dns_response_packet *dns_p, /* Check if the IP found in the record is already affected to a * member of a group. If not, the score should be incremented * by 2. */ - if (owner && snr_check_ip_callback(owner, ip, &ip_type)) { - if (!allowed_duplicated_ip) { - continue; - } + if (owner) { + struct server *srv; + int already_used = 0; + + list_for_each_entry(srv, &record->attached_servers, ip_rec_item) { + if (srv == owner) + continue; + if (srv->proxy == owner->proxy) { + already_used = 1; + break; + } + } + if (already_used) { + if (!allowed_duplicated_ip) { + continue; + } + } + else { + score += 2; + } } else { score += 2; } @@ -1498,9 +1524,15 @@ int dns_get_ip_from_response(struct dns_response_packet *dns_p, newip4 = ip; else newip6 = ip; + found_record = record; currentip_found = currentip_sel; - if (score == 15) - return DNS_UPD_NO; + if (score == 15) { + /* this was not registered on the current record but it matches + * let's fix it (it may comes from state file */ + if (owner) + LIST_ADDQ(&found_record->attached_servers, &owner->ip_rec_item); + return DNS_UPD_NO; + } max_score = score; } } /* list for each record entries */ @@ -1553,6 +1585,12 @@ int dns_get_ip_from_response(struct dns_response_packet *dns_p, return DNS_UPD_NO; not_found: + + /* the ip of this record was chosen for the server */ + if (owner && found_record) { + LIST_ADDQ(&found_record->attached_servers, &owner->ip_rec_item); + } + list_for_each_entry(record, &dns_p->answer_list, list) { /* Move the first record to the end of the list, for internal * round robin */ @@ -1892,12 +1930,18 @@ void dns_unlink_resolution(struct dns_requester *requester, int safe) { struct dns_resolution *res; struct dns_requester *req; + struct server *srv; /* Nothing to do */ if (!requester || !requester->resolution) return; res = requester->resolution; + /* remove ref from the resolution answer item list to the requester */ + if ((srv = objt_server(requester->owner)) != NULL) { + LIST_DEL_INIT(&srv->ip_rec_item); + } + /* Clean up the requester */ LIST_DEL(&requester->list); requester->resolution = NULL; diff --git a/src/server.c b/src/server.c index 9a450a7..b0a5874 100644 --- a/src/server.c +++ b/src/server.c @@ -1741,6 +1741,7 @@ struct server *new_server(struct proxy *proxy) for (i = 0; i < MAX_THREADS; i++) MT_LIST_INIT(&srv->actconns[i]); srv->pendconns = EB_ROOT; + LIST_INIT(&srv->ip_rec_item); srv->next_state = SRV_ST_RUNNING; /* early server setup */ srv->last_change = now.tv_sec; @@ -4042,6 +4043,7 @@ int snr_resolution_error_cb(struct dns_requester *requester, int error_code) if (!snr_update_srv_status(s, 1)) { memset(&s->addr, 0, sizeof(s->addr)); HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock); + LIST_DEL_INIT(&s->ip_rec_item); return 0; } HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock); -- 1.7.10.4