[Samba] samba 4.7.3 DLZ performance regression

[Andrew Bartlett]

On Fri, 2017-12-15 at 10:16 +1300, Garming Sam via samba wrote:
> Hi,
> 
> We've just had another person notice this issue and we're currently
> looking into it. If we need someone to test the patch fix, or if we need
> more information, we can let you know.
> 
> I've filed a bug as well:
> 
> https://bugzilla.samba.org/show_bug.cgi?id=13191

Attached is a possible set of patches. 

As indicated, a search for a not-existing name, be it one that will
match with a wildcard or one that wont, will still be slow.

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 b8f7342774507350b7897a82030ddfeee57ccf11 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 15 Dec 2017 11:40:28 +1300
Subject: [PATCH 1/3] dns_server: Do not look for a wildcard for @

This query is made for every record returned via BIND9 DLZ.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13191

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dns_server/dnsserver_common.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
index 217e65b39f4..093898361d0 100644
--- a/source4/dns_server/dnsserver_common.c
+++ b/source4/dns_server/dnsserver_common.c
@@ -547,6 +547,16 @@ WERROR dns_common_wildcard_lookup(struct ldb_context *samdb,
 		return DNS_ERR(NAME_ERROR);
 	}
 
+	/* Don't look for a wildcard for @ */
+	if (name->length == 1 && name->data[0] == '@') {
+		return dns_common_lookup(samdb,
+					 mem_ctx,
+					 dn,
+					 records,
+					 num_records,
+					 NULL);
+	}
+
 	werr =  dns_name_check(
 			mem_ctx,
 			strlen((const char*)name->data),
-- 
2.11.0


From 9a22603499230e51dd8173a2ebd31c63e6c8911f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 15 Dec 2017 11:41:15 +1300
Subject: [PATCH 2/3] dns_server: Do not embed a trailing NUL in the query

The data to match on does not include the NUL, just the significant bytes

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13191

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dns_server/dnsserver_common.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
index 093898361d0..574d1510230 100644
--- a/source4/dns_server/dnsserver_common.c
+++ b/source4/dns_server/dnsserver_common.c
@@ -250,9 +250,11 @@ static struct ldb_parse_tree *build_equality_operation(
 	struct ldb_parse_tree *el = NULL;  /* Equality node being built */
 	struct ldb_val *value = NULL;      /* Value the attr will be compared
 					      with */
-	size_t length = 0;                 /* calculated length of the value
-	                                      including option '*' prefix and
-					      '\0' string terminator */
+	size_t length = 0;                 /* calculated length of the
+	                                      value including option
+	                                      '*' prefix but no '\0'
+	                                      string terminator (this
+	                                      is an ldb_val) */
 
 	el = talloc(mem_ctx, struct ldb_parse_tree);
 	if (el == NULL) {
@@ -263,7 +265,7 @@ static struct ldb_parse_tree *build_equality_operation(
 	el->operation = LDB_OP_EQUALITY;
 	el->u.equality.attr = talloc_strdup(mem_ctx, attr);
 	value = &el->u.equality.value;
-	length = (add_asterix) ? size + 2 : size + 1;
+	length = (add_asterix) ? size + 1 : size;
 	value->data = talloc_zero_array(el, uint8_t, length);
 	if (el == NULL) {
 		DBG_ERR("Unable to allocate value->data\n");
-- 
2.11.0


From 007efcf5d9cc15a200581ff7c8f24137ec0e5a8e Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 15 Dec 2017 12:30:50 +1300
Subject: [PATCH 3/3] dns_server: Do the exact match query first, then do the
 wildcard lookup

The wildcard lookup is SCOPE_ONELEVEL which in current ldb will load the full index for that zone.

This is inefficient as we do not currently intersect that index with the
index on DN=

Not that a not-found and wildcard response will still go via the ONELEVEL
index for now so this only helps for correct, directly matching queries.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13191

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dns_server/dnsserver_common.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
index 574d1510230..368d32326b0 100644
--- a/source4/dns_server/dnsserver_common.c
+++ b/source4/dns_server/dnsserver_common.c
@@ -567,6 +567,20 @@ WERROR dns_common_wildcard_lookup(struct ldb_context *samdb,
 		return werr;
 	}
 
+	/* 
+	 * Do a point search first, then fall back to a wildcard
+	 * lookup if it does not exist
+	 */
+	werr = dns_common_lookup(samdb,
+				 mem_ctx,
+				 dn,
+				 records,
+				 num_records,
+				 NULL);
+	if (!W_ERROR_EQUAL(werr, WERR_DNS_ERROR_NAME_DOES_NOT_EXIST)) {
+		return werr;
+	}
+	
 	ret = dns_wildcard_lookup(samdb, mem_ctx, dn, &msg);
 	if (ret == LDB_ERR_OPERATIONS_ERROR) {
 		return DNS_ERR(SERVER_FAILURE);
-- 
2.11.0