MEDIUM: resolvers: replace bogus resolv_hostname_cmp() with memcmp()
resolv_hostname_cmp() is bogus, it is applied on labels and not plain
names, but doesn't make any distinction between length prefixes and
characters, so it compares the labels lengths via tolower() as well.
The only reason for which it doesn't break is because labels cannot
be larger than 63 bytes, and that none of the common encoding systems
have upper case letters in the lower 63 bytes, that could be turned
into a different value via tolower().
Now that all labels are stored in lower case, we don't need to burn
CPU cycles in tolower() at run time and can use memcmp() instead of
resolv_hostname_cmp(). This results in a ~22% lower CPU usage on large
farms using SRV records:
before:
18.33% haproxy [.] resolv_validate_dns_response
10.58% haproxy [.] process_resolvers
10.28% haproxy [.] resolv_hostname_cmp
7.50% libc-2.30.so [.] tolower
46.69% total
after:
24.73% haproxy [.] resolv_validate_dns_response
7.78% libc-2.30.so [.] __memcmp_avx2_movbe
3.65% haproxy [.] process_resolvers
36.16% total
diff --git a/src/resolvers.c b/src/resolvers.c
index 6fe38fb..ae1eaee 100644
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -154,19 +154,6 @@
return NULL;
}
-/* Compare hostnames in a case-insensitive way .
- * Returns 0 if they are the same, non-zero otherwise
- */
-static __inline int resolv_hostname_cmp(const char *name1, const char *name2, int len)
-{
- int i;
-
- for (i = 0; i < len; i++)
- if (tolower((unsigned char)name1[i]) != tolower((unsigned char)name2[i]))
- return -1;
- return 0;
-}
-
/* Returns a pointer on the SRV request matching the name <name> for the proxy
* <px>. NULL is returned if no match is found.
*/
@@ -251,7 +238,7 @@
/* search an ANSWER record whose target points to the server's hostname and whose port is
* the same as server's svc_port */
list_for_each_entry(item, &res->response.answer_list, list) {
- if (resolv_hostname_cmp(srv->hostname_dn, item->data.target, srv->hostname_dn_len) == 0 &&
+ if (memcmp(srv->hostname_dn, item->data.target, srv->hostname_dn_len) == 0 &&
(srv->svc_port == item->port))
return item;
}
@@ -724,7 +711,7 @@
HA_SPIN_LOCK(SERVER_LOCK, &srv->lock);
if ((item->data_len != srv->hostname_dn_len)
- || resolv_hostname_cmp(srv->hostname_dn, item->data.target, item->data_len)) {
+ || memcmp(srv->hostname_dn, item->data.target, item->data_len) != 0) {
HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
break;
}
@@ -1004,7 +991,7 @@
/* Check if the current record dname is valid. previous_dname
* points either to queried dname or last CNAME target */
- if (query->type != DNS_RTYPE_SRV && resolv_hostname_cmp(previous_dname, tmpname, len) != 0) {
+ if (query->type != DNS_RTYPE_SRV && memcmp(previous_dname, tmpname, len) != 0) {
if (i == 0) {
/* First record, means a mismatch issue between
* queried dname and dname found in the first
@@ -1172,7 +1159,7 @@
case DNS_RTYPE_SRV:
if (answer_record->data_len == tmp_record->data_len &&
- !resolv_hostname_cmp(answer_record->data.target, tmp_record->data.target, answer_record->data_len) &&
+ memcmp(answer_record->data.target, tmp_record->data.target, answer_record->data_len) == 0 &&
answer_record->port == tmp_record->port) {
tmp_record->weight = answer_record->weight;
found = 1;
@@ -1342,7 +1329,7 @@
ar_item = tmp_record->ar_item;
if (ar_item->type != answer_record->type || ar_item->last_seen == now_ms ||
len != tmp_record->data_len ||
- resolv_hostname_cmp(answer_record->name, tmp_record->data.target, tmp_record->data_len))
+ memcmp(answer_record->name, tmp_record->data.target, tmp_record->data_len) != 0)
continue;
switch(ar_item->type) {
@@ -1381,7 +1368,7 @@
list_for_each_entry(tmp_record, &r_res->answer_list, list) {
if (tmp_record->type == DNS_RTYPE_SRV &&
tmp_record->ar_item == NULL &&
- !resolv_hostname_cmp(tmp_record->data.target, answer_record->name, tmp_record->data_len)) {
+ memcmp(tmp_record->data.target, answer_record->name, tmp_record->data_len) == 0) {
/* Always use the received additional record to refresh info */
if (tmp_record->ar_item)
pool_free(resolv_answer_item_pool, tmp_record->ar_item);
@@ -1759,7 +1746,7 @@
continue;
if ((query_type == res->prefered_query_type) &&
hostname_dn_len == res->hostname_dn_len &&
- !resolv_hostname_cmp(*hostname_dn, res->hostname_dn, hostname_dn_len))
+ memcmp(*hostname_dn, res->hostname_dn, hostname_dn_len) == 0)
return res;
}
@@ -1769,7 +1756,7 @@
continue;
if ((query_type == res->prefered_query_type) &&
hostname_dn_len == res->hostname_dn_len &&
- !resolv_hostname_cmp(*hostname_dn, res->hostname_dn, hostname_dn_len))
+ memcmp(*hostname_dn, res->hostname_dn, hostname_dn_len) == 0)
return res;
}
@@ -2165,7 +2152,7 @@
* sent. We can check only the first query of the list. We send
* one query at a time so we get one query in the response */
query = LIST_NEXT(&res->response.query_list, struct resolv_query_item *, list);
- if (query && resolv_hostname_cmp(query->name, res->hostname_dn, res->hostname_dn_len) != 0) {
+ if (query && memcmp(query->name, res->hostname_dn, res->hostname_dn_len) != 0) {
dns_resp = RSLV_RESP_WRONG_NAME;
ns->counters->other++;
goto report_res_error;