The Samba-Bugzilla – Attachment 14151 Details for
Bug 13292
winbind requests could get stuck in the queue of a busy child, while later requests could get served fine by other children.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patches for v4-7-test
tmp47.diff.txt (text/plain), 48.95 KB, created by
Stefan Metzmacher
on 2018-04-17 06:27:45 UTC
(
hide
)
Description:
Patches for v4-7-test
Filename:
MIME Type:
Creator:
Stefan Metzmacher
Created:
2018-04-17 06:27:45 UTC
Size:
48.95 KB
patch
obsolete
>From c9d90f001dad00039b9c48105fa0cd83ff58990c Mon Sep 17 00:00:00 2001 >From: Volker Lendecke <vl@samba.org> >Date: Fri, 9 Feb 2018 10:27:55 +0100 >Subject: [PATCH 01/14] winbind: Improve child selection > >This improves the situation when a client request blocks a winbind >child. This might be a slow samlogon or lookupnames to a domain that's >far away. With random selection of the child for new request coming in >we could end up with a long queue when other, non-blocked children >could serve those new requests. Choose the shortest queue. > >This is an immediate and simple fix. Step two will be to have a >per-domain and not a per-child queue. Right now we're pre-selecting >the check-out queue at Fry's randomly without looking at the queue >length. With this change we're picking the shortest queue. The better >change will be what Fry's really does: One central queue and red/green >lights on the busy/free checkout counters. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=13290 > >Signed-off-by: Volker Lendecke <vl@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> > >Autobuild-User(master): Volker Lendecke <vl@samba.org> >Autobuild-Date(master): Mon Feb 12 19:51:35 CET 2018 on sn-devel-144 > >(cherry picked from commit b4384b7f0ecf3b47dd60acaf77636b679e3adc05) >--- > source3/winbindd/winbindd_dual.c | 36 +++++++++++++++++------------------- > 1 file changed, 17 insertions(+), 19 deletions(-) > >diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c >index eaa7d9f..59bf389 100644 >--- a/source3/winbindd/winbindd_dual.c >+++ b/source3/winbindd/winbindd_dual.c >@@ -248,33 +248,31 @@ static void wb_child_request_cleanup(struct tevent_req *req, > DLIST_REMOVE(winbindd_children, state->child); > } > >-static bool winbindd_child_busy(struct winbindd_child *child) >-{ >- return tevent_queue_length(child->queue) > 0; >-} >- >-static struct winbindd_child *find_idle_child(struct winbindd_domain *domain) >+struct winbindd_child *choose_domain_child(struct winbindd_domain *domain) > { >+ struct winbindd_child *shortest = &domain->children[0]; >+ struct winbindd_child *current; > int i; > > for (i=0; i<lp_winbind_max_domain_connections(); i++) { >- if (!winbindd_child_busy(&domain->children[i])) { >- return &domain->children[i]; >- } >- } >+ size_t shortest_len, current_len; > >- return NULL; >-} >+ current = &domain->children[i]; >+ current_len = tevent_queue_length(current->queue); > >-struct winbindd_child *choose_domain_child(struct winbindd_domain *domain) >-{ >- struct winbindd_child *result; >+ if (current_len == 0) { >+ /* idle child */ >+ return current; >+ } > >- result = find_idle_child(domain); >- if (result != NULL) { >- return result; >+ shortest_len = tevent_queue_length(shortest->queue); >+ >+ if (current_len < shortest_len) { >+ shortest = current; >+ } > } >- return &domain->children[rand() % lp_winbind_max_domain_connections()]; >+ >+ return shortest; > } > > struct dcerpc_binding_handle *dom_child_handle(struct winbindd_domain *domain) >-- >1.9.1 > > >From 956785b7712569624b34cd952b5642b6aa28effb Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Fri, 16 Feb 2018 15:02:42 +0100 >Subject: [PATCH 02/14] winbind: use tevent_queue_wait_send/recv in > wb_child_request_*() > >We need a way to keep the child->queue blocked without relying on >the current 'req' (wb_child_request_state). > >The next commit will make use of this. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13290 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit d29dda141e08af42c535e8718226f95c45aadab8) >--- > source3/winbindd/winbindd_dual.c | 31 +++++++++++++++++++++++-------- > 1 file changed, 23 insertions(+), 8 deletions(-) > >diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c >index 59bf389..1fa9e18 100644 >--- a/source3/winbindd/winbindd_dual.c >+++ b/source3/winbindd/winbindd_dual.c >@@ -119,6 +119,7 @@ static NTSTATUS child_write_response(int sock, struct winbindd_response *wrsp) > > struct wb_child_request_state { > struct tevent_context *ev; >+ struct tevent_req *queue_subreq; > struct tevent_req *subreq; > struct winbindd_child *child; > struct winbindd_request *request; >@@ -127,8 +128,7 @@ struct wb_child_request_state { > > static bool fork_domain_child(struct winbindd_child *child); > >-static void wb_child_request_trigger(struct tevent_req *req, >- void *private_data); >+static void wb_child_request_waited(struct tevent_req *subreq); > static void wb_child_request_done(struct tevent_req *subreq); > > static void wb_child_request_cleanup(struct tevent_req *req, >@@ -141,6 +141,7 @@ struct tevent_req *wb_child_request_send(TALLOC_CTX *mem_ctx, > { > struct tevent_req *req; > struct wb_child_request_state *state; >+ struct tevent_req *subreq; > > req = tevent_req_create(mem_ctx, &state, > struct wb_child_request_state); >@@ -152,23 +153,36 @@ struct tevent_req *wb_child_request_send(TALLOC_CTX *mem_ctx, > state->child = child; > state->request = request; > >- if (!tevent_queue_add(child->queue, ev, req, >- wb_child_request_trigger, NULL)) { >- tevent_req_oom(req); >+ subreq = tevent_queue_wait_send(state, ev, child->queue); >+ if (tevent_req_nomem(subreq, req)) { > return tevent_req_post(req, ev); > } >+ tevent_req_set_callback(subreq, wb_child_request_waited, req); >+ state->queue_subreq = subreq; > > tevent_req_set_cleanup_fn(req, wb_child_request_cleanup); > > return req; > } > >-static void wb_child_request_trigger(struct tevent_req *req, >- void *private_data) >+static void wb_child_request_waited(struct tevent_req *subreq) > { >+ struct tevent_req *req = tevent_req_callback_data( >+ subreq, struct tevent_req); > struct wb_child_request_state *state = tevent_req_data( > req, struct wb_child_request_state); >- struct tevent_req *subreq; >+ bool ok; >+ >+ ok = tevent_queue_wait_recv(subreq); >+ if (!ok) { >+ tevent_req_oom(req); >+ return; >+ } >+ /* >+ * We need to keep state->queue_subreq >+ * in order to block the queue. >+ */ >+ subreq = NULL; > > if ((state->child->sock == -1) && (!fork_domain_child(state->child))) { > tevent_req_error(req, errno); >@@ -231,6 +245,7 @@ static void wb_child_request_cleanup(struct tevent_req *req, > } > > TALLOC_FREE(state->subreq); >+ TALLOC_FREE(state->queue_subreq); > > if (req_state == TEVENT_REQ_DONE) { > /* transmitted request and got response */ >-- >1.9.1 > > >From 3930e31e48a320de89a42899aca6b48648fe3f7b Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Fri, 16 Feb 2018 15:05:57 +0100 >Subject: [PATCH 03/14] winbind: protect a pending wb_child_request against a > talloc_free() > >If the (winbind) client gave up we call TALLOC_FREE(state->mem_ctx) >in remove_client(). This triggers a recursive talloc_free() for all >in flight requests. > >In order to maintain the winbindd parent-child protocol, we need >to keep the orphaned wb_simple_trans request until the parent >got the response from the child. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13290 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit 43af57d8728883c5ddbe169e1483181246fb68a8) >--- > source3/winbindd/winbindd_dual.c | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > >diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c >index 1fa9e18..e3b3b09 100644 >--- a/source3/winbindd/winbindd_dual.c >+++ b/source3/winbindd/winbindd_dual.c >@@ -130,6 +130,7 @@ static bool fork_domain_child(struct winbindd_child *child); > > static void wb_child_request_waited(struct tevent_req *subreq); > static void wb_child_request_done(struct tevent_req *subreq); >+static void wb_child_request_orphaned(struct tevent_req *subreq); > > static void wb_child_request_cleanup(struct tevent_req *req, > enum tevent_req_state req_state); >@@ -220,6 +221,12 @@ static void wb_child_request_done(struct tevent_req *subreq) > tevent_req_done(req); > } > >+static void wb_child_request_orphaned(struct tevent_req *subreq) >+{ >+ DBG_WARNING("cleanup orphaned subreq[%p]\n", subreq); >+ TALLOC_FREE(subreq); >+} >+ > int wb_child_request_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, > struct winbindd_response **presponse, int *err) > { >@@ -244,6 +251,28 @@ static void wb_child_request_cleanup(struct tevent_req *req, > return; > } > >+ if (req_state == TEVENT_REQ_RECEIVED) { >+ struct tevent_req *subreq = NULL; >+ >+ /* >+ * Our caller gave up, but we need to keep >+ * the low level request (wb_simple_trans) >+ * in order to maintain the parent child protocol. >+ * >+ * We also need to keep the child queue blocked >+ * until we got the response from the child. >+ */ >+ >+ subreq = talloc_move(state->child->queue, &state->subreq); >+ talloc_move(subreq, &state->queue_subreq); >+ tevent_req_set_callback(subreq, >+ wb_child_request_orphaned, >+ NULL); >+ >+ DBG_WARNING("keep orphaned subreq[%p]\n", subreq); >+ return; >+ } >+ > TALLOC_FREE(state->subreq); > TALLOC_FREE(state->queue_subreq); > >-- >1.9.1 > > >From a7c102971981edafab5ed042a51cb7c0883b964e Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Fri, 16 Feb 2018 16:09:58 +0100 >Subject: [PATCH 04/14] winbind: call lp_winbind_enum_{users,groups}() already > in set{pw,gr}ent() > >This way we don't keep winbindd_cli_state->{pw,gr}ent_state arround forever, >if the client forgets an explicit end{pw,gr}ent(). > >This allows client_is_idle() return true in more cases. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13293 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit 6548b82b5c1ed30ce14e17e4ba9d4bc24ab49c42) >--- > source3/winbindd/winbindd_getgrent.c | 5 ----- > source3/winbindd/winbindd_getpwent.c | 5 ----- > source3/winbindd/winbindd_setgrent.c | 5 +++++ > source3/winbindd/winbindd_setpwent.c | 5 +++++ > 4 files changed, 10 insertions(+), 10 deletions(-) > >diff --git a/source3/winbindd/winbindd_getgrent.c b/source3/winbindd/winbindd_getgrent.c >index 21da75b..6a99c6b 100644 >--- a/source3/winbindd/winbindd_getgrent.c >+++ b/source3/winbindd/winbindd_getgrent.c >@@ -50,11 +50,6 @@ struct tevent_req *winbindd_getgrent_send(TALLOC_CTX *mem_ctx, > > DEBUG(3, ("[%5lu]: getgrent\n", (unsigned long)cli->pid)); > >- if (!lp_winbind_enum_groups()) { >- tevent_req_nterror(req, NT_STATUS_NO_MORE_ENTRIES); >- return tevent_req_post(req, ev); >- } >- > if (cli->grent_state == NULL) { > tevent_req_nterror(req, NT_STATUS_NO_MORE_ENTRIES); > return tevent_req_post(req, ev); >diff --git a/source3/winbindd/winbindd_getpwent.c b/source3/winbindd/winbindd_getpwent.c >index 3c035ea..d33f5f9 100644 >--- a/source3/winbindd/winbindd_getpwent.c >+++ b/source3/winbindd/winbindd_getpwent.c >@@ -49,11 +49,6 @@ struct tevent_req *winbindd_getpwent_send(TALLOC_CTX *mem_ctx, > > DEBUG(3, ("[%5lu]: getpwent\n", (unsigned long)cli->pid)); > >- if (!lp_winbind_enum_users()) { >- tevent_req_nterror(req, NT_STATUS_NO_MORE_ENTRIES); >- return tevent_req_post(req, ev); >- } >- > if (cli->pwent_state == NULL) { > tevent_req_nterror(req, NT_STATUS_NO_MORE_ENTRIES); > return tevent_req_post(req, ev); >diff --git a/source3/winbindd/winbindd_setgrent.c b/source3/winbindd/winbindd_setgrent.c >index 79aa8c3..ab7fa98 100644 >--- a/source3/winbindd/winbindd_setgrent.c >+++ b/source3/winbindd/winbindd_setgrent.c >@@ -39,6 +39,11 @@ struct tevent_req *winbindd_setgrent_send(TALLOC_CTX *mem_ctx, > } > TALLOC_FREE(cli->grent_state); > >+ if (!lp_winbind_enum_groups()) { >+ tevent_req_done(req); >+ return tevent_req_post(req, ev); >+ } >+ > cli->grent_state = talloc_zero(cli, struct getgrent_state); > if (tevent_req_nomem(cli->grent_state, req)) { > return tevent_req_post(req, ev); >diff --git a/source3/winbindd/winbindd_setpwent.c b/source3/winbindd/winbindd_setpwent.c >index af28758..4591731 100644 >--- a/source3/winbindd/winbindd_setpwent.c >+++ b/source3/winbindd/winbindd_setpwent.c >@@ -39,6 +39,11 @@ struct tevent_req *winbindd_setpwent_send(TALLOC_CTX *mem_ctx, > } > TALLOC_FREE(cli->pwent_state); > >+ if (!lp_winbind_enum_users()) { >+ tevent_req_done(req); >+ return tevent_req_post(req, ev); >+ } >+ > cli->pwent_state = talloc_zero(cli, struct getpwent_state); > if (tevent_req_nomem(cli->pwent_state, req)) { > return tevent_req_post(req, ev); >-- >1.9.1 > > >From e893cd0c84d33c6fab078abdce69a9621828ba71 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Fri, 16 Feb 2018 16:13:16 +0100 >Subject: [PATCH 05/14] winbind: cleanup winbindd_cli_state->grent_state if > winbindd_getgrent_recv() returns an error > >A client may skip the explicit endgrent() if getgrent() fails. > >This allows client_is_idle() return true in more cases. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13293 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit b7789da8468c3f070727011639d5f74aca76cb59) >--- > source3/winbindd/winbindd_getgrent.c | 4 ++++ > 1 file changed, 4 insertions(+) > >diff --git a/source3/winbindd/winbindd_getgrent.c b/source3/winbindd/winbindd_getgrent.c >index 6a99c6b..ce1d1e6 100644 >--- a/source3/winbindd/winbindd_getgrent.c >+++ b/source3/winbindd/winbindd_getgrent.c >@@ -141,6 +141,7 @@ NTSTATUS winbindd_getgrent_recv(struct tevent_req *req, > int i; > > if (tevent_req_is_nterror(req, &status)) { >+ TALLOC_FREE(state->cli->grent_state); > DEBUG(5, ("getgrent failed: %s\n", nt_errstr(status))); > return status; > } >@@ -151,6 +152,7 @@ NTSTATUS winbindd_getgrent_recv(struct tevent_req *req, > > memberstrings = talloc_array(talloc_tos(), char *, state->num_groups); > if (memberstrings == NULL) { >+ TALLOC_FREE(state->cli->grent_state); > return NT_STATUS_NO_MEMORY; > } > >@@ -165,6 +167,7 @@ NTSTATUS winbindd_getgrent_recv(struct tevent_req *req, > > if (!NT_STATUS_IS_OK(status)) { > TALLOC_FREE(memberstrings); >+ TALLOC_FREE(state->cli->grent_state); > return status; > } > TALLOC_FREE(state->members[i]); >@@ -180,6 +183,7 @@ NTSTATUS winbindd_getgrent_recv(struct tevent_req *req, > result = talloc_realloc(state, state->groups, char, > base_memberofs + total_memberlen); > if (result == NULL) { >+ TALLOC_FREE(state->cli->grent_state); > return NT_STATUS_NO_MEMORY; > } > state->groups = (struct winbindd_gr *)result; >-- >1.9.1 > > >From d9a0b25fd3e298ef751c8a26a1cf12595a376ac6 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Fri, 16 Feb 2018 16:13:16 +0100 >Subject: [PATCH 06/14] winbind: cleanup winbindd_cli_state->pwent_state if > winbindd_getpwent_recv() returns an error > >A client may skip the explicit endpwent() if getgrent() fails. > >This allows client_is_idle() return true in more cases. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13293 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit b158d4e4c1c3fee0a8884bc5e8f0c5a5ce49687f) >--- > source3/winbindd/winbindd_getpwent.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/source3/winbindd/winbindd_getpwent.c b/source3/winbindd/winbindd_getpwent.c >index d33f5f9..9e5190a 100644 >--- a/source3/winbindd/winbindd_getpwent.c >+++ b/source3/winbindd/winbindd_getpwent.c >@@ -124,6 +124,7 @@ NTSTATUS winbindd_getpwent_recv(struct tevent_req *req, > NTSTATUS status; > > if (tevent_req_is_nterror(req, &status)) { >+ TALLOC_FREE(state->cli->pwent_state); > DEBUG(5, ("getpwent failed: %s\n", nt_errstr(status))); > return status; > } >-- >1.9.1 > > >From ce8c4af3a7e5c7a3913fdf722fd94e9652d5eed9 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 15 Feb 2018 16:00:33 +0100 >Subject: [PATCH 07/14] winbind: avoid using fstrcpy(dcname,...) in > _dual_init_connection > >domain->dcname was converted from fstring to char * by commit >14bae61ba36814ea5eca7c51cf1cc039e9e6803f. > >Luckily this was only ever called with an empty string in >state->request->data.init_conn.dcname. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13294 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >(cherry picked from commit d73e3d451976e692c6c346f98547d7123f7b9006) >--- > source3/winbindd/winbindd_util.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > >diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c >index 370b70c..89b85ea 100644 >--- a/source3/winbindd/winbindd_util.c >+++ b/source3/winbindd/winbindd_util.c >@@ -653,7 +653,12 @@ enum winbindd_result winbindd_dual_init_connection(struct winbindd_domain *domai > [sizeof(state->request->data.init_conn.dcname)-1]='\0'; > > if (strlen(state->request->data.init_conn.dcname) > 0) { >- fstrcpy(domain->dcname, state->request->data.init_conn.dcname); >+ TALLOC_FREE(domain->dcname); >+ domain->dcname = talloc_strdup(domain, >+ state->request->data.init_conn.dcname); >+ if (domain->dcname == NULL) { >+ return WINBINDD_ERROR; >+ } > } > > init_dc_connection(domain, false); >-- >1.9.1 > > >From 61fa3216f6d8fa7a62ce45e7957be5f41d379e4a Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Wed, 14 Feb 2018 15:09:51 +0100 >Subject: [PATCH 08/14] winbind: use state->{ev,request} in > wb_domain_request_send() > >This will reduce the diff for the following changes. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13295 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit 4d804f5f3e65df0e2f646d4f88793cab8e2f32d1) >--- > source3/winbindd/winbindd_dual.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > >diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c >index e3b3b09..5045fa0 100644 >--- a/source3/winbindd/winbindd_dual.c >+++ b/source3/winbindd/winbindd_dual.c >@@ -354,11 +354,15 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx, > return NULL; > } > >+ state->domain = domain; >+ state->ev = ev; >+ state->request = request; >+ > state->child = choose_domain_child(domain); > > if (domain->initialized) { >- subreq = wb_child_request_send(state, ev, state->child, >- request); >+ subreq = wb_child_request_send(state, state->ev, state->child, >+ state->request); > if (tevent_req_nomem(subreq, req)) { > return tevent_req_post(req, ev); > } >@@ -366,10 +370,6 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx, > return req; > } > >- state->domain = domain; >- state->ev = ev; >- state->request = request; >- > state->init_req = talloc_zero(state, struct winbindd_request); > if (tevent_req_nomem(state->init_req, req)) { > return tevent_req_post(req, ev); >@@ -382,7 +382,7 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx, > state->init_req->data.init_conn.is_primary = domain->primary; > fstrcpy(state->init_req->data.init_conn.dcname, ""); > >- subreq = wb_child_request_send(state, ev, state->child, >+ subreq = wb_child_request_send(state, state->ev, state->child, > state->init_req); > if (tevent_req_nomem(subreq, req)) { > return tevent_req_post(req, ev); >@@ -403,7 +403,8 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx, > state->init_req->cmd = WINBINDD_GETDCNAME; > fstrcpy(state->init_req->domain_name, domain->name); > >- subreq = wb_child_request_send(state, ev, state->child, request); >+ subreq = wb_child_request_send(state, state->ev, state->child, >+ state->request); > if (tevent_req_nomem(subreq, req)) { > return tevent_req_post(req, ev); > } >-- >1.9.1 > > >From caf279a97fee935cf62649a6f88f72edbc1650d9 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Wed, 14 Feb 2018 15:11:50 +0100 >Subject: [PATCH 09/14] winbind: improve wb_domain_request_send() to use > wb_dsgetdcname_send() for a foreign domain > >Commit ed3bc614cccec6167c64ac58d78344b6426cd019 got the logic wrong while >trying to implement the logic we had in init_child_connection(), >which was removed by commit d61f3626b79e0523beadff355453145aa7b0195c. > >Instead of doing a WINBINDD_GETDCNAME request (which would caused an error >because the implementation was removed in commit >958fdaf5c3ba17969a5110e6b2b08babb9096d7e), we sent the callers request >and interpreted the result as WINBINDD_GETDCNAME response, which >led to an empty dcname variable. As result the domain child >opened a connection to the primary domain in order to lookup >a dc. > >If we want to connect the primary domain from the parent via >a domain child of the primary domain. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13295 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit 1f41193e005df37401a28004f0a95d4d73b98ccd) >--- > source3/winbindd/winbindd_dual.c | 40 ++++++++++++++++++++++------------------ > 1 file changed, 22 insertions(+), 18 deletions(-) > >diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c >index 5045fa0..06e3c95 100644 >--- a/source3/winbindd/winbindd_dual.c >+++ b/source3/winbindd/winbindd_dual.c >@@ -393,18 +393,18 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx, > } > > /* >- * Ask our DC for a DC name >+ * This is *not* the primary domain, >+ * let's ask our DC about a DC name. >+ * >+ * We prefer getting a dns name in dc_unc, >+ * which is indicated by DS_RETURN_DNS_NAME. >+ * For NT4 domains we still get the netbios name. > */ >- domain = find_our_domain(); >- >- /* This is *not* the primary domain, let's ask our DC about a DC >- * name */ >- >- state->init_req->cmd = WINBINDD_GETDCNAME; >- fstrcpy(state->init_req->domain_name, domain->name); >- >- subreq = wb_child_request_send(state, state->ev, state->child, >- state->request); >+ subreq = wb_dsgetdcname_send(state, state->ev, >+ state->domain->name, >+ NULL, /* domain_guid */ >+ NULL, /* site_name */ >+ DS_RETURN_DNS_NAME); /* flags */ > if (tevent_req_nomem(subreq, req)) { > return tevent_req_post(req, ev); > } >@@ -418,22 +418,26 @@ static void wb_domain_request_gotdc(struct tevent_req *subreq) > subreq, struct tevent_req); > struct wb_domain_request_state *state = tevent_req_data( > req, struct wb_domain_request_state); >- struct winbindd_response *response; >- int ret, err; >+ struct netr_DsRGetDCNameInfo *dcinfo = NULL; >+ NTSTATUS status; >+ const char *dcname = NULL; > >- ret = wb_child_request_recv(subreq, talloc_tos(), &response, &err); >+ status = wb_dsgetdcname_recv(subreq, state, &dcinfo); > TALLOC_FREE(subreq); >- if (ret == -1) { >- tevent_req_error(req, err); >+ if (tevent_req_nterror(req, status)) { > return; > } >+ dcname = dcinfo->dc_unc; >+ while (dcname != NULL && *dcname == '\\') { >+ dcname++; >+ } > state->init_req->cmd = WINBINDD_INIT_CONNECTION; > fstrcpy(state->init_req->domain_name, state->domain->name); > state->init_req->data.init_conn.is_primary = False; > fstrcpy(state->init_req->data.init_conn.dcname, >- response->data.dc_name); >+ dcname); > >- TALLOC_FREE(response); >+ TALLOC_FREE(dcinfo); > > subreq = wb_child_request_send(state, state->ev, state->child, > state->init_req); >-- >1.9.1 > > >From a41a6048ed3929091b243cc5969a9abff278d6b5 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Wed, 14 Feb 2018 13:24:54 +0100 >Subject: [PATCH 10/14] winbind: add idmap_child_handle() and use it instead of > child->binding_handle > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13292 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit c2d78a0a0a3f9b9ade61cf707f23e59a1a16c61b) >--- > source3/winbindd/wb_sids2xids.c | 6 +++--- > source3/winbindd/winbindd_allocate_gid.c | 6 +++--- > source3/winbindd/winbindd_allocate_uid.c | 6 +++--- > source3/winbindd/winbindd_idmap.c | 5 +++++ > source3/winbindd/winbindd_proto.h | 1 + > 5 files changed, 15 insertions(+), 9 deletions(-) > >diff --git a/source3/winbindd/wb_sids2xids.c b/source3/winbindd/wb_sids2xids.c >index b8ad300..c687f70 100644 >--- a/source3/winbindd/wb_sids2xids.c >+++ b/source3/winbindd/wb_sids2xids.c >@@ -167,7 +167,7 @@ static void wb_sids2xids_lookupsids_done(struct tevent_req *subreq) > req, struct wb_sids2xids_state); > struct lsa_RefDomainList *domains = NULL; > struct lsa_TransNameArray *names = NULL; >- struct winbindd_child *child; >+ struct dcerpc_binding_handle *child_binding_handle = NULL; > NTSTATUS status; > int i; > >@@ -237,7 +237,7 @@ static void wb_sids2xids_lookupsids_done(struct tevent_req *subreq) > TALLOC_FREE(names); > TALLOC_FREE(domains); > >- child = idmap_child(); >+ child_binding_handle = idmap_child_handle(); > > state->dom_ids = wb_sids2xids_extract_for_domain_index( > state, &state->ids, state->dom_index); >@@ -252,7 +252,7 @@ static void wb_sids2xids_lookupsids_done(struct tevent_req *subreq) > }; > > subreq = dcerpc_wbint_Sids2UnixIDs_send( >- state, state->ev, child->binding_handle, &state->idmap_dom, >+ state, state->ev, child_binding_handle, &state->idmap_dom, > state->dom_ids); > if (tevent_req_nomem(subreq, req)) { > return; >diff --git a/source3/winbindd/winbindd_allocate_gid.c b/source3/winbindd/winbindd_allocate_gid.c >index a9236bb..85aa136 100644 >--- a/source3/winbindd/winbindd_allocate_gid.c >+++ b/source3/winbindd/winbindd_allocate_gid.c >@@ -34,7 +34,7 @@ struct tevent_req *winbindd_allocate_gid_send(TALLOC_CTX *mem_ctx, > { > struct tevent_req *req, *subreq; > struct winbindd_allocate_gid_state *state; >- struct winbindd_child *child; >+ struct dcerpc_binding_handle *child_binding_handle = NULL; > > req = tevent_req_create(mem_ctx, &state, > struct winbindd_allocate_gid_state); >@@ -44,9 +44,9 @@ struct tevent_req *winbindd_allocate_gid_send(TALLOC_CTX *mem_ctx, > > DEBUG(3, ("allocate_gid\n")); > >- child = idmap_child(); >+ child_binding_handle = idmap_child_handle(); > >- subreq = dcerpc_wbint_AllocateGid_send(state, ev, child->binding_handle, >+ subreq = dcerpc_wbint_AllocateGid_send(state, ev, child_binding_handle, > &state->gid); > if (tevent_req_nomem(subreq, req)) { > return tevent_req_post(req, ev); >diff --git a/source3/winbindd/winbindd_allocate_uid.c b/source3/winbindd/winbindd_allocate_uid.c >index 99c0bdac..69ce61c 100644 >--- a/source3/winbindd/winbindd_allocate_uid.c >+++ b/source3/winbindd/winbindd_allocate_uid.c >@@ -34,7 +34,7 @@ struct tevent_req *winbindd_allocate_uid_send(TALLOC_CTX *mem_ctx, > { > struct tevent_req *req, *subreq; > struct winbindd_allocate_uid_state *state; >- struct winbindd_child *child; >+ struct dcerpc_binding_handle *child_binding_handle = NULL; > > req = tevent_req_create(mem_ctx, &state, > struct winbindd_allocate_uid_state); >@@ -44,9 +44,9 @@ struct tevent_req *winbindd_allocate_uid_send(TALLOC_CTX *mem_ctx, > > DEBUG(3, ("allocate_uid\n")); > >- child = idmap_child(); >+ child_binding_handle = idmap_child_handle(); > >- subreq = dcerpc_wbint_AllocateUid_send(state, ev, child->binding_handle, >+ subreq = dcerpc_wbint_AllocateUid_send(state, ev, child_binding_handle, > &state->uid); > if (tevent_req_nomem(subreq, req)) { > return tevent_req_post(req, ev); >diff --git a/source3/winbindd/winbindd_idmap.c b/source3/winbindd/winbindd_idmap.c >index 0280260..2ee436b 100644 >--- a/source3/winbindd/winbindd_idmap.c >+++ b/source3/winbindd/winbindd_idmap.c >@@ -34,6 +34,11 @@ struct winbindd_child *idmap_child(void) > return &static_idmap_child; > } > >+struct dcerpc_binding_handle *idmap_child_handle(void) >+{ >+ return static_idmap_child.binding_handle; >+} >+ > static const struct winbindd_child_dispatch_table idmap_dispatch_table[] = { > { > .name = "PING", >diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h >index e45d9b5..29b0e7e 100644 >--- a/source3/winbindd/winbindd_proto.h >+++ b/source3/winbindd/winbindd_proto.h >@@ -372,6 +372,7 @@ NTSTATUS winbindd_print_groupmembers(struct talloc_dict *members, > > void init_idmap_child(void); > struct winbindd_child *idmap_child(void); >+struct dcerpc_binding_handle *idmap_child_handle(void); > struct idmap_domain *idmap_find_domain_with_sid(const char *domname, > const struct dom_sid *sid); > const char *idmap_config_const_string(const char *domname, const char *option, >-- >1.9.1 > > >From 3d2ee84f8bb0066674ceb7addc5783b41420980c Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Wed, 14 Feb 2018 13:24:54 +0100 >Subject: [PATCH 11/14] winbind: add locator_child_handle() and use it instead > of child->binding_handle > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13292 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit 44ebaaac8933f5fc16a043b8c15a9449746af47b) >--- > source3/winbindd/wb_dsgetdcname.c | 8 ++++---- > source3/winbindd/winbindd_dsgetdcname.c | 6 +++--- > source3/winbindd/winbindd_locator.c | 5 +++++ > source3/winbindd/winbindd_proto.h | 1 + > 4 files changed, 13 insertions(+), 7 deletions(-) > >diff --git a/source3/winbindd/wb_dsgetdcname.c b/source3/winbindd/wb_dsgetdcname.c >index 125e98a..bfc6aa1 100644 >--- a/source3/winbindd/wb_dsgetdcname.c >+++ b/source3/winbindd/wb_dsgetdcname.c >@@ -37,7 +37,7 @@ struct tevent_req *wb_dsgetdcname_send(TALLOC_CTX *mem_ctx, > { > struct tevent_req *req, *subreq; > struct wb_dsgetdcname_state *state; >- struct winbindd_child *child; >+ struct dcerpc_binding_handle *child_binding_handle = NULL; > struct GUID guid; > struct GUID *guid_ptr = NULL; > >@@ -72,10 +72,10 @@ struct tevent_req *wb_dsgetdcname_send(TALLOC_CTX *mem_ctx, > /* > * We have to figure out the DC ourselves > */ >- child = locator_child(); >+ child_binding_handle = locator_child_handle(); > } else { > struct winbindd_domain *domain = find_our_domain(); >- child = choose_domain_child(domain); >+ child_binding_handle = dom_child_handle(domain); > } > > if (domain_guid != NULL) { >@@ -85,7 +85,7 @@ struct tevent_req *wb_dsgetdcname_send(TALLOC_CTX *mem_ctx, > } > > subreq = dcerpc_wbint_DsGetDcName_send( >- state, ev, child->binding_handle, domain_name, guid_ptr, site_name, >+ state, ev, child_binding_handle, domain_name, guid_ptr, site_name, > flags, &state->dcinfo); > if (tevent_req_nomem(subreq, req)) { > return tevent_req_post(req, ev); >diff --git a/source3/winbindd/winbindd_dsgetdcname.c b/source3/winbindd/winbindd_dsgetdcname.c >index 8eb1de7..fd9270f 100644 >--- a/source3/winbindd/winbindd_dsgetdcname.c >+++ b/source3/winbindd/winbindd_dsgetdcname.c >@@ -35,7 +35,7 @@ struct tevent_req *winbindd_dsgetdcname_send(TALLOC_CTX *mem_ctx, > struct winbindd_request *request) > { > struct tevent_req *req, *subreq; >- struct winbindd_child *child; >+ struct dcerpc_binding_handle *child_binding_handle = NULL; > struct winbindd_dsgetdcname_state *state; > struct GUID *guid_ptr = NULL; > uint32_t ds_flags = 0; >@@ -65,10 +65,10 @@ struct tevent_req *winbindd_dsgetdcname_send(TALLOC_CTX *mem_ctx, > guid_ptr = &state->guid; > } > >- child = locator_child(); >+ child_binding_handle = locator_child_handle(); > > subreq = dcerpc_wbint_DsGetDcName_send( >- state, ev, child->binding_handle, >+ state, ev, child_binding_handle, > request->data.dsgetdcname.domain_name, guid_ptr, > request->data.dsgetdcname.site_name, > ds_flags, &state->dc_info); >diff --git a/source3/winbindd/winbindd_locator.c b/source3/winbindd/winbindd_locator.c >index 59e8614..55b6455 100644 >--- a/source3/winbindd/winbindd_locator.c >+++ b/source3/winbindd/winbindd_locator.c >@@ -34,6 +34,11 @@ struct winbindd_child *locator_child(void) > return &static_locator_child; > } > >+struct dcerpc_binding_handle *locator_child_handle(void) >+{ >+ return static_locator_child.binding_handle; >+} >+ > static const struct winbindd_child_dispatch_table locator_dispatch_table[] = { > { > .name = "PING", >diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h >index 29b0e7e..ae48796 100644 >--- a/source3/winbindd/winbindd_proto.h >+++ b/source3/winbindd/winbindd_proto.h >@@ -388,6 +388,7 @@ bool lp_scan_idmap_domains(bool (*fn)(const char *domname, > > void init_locator_child(void); > struct winbindd_child *locator_child(void); >+struct dcerpc_binding_handle *locator_child_handle(void); > > /* The following definitions come from winbindd/winbindd_misc.c */ > >-- >1.9.1 > > >From 28c94f99baa2eade0b8ed89dedd82f7c6b138a7e Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Tue, 20 Feb 2018 14:43:38 +0100 >Subject: [PATCH 12/14] winbind: make choose_domain_child() static > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13292 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit 5116aff286bdffe4abc9ddda09cf64ab999fd13e) >--- > source3/winbindd/winbindd_dual.c | 2 +- > source3/winbindd/winbindd_proto.h | 1 - > 2 files changed, 1 insertion(+), 2 deletions(-) > >diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c >index 06e3c95..6d249fe 100644 >--- a/source3/winbindd/winbindd_dual.c >+++ b/source3/winbindd/winbindd_dual.c >@@ -292,7 +292,7 @@ static void wb_child_request_cleanup(struct tevent_req *req, > DLIST_REMOVE(winbindd_children, state->child); > } > >-struct winbindd_child *choose_domain_child(struct winbindd_domain *domain) >+static struct winbindd_child *choose_domain_child(struct winbindd_domain *domain) > { > struct winbindd_child *shortest = &domain->children[0]; > struct winbindd_child *current; >diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h >index ae48796..a300a53 100644 >--- a/source3/winbindd/winbindd_proto.h >+++ b/source3/winbindd/winbindd_proto.h >@@ -295,7 +295,6 @@ void setup_domain_child(struct winbindd_domain *domain); > /* The following definitions come from winbindd/winbindd_dual.c */ > > struct dcerpc_binding_handle *dom_child_handle(struct winbindd_domain *domain); >-struct winbindd_child *choose_domain_child(struct winbindd_domain *domain); > > struct tevent_req *wb_child_request_send(TALLOC_CTX *mem_ctx, > struct tevent_context *ev, >-- >1.9.1 > > >From e13ef0bd9c8f6436b337a5b89fec4e79ee8086fc Mon Sep 17 00:00:00 2001 >From: Volker Lendecke <vl@samba.org> >Date: Tue, 13 Feb 2018 16:04:44 +0100 >Subject: [PATCH 13/14] winbind: Maintain a binding handle per domain and > always go via wb_domain_request_send() > >Pair-Programmed-With: Stefan Metzmacher <metze@samba.org> > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13292 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Signed-off-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit b518cb0597d269002105644302c58ca8f9f0f717) >--- > source3/winbindd/winbindd.h | 2 ++ > source3/winbindd/winbindd_dual.c | 11 +++---- > source3/winbindd/winbindd_dual_ndr.c | 61 +++++++++++++++++++++++++++++++----- > source3/winbindd/winbindd_util.c | 6 ++++ > 4 files changed, 66 insertions(+), 14 deletions(-) > >diff --git a/source3/winbindd/winbindd.h b/source3/winbindd/winbindd.h >index 1c7e4e9..25e4812 100644 >--- a/source3/winbindd/winbindd.h >+++ b/source3/winbindd/winbindd.h >@@ -185,6 +185,8 @@ struct winbindd_domain { > > struct winbindd_child *children; > >+ struct dcerpc_binding_handle *binding_handle; >+ > /* Callback we use to try put us back online. */ > > uint32_t check_online_timeout; >diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c >index 6d249fe..89823b7 100644 >--- a/source3/winbindd/winbindd_dual.c >+++ b/source3/winbindd/winbindd_dual.c >@@ -321,10 +321,7 @@ static struct winbindd_child *choose_domain_child(struct winbindd_domain *domain > > struct dcerpc_binding_handle *dom_child_handle(struct winbindd_domain *domain) > { >- struct winbindd_child *child; >- >- child = choose_domain_child(domain); >- return child->binding_handle; >+ return domain->binding_handle; > } > > struct wb_domain_request_state { >@@ -608,8 +605,10 @@ void setup_child(struct winbindd_domain *domain, struct winbindd_child *child, > child->table = table; > child->queue = tevent_queue_create(NULL, "winbind_child"); > SMB_ASSERT(child->queue != NULL); >- child->binding_handle = wbint_binding_handle(NULL, domain, child); >- SMB_ASSERT(child->binding_handle != NULL); >+ if (domain == NULL) { >+ child->binding_handle = wbint_binding_handle(NULL, NULL, child); >+ SMB_ASSERT(child->binding_handle != NULL); >+ } > } > > void winbind_child_died(pid_t pid) >diff --git a/source3/winbindd/winbindd_dual_ndr.c b/source3/winbindd/winbindd_dual_ndr.c >index 250d9d3..fca1996 100644 >--- a/source3/winbindd/winbindd_dual_ndr.c >+++ b/source3/winbindd/winbindd_dual_ndr.c >@@ -42,7 +42,7 @@ static bool wbint_bh_is_connected(struct dcerpc_binding_handle *h) > struct wbint_bh_state *hs = dcerpc_binding_handle_data(h, > struct wbint_bh_state); > >- if (!hs->child) { >+ if ((hs->domain == NULL) && (hs->child == NULL)) { > return false; > } > >@@ -65,7 +65,8 @@ struct wbint_bh_raw_call_state { > DATA_BLOB out_data; > }; > >-static void wbint_bh_raw_call_done(struct tevent_req *subreq); >+static void wbint_bh_raw_call_child_done(struct tevent_req *subreq); >+static void wbint_bh_raw_call_domain_done(struct tevent_req *subreq); > > static struct tevent_req *wbint_bh_raw_call_send(TALLOC_CTX *mem_ctx, > struct tevent_context *ev, >@@ -112,17 +113,28 @@ static struct tevent_req *wbint_bh_raw_call_send(TALLOC_CTX *mem_ctx, > state->request.extra_data.data = (char *)state->in_data.data; > state->request.extra_len = state->in_data.length; > >- subreq = wb_child_request_send(state, ev, hs->child, >- &state->request); >+ if (hs->child != NULL) { >+ subreq = wb_child_request_send(state, ev, hs->child, >+ &state->request); >+ if (tevent_req_nomem(subreq, req)) { >+ return tevent_req_post(req, ev); >+ } >+ tevent_req_set_callback( >+ subreq, wbint_bh_raw_call_child_done, req); >+ return req; >+ } >+ >+ subreq = wb_domain_request_send(state, ev, hs->domain, >+ &state->request); > if (tevent_req_nomem(subreq, req)) { > return tevent_req_post(req, ev); > } >- tevent_req_set_callback(subreq, wbint_bh_raw_call_done, req); >+ tevent_req_set_callback(subreq, wbint_bh_raw_call_domain_done, req); > > return req; > } > >-static void wbint_bh_raw_call_done(struct tevent_req *subreq) >+static void wbint_bh_raw_call_child_done(struct tevent_req *subreq) > { > struct tevent_req *req = > tevent_req_callback_data(subreq, >@@ -156,6 +168,40 @@ static void wbint_bh_raw_call_done(struct tevent_req *subreq) > tevent_req_done(req); > } > >+static void wbint_bh_raw_call_domain_done(struct tevent_req *subreq) >+{ >+ struct tevent_req *req = >+ tevent_req_callback_data(subreq, >+ struct tevent_req); >+ struct wbint_bh_raw_call_state *state = >+ tevent_req_data(req, >+ struct wbint_bh_raw_call_state); >+ int ret, err; >+ >+ ret = wb_domain_request_recv(subreq, state, &state->response, &err); >+ TALLOC_FREE(subreq); >+ if (ret == -1) { >+ NTSTATUS status = map_nt_error_from_unix(err); >+ tevent_req_nterror(req, status); >+ return; >+ } >+ >+ state->out_data = data_blob_talloc(state, >+ state->response->extra_data.data, >+ state->response->length - sizeof(struct winbindd_response)); >+ if (state->response->extra_data.data && !state->out_data.data) { >+ tevent_req_oom(req); >+ return; >+ } >+ >+ if (state->domain != NULL) { >+ wcache_store_ndr(state->domain, state->opnum, >+ &state->in_data, &state->out_data); >+ } >+ >+ tevent_req_done(req); >+} >+ > static NTSTATUS wbint_bh_raw_call_recv(struct tevent_req *req, > TALLOC_CTX *mem_ctx, > uint8_t **out_data, >@@ -207,9 +253,8 @@ static struct tevent_req *wbint_bh_disconnect_send(TALLOC_CTX *mem_ctx, > > /* > * TODO: do a real async disconnect ... >- * >- * For now the caller needs to free rpc_cli > */ >+ hs->domain = NULL; > hs->child = NULL; > > tevent_req_done(req); >diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c >index 89b85ea..a7e769e 100644 >--- a/source3/winbindd/winbindd_util.c >+++ b/source3/winbindd/winbindd_util.c >@@ -222,6 +222,12 @@ add_trusted_domain_from_tdc(const struct winbindd_tdc_domain *tdc) > return NULL; > } > >+ domain->binding_handle = wbint_binding_handle(domain, domain, NULL); >+ if (domain->binding_handle == NULL) { >+ TALLOC_FREE(domain); >+ return NT_STATUS_NO_MEMORY; >+ } >+ > domain->name = talloc_strdup(domain, domain_name); > if (domain->name == NULL) { > TALLOC_FREE(domain); >-- >1.9.1 > > >From b71bcaf7abae4ebbb8b042093f77279a6bd310d3 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Wed, 14 Feb 2018 15:04:01 +0100 >Subject: [PATCH 14/14] winbind: Use one queue for all domain children > >If we have multiple domain children, it's important >that the first idle child takes over the next waiting request. > >Before we had the problem that a request could get stuck in the >queue of a busy child, while later requests could get served fine by >other children. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13292 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> > >Autobuild-User(master): Stefan Metzmacher <metze@samba.org> >Autobuild-Date(master): Fri Feb 23 09:04:23 CET 2018 on sn-devel-144 > >(cherry picked from commit 7f2d45a6c2a88dd8833fc66d314ec21507dd52c3) >--- > source3/winbindd/winbindd.h | 1 + > source3/winbindd/winbindd_dual.c | 127 ++++++++++++++++++++++++++++++++++++--- > source3/winbindd/winbindd_util.c | 6 ++ > 3 files changed, 125 insertions(+), 9 deletions(-) > >diff --git a/source3/winbindd/winbindd.h b/source3/winbindd/winbindd.h >index 25e4812..ebb9aa1 100644 >--- a/source3/winbindd/winbindd.h >+++ b/source3/winbindd/winbindd.h >@@ -185,6 +185,7 @@ struct winbindd_domain { > > struct winbindd_child *children; > >+ struct tevent_queue *queue; > struct dcerpc_binding_handle *binding_handle; > > /* Callback we use to try put us back online. */ >diff --git a/source3/winbindd/winbindd_dual.c b/source3/winbindd/winbindd_dual.c >index 89823b7..512ecd0 100644 >--- a/source3/winbindd/winbindd_dual.c >+++ b/source3/winbindd/winbindd_dual.c >@@ -223,8 +223,21 @@ static void wb_child_request_done(struct tevent_req *subreq) > > static void wb_child_request_orphaned(struct tevent_req *subreq) > { >+ struct winbindd_child *child = >+ (struct winbindd_child *)tevent_req_callback_data_void(subreq); >+ > DBG_WARNING("cleanup orphaned subreq[%p]\n", subreq); > TALLOC_FREE(subreq); >+ >+ if (child->domain != NULL) { >+ /* >+ * If the child is attached to a domain, >+ * we need to make sure the domain queue >+ * can move forward, after the orphaned >+ * request is done. >+ */ >+ tevent_queue_start(child->domain->queue); >+ } > } > > int wb_child_request_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, >@@ -267,7 +280,7 @@ static void wb_child_request_cleanup(struct tevent_req *req, > talloc_move(subreq, &state->queue_subreq); > tevent_req_set_callback(subreq, > wb_child_request_orphaned, >- NULL); >+ state->child); > > DBG_WARNING("keep orphaned subreq[%p]\n", subreq); > return; >@@ -276,6 +289,16 @@ static void wb_child_request_cleanup(struct tevent_req *req, > TALLOC_FREE(state->subreq); > TALLOC_FREE(state->queue_subreq); > >+ if (state->child->domain != NULL) { >+ /* >+ * If the child is attached to a domain, >+ * we need to make sure the domain queue >+ * can move forward, after the request >+ * is done. >+ */ >+ tevent_queue_start(state->child->domain->queue); >+ } >+ > if (req_state == TEVENT_REQ_DONE) { > /* transmitted request and got response */ > return; >@@ -326,13 +349,35 @@ struct dcerpc_binding_handle *dom_child_handle(struct winbindd_domain *domain) > > struct wb_domain_request_state { > struct tevent_context *ev; >+ struct tevent_queue_entry *queue_entry; > struct winbindd_domain *domain; > struct winbindd_child *child; > struct winbindd_request *request; > struct winbindd_request *init_req; > struct winbindd_response *response; >+ struct tevent_req *pending_subreq; > }; > >+static void wb_domain_request_cleanup(struct tevent_req *req, >+ enum tevent_req_state req_state) >+{ >+ struct wb_domain_request_state *state = tevent_req_data( >+ req, struct wb_domain_request_state); >+ >+ /* >+ * If we're completely done or got a failure. >+ * we should remove ourself from the domain queue, >+ * after removing the child subreq from the child queue >+ * and give the next one in the queue the chance >+ * to check for an idle child. >+ */ >+ TALLOC_FREE(state->pending_subreq); >+ TALLOC_FREE(state->queue_entry); >+ tevent_queue_start(state->domain->queue); >+} >+ >+static void wb_domain_request_trigger(struct tevent_req *req, >+ void *private_data); > static void wb_domain_request_gotdc(struct tevent_req *subreq); > static void wb_domain_request_initialized(struct tevent_req *subreq); > static void wb_domain_request_done(struct tevent_req *subreq); >@@ -342,7 +387,7 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx, > struct winbindd_domain *domain, > struct winbindd_request *request) > { >- struct tevent_req *req, *subreq; >+ struct tevent_req *req; > struct wb_domain_request_state *state; > > req = tevent_req_create(mem_ctx, &state, >@@ -355,21 +400,66 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx, > state->ev = ev; > state->request = request; > >+ tevent_req_set_cleanup_fn(req, wb_domain_request_cleanup); >+ >+ state->queue_entry = tevent_queue_add_entry( >+ domain->queue, state->ev, req, >+ wb_domain_request_trigger, NULL); >+ if (tevent_req_nomem(state->queue_entry, req)) { >+ return tevent_req_post(req, ev); >+ } >+ >+ return req; >+} >+ >+static void wb_domain_request_trigger(struct tevent_req *req, >+ void *private_data) >+{ >+ struct wb_domain_request_state *state = tevent_req_data( >+ req, struct wb_domain_request_state); >+ struct winbindd_domain *domain = state->domain; >+ struct tevent_req *subreq = NULL; >+ size_t shortest_queue_length; >+ > state->child = choose_domain_child(domain); >+ shortest_queue_length = tevent_queue_length(state->child->queue); >+ if (shortest_queue_length > 0) { >+ /* >+ * All children are busy, we need to stop >+ * the queue and untrigger our own queue >+ * entry. Once a pending request >+ * is done it calls tevent_queue_start >+ * and we get retriggered. >+ */ >+ state->child = NULL; >+ tevent_queue_stop(state->domain->queue); >+ tevent_queue_entry_untrigger(state->queue_entry); >+ return; >+ } > > if (domain->initialized) { > subreq = wb_child_request_send(state, state->ev, state->child, > state->request); > if (tevent_req_nomem(subreq, req)) { >- return tevent_req_post(req, ev); >+ return; > } > tevent_req_set_callback(subreq, wb_domain_request_done, req); >- return req; >+ state->pending_subreq = subreq; >+ >+ /* >+ * Once the domain is initialized and >+ * once we placed our real request into the child queue, >+ * we can remove ourself from the domain queue >+ * and give the next one in the queue the chance >+ * to check for an idle child. >+ */ >+ TALLOC_FREE(state->queue_entry); >+ return; > } > > state->init_req = talloc_zero(state, struct winbindd_request); > if (tevent_req_nomem(state->init_req, req)) { >- return tevent_req_post(req, ev); >+ return; > } > > if (IS_DC || domain->primary || domain->internal) { >@@ -382,11 +472,12 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx, > subreq = wb_child_request_send(state, state->ev, state->child, > state->init_req); > if (tevent_req_nomem(subreq, req)) { >- return tevent_req_post(req, ev); >+ return; > } > tevent_req_set_callback(subreq, wb_domain_request_initialized, > req); >- return req; >+ state->pending_subreq = subreq; >+ return; > } > > /* >@@ -403,10 +494,11 @@ struct tevent_req *wb_domain_request_send(TALLOC_CTX *mem_ctx, > NULL, /* site_name */ > DS_RETURN_DNS_NAME); /* flags */ > if (tevent_req_nomem(subreq, req)) { >- return tevent_req_post(req, ev); >+ return; > } > tevent_req_set_callback(subreq, wb_domain_request_gotdc, req); >- return req; >+ state->pending_subreq = subreq; >+ return; > } > > static void wb_domain_request_gotdc(struct tevent_req *subreq) >@@ -419,6 +511,8 @@ static void wb_domain_request_gotdc(struct tevent_req *subreq) > NTSTATUS status; > const char *dcname = NULL; > >+ state->pending_subreq = NULL; >+ > status = wb_dsgetdcname_recv(subreq, state, &dcinfo); > TALLOC_FREE(subreq); > if (tevent_req_nterror(req, status)) { >@@ -442,6 +536,7 @@ static void wb_domain_request_gotdc(struct tevent_req *subreq) > return; > } > tevent_req_set_callback(subreq, wb_domain_request_initialized, req); >+ state->pending_subreq = subreq; > } > > static void wb_domain_request_initialized(struct tevent_req *subreq) >@@ -453,6 +548,8 @@ static void wb_domain_request_initialized(struct tevent_req *subreq) > struct winbindd_response *response; > int ret, err; > >+ state->pending_subreq = NULL; >+ > ret = wb_child_request_recv(subreq, talloc_tos(), &response, &err); > TALLOC_FREE(subreq); > if (ret == -1) { >@@ -500,6 +597,16 @@ static void wb_domain_request_initialized(struct tevent_req *subreq) > return; > } > tevent_req_set_callback(subreq, wb_domain_request_done, req); >+ state->pending_subreq = subreq; >+ >+ /* >+ * Once the domain is initialized and >+ * once we placed our real request into the child queue, >+ * we can remove ourself from the domain queue >+ * and give the next one in the queue the chance >+ * to check for an idle child. >+ */ >+ TALLOC_FREE(state->queue_entry); > } > > static void wb_domain_request_done(struct tevent_req *subreq) >@@ -510,6 +617,8 @@ static void wb_domain_request_done(struct tevent_req *subreq) > req, struct wb_domain_request_state); > int ret, err; > >+ state->pending_subreq = NULL; >+ > ret = wb_child_request_recv(subreq, talloc_tos(), &state->response, > &err); > TALLOC_FREE(subreq); >diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c >index a7e769e..cc92112 100644 >--- a/source3/winbindd/winbindd_util.c >+++ b/source3/winbindd/winbindd_util.c >@@ -222,6 +222,12 @@ add_trusted_domain_from_tdc(const struct winbindd_tdc_domain *tdc) > return NULL; > } > >+ domain->queue = tevent_queue_create(domain, "winbind_domain"); >+ if (domain->queue == NULL) { >+ TALLOC_FREE(domain); >+ return NT_STATUS_NO_MEMORY; >+ } >+ > domain->binding_handle = wbint_binding_handle(domain, domain, NULL); > if (domain->binding_handle == NULL) { > TALLOC_FREE(domain); >-- >1.9.1 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=14151">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
vl
:
review+
Actions:
View
Attachments on
bug 13292
:
14002
|
14151
|
14157