From: Emeric Brun Date: Fri, 11 Jun 2021 08:48:45 +0000 (+0200) Subject: MEDIUM: resolvers: add a ref between servers and srv request or used SRV record X-Git-Tag: v2.3.11~28 X-Git-Url: http://git.haproxy.org/?a=commitdiff_plain;h=32131d40fdd1e67fc06dcb21fd8b62f08677b624;p=haproxy-2.3.git MEDIUM: resolvers: add a ref between servers and srv request or used SRV record This patch add a ref into servers to register them onto the record answer item used to set their hostnames. It also adds a head list into 'srvrq' to register servers free to be affected to a SRV record. A head of a tree is also added to srvrq to put servers which present a hotname in server state file. To re-link them fastly to the matching record as soon an item present the same name. This results in better performances on SRV record response parsing. 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 3406766d57fc11478d54a6fa2d048cbfe4524a4e) Signed-off-by: Willy Tarreau (cherry picked from commit 0c4a8a3cfc0e7cf8df28012222f930e55bcb7df4) [cf: Changes applied in src/dns.c instead of src/resolvers.c] Signed-off-by: Christopher Faulet --- diff --git a/include/haproxy/dns-t.h b/include/haproxy/dns-t.h index f44240b..081332e 100644 --- a/include/haproxy/dns-t.h +++ b/include/haproxy/dns-t.h @@ -357,6 +357,8 @@ struct dns_srvrq { char *hostname_dn; /* server hostname in Domain Name format */ int hostname_dn_len; /* string length of the server hostname in Domain Name format */ struct dns_requester *dns_requester; /* used to link to its DNS resolution */ + struct list attached_servers; /* List of the servers free to use */ + struct eb_root named_servers; /* tree of servers indexed by hostnames found in server state file */ struct list list; /* Next SRV RQ for the same proxy */ }; diff --git a/include/haproxy/dns.h b/include/haproxy/dns.h index d5d76ac..a73dafa 100644 --- a/include/haproxy/dns.h +++ b/include/haproxy/dns.h @@ -46,6 +46,7 @@ int dns_get_ip_from_response(struct dns_response_packet *dns_p, void dns_purge_resolution_answer_records(struct dns_resolution *resolution); int dns_link_resolution(void *requester, int requester_type, int requester_locked); void dns_unlink_resolution(struct dns_requester *requester, int safe); +void dns_detach_from_resolution_answer_items(struct dns_resolution *res, struct dns_requester *req, int safe); void dns_trigger_resolution(struct dns_requester *requester); enum act_parse_ret dns_parse_do_resolve(const char **args, int *orig_arg, struct proxy *px, struct act_rule *rule, char **err); int check_action_do_resolve(struct act_rule *rule, struct proxy *px, char **err); diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h index e401744..7f42989 100644 --- a/include/haproxy/server-t.h +++ b/include/haproxy/server-t.h @@ -330,7 +330,9 @@ struct server { } ssl_ctx; #endif struct dns_srvrq *srvrq; /* Pointer representing the DNS SRV requeest, if any */ + struct list srv_rec_item; /* to attach server to a srv record item */ struct list ip_rec_item; /* to attach server to a A or AAAA record item */ + struct ebpt_node host_dn; /* hostdn store for srvrq and state file matching*/ 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 fe1f9ea..eb58cf3 100644 --- a/src/dns.c +++ b/src/dns.c @@ -19,6 +19,8 @@ #include +#include + #include #include #include @@ -202,6 +204,8 @@ struct dns_srvrq *new_dns_srvrq(struct server *srv, char *fqdn) proxy_type_str(px), px->id, srv->id); goto err; } + LIST_INIT(&srvrq->attached_servers); + srvrq->named_servers = EB_ROOT; LIST_ADDQ(&dns_srvrq_list, &srvrq->list); return srvrq; @@ -655,28 +659,22 @@ static void dns_check_dns_response(struct dns_resolution *res) } } 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); - } + /* Remove any associated server */ + list_for_each_entry_safe(srv, srvback, &item->attached_servers, srv_rec_item) { + dns_unlink_resolution(srv->dns_requester, 0); + HA_SPIN_LOCK(SERVER_LOCK, &srv->lock); + 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; + srv->flags |= SRV_F_NO_RESOLUTION; + HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock); + LIST_DEL(&srv->srv_rec_item); + LIST_ADDQ(&srv->srvrq->attached_servers, &srv->srv_rec_item); } } @@ -694,30 +692,81 @@ static void dns_check_dns_response(struct dns_resolution *res) /* Now process SRV records */ list_for_each_entry_safe(req, reqback, &res->requesters, list) { + struct ebpt_node *node; + char target[DNS_MAX_NAME_SIZE+1]; + + int i; if ((srvrq = objt_dns_srvrq(req->owner)) == NULL) continue; - /* Check if a server already uses that hostname */ - 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)) { - break; + /* Check if a server already uses that record */ + srv = NULL; + list_for_each_entry(srv, &item->attached_servers, srv_rec_item) { + if (srv->srvrq == srvrq) { + HA_SPIN_LOCK(SERVER_LOCK, &srv->lock); + goto srv_found; } - HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock); } - /* If not, try to find a server with undefined hostname */ - if (!srv) { - for (srv = srvrq->proxy->srv; srv != NULL; srv = srv->next) { + + /* If not empty we try to match a server + * in server state file tree with the same hostname + */ + if (!eb_is_empty(&srvrq->named_servers)) { + srv = NULL; + + /* convert the key to lookup in lower case */ + for (i = 0 ; item->target[i] ; i++) + target[i] = tolower(item->target[i]); + + node = ebis_lookup(&srvrq->named_servers, target); + if (node) { + srv = ebpt_entry(node, struct server, host_dn); HA_SPIN_LOCK(SERVER_LOCK, &srv->lock); - if (srv->srvrq == srvrq && !srv->hostname_dn) - break; - HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock); + + /* an entry was found with the same hostname + * let check this node if the port matches + * and try next node if the hostname + * is still the same + */ + while (1) { + if (srv->svc_port == item->port) { + /* server found, we remove it from tree */ + ebpt_delete(node); + free(srv->host_dn.key); + goto srv_found; + } + + HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock); + + node = ebpt_next(node); + if (!node) + break; + + srv = ebpt_entry(node, struct server, host_dn); + HA_SPIN_LOCK(SERVER_LOCK, &srv->lock); + + if ((item->data_len != srv->hostname_dn_len) + || dns_hostname_cmp(srv->hostname_dn, item->target, item->data_len)) { + HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock); + break; + } + } } } + /* Pick the first server listed in srvrq (those ones don't + * have hostname and are free to use) + */ + srv = NULL; + list_for_each_entry(srv, &srvrq->attached_servers, srv_rec_item) { + LIST_DEL_INIT(&srv->srv_rec_item); + HA_SPIN_LOCK(SERVER_LOCK, &srv->lock); + goto srv_found; + } + srv = NULL; + +srv_found: /* And update this server, if found (srv is locked here) */ if (srv) { /* re-enable DNS resolution for this server by default */ @@ -763,6 +812,9 @@ static void dns_check_dns_response(struct dns_resolution *res) send_log(srv->proxy, LOG_NOTICE, "%s", msg); } + if (!LIST_ADDED(&srv->srv_rec_item)) + LIST_ADDQ(&item->attached_servers, &srv->srv_rec_item); + if (!(srv->flags & SRV_F_NO_RESOLUTION)) { /* If there is no AR item responsible of the FQDN resolution, * trigger a dedicated DNS resolution @@ -1922,6 +1974,46 @@ int dns_link_resolution(void *requester, int requester_type, int requester_locke return -1; } +/* This function removes all server/srvrq references on answer items + * if is set to 1, in case of srvrq, sub server resquesters unlink + * is called using safe == 1 to make it usable into callbacks + */ +void dns_detach_from_resolution_answer_items(struct dns_resolution *res, struct dns_requester *req, int safe) +{ + struct dns_answer_item *item, *itemback; + struct server *srv, *srvback; + struct dns_srvrq *srvrq; + + if ((srv = objt_server(req->owner)) != NULL) { + LIST_DEL_INIT(&srv->ip_rec_item); + } + else if ((srvrq = objt_dns_srvrq(req->owner)) != NULL) { + list_for_each_entry_safe(item, itemback, &res->response.answer_list, list) { + if (item->type == DNS_RTYPE_SRV) { + list_for_each_entry_safe(srv, srvback, &item->attached_servers, srv_rec_item) { + if (srv->srvrq == srvrq) { + HA_SPIN_LOCK(SERVER_LOCK, &srv->lock); + dns_unlink_resolution(srv->dns_requester, safe); + 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; + srv->flags |= SRV_F_NO_RESOLUTION; + HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock); + LIST_DEL(&srv->srv_rec_item); + LIST_ADDQ(&srvrq->attached_servers, &srv->srv_rec_item); + } + } + } + } + } +} + + /* Removes a requester from a DNS resolution. It takes takes care of all the * consequences. It also cleans up some parameters from the requester. * if is set to 1, the corresponding resolution is not released. @@ -1930,7 +2022,6 @@ 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) @@ -1938,9 +2029,7 @@ void dns_unlink_resolution(struct dns_requester *requester, int safe) 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); - } + dns_detach_from_resolution_answer_items(res, requester, safe); /* Clean up the requester */ LIST_DEL(&requester->list); diff --git a/src/server.c b/src/server.c index b0a5874..b30c4e9 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->srv_rec_item); LIST_INIT(&srv->ip_rec_item); srv->next_state = SRV_ST_RUNNING; /* early server setup */ @@ -1890,6 +1891,9 @@ static int server_template_init(struct server *srv, struct proxy *px) goto err; } #endif + /* append to list of servers available to receive an hostname */ + LIST_ADDQ(&newsrv->srvrq->attached_servers, &newsrv->srv_rec_item); + /* Set this new server ID. */ srv_set_id_from_prefix(newsrv, srv->tmpl_info.prefix, i); @@ -2917,6 +2921,8 @@ static void srv_update_state(struct server *srv, int version, char **params) */ else if (fqdn && !srv->hostname && srvrecord) { int res; + int i; + char *tmp; /* we can't apply previous state if SRV record has changed */ if (srv->srvrq && strcmp(srv->srvrq->name, srvrecord) != 0) { @@ -2947,6 +2953,21 @@ static void srv_update_state(struct server *srv, int version, char **params) !(srv->flags & SRV_F_CHECKPORT)) srv->check.port = port; + /* Remove from available list and insert in tree + * since this server has an hostname + */ + LIST_DEL_INIT(&srv->srv_rec_item); + srv->host_dn.key = tmp = strdup(srv->hostname_dn); + + /* convert the key in lowercase because tree + * lookup is case sensitive but we don't care + */ + for (i = 0; tmp[i]; i++) + tmp[i] = tolower(tmp[i]); + + /* insert in tree */ + ebis_insert(&srv->srvrq->named_servers, &srv->host_dn); + /* Unset SRV_F_MAPPORTS for SRV records. * SRV_F_MAPPORTS is unfortunately set by parse_server() * because no ports are provided in the configuration file. @@ -3854,18 +3875,11 @@ int snr_resolution_cb(struct dns_requester *requester, struct dns_nameserver *na return 1; if (s->srvrq) { - struct dns_answer_item *srv_item; - - /* If DNS resolution is disabled ignore it. */ - if (s->flags & SRV_F_NO_RESOLUTION) - return 1; - - /* The server is based on a SRV record, thus, find the - * associated answer record. If not found, it means the SRV item - * has expired and this resolution must be ignored. + /* If DNS resolution is disabled ignore it. + * This is the case if the server was associated to + * a SRV record and this record is now expired. */ - srv_item = find_srvrq_answer_record(requester); - if (!srv_item) + if (s->flags & SRV_F_NO_RESOLUTION) return 1; } @@ -3959,7 +3973,6 @@ int snr_resolution_cb(struct dns_requester *requester, struct dns_nameserver *na */ int srvrq_resolution_error_cb(struct dns_requester *requester, int error_code) { - struct server *s; struct dns_srvrq *srvrq; struct dns_resolution *res; struct dns_resolvers *resolvers; @@ -4003,22 +4016,8 @@ int srvrq_resolution_error_cb(struct dns_requester *requester, int error_code) return 1; } - /* Remove any associated server */ - for (s = srvrq->proxy->srv; s != NULL; s = s->next) { - HA_SPIN_LOCK(SERVER_LOCK, &s->lock); - if (s->srvrq == srvrq) { - dns_unlink_resolution(s->dns_requester, 1); - srvrq_update_srv_status(s, 1); - free(s->hostname); - free(s->hostname_dn); - s->hostname = NULL; - s->hostname_dn = NULL; - s->hostname_dn_len = 0; - memset(&s->addr, 0, sizeof(s->addr)); - s->svc_port = 0; - } - HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock); - } + /* Remove any associated server ref */ + dns_detach_from_resolution_answer_items(res, requester, 1); return 0; } @@ -4043,7 +4042,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); + dns_detach_from_resolution_answer_items(requester->resolution, requester, 1); return 0; } HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock);