From 12ca658dbe5bc3a3f47de06b2c19d5f0ee08941e Mon Sep 17 00:00:00 2001 From: Emeric Brun Date: Thu, 10 Jun 2021 15:25:25 +0200 Subject: [PATCH] BUG/MINOR: resolvers: answser item list was randomly purged or errors In case of SRV records, The answer item list was purged by the error callback of the first requester which considers the error could not be safely ignored. It makes this item list unavailable for subsequent requesters even if they consider the error could be ignored. On A resolution or do_resolve action error, the answer items were never trashed. This patch re-work the error callbacks and the code to check the return code If a callback return 1, we consider the error was ignored and the answer item list must be kept. At the opposite, If all error callbacks of all requesters of the same resolution returns 0 the list will be purged This patch should be backported as far as 2.0. --- src/action.c | 6 ++++++ src/resolvers.c | 11 +++++++++-- src/server.c | 23 +++++++++++++---------- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/action.c b/src/action.c index e44ac5317..1333fb3c1 100644 --- a/src/action.c +++ b/src/action.c @@ -157,6 +157,12 @@ int act_resolution_cb(struct resolv_requester *requester, struct dns_counters *c return 0; } +/* + * Do resolve error management callback + * returns: + * 0 if we can trash answser items. + * 1 when safely ignored and we must kept answer items + */ int act_resolution_error_cb(struct resolv_requester *requester, int error_code) { struct stream *stream; diff --git a/src/resolvers.c b/src/resolvers.c index 9e08b64a0..85b10d6a5 100644 --- a/src/resolvers.c +++ b/src/resolvers.c @@ -1900,6 +1900,7 @@ static int resolv_process_responses(struct dns_nameserver *ns) unsigned short query_id; struct eb32_node *eb; struct resolv_requester *req; + int keep_answer_items; resolvers = ns->parent; HA_SPIN_LOCK(DNS_LOCK, &resolvers->lock); @@ -2034,8 +2035,11 @@ static int resolv_process_responses(struct dns_nameserver *ns) goto report_res_success; report_res_error: + keep_answer_items = 0; list_for_each_entry(req, &res->requesters, list) - req->requester_error_cb(req, dns_resp); + keep_answer_items |= req->requester_error_cb(req, dns_resp); + if (!keep_answer_items) + resolv_purge_resolution_answer_records(res); resolv_reset_resolution(res); LIST_DELETE(&res->list); LIST_APPEND(&resolvers->resolutions.wait, &res->list); @@ -2097,12 +2101,15 @@ static struct task *process_resolvers(struct task *t, void *context, unsigned in * the list */ if (!res->try) { struct resolv_requester *req; + int keep_answer_items = 0; /* Notify the result to the requesters */ if (!res->nb_responses) res->status = RSLV_STATUS_TIMEOUT; list_for_each_entry(req, &res->requesters, list) - req->requester_error_cb(req, res->status); + keep_answer_items |= req->requester_error_cb(req, res->status); + if (!keep_answer_items) + resolv_purge_resolution_answer_records(res); /* Clean up resolution info and remove it from the * current list */ diff --git a/src/server.c b/src/server.c index 62f0bcb89..c2c52e7a0 100644 --- a/src/server.c +++ b/src/server.c @@ -3436,8 +3436,8 @@ int snr_resolution_cb(struct resolv_requester *requester, struct dns_counters *c /* * SRV record error management callback * returns: - * 0 on error - * 1 when no error or safe ignore + * 0 if we can trash answser items. + * 1 when safely ignored and we must kept answer items * * Grabs the server's lock. */ @@ -3452,7 +3452,7 @@ int srvrq_resolution_error_cb(struct resolv_requester *requester, int error_code /* SRV records */ srvrq = objt_resolv_srvrq(requester->owner); if (!srvrq) - return 1; + return 0; resolvers = srvrq->resolvers; res = requester->resolution; @@ -3502,16 +3502,14 @@ int srvrq_resolution_error_cb(struct resolv_requester *requester, int error_code HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock); } - resolv_purge_resolution_answer_records(res); - - return 1; + return 0; } /* * Server Name Resolution error management callback * returns: - * 0 on error - * 1 when no error or safe ignore + * 0 if we can trash answser items. + * 1 when safely ignored and we must kept answer items * * Grabs the server's lock. */ @@ -3521,11 +3519,16 @@ int snr_resolution_error_cb(struct resolv_requester *requester, int error_code) s = objt_server(requester->owner); if (!s) - return 1; + return 0; + HA_SPIN_LOCK(SERVER_LOCK, &s->lock); - if (!snr_update_srv_status(s, 1)) + if (!snr_update_srv_status(s, 1)) { memset(&s->addr, 0, sizeof(s->addr)); + HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock); + return 0; + } HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock); + return 1; }