From 120c09b125f656d6d03b979560f3ef6652217691 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 26 Jun 2008 14:02:39 -0700 Subject: [PATCH] From Steve Danneman @ Isilon. Attached is the companion patch to (037b9689d9042a398cb91e4628a82fcdfa913c21), which made handling of WINBINDD_LIST_GROUPS asynchronous. Because most all of the list_groups code was reusable, I abstracted it, and implemented both list_groups and list_users on top of it. On my large test domain a "wbinfo -u" call went from 70 seconds to 30 seconds with this patch. Plus, the parent process is no longer blocked from receiving new requests during that time. Steven Danneman | Software Development Engineer Isilon Systems P +1-206-315-7500 F +1-206-315-7501 www.isilon.com (This used to be commit 5188f2861137ff06d5399561d55d7d00c3a08644) --- source3/winbindd/winbindd.h | 7 +- source3/winbindd/winbindd_async.c | 85 +++++++++++++++++--- source3/winbindd/winbindd_domain.c | 4 + source3/winbindd/winbindd_group.c | 104 +----------------------- source3/winbindd/winbindd_misc.c | 123 +++++++++++++++++++++++++++++ source3/winbindd/winbindd_proto.h | 20 ++--- source3/winbindd/winbindd_user.c | 94 +--------------------- 7 files changed, 220 insertions(+), 217 deletions(-) diff --git a/source3/winbindd/winbindd.h b/source3/winbindd/winbindd.h index 301d877e615..b2fe8b67e7c 100644 --- a/source3/winbindd/winbindd.h +++ b/source3/winbindd/winbindd.h @@ -363,7 +363,12 @@ struct winbindd_tdc_domain { uint32 trust_type; }; - +/* Switch for listing users or groups */ +enum ent_type { + LIST_USERS = 0, + LIST_GROUPS, +}; + #include "winbindd/winbindd_proto.h" #define WINBINDD_ESTABLISH_LOOP 30 diff --git a/source3/winbindd/winbindd_async.c b/source3/winbindd/winbindd_async.c index bc0f9d836b4..e47666462e2 100644 --- a/source3/winbindd/winbindd_async.c +++ b/source3/winbindd/winbindd_async.c @@ -453,8 +453,8 @@ enum winbindd_result winbindd_dual_lookupname(struct winbindd_domain *domain, return WINBINDD_OK; } -/* This is the first callback after enumerating groups from a domain */ -static void listgroups_recv(TALLOC_CTX *mem_ctx, bool success, +/* This is the first callback after enumerating users/groups from a domain */ +static void listent_recv(TALLOC_CTX *mem_ctx, bool success, struct winbindd_response *response, void *c, void *private_data) { @@ -462,7 +462,7 @@ static void listgroups_recv(TALLOC_CTX *mem_ctx, bool success, (void (*)(void *, bool, fstring, char*))c; if (!success || response->result != WINBINDD_OK) { - DEBUG(5, ("list_groups() failed!\n")); + DEBUG(5, ("list_ent() failed!\n")); cont(private_data, False, response->data.name.dom_name, NULL); return; } @@ -473,30 +473,95 @@ static void listgroups_recv(TALLOC_CTX *mem_ctx, bool success, SAFE_FREE(response->extra_data.data); } -/* Request the name of all groups in a single domain */ -void winbindd_listgroups_async(TALLOC_CTX *mem_ctx, +/* Request the name of all users/groups in a single domain */ +void winbindd_listent_async(TALLOC_CTX *mem_ctx, struct winbindd_domain *domain, void (*cont)(void *private_data, bool success, fstring dom_name, char* extra_data), - void *private_data) + void *private_data, enum ent_type type) { struct winbindd_request request; ZERO_STRUCT(request); - request.cmd = WINBINDD_LIST_GROUPS; + if (type == LIST_USERS) + request.cmd = WINBINDD_LIST_USERS; + else if (type == LIST_GROUPS) + request.cmd = WINBINDD_LIST_GROUPS; - do_async_domain(mem_ctx, domain, &request, listgroups_recv, + do_async_domain(mem_ctx, domain, &request, listent_recv, (void *)cont, private_data); } + +enum winbindd_result winbindd_dual_list_users(struct winbindd_domain *domain, + struct winbindd_cli_state *state) +{ + WINBIND_USERINFO *info; + NTSTATUS status; + struct winbindd_methods *methods; + uint32 num_entries = 0; + char *extra_data = NULL; + uint32_t extra_data_len = 0, i; + + /* Must copy domain into response first for debugging in parent */ + fstrcpy(state->response.data.name.dom_name, domain->name); + + /* Query user info */ + methods = domain->methods; + status = methods->query_user_list(domain, state->mem_ctx, + &num_entries, &info); + + if (!NT_STATUS_IS_OK(status)) + return WINBINDD_ERROR; + + if (num_entries == 0) + return WINBINDD_OK; + + /* Allocate some memory for extra data. Note that we limit + account names to sizeof(fstring) = 256 characters. + +1 for the ',' between group names */ + extra_data = (char *)SMB_REALLOC(extra_data, + (sizeof(fstring) + 1) * num_entries); + + if (!extra_data) { + DEBUG(0,("failed to enlarge buffer!\n")); + return WINBINDD_ERROR; + } + + /* Pack user list into extra data fields */ + for (i = 0; i < num_entries; i++) { + fstring acct_name, name; + + if (info[i].acct_name == NULL) + fstrcpy(acct_name, ""); + else + fstrcpy(acct_name, info[i].acct_name); + + fill_domain_username(name, domain->name, acct_name, True); + /* Append to extra data */ + memcpy(&extra_data[extra_data_len], name, strlen(name)); + extra_data_len += strlen(name); + extra_data[extra_data_len++] = ','; + } + + /* Assign extra_data fields in response structure */ + if (extra_data) { + /* remove trailing ',' */ + extra_data[extra_data_len - 1] = '\0'; + state->response.extra_data.data = extra_data; + state->response.length += extra_data_len; + } + + return WINBINDD_OK; +} enum winbindd_result winbindd_dual_list_groups(struct winbindd_domain *domain, struct winbindd_cli_state *state) { struct getent_state groups = {}; char *extra_data = NULL; - unsigned int extra_data_len = 0, i; + uint32_t extra_data_len = 0, i; - /* Must copy domain into response first for bookeeping in parent */ + /* Must copy domain into response first for debugging in parent */ fstrcpy(state->response.data.name.dom_name, domain->name); fstrcpy(groups.domain_name, domain->name); diff --git a/source3/winbindd/winbindd_domain.c b/source3/winbindd/winbindd_domain.c index 4d10f49ca21..2e8c6175ca0 100644 --- a/source3/winbindd/winbindd_domain.c +++ b/source3/winbindd/winbindd_domain.c @@ -49,6 +49,10 @@ static const struct winbindd_child_dispatch_table domain_dispatch_table[] = { .name = "LOOKUPRIDS", .struct_cmd = WINBINDD_LOOKUPRIDS, .struct_fn = winbindd_dual_lookuprids, + },{ + .name = "LIST_USERS", + .struct_cmd = WINBINDD_LIST_USERS, + .struct_fn = winbindd_dual_list_users, },{ .name = "LIST_GROUPS", .struct_cmd = WINBINDD_LIST_GROUPS, diff --git a/source3/winbindd/winbindd_group.c b/source3/winbindd/winbindd_group.c index d10609a83f1..f9f946f3f3d 100644 --- a/source3/winbindd/winbindd_group.c +++ b/source3/winbindd/winbindd_group.c @@ -1339,112 +1339,10 @@ void winbindd_getgrent(struct winbindd_cli_state *state) request_error(state); } -struct listgroups_state { - TALLOC_CTX *mem_ctx; - struct winbindd_cli_state *cli_state; - unsigned int domain_count; - char *extra_data; - unsigned int extra_data_len; -}; - -static void listgroups_recv(void *private_data, bool success, fstring dom_name, - char *extra_data); - /* List domain groups without mapping to unix ids */ void winbindd_list_groups(struct winbindd_cli_state *state) { - struct winbindd_domain *domain; - const char *which_domain; - struct listgroups_state *groups_state; - - DEBUG(3, ("[%5lu]: list groups\n", (unsigned long)state->pid)); - - /* Ensure null termination */ - state->request.domain_name[sizeof(state->request.domain_name)-1]='\0'; - which_domain = state->request.domain_name; - - /* Initialize listgroups_state */ - groups_state = TALLOC_P(state->mem_ctx, struct listgroups_state); - if (groups_state == NULL) { - DEBUG(0, ("talloc failed\n")); - request_error(state); - return; - } - - groups_state->mem_ctx = state->mem_ctx; - groups_state->cli_state = state; - groups_state->domain_count = 0; - groups_state->extra_data = NULL; - groups_state->extra_data_len = 0; - - /* Must count the full list of expected domains before we request data - * from any of them. Otherwise it's possible for a connection to the - * first domain to fail, call listgroups_recv(), and return to the - * client without checking any other domains. */ - for (domain = domain_list(); domain; domain = domain->next) { - /* if we have a domain name restricting the request and this - one in the list doesn't match, then just bypass the remainder - of the loop */ - if ( *which_domain && !strequal(which_domain, domain->name) ) - continue; - - groups_state->domain_count++; - } - - /* Make sure we're enumerating at least one domain */ - if (!groups_state->domain_count) { - request_ok(state); - return; - } - - /* Enumerate list of trusted domains and request group list from each */ - for (domain = domain_list(); domain; domain = domain->next) { - if ( *which_domain && !strequal(which_domain, domain->name) ) - continue; - - winbindd_listgroups_async(state->mem_ctx, domain, - listgroups_recv, groups_state); - } -} - -static void listgroups_recv(void *private_data, bool success, fstring dom_name, - char *extra_data) -{ - /* extra_data comes to us as a '\0' terminated string of comma - separated groups */ - struct listgroups_state *state = talloc_get_type_abort( - private_data, struct listgroups_state); - - /* Append groups from one domain onto the whole list */ - if (extra_data) { - DEBUG(5, ("listgroups_recv: %s returned groups.\n", dom_name)); - if (!state->extra_data) - state->extra_data = talloc_asprintf(state->mem_ctx, - "%s", extra_data); - else - state->extra_data = talloc_asprintf_append_buffer( - state->extra_data, - ",%s", extra_data); - /* Add one for the '\0' and each additional ',' */ - state->extra_data_len += strlen(extra_data) + 1; - } - else { - DEBUG(5, ("listgroups_recv: %s returned no groups.\n", - dom_name)); - } - - if (--state->domain_count) - /* Still waiting for some child domains to return */ - return; - - /* Return list of all groups to the client */ - if (state->extra_data) { - state->cli_state->response.extra_data.data = - SMB_STRDUP(state->extra_data); - state->cli_state->response.length += state->extra_data_len; - } - - request_ok(state->cli_state); + winbindd_list_ent(state, LIST_GROUPS); } /* Get user supplementary groups. This is much quicker than trying to diff --git a/source3/winbindd/winbindd_misc.c b/source3/winbindd/winbindd_misc.c index 8933cf27945..01a4054d442 100644 --- a/source3/winbindd/winbindd_misc.c +++ b/source3/winbindd/winbindd_misc.c @@ -97,6 +97,129 @@ enum winbindd_result winbindd_dual_check_machine_acct(struct winbindd_domain *do return NT_STATUS_IS_OK(result) ? WINBINDD_OK : WINBINDD_ERROR; } +/* Helpers for listing user and group names */ + +const char *ent_type_strings[] = {"users", + "groups"}; + +static const char *get_ent_type_string(enum ent_type type) +{ + return ent_type_strings[type]; +} + +struct listent_state { + TALLOC_CTX *mem_ctx; + struct winbindd_cli_state *cli_state; + enum ent_type type; + int domain_count; + char *extra_data; + uint32_t extra_data_len; +}; + +static void listent_recv(void *private_data, bool success, fstring dom_name, + char *extra_data); + +/* List domain users/groups without mapping to unix ids */ +void winbindd_list_ent(struct winbindd_cli_state *state, enum ent_type type) +{ + struct winbindd_domain *domain; + const char *which_domain; + struct listent_state *ent_state; + + DEBUG(3, ("[%5lu]: list %s\n", (unsigned long)state->pid, + get_ent_type_string(type))); + + /* Ensure null termination */ + state->request.domain_name[sizeof(state->request.domain_name)-1]='\0'; + which_domain = state->request.domain_name; + + /* Initialize listent_state */ + ent_state = TALLOC_P(state->mem_ctx, struct listent_state); + if (ent_state == NULL) { + DEBUG(0, ("talloc failed\n")); + request_error(state); + return; + } + + ent_state->mem_ctx = state->mem_ctx; + ent_state->cli_state = state; + ent_state->type = type; + ent_state->domain_count = 0; + ent_state->extra_data = NULL; + ent_state->extra_data_len = 0; + + /* Must count the full list of expected domains before we request data + * from any of them. Otherwise it's possible for a connection to the + * first domain to fail, call listent_recv(), and return to the + * client without checking any other domains. */ + for (domain = domain_list(); domain; domain = domain->next) { + /* if we have a domain name restricting the request and this + one in the list doesn't match, then just bypass the remainder + of the loop */ + if ( *which_domain && !strequal(which_domain, domain->name) ) + continue; + + ent_state->domain_count++; + } + + /* Make sure we're enumerating at least one domain */ + if (!ent_state->domain_count) { + request_ok(state); + return; + } + + /* Enumerate list of trusted domains and request user/group list from + * each */ + for (domain = domain_list(); domain; domain = domain->next) { + if ( *which_domain && !strequal(which_domain, domain->name) ) + continue; + + winbindd_listent_async(state->mem_ctx, domain, + listent_recv, ent_state, type); + } +} + +static void listent_recv(void *private_data, bool success, fstring dom_name, + char *extra_data) +{ + /* extra_data comes to us as a '\0' terminated string of comma + separated users or groups */ + struct listent_state *state = talloc_get_type_abort( + private_data, struct listent_state); + + /* Append users/groups from one domain onto the whole list */ + if (extra_data) { + DEBUG(5, ("listent_recv: %s returned %s.\n", + dom_name, get_ent_type_string(state->type))); + if (!state->extra_data) + state->extra_data = talloc_asprintf(state->mem_ctx, + "%s", extra_data); + else + state->extra_data = talloc_asprintf_append( + state->extra_data, + ",%s", extra_data); + /* Add one for the '\0' and each additional ',' */ + state->extra_data_len += strlen(extra_data) + 1; + } + else { + DEBUG(5, ("listent_recv: %s returned no %s.\n", + dom_name, get_ent_type_string(state->type))); + } + + if (--state->domain_count) + /* Still waiting for some child domains to return */ + return; + + /* Return list of all users/groups to the client */ + if (state->extra_data) { + state->cli_state->response.extra_data.data = + SMB_STRDUP(state->extra_data); + state->cli_state->response.length += state->extra_data_len; + } + + request_ok(state->cli_state); +} + /* Constants and helper functions for determining domain trust types */ enum trust_type { diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h index 356d4aaf79a..84ea67b9ec8 100644 --- a/source3/winbindd/winbindd_proto.h +++ b/source3/winbindd/winbindd_proto.h @@ -104,6 +104,15 @@ void winbindd_lookupname_async(TALLOC_CTX *mem_ctx, void *private_data); enum winbindd_result winbindd_dual_lookupname(struct winbindd_domain *domain, struct winbindd_cli_state *state); +void winbindd_listent_async(TALLOC_CTX *mem_ctx, + struct winbindd_domain *domain, + void (*cont)(void *private_data, bool success, + fstring dom_name, char* extra_data), + void *private_data, enum ent_type type); +enum winbindd_result winbindd_dual_list_users(struct winbindd_domain *domain, + struct winbindd_cli_state *state); +enum winbindd_result winbindd_dual_list_groups(struct winbindd_domain *domain, + struct winbindd_cli_state *state); bool print_sidlist(TALLOC_CTX *mem_ctx, const DOM_SID *sids, size_t num_sids, char **result, ssize_t *len); enum winbindd_result winbindd_dual_lookuprids(struct winbindd_domain *domain, @@ -133,16 +142,6 @@ void query_user_async(TALLOC_CTX *mem_ctx, struct winbindd_domain *domain, uint32 group_rid), void *private_data); -void winbindd_listgroups_async(TALLOC_CTX *mem_ctx, - struct winbindd_domain *domain, - void (*cont)(void *private_data, bool success, - fstring dom_name, char* extra_data), - void *private_data); - -enum winbindd_result winbindd_dual_list_groups(struct winbindd_domain *domain, - struct winbindd_cli_state *state); - - /* The following definitions come from winbindd/winbindd_cache.c */ void winbindd_check_cache_size(time_t t); @@ -394,6 +393,7 @@ void winbindd_dsgetdcname(struct winbindd_cli_state *state); void winbindd_check_machine_acct(struct winbindd_cli_state *state); enum winbindd_result winbindd_dual_check_machine_acct(struct winbindd_domain *domain, struct winbindd_cli_state *state); +void winbindd_list_ent(struct winbindd_cli_state *state, enum ent_type type); void winbindd_list_trusted_domains(struct winbindd_cli_state *state); enum winbindd_result winbindd_dual_list_trusted_domains(struct winbindd_domain *domain, struct winbindd_cli_state *state); diff --git a/source3/winbindd/winbindd_user.c b/source3/winbindd/winbindd_user.c index 6241d84fe6d..45918383b75 100644 --- a/source3/winbindd/winbindd_user.c +++ b/source3/winbindd/winbindd_user.c @@ -778,99 +778,7 @@ void winbindd_getpwent(struct winbindd_cli_state *state) } /* List domain users without mapping to unix ids */ - void winbindd_list_users(struct winbindd_cli_state *state) { - struct winbindd_domain *domain; - WINBIND_USERINFO *info; - const char *which_domain; - uint32 num_entries = 0, total_entries = 0; - char *extra_data = NULL; - int extra_data_len = 0; - enum winbindd_result rv = WINBINDD_ERROR; - - DEBUG(3, ("[%5lu]: list users\n", (unsigned long)state->pid)); - - /* Ensure null termination */ - state->request.domain_name[sizeof(state->request.domain_name)-1]='\0'; - which_domain = state->request.domain_name; - - /* Enumerate over trusted domains */ - - for (domain = domain_list(); domain; domain = domain->next) { - NTSTATUS status; - struct winbindd_methods *methods; - unsigned int i; - - /* if we have a domain name restricting the request and this - one in the list doesn't match, then just bypass the remainder - of the loop */ - - if ( *which_domain && !strequal(which_domain, domain->name) ) - continue; - - methods = domain->methods; - - /* Query display info */ - status = methods->query_user_list(domain, state->mem_ctx, - &num_entries, &info); - - if (!NT_STATUS_IS_OK(status)) { - continue; - } - - if (num_entries == 0) - continue; - - /* Allocate some memory for extra data */ - total_entries += num_entries; - - extra_data = (char *)SMB_REALLOC( - extra_data, sizeof(fstring) * total_entries); - - if (!extra_data) { - DEBUG(0,("failed to enlarge buffer!\n")); - goto done; - } - - /* Pack user list into extra data fields */ - - for (i = 0; i < num_entries; i++) { - fstring acct_name, name; - - if (!info[i].acct_name) { - fstrcpy(acct_name, ""); - } else { - fstrcpy(acct_name, info[i].acct_name); - } - - fill_domain_username(name, domain->name, acct_name, True); - - /* Append to extra data */ - memcpy(&extra_data[extra_data_len], name, - strlen(name)); - extra_data_len += strlen(name); - extra_data[extra_data_len++] = ','; - } - } - - /* Assign extra_data fields in response structure */ - - if (extra_data) { - extra_data[extra_data_len - 1] = '\0'; - state->response.extra_data.data = extra_data; - state->response.length += extra_data_len; - } - - /* No domains responded but that's still OK so don't return an - error. */ - - rv = WINBINDD_OK; - - done: - - if (rv == WINBINDD_OK) - request_ok(state); - else - request_error(state); + winbindd_list_ent(state, LIST_USERS); } -- 2.34.1