[WIP][PATCH] Improve AD LDAP search performance via SD caching

[Andrew Bartlett]

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.

Thanks,

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 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] 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