[PATCH] Improve performance of GUID_to_ndr_blob() and similar

Volker Lendecke vl at samba.org
Mon Jul 25 05:55:03 UTC 2016


On Mon, Jul 25, 2016 at 04:10:41PM +1200, Andrew Bartlett wrote:
> Here we avoid a talloc, reduce the other from 1024 to 16 bytes and
> avoid a talloc_steal().
> 
> This helps in tight loops in the AD DC ldb layers, particularly during
> DRS replication of large numbers of links.  These O(N^2) loops should
> be addressed as well, but this fix will also help in many other cases. 
> 
> Then, in the other patches, avoid talloc when not required elsewhere in
> ldb extended DN parsing.
> 
> Please review and push.

This looks good, but I would like to see the attached diff applied
before it is pushed.

Thanks, Volker
-------------- next part --------------
 lib/ldb-samba/ldif_handlers.c |  7 ++++---
 librpc/ndr/ndr.c              | 11 ++++++-----
 librpc/ndr/uuid.c             |  4 ++--
 source4/dsdb/common/util.c    |  3 ++-
 source4/torture/ndr/ndr.c     | 24 ++++++++++++++----------
 5 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/lib/ldb-samba/ldif_handlers.c b/lib/ldb-samba/ldif_handlers.c
index 085c4da..ebffd6a 100644
--- a/lib/ldb-samba/ldif_handlers.c
+++ b/lib/ldb-samba/ldif_handlers.c
@@ -100,13 +100,14 @@ static int ldif_read_objectSid(struct ldb_context *ldb, void *mem_ctx,
 			return -1;
 		}
 
-		*out = data_blob_talloc(mem_ctx, NULL, ndr_size_dom_sid(&sid, 0));
+		*out = data_blob_talloc(mem_ctx, NULL,
+					ndr_size_dom_sid(&sid, 0));
 		if (out->data == NULL) {
 			return -1;
 		}
 
-		ndr_err = ndr_push_struct_into_fixed_blob(out, &sid,
-							  (ndr_push_flags_fn_t)ndr_push_dom_sid);
+		ndr_err = ndr_push_struct_into_fixed_blob(
+			out, &sid, (ndr_push_flags_fn_t)ndr_push_dom_sid);
 		if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
 			return -1;
 		}
diff --git a/librpc/ndr/ndr.c b/librpc/ndr/ndr.c
index c0ab789..4f0e2cb 100644
--- a/librpc/ndr/ndr.c
+++ b/librpc/ndr/ndr.c
@@ -260,7 +260,8 @@ _PUBLIC_ enum ndr_err_code ndr_push_expand(struct ndr_push *ndr, uint32_t extra_
 		}
 		return ndr_push_error(ndr,
 				      NDR_ERR_BUFSIZE,
-				      "Overflow of fixed buffer in push_expand to %u",
+				      "Overflow of fixed buffer in "
+				      "push_expand to %u",
 				      size);
 	}
 
