From 3023e981998da3dc6fb03de7bd955d5775f68a7b Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Thu, 11 Sep 2025 15:20:35 +0200 Subject: [PATCH] BUG/MINOR: resolvers: Restore round-robin selection on records in DNS answers Since the commit dcb696cd3 ("MEDIUM: resolvers: hash the records before inserting them into the tree"), When several records are found in a DNS answer, the round robin selection over these records is no longer performed. Indeed, before a list of records was used. To ensure each records was selected one after the other, at each selection, the first record of the list was moved at the end. When this list was replaced bu a tree, the same mechanism was preserved. However, the record is indexed using its key, a hash of the record. So its position never changes. When it is removed and reinserted in the tree, its position remains the same. When we walk though the tree, starting from the root, the records are always evaluated in the same order. So, even if there are several records in a DNS answer, the same IP address is always selected. It is quite easy to trigger the issue with a do-resolv action. To fix the issue, the node to perform the next selection is now saved. So instead of restarting from the root each time, we can restart from the next node of the previous call. Thanks to Damien Claisse for the issue analysis and for the reproducer. This patch should fix the issue #3116. It must be backported as far as 2.6. --- include/haproxy/resolvers-t.h | 1 + src/resolvers.c | 33 ++++++++++++++++----------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/include/haproxy/resolvers-t.h b/include/haproxy/resolvers-t.h index 914b37530..7c411b3db 100644 --- a/include/haproxy/resolvers-t.h +++ b/include/haproxy/resolvers-t.h @@ -133,6 +133,7 @@ struct resolv_answer_item { struct resolv_response { struct dns_header header; struct eb_root answer_tree; + struct eb32_node *next; /* node to start eval on the next lookup to perform a round robin selection on entries (may be NULL) */ /* authority ignored for now */ }; diff --git a/src/resolvers.c b/src/resolvers.c index 7753422a9..a42ae9133 100644 --- a/src/resolvers.c +++ b/src/resolvers.c @@ -1614,7 +1614,7 @@ int resolv_get_ip_from_response(struct resolv_response *r_res, struct server *owner) { struct resolv_answer_item *record, *found_record = NULL; - struct eb32_node *eb32; + struct eb32_node *eb32, *end; int family_priority; int currentip_found; unsigned char *newip4, *newip6; @@ -1647,11 +1647,16 @@ int resolv_get_ip_from_response(struct resolv_response *r_res, * 1 - current ip. * The result with the biggest score is returned. */ - - for (eb32 = eb32_first(&r_res->answer_tree); eb32 != NULL; eb32 = eb32_next(eb32)) { + eb32 = (!r_res->next) ? eb32_first(&r_res->answer_tree) : r_res->next; + end = r_res->next; + r_res->next = eb32_next(eb32); /* get node for the next lookup */ + do { void *ip; unsigned char ip_type; + if (eb32 == NULL) + eb32 = eb32_first(&r_res->answer_tree); + record = eb32_entry(eb32, typeof(*record), link); if (record->type == DNS_RTYPE_A && (resolv_active_families() & RSLV_ACCEPT_IPV4)) { ip_type = AF_INET; @@ -1662,7 +1667,7 @@ int resolv_get_ip_from_response(struct resolv_response *r_res, ip = &record->data.in6.sin6_addr; } else - continue; + goto next; score = 0; /* Check for preferred ip protocol. */ @@ -1674,7 +1679,7 @@ int resolv_get_ip_from_response(struct resolv_response *r_res, /* Compare only the same addresses class. */ if (resolv_opts->pref_net[j].family != ip_type) - continue; + goto next; if ((ip_type == AF_INET && in_net_ipv4(ip, @@ -1698,7 +1703,7 @@ int resolv_get_ip_from_response(struct resolv_response *r_res, list_for_each_entry(srv, &record->attached_servers, ip_rec_item) { if (srv == owner) - continue; + goto next; if (srv->proxy == owner->proxy) { already_used = 1; break; @@ -1706,7 +1711,7 @@ int resolv_get_ip_from_response(struct resolv_response *r_res, } if (already_used) { if (!allowed_duplicated_ip) { - continue; + goto next; } } else { @@ -1748,7 +1753,9 @@ int resolv_get_ip_from_response(struct resolv_response *r_res, } max_score = score; } - } /* list for each record entries */ + next: + eb32 = eb32_next(eb32); + } while (eb32 != end); /* list for each record entries */ /* No IP found in the response */ if (!newip4 && !newip6) @@ -1794,15 +1801,6 @@ int resolv_get_ip_from_response(struct resolv_response *r_res, LIST_APPEND(&found_record->attached_servers, &owner->ip_rec_item); } - eb32 = eb32_first(&r_res->answer_tree); - if (eb32) { - /* Move the first record to the end of the list, for internal - * round robin. - */ - eb32_delete(eb32); - eb32_insert(&r_res->answer_tree, eb32); - } - return (currentip_found ? RSLV_UPD_NO : RSLV_UPD_SRVIP_NOT_FOUND); } @@ -1973,6 +1971,7 @@ static struct resolv_resolution *resolv_pick_resolution(struct resolvers *resolv LIST_INIT(&res->requesters); res->response.answer_tree = EB_ROOT; + res->response.next = NULL; res->prefered_query_type = query_type; res->query_type = query_type;