[SSSD] [PATCH] Improve handling of DNS timeout

Jakub Hrozek jhrozek at redhat.com
Tue Dec 20 15:49:09 UTC 2011


Hi,

I'm not sure if it's still time to include this change in 1.7 since we
froze the strings yesterday, but if not, feel free to postpone to 1.8.

[PATCH 1/3] Resolver: Introduce a per-request timeout
In addition to passing a timeout to c-ares (which controls how long
would c-ares talk to an individual name server), also set up a tevent
timeout for the whole request.

The rationale is that the user of the async interface does not know or
case how many name servers there are and would like to just limit how
long the whole request could take. This should help in situations where
the DNS server is very slow or drops packets.

[PATCH 2/3] Do not touch resolve_service_state in fo_resolve_service_done
In the fail over code, we correctly used the "server->common" pointer as
memory context for the resolving but then passed a service resolving
state as a private context.

If the service resolving request was canceled before the resolving
request finished, fo_resolve_service_done would access freed memory.

This was a long-standing bug in the fail over code we never hit because
the service resolve request could end only when the resolve request
ended. I hit the bug when I introduced the timeouts in the later patch.

[PATCH 3/3] Failover: Introduce a per-service timeout
Changes the semantics of dns_resolver_timeout to be "how long a
service resolution can take" from "how long a name resolution can take".
I think it is better from the point of view of the fail over user.

Also introduces a new option dns_resolver_op_timeout which controls the
resolver timeout (which is what dns_resolver_timeout used to do). It is
intentionally undocumented.

https://fedorahosted.org/sssd/ticket/976
-------------- next part --------------
From a829c62fcb2e5f3c83c9795fe973a0bb052d2a3f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 19 Dec 2011 18:06:47 +0100
Subject: [PATCH 1/3] Resolver: Introduce a per-request timeout

---
 src/resolv/async_resolv.c |  231 +++++++++++++++++++++++++++++++++++++++------
 1 files changed, 200 insertions(+), 31 deletions(-)

diff --git a/src/resolv/async_resolv.c b/src/resolv/async_resolv.c
index 7b7ca41e5956b527e2256a740b494d3bcd66f19a..1fbd671a99a7d4c8151681f06f56f63a56145bbf 100644
--- a/src/resolv/async_resolv.c
+++ b/src/resolv/async_resolv.c
@@ -64,6 +64,8 @@
 #define DNS__16BIT(p)                   (((p)[0] << 8) | (p)[1])
 #define DNS_HEADER_ANCOUNT(h)           DNS__16BIT((h) + 6)
 
