Hi,
here it is. I included some description in the commit log, it is
necessary to read it in order to understand why I did some things this
way. Any suggestions to make the patch better are most welcome.
Martin
>From 228bfdcc000e93be5908ef9ba28aee13385c258d Mon Sep 17 00:00:00 2001
From: Martin Nagy <mn...@redhat.com>
Date: Tue, 20 Apr 2010 21:08:05 +0200
Subject: [PATCH] Make ldap simple bind asynchronous
We make openldap use asynchronous connections by setting the
LDAP_OPT_CONNECT_ASYNC option. When ldap_sasl_bind() is first called, it
can either return LDAP_X_CONNECTING without blocking, or succeed
immediately without blocking (e.g. when connecting to localhost). If
LDAP_X_CONNECTING is returned, the function has to be called again. We
create a callback that is called whenever openldap creates a new file
descriptor. This callback registers the file descriptor with tevent to
watch for its availability for reading and writing.
The problem is that this callback is called from ldap_sasl_bind() and
has no means to know the tevent_req that is associated with the bind
task. We solve this by partitioning simple_bind_send() into three
functions: simple_bind_send() creates the tevent_req and registers an
immediate wakeup to simple_bind_wakeup_bind() which invokes openldap and
can be called two times. So simple_bind_wakeup_bind() saves its
tevent_req to a place that is accessible by the openldap callback. Then,
when the file descriptor is ready for writing, the tevent fd callback
can notify the callback. The third function, simple_bind_wakeup_op_add(),
finishes the whole transaction by calling sdap_op_add().
This is a source of another problem: What if the simple bind tevent_req
is freed? The subreq will become a dangling pointer and will erroneously
be invoked by the tevent fd callback. To avoid this, we talloc the
fd_event_item from the wakeup tevent_req. Another problem caused by this
is that we need to make sure the tevent fd callback has a chance to be
called and may use talloc_steal() before the request is finished. This
of course causes yet another problem, since ldap_sasl_bind() can
occasionally return without blocking, so simple_bind_wakeup_bind() has
to know if it was called already or not. If not, it means that the
request is not stolen yet, and we have to wait for that.
---
src/providers/ldap/sdap.h | 2 +
src/providers/ldap/sdap_async.c | 17 ++++-
src/providers/ldap/sdap_async_connection.c | 121 ++++++++++++++++++++++------
src/providers/ldap/sdap_async_private.h | 7 ++
src/providers/ldap/sdap_fd_events.c | 78 ++++++++++++++++--
5 files changed, 192 insertions(+), 33 deletions(-)
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index aca9845..ad8e559 100644
--- a/src/providers/ldap/sdap.h
+++ b/src/providers/ldap/sdap.h
@@ -59,12 +59,14 @@ struct fd_event_item {
int fd;
struct tevent_fd *fde;
+ struct tevent_req *wakeup_req;
};
struct ldap_cb_data {
struct sdap_handle *sh;
struct tevent_context *ev;
struct fd_event_item *fd_list;
+ struct tevent_req *fd_wakeup_req;
};
struct sdap_handle {
diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index a25c50b..14e6931 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -124,6 +124,21 @@ static void sdap_process_next_reply(struct tevent_context *ev,
struct tevent_timer *te,
struct timeval tv, void *pvt);
+void sdap_async_ldap_result(struct tevent_context *ev,
+ struct tevent_fd *fde,
+ uint16_t flags, void *pvt)
+{
+ struct ldap_cb_data *cb_data = talloc_get_type(pvt, struct ldap_cb_data);
+
+ if (flags & TEVENT_FD_WRITE) {
+ sdap_finish_bind(cb_data, fde);
+ }
+
+ if (flags & TEVENT_FD_READ) {
+ sdap_process_result(ev, cb_data->sh);
+ }
+}
+
void sdap_ldap_result(struct tevent_context *ev, struct tevent_fd *fde,
uint16_t flags, void *pvt)
{
@@ -137,7 +152,7 @@ static void sdap_ldap_next_result(struct tevent_context *ev,
sdap_process_result(ev, pvt);
}
-static void sdap_process_result(struct tevent_context *ev, void *pvt)
+void sdap_process_result(struct tevent_context *ev, void *pvt)
{
struct sdap_handle *sh = talloc_get_type(pvt, struct sdap_handle);
struct timeval no_timeout = {0, 0};
diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c
index 33edf2f..5f1c012 100644
--- a/src/providers/ldap/sdap_async_connection.c
+++ b/src/providers/ldap/sdap_async_connection.c
@@ -252,11 +252,11 @@ struct simple_bind_state {
struct sdap_msg *reply;
struct sdap_ppolicy_data *ppolicy;
int result;
+ bool sasl_bind_called;
+ int msgid;
};
-static void simple_bind_done(struct sdap_op *op,
- struct sdap_msg *reply,
- int error, void *pvt);
+static void simple_bind_wakeup_bind(struct tevent_req *subreq);
static struct tevent_req *simple_bind_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
@@ -265,16 +265,13 @@ static struct tevent_req *simple_bind_send(TALLOC_CTX *memctx,
struct berval *pw)
{
struct tevent_req *req;
+ struct tevent_req *subreq;
struct simple_bind_state *state;
- int ret = EOK;
- int msgid;
- int ldap_err;
- LDAPControl *request_controls[2];
req = tevent_req_create(memctx, &state, struct simple_bind_state);
if (!req) return NULL;
- state->reply = talloc(state, struct sdap_msg);
+ state->reply = talloc_zero(state, struct sdap_msg);
if (!state->reply) {
talloc_zfree(req);
return NULL;
@@ -284,6 +281,34 @@ static struct tevent_req *simple_bind_send(TALLOC_CTX *memctx,
state->sh = sh;
state->user_dn = user_dn;
state->pw = pw;
+ state->sasl_bind_called = false;
+
+ subreq = tevent_wakeup_send(state, ev, tevent_timeval_current_ofs(0, 0));
+ if (subreq == NULL) {
+ talloc_zfree(req);
+ return NULL;
+ }
+ tevent_req_set_callback(subreq, simple_bind_wakeup_bind, req);
+
+ return req;
+}
+
+static void simple_bind_wakeup_op_add(struct tevent_req *subreq);
+
+static void simple_bind_wakeup_bind(struct tevent_req *subreq)
+{
+ struct simple_bind_state *state;
+ int ret;
+ int ldap_err;
+ struct sdap_handle *sh;
+ LDAPControl *request_controls[2];
+ struct tevent_req *req;
+
+ req = tevent_req_callback_data(subreq, struct tevent_req);
+ state = tevent_req_data(req, struct simple_bind_state);
+ sh = state->sh;
+
+ DEBUG(4, ("Executing simple bind as: %s\n", state->user_dn));
ret = sss_ldap_control_create(LDAP_CONTROL_PASSWORDPOLICYREQUEST,
0, NULL, 0, &request_controls[0]);
@@ -293,14 +318,24 @@ static struct tevent_req *simple_bind_send(TALLOC_CTX *memctx,
}
request_controls[1] = NULL;
- DEBUG(4, ("Executing simple bind as: %s\n", state->user_dn));
-
- ret = ldap_sasl_bind(state->sh->ldap, state->user_dn, LDAP_SASL_SIMPLE,
- state->pw, request_controls, NULL, &msgid);
+ set_fd_wakeup_req(sh, subreq);
+ ret = ldap_sasl_bind(sh->ldap, state->user_dn, LDAP_SASL_SIMPLE,
+ state->pw, request_controls, NULL, &state->msgid);
ldap_control_free(request_controls[0]);
- if (ret == -1 || msgid == -1) {
- ret = ldap_get_option(state->sh->ldap,
- LDAP_OPT_RESULT_CODE, &ldap_err);
+ set_fd_wakeup_req(sh, NULL);
+
+ if (ret == LDAP_X_CONNECTING) {
+ /* The ldap socket is not connected yet, we will let the ldap
+ * connect callback call us again. */
+ DEBUG(4, ("Simple bind is in progress\n"));
+ if (state->sasl_bind_called) {
+ DEBUG(1, ("Bug: ldap_sasl_bind() shouldn't have been called "
+ "more than once\n"));
+ }
+ state->sasl_bind_called = true;
+ return;
+ } else if (ret == -1 || state->msgid == -1) {
+ ret = ldap_get_option(sh->ldap, LDAP_OPT_RESULT_CODE, &ldap_err);
if (ret != LDAP_OPT_SUCCESS) {
DEBUG(1, ("ldap_bind failed (couldn't get ldap error)\n"));
ret = LDAP_LOCAL_ERROR;
@@ -311,31 +346,65 @@ static struct tevent_req *simple_bind_send(TALLOC_CTX *memctx,
}
goto fail;
}
- DEBUG(8, ("ldap simple bind sent, msgid = %d\n", msgid));
+ DEBUG(8, ("ldap simple bind sent, msgid = %d\n", state->msgid));
if (!sh->connected) {
- ret = sdap_set_connected(sh, ev);
+ ret = sdap_set_connected(sh, state->ev);
if (ret) goto fail;
}
- /* FIXME: get timeouts from configuration, for now 5 secs. */
- ret = sdap_op_add(state, ev, sh, msgid,
- simple_bind_done, req, 5, &state->op);
- if (ret) {
- DEBUG(1, ("Failed to set up operation!\n"));
- goto fail;
+ /* Set our callback to the next step. */
+ tevent_req_set_callback(subreq, simple_bind_wakeup_op_add, req);
+
+ if (!state->sasl_bind_called) {
+ /* The fd callback was not called yet. This means that subreq is
+ * still used as a memory context for fd_event_item. We will give
+ * it a chance to talloc_steal() it and invoke the wakeup for the
+ * next step. */
+ return;
}
- return req;
+ /* The fd callback was called and we can safely complete the bind
+ * ourselves, we will invoke the next step. */
+ tevent_req_notify_callback(subreq);
+ return;
fail:
+ tevent_wakeup_recv(subreq);
+ talloc_zfree(subreq);
+
if (ret == LDAP_SERVER_DOWN) {
tevent_req_error(req, ETIMEDOUT);
} else {
tevent_req_error(req, EIO);
}
- tevent_req_post(req, ev);
- return req;
+}
+
+static void simple_bind_done(struct sdap_op *op,
+ struct sdap_msg *reply,
+ int error, void *pvt);
+
+static void simple_bind_wakeup_op_add(struct tevent_req *subreq)
+{
+ struct simple_bind_state *state;
+ struct tevent_req *req;
+ int ret;
+
+ req = tevent_req_callback_data(subreq, struct tevent_req);
+ state = tevent_req_data(req, struct simple_bind_state);
+
+ DEBUG(4, ("Waiting for response from the server\n"));
+ tevent_wakeup_recv(subreq);
+ talloc_zfree(subreq);
+
+ /* FIXME: get timeouts from configuration, for now 5 secs. */
+ ret = sdap_op_add(state, state->ev, state->sh, state->msgid,
+ simple_bind_done, req, 5, &state->op);
+ if (ret) {
+ DEBUG(1, ("Failed to set up operation!\n"));
+ tevent_req_error(req, EIO);
+ tevent_req_post(req, state->ev);
+ }
}
static void simple_bind_done(struct sdap_op *op,
diff --git a/src/providers/ldap/sdap_async_private.h b/src/providers/ldap/sdap_async_private.h
index 75597c6..e8d0e54 100644
--- a/src/providers/ldap/sdap_async_private.h
+++ b/src/providers/ldap/sdap_async_private.h
@@ -30,6 +30,13 @@ struct sdap_handle *sdap_handle_create(TALLOC_CTX *memctx);
void sdap_ldap_result(struct tevent_context *ev, struct tevent_fd *fde,
uint16_t flags, void *pvt);
+void set_fd_wakeup_req(struct sdap_handle *sh,
+ struct tevent_req *wakeup_req);
+void sdap_async_ldap_result(struct tevent_context *ev,
+ struct tevent_fd *fde,
+ uint16_t flags, void *pvt);
+void sdap_finish_bind(struct ldap_cb_data *cb_data,
+ struct tevent_fd *fde);
int setup_ldap_connection_callbacks(struct sdap_handle *sh,
struct tevent_context *ev);
diff --git a/src/providers/ldap/sdap_fd_events.c b/src/providers/ldap/sdap_fd_events.c
index 4b9a8b2..495a233 100644
--- a/src/providers/ldap/sdap_fd_events.c
+++ b/src/providers/ldap/sdap_fd_events.c
@@ -41,6 +41,49 @@ struct sdap_fd_events {
};
#ifdef HAVE_LDAP_CONNCB
+void sdap_finish_bind(struct ldap_cb_data *cb_data,
+ struct tevent_fd *fde);
+
+# ifdef LDAP_OPT_CONNECT_ASYNC
+void sdap_finish_bind(struct ldap_cb_data *cb_data,
+ struct tevent_fd *fde)
+{
+ struct fd_event_item *fd_event_item;
+
+ DEBUG(8, ("Trace: sh[%p], ldap[%p], fde[%p]\n",
+ cb_data->sh, cb_data->sh->ldap, fde));
+
+ DLIST_FOR_EACH(fd_event_item, cb_data->fd_list) {
+ if (fd_event_item->fde == fde) {
+ break;
+ }
+ }
+ if (fd_event_item != NULL && fd_event_item->wakeup_req != NULL) {
+ talloc_steal(cb_data, fd_event_item);
+ tevent_req_notify_callback(fd_event_item->wakeup_req);
+ } else if (fd_event_item == NULL) {
+ DEBUG(1, ("Bug: Couldn't find fd_event_item\n"));
+ }
+ tevent_fd_set_flags(fde, tevent_fd_get_flags(fde) & ~TEVENT_FD_WRITE);
+}
+
+void set_fd_wakeup_req(struct sdap_handle *sh,
+ struct tevent_req *wakeup_req)
+{
+ struct ldap_cb_data *cb_data;
+
+ cb_data = talloc_get_type(sh->sdap_fd_events->conncb->lc_arg, struct ldap_cb_data);
+ cb_data->fd_wakeup_req = wakeup_req;
+}
+# else /* !LDAP_OPT_CONNECT_ASYNC */
+void sdap_finish_bind(struct ldap_cb_data *cb_data,
+ struct tevent_fd *fde)
+{
+ (void)cb_data;
+ tevent_fd_set_flags(fde, tevent_fd_get_flags(fde) & ~TEVENT_FD_WRITE);
+}
+# endif /* LDAP_OPT_CONNECT_ASYNC */
+
static int remove_connection_callback(TALLOC_CTX *mem_ctx)
{
int lret;
@@ -83,21 +126,35 @@ static int sdap_ldap_connect_callback_add(LDAP *ld, Sockbuf *sb,
DEBUG(9, ("New LDAP connection to [%s] with fd [%d].\n",
ldap_url_desc2str(srv), ber_fd));
- fd_event_item = talloc_zero(cb_data, struct fd_event_item);
+ /* We allocate the fd_event_item on a provided memory context.
+ * After the connection is established, we will talloc_steal()
+ * it to cb_data. */
+ if (cb_data->fd_wakeup_req) {
+ fd_event_item = talloc_zero(cb_data->fd_wakeup_req,
+ struct fd_event_item);
+ } else {
+ fd_event_item = talloc_zero(cb_data, struct fd_event_item);
+ }
if (fd_event_item == NULL) {
DEBUG(1, ("talloc failed.\n"));
return ENOMEM;
}
fd_event_item->fde = tevent_add_fd(cb_data->ev, fd_event_item, ber_fd,
+#ifdef LDAP_OPT_CONNECT_ASYNC
+ TEVENT_FD_READ | TEVENT_FD_WRITE,
+ sdap_async_ldap_result,
+#else
TEVENT_FD_READ, sdap_ldap_result,
- cb_data->sh);
+#endif
+ cb_data);
if (fd_event_item->fde == NULL) {
DEBUG(1, ("tevent_add_fd failed.\n"));
talloc_free(fd_event_item);
return ENOMEM;
}
fd_event_item->fd = ber_fd;
+ fd_event_item->wakeup_req = cb_data->fd_wakeup_req;
DLIST_ADD(cb_data->fd_list, fd_event_item);
@@ -140,7 +197,7 @@ static void sdap_ldap_connect_callback_del(LDAP *ld, Sockbuf *sb,
return;
}
-#else
+#else /* !HAVE_LDAP_CONNCB */
static int get_fd_from_ldap(LDAP *ldap, int *fd)
{
@@ -190,10 +247,19 @@ static int sdap_install_ldap_callbacks(struct sdap_handle *sh,
sh, (int)sh->connected, sh->ops, sh->sdap_fd_events->fde,
sh->ldap));
+#ifdef LDAP_OPT_CONNECT_ASYNC
+ ret = ldap_set_option(sh->ldap, LDAP_OPT_CONNECT_ASYNC, LDAP_OPT_ON);
+ if (ret != LDAP_OPT_SUCCESS) {
+ DEBUG(1, ("Failed to set connection as asynchronous\n"));
+ talloc_zfree(sh->sdap_fd_events);
+ return EIO;
+ }
+#endif
+
return EOK;
}
-#endif
+#endif /* HAVE_LDAP_CONNCB */
errno_t setup_ldap_connection_callbacks(struct sdap_handle *sh,
@@ -248,10 +314,10 @@ errno_t setup_ldap_connection_callbacks(struct sdap_handle *sh,
fail:
talloc_zfree(sh->sdap_fd_events);
return ret;
-#else
+#else /* !HAVE_LDAP_CONNCB */
DEBUG(9, ("LDAP connection callbacks are not supported.\n"));
return EOK;
-#endif
+#endif /* HAVE_LDAP_CONNCB */
}
errno_t sdap_set_connected(struct sdap_handle *sh, struct tevent_context *ev)
--
1.6.2.5
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
