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.

diff --git a/src/action.c b/src/action.c
index e44ac5317e958384e13f0a3063ffe926ec772f49..1333fb3c1674cd081637a58eda9253da50f4e291 100644 (file)
--- 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 9e08b64a07f431866cb168f406f0885847e40226..85b10d6a579f01d94ca71e7b39e7bd4117100ee5 100644 (file)
--- 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 62f0bcb89fad1c746d28616ada1d68bbad1db18a..c2c52e7a01ab92486bde594e8c10a7daa972e23a 100644 (file)
--- 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;
 }