Hi,
based on an issue found by Shanks (#757) I had a closer look at some
parts of the enumeration code in the nss responder and found a different
issue (#758) and some non functional code.
0001: should fix #758.
0002: should fix #757. If only the local provider was configured the
code did not return to the main loop during execution and the callback
of the corresponding tevent request was never called.I think this is a
regression from the introduction of the synchronous sysdb interface. To
fix it I added a timer event which just calls *_step() again to make
sure we always return to the main loop at least once. Of someone knows
of a more elegant solution I'm all ears. Btw. the code cries for
refactoring.
0003: the code arround last_user_enum and last_group_enum was not
functional anymore and the enumeration timeout is handled by
*_result_timeout().
From 75f995a30555b74bacabde90c92e4ded8e6d4d7c Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Mon, 3 Jan 2011 16:42:00 +0100
Subject: [PATCH 1/3] Return groups and users from all domains during enumeration
---
src/responder/nss/nsssrv_cmd.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 65d9c58..2decf84 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -1386,7 +1386,8 @@ static int nss_cmd_retpwent(struct cli_ctx *cctx, int num)
if (n <= 0 && (cctx->pwent_dom_idx+1 < pctx->num)) {
cctx->pwent_dom_idx++;
pdom = &pctx->doms[cctx->pwent_dom_idx];
- n = pdom->res->count - cctx->pwent_cur;
+ n = pdom->res->count;
+ cctx->pwent_cur = 0;
}
if (!n) break;
@@ -2564,10 +2565,11 @@ static int nss_cmd_retgrent(struct cli_ctx *cctx, int
num)
gdom = &gctx->doms[cctx->grent_dom_idx];
n = gdom->res->count - cctx->grent_cur;
- if (n <= 0 && (cctx->grent_cur+1 < gctx->num)) {
+ if (n <= 0 && (cctx->grent_dom_idx+1 < gctx->num)) {
cctx->grent_dom_idx++;
gdom = &gctx->doms[cctx->grent_dom_idx];
- n = gdom->res->count - cctx->grent_cur;
+ n = gdom->res->count;
+ cctx->grent_cur = 0;
}
if (!n) break;
--
1.7.3.3
From f829604a8e7ca5a77bfac5834ca32bb465dd6538 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Mon, 3 Jan 2011 16:43:08 +0100
Subject: [PATCH 2/3] Post setgrent requests if necessary
---
src/responder/nss/nsssrv_cmd.c | 122 ++++++++++++++++++++++++++----------
src/responder/nss/nsssrv_private.h | 1 +
2 files changed, 89 insertions(+), 34 deletions(-)
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 2decf84..ea89a9d 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -1025,8 +1025,7 @@ struct tevent_req *nss_cmd_setpwent_send(TALLOC_CTX
*mem_ctx,
goto error;
}
- state->dctx->check_provider =
- NEED_CHECK_PROVIDER(state->dctx->domain->provider);
+ state->dctx->check_provider = true;
/* Is the result context already available */
if (state->nctx->pctx) {
@@ -1081,6 +1080,7 @@ struct tevent_req *nss_cmd_setpwent_send(TALLOC_CTX
*mem_ctx,
step_ctx->getent_ctx = state->getent_ctx;
step_ctx->rctx = client->rctx;
step_ctx->enum_cached = enum_cached;
+ step_ctx->cmd = SSS_NSS_SETPWENT;
ret = nss_cmd_setpwent_step(step_ctx);
if (ret != EOK) goto error;
@@ -1099,6 +1099,11 @@ static void setpwent_result_timeout(struct
tevent_context *ev,
struct tevent_timer *te,
struct timeval current_time,
void *pvt);
+static void no_provider_check_handler(struct tevent_context *ev,
+ struct tevent_timer *te,
+ struct timeval current_time,
+ void *private_data);
+
static errno_t nss_cmd_setpwent_step(struct setent_step_ctx *step_ctx)
{
errno_t ret;
@@ -1124,11 +1129,7 @@ static errno_t nss_cmd_setpwent_step(struct
setent_step_ctx *step_ctx)
if (dom != dctx->domain) {
/* make sure we reset the check_provider flag when we check
* a new domain */
- if (step_ctx->enum_cached) {
- dctx->check_provider = false;
- } else {
- dctx->check_provider = NEED_CHECK_PROVIDER(dom->provider);
- }
+ dctx->check_provider = true;
}
/* make sure to update the dctx if we changed domain */
@@ -1147,18 +1148,30 @@ static errno_t nss_cmd_setpwent_step(struct
setent_step_ctx *step_ctx)
if (dctx->check_provider) {
/* Only do this once per provider */
dctx->check_provider = false;
- timeout = SSS_CLI_SOCKET_TIMEOUT;
- ret = sss_dp_send_acct_req(rctx, step_ctx,
- nss_cmd_setpwent_dp_callback, step_ctx,
- timeout, dom->name, true,
- SSS_DP_USER, NULL, 0);
- if (ret == EOK) {
- return ret;
+ if (NEED_CHECK_PROVIDER(dom->provider)) {
+ timeout = SSS_CLI_SOCKET_TIMEOUT;
+
+ ret = sss_dp_send_acct_req(rctx, step_ctx,
+ nss_cmd_setpwent_dp_callback,
+ step_ctx, timeout, dom->name, true,
+ SSS_DP_USER, NULL, 0);
+ if (ret == EOK) {
+ return ret;
+ } else {
+ DEBUG(2, ("Enum Cache refresh for domain [%s] failed."
+ " Trying to return what we have in cache!\n",
+ dom->name));
+ }
} else {
- DEBUG(2, ("Enum Cache refresh for domain [%s] failed."
- " Trying to return what we have in cache!\n",
- dom->name));
+ tv = tevent_timeval_current();
+ te = tevent_add_timer(rctx->ev, step_ctx, tv,
+ no_provider_check_handler, step_ctx);
+ if (te == NULL) {
+ return ENOMEM;
+ }
+
+ return EOK;
}
}
@@ -2278,8 +2291,7 @@ struct tevent_req *nss_cmd_setgrent_send(TALLOC_CTX
*mem_ctx,
goto error;
}
- state->dctx->check_provider =
- NEED_CHECK_PROVIDER(state->dctx->domain->provider);
+ state->dctx->check_provider = true;
/* Is the result context already available */
if (state->nctx->gctx) {
@@ -2334,6 +2346,7 @@ struct tevent_req *nss_cmd_setgrent_send(TALLOC_CTX
*mem_ctx,
step_ctx->getent_ctx = state->getent_ctx;
step_ctx->rctx = client->rctx;
step_ctx->enum_cached = enum_cached;
+ step_ctx->cmd = SSS_NSS_SETGRENT;
ret = nss_cmd_setgrent_step(step_ctx);
if (ret != EOK) goto error;
@@ -2377,11 +2390,7 @@ static errno_t nss_cmd_setgrent_step(struct
setent_step_ctx *step_ctx)
if (dom != dctx->domain) {
/* make sure we reset the check_provider flag when we check
* a new domain */
- if (step_ctx->enum_cached) {
- dctx->check_provider = false;
- } else {
- dctx->check_provider = NEED_CHECK_PROVIDER(dom->provider);
- }
+ dctx->check_provider = true;
}
/* make sure to update the dctx if we changed domain */
@@ -2400,18 +2409,30 @@ static errno_t nss_cmd_setgrent_step(struct
setent_step_ctx *step_ctx)
if (dctx->check_provider) {
/* Only do this once per provider */
dctx->check_provider = false;
- timeout = SSS_CLI_SOCKET_TIMEOUT;
- ret = sss_dp_send_acct_req(rctx, step_ctx,
- nss_cmd_setgrent_dp_callback, step_ctx,
- timeout, dom->name, true,
- SSS_DP_GROUP, NULL, 0);
- if (ret == EOK) {
- return ret;
+ if (NEED_CHECK_PROVIDER(dom->provider)) {
+ timeout = SSS_CLI_SOCKET_TIMEOUT;
+
+ ret = sss_dp_send_acct_req(rctx, step_ctx,
+ nss_cmd_setgrent_dp_callback,
+ step_ctx, timeout, dom->name, true,
+ SSS_DP_GROUP, NULL, 0);
+ if (ret == EOK) {
+ return ret;
+ } else {
+ DEBUG(2, ("Enum Cache refresh for domain [%s] failed."
+ " Trying to return what we have in cache!\n",
+ dom->name));
+ }
} else {
- DEBUG(2, ("Enum Cache refresh for domain [%s] failed."
- " Trying to return what we have in cache!\n",
- dom->name));
+ tv = tevent_timeval_current();
+ te = tevent_add_timer(rctx->ev, step_ctx, tv,
+ no_provider_check_handler, step_ctx);
+ if (te == NULL) {
+ return ENOMEM;
+ }
+
+ return EOK;
}
}
@@ -2545,6 +2566,39 @@ static void nss_cmd_setgrent_done(struct tevent_req *req)
nss_cmd_done(cmdctx, ret);
}
+static void no_provider_check_handler(struct tevent_context *ev,
+ struct tevent_timer *te,
+ struct timeval current_time,
+ void *private_data)
+{
+ struct setent_step_ctx *step_ctx = talloc_get_type(private_data,
+ struct setent_step_ctx);
+ int ret;
+
+ switch (step_ctx->cmd) {
+ case SSS_NSS_SETPWENT:
+ DEBUG(9, ("Calling nss_cmd_setpwent_step again.\n"));
+ ret = nss_cmd_setpwent_step(step_ctx);
+ break;
+ case SSS_NSS_SETGRENT:
+ DEBUG(9, ("Calling nss_cmd_setgrent_step again.\n"));
+ ret = nss_cmd_setgrent_step(step_ctx);
+ break;
+ default:
+ DEBUG(1, ("Unexpected nss command.\n"))
+ return;
+ }
+
+ if (ret) {
+ /* Notify any waiting processes of failure */
+ while(step_ctx->nctx->pctx->reqs) {
+ tevent_req_error(step_ctx->nctx->pctx->reqs->req, ret);
+ /* Freeing each entry in the list removes it from the dlist */
+ talloc_free(step_ctx->nctx->pctx->reqs);
+ }
+ }
+}
+
static int nss_cmd_retgrent(struct cli_ctx *cctx, int num)
{
struct nss_ctx *nctx;
diff --git a/src/responder/nss/nsssrv_private.h
b/src/responder/nss/nsssrv_private.h
index 4d9f947..be65877 100644
--- a/src/responder/nss/nsssrv_private.h
+++ b/src/responder/nss/nsssrv_private.h
@@ -86,6 +86,7 @@ struct setent_step_ctx {
struct resp_ctx *rctx;
bool enum_cached;
bool check_next;
+ enum sss_cli_command cmd;
/* Netgroup-specific */
char *name;
--
1.7.3.3
From 7baca1d2475b7169392cd206cbb08d84a32f64ae Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Tue, 4 Jan 2011 10:38:40 +0100
Subject: [PATCH 3/3] Remove unused enumeration cache timeout checks
The existence of the getent_ctx is used to track the enumeration cache
timeout.
---
src/responder/nss/nsssrv.h | 2 --
src/responder/nss/nsssrv_cmd.c | 20 --------------------
src/responder/nss/nsssrv_private.h | 1 -
3 files changed, 0 insertions(+), 23 deletions(-)
diff --git a/src/responder/nss/nsssrv.h b/src/responder/nss/nsssrv.h
index 0b7265b..062d937 100644
--- a/src/responder/nss/nsssrv.h
+++ b/src/responder/nss/nsssrv.h
@@ -49,8 +49,6 @@ struct nss_ctx {
int cache_refresh_percent;
int enum_cache_timeout;
- time_t last_user_enum;
- time_t last_group_enum;
struct getent_ctx *pctx;
struct getent_ctx *gctx;
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index ea89a9d..caa74dc 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -981,8 +981,6 @@ struct tevent_req *nss_cmd_setpwent_send(TALLOC_CTX
*mem_ctx,
struct setent_ctx *state;
struct sss_domain_info *dom;
struct setent_step_ctx *step_ctx;
- bool enum_cached = false;
- time_t now = time(NULL);
DEBUG(4, ("Received setpwent request\n"));
nctx = talloc_get_type(client->rctx->pvt_ctx, struct nss_ctx);
@@ -1000,13 +998,6 @@ struct tevent_req *nss_cmd_setpwent_send(TALLOC_CTX
*mem_ctx,
state->nctx = nctx;
state->client = client;
- /* do not query backends if we have a recent enumeration */
- if (nctx->enum_cache_timeout) {
- if (nctx->last_user_enum + nctx->enum_cache_timeout > now) {
- enum_cached = true;
- }
- }
-
state->dctx = talloc_zero(state, struct nss_dom_ctx);
if (!state->dctx) {
ret = ENOMEM;
@@ -1079,7 +1070,6 @@ struct tevent_req *nss_cmd_setpwent_send(TALLOC_CTX
*mem_ctx,
step_ctx->nctx = state->nctx;
step_ctx->getent_ctx = state->getent_ctx;
step_ctx->rctx = client->rctx;
- step_ctx->enum_cached = enum_cached;
step_ctx->cmd = SSS_NSS_SETPWENT;
ret = nss_cmd_setpwent_step(step_ctx);
@@ -2247,8 +2237,6 @@ struct tevent_req *nss_cmd_setgrent_send(TALLOC_CTX
*mem_ctx,
struct setent_ctx *state;
struct sss_domain_info *dom;
struct setent_step_ctx *step_ctx;
- bool enum_cached = false;
- time_t now = time(NULL);
DEBUG(4, ("Received setgrent request\n"));
nctx = talloc_get_type(client->rctx->pvt_ctx, struct nss_ctx);
@@ -2266,13 +2254,6 @@ struct tevent_req *nss_cmd_setgrent_send(TALLOC_CTX
*mem_ctx,
state->nctx = nctx;
state->client = client;
- /* do not query backends if we have a recent enumeration */
- if (nctx->enum_cache_timeout) {
- if (nctx->last_user_enum + nctx->enum_cache_timeout > now) {
- enum_cached = true;
- }
- }
-
state->dctx = talloc_zero(state, struct nss_dom_ctx);
if (!state->dctx) {
ret = ENOMEM;
@@ -2345,7 +2326,6 @@ struct tevent_req *nss_cmd_setgrent_send(TALLOC_CTX
*mem_ctx,
step_ctx->nctx = state->nctx;
step_ctx->getent_ctx = state->getent_ctx;
step_ctx->rctx = client->rctx;
- step_ctx->enum_cached = enum_cached;
step_ctx->cmd = SSS_NSS_SETGRENT;
ret = nss_cmd_setgrent_step(step_ctx);
diff --git a/src/responder/nss/nsssrv_private.h
b/src/responder/nss/nsssrv_private.h
index be65877..b93a1c4 100644
--- a/src/responder/nss/nsssrv_private.h
+++ b/src/responder/nss/nsssrv_private.h
@@ -84,7 +84,6 @@ struct setent_step_ctx {
struct nss_dom_ctx *dctx;
struct getent_ctx *getent_ctx;
struct resp_ctx *rctx;
- bool enum_cached;
bool check_next;
enum sss_cli_command cmd;
--
1.7.3.3
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