@@ -1282,9 +1283,8 @@ _PUBLIC_ enum ndr_err_code ndr_push_struct_blob(DATA_BLOB *blob, TALLOC_CTX *mem
   used for things like GUIDs, which we expect to be a fixed size, and
   SIDs that we can pre-calculate the size for.
 */
-_PUBLIC_ enum ndr_err_code ndr_push_struct_into_fixed_blob(DATA_BLOB *blob,
-							   const void *p,
-							   ndr_push_flags_fn_t fn)
+_PUBLIC_ enum ndr_err_code ndr_push_struct_into_fixed_blob(
+	DATA_BLOB *blob, const void *p, ndr_push_flags_fn_t fn)
 {
 	struct ndr_push ndr = {
 		.data = blob->data,
@@ -1296,7 +1296,8 @@ _PUBLIC_ enum ndr_err_code ndr_push_struct_into_fixed_blob(DATA_BLOB *blob,
 
 	if (ndr.offset != blob->length) {
 		return ndr_push_error(&ndr, NDR_ERR_BUFSIZE,
-				      "buffer was either to large or small ofs[%u] size[%zu]",
+				      "buffer was either to large or small "
+				      "ofs[%u] size[%zu]",
 				      ndr.offset, blob->length);
 	}
 
diff --git a/librpc/ndr/uuid.c b/librpc/ndr/uuid.c
index 96ab00a..a3f68d1 100644
--- a/librpc/ndr/uuid.c
+++ b/librpc/ndr/uuid.c
@@ -35,8 +35,8 @@ _PUBLIC_ NTSTATUS GUID_to_ndr_blob(const struct GUID *guid, TALLOC_CTX *mem_ctx,
 	if (b->data == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
-	ndr_err = ndr_push_struct_into_fixed_blob(b, guid,
-						  (ndr_push_flags_fn_t)ndr_push_GUID);
+	ndr_err = ndr_push_struct_into_fixed_blob(
+		b, guid, (ndr_push_flags_fn_t)ndr_push_GUID);
 	return ndr_map_error2ntstatus(ndr_err);
 }
 
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index f246faf..448b20a 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -3797,7 +3797,8 @@ NTSTATUS dsdb_get_extended_dn_sid(struct ldb_dn *dn, struct dom_sid *sid, const
 uint32_t dsdb_dn_rmd_flags(struct ldb_dn *dn)
 {
 	uint32_t rmd_flags = 0;
-	NTSTATUS status = dsdb_get_extended_dn_uint32(dn, &rmd_flags, "RMD_FLAGS");
+	NTSTATUS status = dsdb_get_extended_dn_uint32(dn, &rmd_flags,
+						      "RMD_FLAGS");
 	if (NT_STATUS_IS_OK(status)) {
 		return rmd_flags;
 	}
diff --git a/source4/torture/ndr/ndr.c b/source4/torture/ndr/ndr.c
index 5b94299..3b8c76f 100644
--- a/source4/torture/ndr/ndr.c
+++ b/source4/torture/ndr/ndr.c
@@ -395,10 +395,12 @@ static bool test_guid_into_blob(struct torture_context *tctx)
 	guid.node[4] = 10;
 	guid.node[5] = 11;
 
-	ndr_err = ndr_push_struct_into_fixed_blob(&b, &guid,
-						  (ndr_push_flags_fn_t)ndr_push_GUID);
-	torture_assert_ndr_err_equal(tctx, ndr_err, NDR_ERR_SUCCESS, "wrong NDR error");
-	torture_assert_data_blob_equal(tctx, b, exp, "GUID packed wrongly");
+	ndr_err = ndr_push_struct_into_fixed_blob(
+		&b, &guid, (ndr_push_flags_fn_t)ndr_push_GUID);
+	torture_assert_ndr_err_equal(tctx, ndr_err, NDR_ERR_SUCCESS,
+				     "wrong NDR error");
+	torture_assert_data_blob_equal(tctx, b, exp,
+				       "GUID packed wrongly");
 
 	return true;
 }
@@ -427,9 +429,10 @@ static bool test_guid_into_long_blob(struct torture_context *tctx)
 	guid.node[5] = 11;
 
 	torture_assert(tctx, b.data != NULL, "data_blob_talloc failed");
-	ndr_err = ndr_push_struct_into_fixed_blob(&b, &guid,
-						  (ndr_push_flags_fn_t)ndr_push_GUID);
-	torture_assert_ndr_err_equal(tctx, ndr_err, NDR_ERR_BUFSIZE, "wrong NDR error");
+	ndr_err = ndr_push_struct_into_fixed_blob(
+		&b, &guid, (ndr_push_flags_fn_t)ndr_push_GUID);
+	torture_assert_ndr_err_equal(tctx, ndr_err, NDR_ERR_BUFSIZE,
+				     "wrong NDR error");
 
 	return true;
 }
@@ -456,9 +459,10 @@ static bool test_guid_into_short_blob(struct torture_context *tctx)
 	guid.node[4] = 10;
 	guid.node[5] = 11;
 
-	ndr_err = ndr_push_struct_into_fixed_blob(&b, &guid,
-						  (ndr_push_flags_fn_t)ndr_push_GUID);
-	torture_assert_ndr_err_equal(tctx, ndr_err, NDR_ERR_BUFSIZE, "wrong NDR error");
+	ndr_err = ndr_push_struct_into_fixed_blob(
+		&b, &guid, (ndr_push_flags_fn_t)ndr_push_GUID);
+	torture_assert_ndr_err_equal(tctx, ndr_err, NDR_ERR_BUFSIZE,
+				     "wrong NDR error");
 
 	return true;
 }


More information about the samba-technical mailing list