+#define RESOLV_TIMEOUTMS  5000
+
 enum host_database default_host_dbs[] = { DB_FILES, DB_DNS, DB_SENTINEL };
 
 struct fd_watch {
@@ -96,6 +98,17 @@ struct resolv_ctx {
     struct tevent_timer *timeout_watcher;
 };
 
+struct request_watch {
+    struct tevent_req *req;
+    struct resolv_request *rr;
+};
+
+struct resolv_request {
+    struct resolv_ctx *ctx;
+    struct request_watch *rwatch;
+    struct tevent_timer *request_timeout;
+};
+
 struct resolv_ctx *context_list;
 
 errno_t
@@ -197,6 +210,10 @@ add_timeout_timer(struct tevent_context *ev, struct resolv_ctx *ctx)
     struct timeval tv = { 0, 0 };
     struct timeval *tvp;
 
+    if (ctx->timeout_watcher) {
+        return;
+    }
+
     tvp = ares_timeout(ctx->channel, NULL, &tv);
 
     if (tvp == NULL) {
@@ -241,20 +258,98 @@ check_fd_timeouts(struct tevent_context *ev, struct tevent_timer *te,
 }
 
 static void
-schedule_timeout_watcher(struct tevent_context *ev, struct resolv_ctx *ctx)
+resolv_request_timeout(struct tevent_context *ev,
+                       struct tevent_timer *te,
+                       struct timeval tv, void *pvt)
 {
+    struct resolv_request *rreq;
+
+    DEBUG(SSSDBG_MINOR_FAILURE, ("The resolve request timed out\n"));
+
+    rreq = talloc_get_type(pvt, struct resolv_request);
+    if (rreq->rwatch == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("The request already completed\n"));
+        return;
+    }
+
+    tevent_req_error(rreq->rwatch->req, ETIMEDOUT);
+    rreq->rwatch = NULL;
+}
+
+static int
+request_watch_destructor(struct request_watch *rwatch)
+{
+    DEBUG(SSSDBG_TRACE_FUNC, ("Deleting request watch\n"));
+    if (rwatch->rr) rwatch->rr->rwatch = NULL;
+    return 0;
+}
+
+static struct resolv_request *
+schedule_request_timeout(struct tevent_context *ev, struct resolv_ctx *ctx,
+                         struct tevent_req *req)
+{
+    struct resolv_request *rreq;
+    struct timeval tv;
+
+    DEBUG(SSSDBG_TRACE_INTERNAL, ("Scheduling a timeout of %d seconds\n",
+                                  ctx->timeout));
+    tv = tevent_timeval_current_ofs(ctx->timeout, 0);
+
+    /* Intentionally allocating on ctx, because the request might go away
+     * before c-ares returns */
+    rreq = talloc(ctx, struct resolv_request);
+    if (!rreq) {
+        talloc_zfree(req);
+        return NULL;
+    }
+    rreq->ctx = ctx;
+    rreq->request_timeout = tevent_add_timer(ev, rreq, tv,
+                                             resolv_request_timeout,
+                                             rreq);
+    if (rreq->request_timeout == NULL) {
+        talloc_free(rreq);
+        return NULL;
+    }
+
+    /* The watch will go away when the request finishes */
+    rreq->rwatch = talloc(req, struct request_watch);
+    if (!rreq->rwatch) {
+        talloc_zfree(req);
+        return NULL;
+    }
+
+    rreq->rwatch->req = req;
+    rreq->rwatch->rr = rreq;
+    talloc_set_destructor(rreq->rwatch, request_watch_destructor);
+
+    return rreq;
+}
+
+static struct resolv_request *
+schedule_timeout_watcher(struct tevent_context *ev, struct resolv_ctx *ctx,
+                         struct tevent_req *req)
+{
+    struct resolv_request *rreq;
+
+    rreq = schedule_request_timeout(ev, ctx, req);
+    if (!rreq) return NULL;
+
     ctx->pending_requests++;
-    if (ctx->timeout_watcher) {
-        return;
-    }
 
-    DEBUG(9, ("Scheduling DNS timeout watcher\n"));
+    DEBUG(SSSDBG_TRACE_INTERNAL, ("Scheduling DNS timeout watcher\n"));
     add_timeout_timer(ev, ctx);
+    return rreq;
 }
 
 static void
-unschedule_timeout_watcher(struct resolv_ctx *ctx)
+unschedule_timeout_watcher(struct resolv_ctx *ctx, struct resolv_request *rreq)
 {
+    /* Unlink the watch if the request is still active */
+    if (rreq->rwatch) {
+        rreq->rwatch->rr = NULL;
+    }
+    talloc_free(rreq); /* Cancels the tevent timeout as well */
+
     if (ctx->pending_requests <= 0) {
         DEBUG(1, ("Pending DNS requests mismatch\n"));
         return;
@@ -383,7 +478,7 @@ recreate_ares_channel(struct resolv_ctx *ctx)
      */
     options.sock_state_cb = fd_event;
     options.sock_state_cb_data = ctx;
-    options.timeout = ctx->timeout * 1000;
+    options.timeout = RESOLV_TIMEOUTMS;
     /* Only affects ares_gethostbyname */
     options.lookups = discard_const("f");
     options.tries = 1;
@@ -784,7 +879,6 @@ resolv_gethostbyname_dns_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
         return NULL;
     }
     tevent_req_set_callback(subreq, resolv_gethostbyname_dns_wakeup, req);
-    schedule_timeout_watcher(ev, ctx);
 
     return req;
 }
@@ -816,25 +910,44 @@ static void
 resolv_gethostbyname_dns_query(struct tevent_req *req,
                                struct gethostbyname_dns_state *state)
 {
+    struct resolv_request *rreq;
+
     DEBUG(4, ("Trying to resolve %s record of '%s' in DNS\n",
               state->family == AF_INET ? "A" : "AAAA", state->name));
 
+    rreq = schedule_timeout_watcher(state->ev, state->resolv_ctx, req);
+    if (!rreq) {
+        tevent_req_error(req, ENOMEM);
+        return;
+    }
+
     ares_search(state->resolv_ctx->channel,
-               state->name, ns_c_in,
-               (state->family == AF_INET) ? ns_t_a : ns_t_aaaa,
-               resolv_gethostbyname_dns_query_done, req);
+                state->name, ns_c_in,
+                (state->family == AF_INET) ? ns_t_a : ns_t_aaaa,
+                resolv_gethostbyname_dns_query_done, rreq);
 }
 
 static void
 resolv_gethostbyname_dns_query_done(void *arg, int status, int timeouts,
                                     unsigned char *abuf, int alen)
 {
-    struct tevent_req *req = talloc_get_type(arg, struct tevent_req);
-    struct gethostbyname_dns_state *state = tevent_req_data(req,
-                                        struct gethostbyname_dns_state);
     errno_t ret;
+    struct gethostbyname_dns_state *state;
+    struct resolv_request *rreq = talloc_get_type(arg, struct resolv_request);
+    struct tevent_req *req;
 
-    unschedule_timeout_watcher(state->resolv_ctx);
+
+    if (rreq->rwatch == NULL) {
+        /* The tevent request was cancelled while the ares call was still in
+         * progress so nobody cares about the result now. Quit. */
+        unschedule_timeout_watcher(rreq->ctx, rreq);
+        return;
+    }
+
+    req = rreq->rwatch->req;
+    unschedule_timeout_watcher(rreq->ctx, rreq);
+
+    state = tevent_req_data(req, struct gethostbyname_dns_state);
 
     state->status = status;
     state->timeouts = timeouts;
@@ -845,7 +958,6 @@ resolv_gethostbyname_dns_query_done(void *arg, int status, int timeouts,
     if (state->retrying == 0 && status == ARES_EDESTRUCTION
         && state->resolv_ctx->channel != NULL) {
         state->retrying = 1;
-        schedule_timeout_watcher(state->ev, state->resolv_ctx);
         resolv_gethostbyname_dns_query(req, state);
         return;
     }
@@ -1439,6 +1551,7 @@ rewrite_talloc_srv_reply(TALLOC_CTX *mem_ctx, struct ares_srv_reply **reply_list
  *******************************************************************/
 
 struct getsrv_state {
+    struct tevent_context *ev;
     struct resolv_ctx *resolv_ctx;
     /* the SRV query - for example _ldap._tcp.example.com */
     const char *query;
@@ -1452,6 +1565,9 @@ struct getsrv_state {
 
 static void
 ares_getsrv_wakeup(struct tevent_req *subreq);
+static void
+resolv_getsrv_query(struct tevent_req *req,
+                    struct getsrv_state *state);
 
 struct tevent_req *
 resolv_getsrv_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
@@ -1478,6 +1594,7 @@ resolv_getsrv_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
     state->status = 0;
     state->timeouts = 0;
     state->retrying = 0;
+    state->ev = ev;
 
     subreq = tevent_wakeup_send(req, ev, tv);
     if (subreq == NULL) {
@@ -1486,7 +1603,6 @@ resolv_getsrv_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
         return NULL;
     }
     tevent_req_set_callback(subreq, ares_getsrv_wakeup, req);
-    schedule_timeout_watcher(ev, ctx);
 
     return req;
 }
@@ -1494,21 +1610,30 @@ resolv_getsrv_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
 static void
 resolv_getsrv_done(void *arg, int status, int timeouts, unsigned char *abuf, int alen)
 {
-    struct tevent_req *req = talloc_get_type(arg, struct tevent_req);
-    struct getsrv_state *state = tevent_req_data(req, struct getsrv_state);
+    struct resolv_request *rreq = talloc_get_type(arg, struct resolv_request);
+    struct tevent_req *req;
+    struct getsrv_state *state;
     int ret;
     struct ares_srv_reply *reply_list;
 
+    if (rreq->rwatch == NULL) {
+        /* The tevent request was cancelled while the ares call was still in
+         * progress so nobody cares about the result now. Quit. */
+        unschedule_timeout_watcher(rreq->ctx, rreq);
+        return;
+    }
+
+    req = rreq->rwatch->req;
+    unschedule_timeout_watcher(rreq->ctx, rreq);
+    state = tevent_req_data(req, struct getsrv_state);
+
     if (state->retrying == 0 && status == ARES_EDESTRUCTION
-            && state->resolv_ctx->channel != NULL) {
+        && state->resolv_ctx->channel != NULL) {
         state->retrying = 1;
-        ares_query(state->resolv_ctx->channel, state->query,
-                   ns_c_in, ns_t_srv, resolv_getsrv_done, req);
+        resolv_getsrv_query(req, state);
         return;
     }
 
-    unschedule_timeout_watcher(state->resolv_ctx);
-
     state->status = status;
     state->timeouts = timeouts;
 
@@ -1574,8 +1699,23 @@ ares_getsrv_wakeup(struct tevent_req *subreq)
         return;
     }
 
+    return resolv_getsrv_query(req, state);
+}
+
+static void
+resolv_getsrv_query(struct tevent_req *req,
+                    struct getsrv_state *state)
+{
+    struct resolv_request *rreq;
+
+    rreq = schedule_timeout_watcher(state->ev, state->resolv_ctx, req);
+    if (!rreq) {
+        tevent_req_error(req, ENOMEM);
+        return;
+    }
+
     ares_query(state->resolv_ctx->channel, state->query,
-               ns_c_in, ns_t_srv, resolv_getsrv_done, req);
+               ns_c_in, ns_t_srv, resolv_getsrv_done, rreq);
 }
 
 /* TXT parsing is not used anywhere in the code yet, so we disable it
@@ -1647,6 +1787,7 @@ rewrite_talloc_txt_reply(TALLOC_CTX *mem_ctx, struct ares_txt_reply **reply_list
  *******************************************************************/
 
 struct gettxt_state {
+    struct tevent_context *ev;
     struct resolv_ctx *resolv_ctx;
     /* the TXT query */
     const char *query;
@@ -1660,6 +1801,9 @@ struct gettxt_state {
 
 static void
 ares_gettxt_wakeup(struct tevent_req *subreq);
+static void
+resolv_gettxt_query(struct tevent_req *req,
+                    struct gettxt_state *state);
 
 struct tevent_req *
 resolv_gettxt_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
@@ -1686,6 +1830,7 @@ resolv_gettxt_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
     state->status = 0;
     state->timeouts = 0;
     state->retrying = 0;
+    state->ev = ev;
 
     subreq = tevent_wakeup_send(req, ev, tv);
     if (subreq == NULL) {
@@ -1694,7 +1839,6 @@ resolv_gettxt_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
         return NULL;
     }
     tevent_req_set_callback(subreq, ares_gettxt_wakeup, req);
-    schedule_timeout_watcher(ev, ctx);
 
     return req;
 }
@@ -1702,11 +1846,23 @@ resolv_gettxt_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
 static void
 resolv_gettxt_done(void *arg, int status, int timeouts, unsigned char *abuf, int alen)
 {
-    struct tevent_req *req = talloc_get_type(arg, struct tevent_req);
-    struct gettxt_state *state = tevent_req_data(req, struct gettxt_state);
+    struct resolv_request *rreq = talloc_get_type(arg, struct resolv_request);
+    struct tevent_req *req;
+    struct gettxt_state *state;
     int ret;
     struct ares_txt_reply *reply_list;
 
+    if (rreq->rwatch == NULL) {
+        /* The tevent request was cancelled while the ares call was still in
+         * progress so nobody cares about the result now. Quit. */
+        unschedule_timeout_watcher(rreq->ctx, rreq);
+        return;
+    }
+
+    req = rreq->rwatch->req;
+    unschedule_timeout_watcher(rreq->ctx, rreq);
+    state = tevent_req_data(req, struct gettxt_state);
+
     if (state->retrying == 0 && status == ARES_EDESTRUCTION
             && state->resolv_ctx->channel != NULL) {
         state->retrying = 1;
@@ -1715,8 +1871,6 @@ resolv_gettxt_done(void *arg, int status, int timeouts, unsigned char *abuf, int
         return;
     }
 
-    unschedule_timeout_watcher(state->resolv_ctx);
-
     state->status = status;
     state->timeouts = timeouts;
 
@@ -1782,8 +1936,23 @@ ares_gettxt_wakeup(struct tevent_req *subreq)
         return;
     }
 
+    return resolv_gettxt_query(req, state);
+}
+
+static void
+resolv_gettxt_query(struct tevent_req *req,
+                    struct gettxt_state *state)
+{
+    struct resolv_request *rreq;
+
+    rreq = schedule_timeout_watcher(state->ev, state->resolv_ctx, req);
+    if (!rreq) {
+        tevent_req_error(req, ENOMEM);
+        return;
+    }
+
     ares_query(state->resolv_ctx->channel, state->query,
-               ns_c_in, ns_t_txt, resolv_gettxt_done, req);
+               ns_c_in, ns_t_txt, resolv_gettxt_done, rreq);
 }
 
 #endif
-- 
1.7.7.4

-------------- next part --------------
From 0e806bb3abadbb88559e1d89905b0f63b3c3dd43 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 20 Dec 2011 14:53:34 +0100
Subject: [PATCH 2/3] Do not touch resolve_service_state in
 fo_resolve_service_done

---
 src/providers/fail_over.c |   25 +++++++++++--------------
 1 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c
index 91d981cf55e905dd472066a395c0a1de3791935d..5cfc1a41c22e0be2bbb2512e86e928d2a20e2b6e 100644
--- a/src/providers/fail_over.c
+++ b/src/providers/fail_over.c
@@ -850,7 +850,8 @@ fo_resolve_service_server(struct tevent_req *req)
             tevent_req_error(req, ENOMEM);
             return true;
         }
-        tevent_req_set_callback(subreq, fo_resolve_service_done, req);
+        tevent_req_set_callback(subreq, fo_resolve_service_done,
+                                state->server->common);
         fo_set_server_status(state->server, SERVER_RESOLVING_NAME);
         /* FALLTHROUGH */
     case SERVER_RESOLVING_NAME:
@@ -873,34 +874,30 @@ fo_resolve_service_server(struct tevent_req *req)
 static void
 fo_resolve_service_done(struct tevent_req *subreq)
 {
-    struct tevent_req *req = tevent_req_callback_data(subreq,
-                                                      struct tevent_req);
-    struct resolve_service_state *state = tevent_req_data(req,
-                                                struct resolve_service_state);
-    struct server_common *common;
+    struct server_common *common = tevent_req_callback_data(subreq,
+                                                        struct server_common);
     int resolv_status;
     struct resolve_service_request *request;
     int ret;
 
-    if (state->server->common->rhostent != NULL) {
-        talloc_zfree(state->server->common->rhostent);
+    if (common->rhostent != NULL) {
+        talloc_zfree(common->rhostent);
     }
 
-    ret = resolv_gethostbyname_recv(subreq, state->server->common,
+    ret = resolv_gethostbyname_recv(subreq, common,
                                     &resolv_status, NULL,
-                                    &state->server->common->rhostent);
+                                    &common->rhostent);
     talloc_zfree(subreq);
     if (ret != EOK) {
         DEBUG(1, ("Failed to resolve server '%s': %s\n",
-                  state->server->common->name,
+                  common->name,
                   resolv_strerror(resolv_status)));
-        set_server_common_status(state->server->common, SERVER_NOT_WORKING);
+        set_server_common_status(common, SERVER_NOT_WORKING);
     } else {
-        set_server_common_status(state->server->common, SERVER_NAME_RESOLVED);
+        set_server_common_status(common, SERVER_NAME_RESOLVED);
     }
 
     /* Take care of all requests for this server. */
-    common = state->server->common; /* state can disappear now */
     while ((request = common->request_list) != NULL) {
         DLIST_REMOVE(common->request_list, request);
         if (resolv_status) {
-- 
1.7.7.4

-------------- next part --------------
From 715ea5d9cab617907d7afd4c73f05d88019998df Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 20 Dec 2011 16:30:02 +0100
Subject: [PATCH 3/3] Failover: Introduce a per-service timeout

https://fedorahosted.org/sssd/ticket/976
---
 src/confdb/confdb.h              |    1 +
 src/providers/data_provider_fo.c |   18 ++++++++++----
 src/providers/fail_over.c        |   46 ++++++++++++++++++++++++++++++++++++++
 src/providers/fail_over.h        |    7 +++++
 4 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index e3f8ba3c49562a49efa3134149b9329bd4416336..7cfc73d2b8a9dd61c796fcc14d69c778dc2a0227 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -117,6 +117,7 @@
 #define CONFDB_DOMAIN_FQ "use_fully_qualified_names"
 #define CONFDB_DOMAIN_ENTRY_CACHE_TIMEOUT "entry_cache_timeout"
 #define CONFDB_DOMAIN_RESOLV_TIMEOUT "dns_resolver_timeout"
+#define CONFDB_DOMAIN_RESOLV_OP_TIMEOUT "dns_resolver_op_timeout"
 #define CONFDB_DOMAIN_DNS_DISCOVERY_NAME "dns_discovery_domain"
 #define CONFDB_DOMAIN_FAMILY_ORDER "lookup_family_order"
 #define CONFDB_DOMAIN_ACCOUNT_CACHE_EXPIRATION "account_cache_expiration"
diff --git a/src/providers/data_provider_fo.c b/src/providers/data_provider_fo.c
index 7a5c82c3e3db866335906b8c4d4002f425b79754..dd73e9d97b85ec4e47b72b6b898aabe49923d6d8 100644
--- a/src/providers/data_provider_fo.c
+++ b/src/providers/data_provider_fo.c
@@ -67,7 +67,14 @@ static int be_fo_get_options(struct be_ctx *ctx,
 {
     errno_t ret;
 
-    /* todo get timeout from configuration */
+    ret = confdb_get_int(ctx->cdb, ctx, ctx->conf_path,
+                         CONFDB_DOMAIN_RESOLV_TIMEOUT,
+                         FO_DEFAULT_SVC_TIMEOUT,
+                         &opts->service_resolv_timeout);
+    if (ret != EOK) {
+        return ret;
+    }
+
     opts->retry_timeout = 30;
     opts->srv_retry_timeout = 14400;
 
@@ -83,7 +90,7 @@ static int be_fo_get_options(struct be_ctx *ctx,
 int be_init_failover(struct be_ctx *ctx)
 {
     int ret;
-    int fo_timeout;
+    int resolv_timeout;
     struct fo_options fopts;
 
     if (ctx->be_fo != NULL) {
@@ -96,13 +103,14 @@ int be_init_failover(struct be_ctx *ctx)
     }
 
     ret = confdb_get_int(ctx->cdb, ctx, ctx->conf_path,
-                         CONFDB_DOMAIN_RESOLV_TIMEOUT,
-                         RESOLV_DEFAULT_TIMEOUT, &fo_timeout);
+                         CONFDB_DOMAIN_RESOLV_OP_TIMEOUT,
+                         RESOLV_DEFAULT_TIMEOUT,
+                         &resolv_timeout);
     if (ret != EOK) {
         return ret;
     }
 
-    ret = resolv_init(ctx, ctx->ev, fo_timeout, &ctx->be_fo->resolv);
+    ret = resolv_init(ctx, ctx->ev, resolv_timeout, &ctx->be_fo->resolv);
     if (ret != EOK) {
         talloc_zfree(ctx->be_fo);
         return ret;
diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c
index 5cfc1a41c22e0be2bbb2512e86e928d2a20e2b6e..637b5b4d3654fc0e93b3efb77e0e996c76e8c7f8 100644
--- a/src/providers/fail_over.c
+++ b/src/providers/fail_over.c
@@ -141,6 +141,7 @@ fo_context_init(TALLOC_CTX *mem_ctx, struct fo_options *opts)
     ctx->opts->srv_retry_timeout = opts->srv_retry_timeout;
     ctx->opts->retry_timeout = opts->retry_timeout;
     ctx->opts->family_order  = opts->family_order;
+    ctx->opts->service_resolv_timeout = opts->service_resolv_timeout;
 
     DEBUG(3, ("Created new fail over context, retry timeout is %d\n",
               ctx->opts->retry_timeout));
@@ -735,10 +736,13 @@ struct resolve_service_state {
 
     struct resolv_ctx *resolv;
     struct tevent_context *ev;
+    struct tevent_timer *timeout_handler;
     struct fo_ctx *fo_ctx;
 };
 
 
+static errno_t fo_resolve_service_activate_timeout(struct tevent_req *req,
+            struct tevent_context *ev, const unsigned long timeout_seconds);
 static void fo_resolve_service_cont(struct tevent_req *subreq);
 static void fo_resolve_service_done(struct tevent_req *subreq);
 static bool fo_resolve_service_server(struct tevent_req *req);
@@ -777,6 +781,14 @@ fo_resolve_service_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
         goto done;
     }
 
+    /* Activate per-service timeout handler */
+    ret = fo_resolve_service_activate_timeout(req, ev,
+                                        ctx->opts->service_resolv_timeout);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Could not set service timeout\n"));
+        goto done;
+    }
+
     if (fo_is_srv_lookup(server)) {
         /* Don't know the server yet, must do a SRV lookup */
         subreq = resolve_srv_send(state, ev, resolv,
@@ -810,6 +822,40 @@ done:
 static void set_server_common_status(struct server_common *common,
                                      enum server_status status);
 
+static void
+fo_resolve_service_timeout(struct tevent_context *ev,
+                           struct tevent_timer *te,
+                           struct timeval tv, void *pvt)
+{
+    struct tevent_req *req = talloc_get_type(pvt, struct tevent_req);
+
+    DEBUG(SSSDBG_MINOR_FAILURE, ("Service resolving timeout reached\n"));
+    tevent_req_error(req, ETIMEDOUT);
+}
+
+static errno_t
+fo_resolve_service_activate_timeout(struct tevent_req *req,
+                                    struct tevent_context *ev,
+                                    const unsigned long timeout_seconds)
+{
+    struct timeval tv;
+    struct resolve_service_state *state = tevent_req_data(req,
+                                        struct resolve_service_state);
+
+    tv = tevent_timeval_current();
+    tv = tevent_timeval_add(&tv, timeout_seconds, 0);
+    state->timeout_handler = tevent_add_timer(ev, state, tv,
+                                           fo_resolve_service_timeout, req);
+    if (state->timeout_handler == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("tevent_add_timer failed.\n"));
+        return ENOMEM;
+    }
+
+    DEBUG(SSSDBG_TRACE_INTERNAL, ("Resolve timeout set to %lu seconds\n",
+          timeout_seconds));
+    return EOK;
+}
+
 /* SRV resolving finished, see if we got server to work with */
 static void
 fo_resolve_service_cont(struct tevent_req *subreq)
diff --git a/src/providers/fail_over.h b/src/providers/fail_over.h
index 3ca6096c45c4b73706f07b593448734a69272678..fee64e82c064671667f5ca8ecde4cd63d13b31b6 100644
--- a/src/providers/fail_over.h
+++ b/src/providers/fail_over.h
@@ -33,6 +33,8 @@
 #define FO_PROTO_TCP "tcp"
 #define FO_PROTO_UDP "udp"
 
+#define FO_DEFAULT_SVC_TIMEOUT 10
+
 /* Some forward declarations that don't have to do anything with fail over. */
 struct hostent;
 struct tevent_context;
@@ -63,6 +65,10 @@ struct fo_server;
  * duration in seconds of how long a server or port will be considered
  * non-working after being marked as such.
  *
+ * The 'service_resolv_timeout' member specifies how long we wait for
+ * service resolution. When this timeout is reached, the resolve request
+ * is cancelled with an error
+ *
  * The 'srv_retry_timeout' member specifies how long a SRV lookup
  * is considered valid until we ask the server again.
  *
@@ -72,6 +78,7 @@ struct fo_server;
 struct fo_options {
     time_t srv_retry_timeout;
     time_t retry_timeout;
+    int service_resolv_timeout;
     enum restrict_family family_order;
 };
 
-- 
1.7.7.4



More information about the sssd-devel mailing list