[WIP][PATCH] Improve AD LDAP search performance via SD caching
Andrew Bartlett
abartlet at samba.org
Wed Jun 14 05:32:25 UTC 2017
On Wed, 2017-06-14 at 17:30 +1200, Andrew Bartlett wrote:
> These patches make our AD LDAP server faster by caching the last SD
> we
> read from the database, and the unpacked representation thereof.
>
> Because we key on the full binary version, we don't need to keep the
> cache consistent with the DB, the same NDR should always become the
> same unpacked structures.
>
> Likewise, with the read lock held, the same parent should always give
> the same result for the same connection.
>
> Please comment.
>
> Marked [WIP] because we are still checking why one of the perf tests
> is
> slower, but most improve quite nicely.
With both patches this time.
Sorry,
Andrew Bartlett
--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team https://samba.org
Samba Development and Support, Catalyst IT
https://catalyst.net.nz/services/samba
-------------- next part --------------
From 7d65421de19b99286ba0b9c21dbd3f9c4393d5c5 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 13 Jun 2017 14:26:49 +1200
Subject: [PATCH 1/2] dsdb: Cache the result of checking the parent ACL
This should help a lot for large one-level searches and for subtree searches that are of
flat tree structures
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dsdb/samdb/ldb_modules/acl_read.c | 77 ++++++++++++++++++++++++++++---
1 file changed, 70 insertions(+), 7 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c
index f15633f28f8..6fd7fc5cd55 100644
--- a/source4/dsdb/samdb/ldb_modules/acl_read.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c
@@ -50,6 +50,10 @@ struct aclread_context {
bool added_objectSid;
bool added_objectClass;
bool indirsync;
+
+ /* cache on the last parent we checked in this search */
+ struct ldb_dn *last_parent_dn;
+ int last_parent_check_ret;
};
struct aclread_private {
@@ -64,6 +68,70 @@ static bool aclread_is_inaccessible(struct ldb_message_element *el) {
return el->flags & LDB_FLAG_INTERNAL_INACCESSIBLE_ATTRIBUTE;
}
+/* the object has a parent, so we have to check for visibility */
+static int aclread_check_parent(struct aclread_context *ac, struct ldb_message *msg, struct ldb_request *req)
+{
+ int ret;
+ struct ldb_dn *parent_dn = NULL;
+
+ /* We may have a cached result from earlier in this search */
+ if (ac->last_parent_dn != NULL) {
+ /*
+ * We try the no-allocation ldb_dn_compare_base()
+ * first however it will not tell parents and
+ * grand-parents apart
+ */
+ int cmp_base = ldb_dn_compare_base(ac->last_parent_dn,
+ msg->dn);
+ if (cmp_base == 0) {
+ /* Now check if it is a direct parent */
+ parent_dn = ldb_dn_get_parent(ac, msg->dn);
+ if (parent_dn == NULL) {
+ return ldb_oom(ldb_module_get_ctx(ac->module));
+ }
+ if (ldb_dn_compare(ac->last_parent_dn,
+ parent_dn) == 0) {
+ TALLOC_FREE(parent_dn);
+
+ /*
+ * If we checked the same parent last
+ * time, then return the cached
+ * result
+ */
+ return ac->last_parent_check_ret;
+ }
+ }
+ }
+
+ {
+ TALLOC_CTX *frame = NULL;
+ frame = talloc_stackframe();
+
+ /*
+ * This may have een set in the block above, don't
+ * re-parse
+ */
+ if (parent_dn == NULL) {
+ parent_dn = ldb_dn_get_parent(ac, msg->dn);
+ if (parent_dn == NULL) {
+ TALLOC_FREE(frame);
+ return ldb_oom(ldb_module_get_ctx(ac->module));
+ }
+ }
+ ret = dsdb_module_check_access_on_dn(ac->module,
+ frame,
+ parent_dn,
+ SEC_ADS_LIST,
+ NULL, req);
+ talloc_unlink(ac, ac->last_parent_dn);
+ ac->last_parent_dn = parent_dn;
+ ac->last_parent_check_ret = ret;
+
+ TALLOC_FREE(frame);
+ }
+ return ret;
+}
+
static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
{
struct ldb_context *ldb;
@@ -123,13 +191,8 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
if (!ldb_dn_is_null(msg->dn) && !(instanceType & INSTANCE_TYPE_IS_NC_HEAD))
{
/* the object has a parent, so we have to check for visibility */
- struct ldb_dn *parent_dn = ldb_dn_get_parent(tmp_ctx, msg->dn);
-
- ret = dsdb_module_check_access_on_dn(ac->module,
- tmp_ctx,
- parent_dn,
- SEC_ADS_LIST,
- NULL, req);
+ ret = aclread_check_parent(ac, msg, req);
+
if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) {
talloc_free(tmp_ctx);
return LDB_SUCCESS;
--
2.11.0
From 67d2d9b3e478ffa39d7dd5ac1afca1987b067e7f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 13 Jun 2017 15:23:14 +1200
Subject: [PATCH 2/2] dsdb: Remember the last ACL we read during a search and
what it expanded to
It may well be the same as the next one we need to check, so we can
avoid parsing it again.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/dsdb/samdb/ldb_modules/acl_read.c | 73 ++++++++++++++++++++++++++++++-
1 file changed, 71 insertions(+), 2 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c
index 6fd7fc5cd55..1f441856061 100644
--- a/source4/dsdb/samdb/ldb_modules/acl_read.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c
@@ -58,6 +58,10 @@ struct aclread_context {
struct aclread_private {
bool enabled;
+
+ /* cache of the last SD we read during any search */
+ struct security_descriptor *sd_cached;
+ struct ldb_val sd_cached_blob;
};
static void aclread_mark_inaccesslible(struct ldb_message_element *el) {
@@ -132,6 +136,71 @@ static int aclread_check_parent(struct aclread_context *ac, struct ldb_message *
return ret;
}
+/*
+ * The sd returned from this function is valid until the next call on
+ * this module context
+ */
+
+static int aclread_get_sd_from_ldb_message(struct aclread_context *ac,
+ struct ldb_message *acl_res,
+ struct security_descriptor **sd)
+{
+ struct ldb_message_element *sd_element;
+ struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
+ struct aclread_private *private_data
+ = talloc_get_type(ldb_module_get_private(ac->module),
+ struct aclread_private);
+ enum ndr_err_code ndr_err;
+
+ sd_element = ldb_msg_find_element(acl_res, "nTSecurityDescriptor");
+ if (sd_element == NULL) {
+ return ldb_error(ldb, LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS,
+ "nTSecurityDescriptor is missing");
+ }
+
+ if (sd_element->num_values != 1) {
+ return ldb_operr(ldb);
+ }
+
+ if (private_data->sd_cached != NULL && private_data->sd_cached_blob.data != NULL &&
+ ldb_val_equal_exact(&sd_element->values[0],
+ &private_data->sd_cached_blob)) {
+ *sd = private_data->sd_cached;
+ return LDB_SUCCESS;
+
+ }
+
+ *sd = talloc(private_data, struct security_descriptor);
+ if(!*sd) {
+ return ldb_oom(ldb);
+ }
+ ndr_err = ndr_pull_struct_blob(&sd_element->values[0], *sd, *sd,
+ (ndr_pull_flags_fn_t)ndr_pull_security_descriptor);
+
+ if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+ TALLOC_FREE(*sd);
+ return ldb_operr(ldb);
+ }
+
+ talloc_unlink(private_data, private_data->sd_cached_blob.data);
+ if (ac->added_nTSecurityDescriptor) {
+ private_data->sd_cached_blob = sd_element->values[0];
+ talloc_steal(private_data, sd_element->values[0].data);
+ } else {
+ private_data->sd_cached_blob = ldb_val_dup(private_data, &sd_element->values[0]);
+ if (private_data->sd_cached_blob.data == NULL) {
+ TALLOC_FREE(*sd);
+ return ldb_operr(ldb);
+ }
+ }
+
+ talloc_unlink(private_data, private_data->sd_cached);
+ private_data->sd_cached = *sd;
+
+ return LDB_SUCCESS;
+}
+
+
static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
{
struct ldb_context *ldb;
@@ -140,7 +209,7 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
struct ldb_message *msg;
int ret, num_of_attrs = 0;
unsigned int i, k = 0;
- struct security_descriptor *sd;
+ struct security_descriptor *sd = NULL;
struct dom_sid *sid = NULL;
TALLOC_CTX *tmp_ctx;
uint32_t instanceType;
@@ -159,7 +228,7 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares)
switch (ares->type) {
case LDB_REPLY_ENTRY:
msg = ares->message;
- ret = dsdb_get_sd_from_ldb_message(ldb, tmp_ctx, msg, &sd);
+ ret = aclread_get_sd_from_ldb_message(ac, msg, &sd);
if (ret != LDB_SUCCESS) {
ldb_debug_set(ldb, LDB_DEBUG_FATAL,
"acl_read: cannot get descriptor of %s: %s\n",
--
2.11.0
More information about the samba-technical
mailing list