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.
(cherry picked from commit 12ca658dbe5bc3a3f47de06b2c19d5f0ee08941e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 43839d0ec40d8aa57c97955fc2dc6bfc1404d3c0)
[cf: Changes applied in src/dns.c instead of src/resolvers.c]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit a8b60f267a9c4c345d7d85dc425e86115b3cd1c4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 3297a75bbc09c222cbc6d9db437470750b3f30f8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/action.c b/src/action.c
index 7684202..319f543 100644
--- a/src/action.c
+++ b/src/action.c
@@ -86,6 +86,12 @@
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 dns_requester *requester, int error_code)
{
struct stream *stream;
diff --git a/src/dns.c b/src/dns.c
index ff5b489..a20b725 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -1563,6 +1563,7 @@
unsigned short query_id;
struct eb32_node *eb;
struct dns_requester *req;
+ int keep_answer_items;
fd = dgram->t.sock.fd;
@@ -1721,8 +1722,11 @@
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)
+ dns_purge_resolution_answer_records(res);
dns_reset_resolution(res);
LIST_DEL(&res->list);
LIST_ADDQ(&resolvers->resolutions.wait, &res->list);
@@ -1854,12 +1858,15 @@
* the list */
if (!res->try) {
struct dns_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)
+ dns_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 296046c..13655ea 100644
--- a/src/server.c
+++ b/src/server.c
@@ -4038,8 +4038,8 @@
/*
* 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.
*/
@@ -4054,7 +4054,7 @@
/* SRV records */
srvrq = objt_dns_srvrq(requester->owner);
if (!srvrq)
- return 1;
+ return 0;
resolvers = srvrq->resolvers;
res = requester->resolution;
@@ -4106,16 +4106,14 @@
HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock);
}
- dns_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.
*/
@@ -4125,11 +4123,16 @@
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;
}