From: Vladimír Čunát <vladimir.cunat@nic.cz>
Date: Mon, 28 Aug 2023 08:32:08 +0000 (+0200)
Subject: lib/rules: avoid using preallocated LMDB writes
X-Git-Tag: v6.0.4~1^2~7
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=aa4fb62f20e4bda26e0aedb4136c327e82ab2593;p=thirdparty%2Fknot-resolver.git

lib/rules: avoid using preallocated LMDB writes

They're a bit more efficient, but they can't work with LMDB multi-sets.
Also, write performance of rules isn't that important and typically
each rule is quite tiny anyway.
---

diff --git a/lib/rules/api.c b/lib/rules/api.c
index 67cbe7886..84339e6f4 100644
--- a/lib/rules/api.c
+++ b/lib/rules/api.c
@@ -548,27 +548,24 @@ int kr_rule_local_data_ins(const knot_rrset_t *rrs, const knot_rdataset_t *sig_r
 int local_data_ins(knot_db_val_t key, const knot_rrset_t *rrs,
 			const knot_rdataset_t *sig_rds, kr_rule_tags_t tags)
 {
-	// Allocate the data in DB.
+	// Prepare the data into a temporary buffer.
 	const int rr_ssize = rdataset_dematerialize_size(&rrs->rrs);
-	const int to_alloc = sizeof(tags) + sizeof(rrs->ttl) + rr_ssize
-			+ rdataset_dematerialize_size(sig_rds);
-	knot_db_val_t val = { .data = NULL, .len = to_alloc };
+	const int val_len = sizeof(tags) + sizeof(rrs->ttl) + rr_ssize
+				+ rdataset_dematerialize_size(sig_rds);
+	uint8_t buf[val_len], *data = buf;
+	memcpy(data, &tags, sizeof(tags));
+	data += sizeof(tags);
+	memcpy(data, &rrs->ttl, sizeof(rrs->ttl));
+	data += sizeof(rrs->ttl);
+	rdataset_dematerialize(&rrs->rrs, data);
+	data += rr_ssize;
+	rdataset_dematerialize(sig_rds, data);
+
+	knot_db_val_t val = { .data = buf, .len = val_len };
 	int ret = ruledb_op(write, &key, &val, 1);
-	if (ret) {
-		// ENOSPC seems to be the only expectable error.
-		kr_assert(ret == kr_error(ENOSPC));
-		return kr_error(ret);
-	}
-
-	// Write all the data.
-	memcpy(val.data, &tags, sizeof(tags));
-	val.data += sizeof(tags);
-	memcpy(val.data, &rrs->ttl, sizeof(rrs->ttl));
-	val.data += sizeof(rrs->ttl);
-	rdataset_dematerialize(&rrs->rrs, val.data);
-	val.data += rr_ssize;
-	rdataset_dematerialize(sig_rds, val.data);
-	return kr_ok();
+	// ENOSPC seems to be the only expectable error.
+	kr_assert(ret == 0 || ret == kr_error(ENOSPC));
+	return ret;
 }
 int kr_rule_local_data_del(const knot_rrset_t *rrs, kr_rule_tags_t tags)
 {
@@ -794,28 +791,25 @@ int kr_rule_local_subtree(const knot_dname_t *apex, enum kr_rule_sub_t type,
 	uint8_t key_data[KEY_MAXLEN];
 	knot_db_val_t key = zla_key(apex, key_data);
 
-	knot_db_val_t val = {
-		.data = NULL,
-		.len = sizeof(tags) + sizeof(ztype),
-	};
+	// Prepare the data into a temporary buffer.
 	const bool has_ttl = ttl != KR_RULE_TTL_DEFAULT;
-	if (has_ttl)
-		val.len += sizeof(ttl);
-	int ret = ruledb_op(write, &key, &val, 1);
-	if (ret) {
-		// ENOSPC seems to be the only expectable error.
-		kr_assert(ret == kr_error(ENOSPC));
-		return kr_error(ret);
-	}
-	memcpy(val.data, &tags, sizeof(tags));
-	val.data += sizeof(tags);
-	memcpy(val.data, &ztype, sizeof(ztype));
-	val.data += sizeof(ztype);
+	const int val_len = sizeof(tags) + sizeof(ztype) + (has_ttl ? sizeof(ttl) : 0);
+	uint8_t buf[val_len], *data = buf;
+	memcpy(data, &tags, sizeof(tags));
+	data += sizeof(tags);
+	memcpy(data, &ztype, sizeof(ztype));
+	data += sizeof(ztype);
 	if (has_ttl) {
-		memcpy(val.data, &ttl, sizeof(ttl));
-		val.data += sizeof(ttl);
+		memcpy(data, &ttl, sizeof(ttl));
+		data += sizeof(ttl);
 	}
-	return kr_ok();
+	kr_require(data == buf + val_len);
+
+	knot_db_val_t val = { .data = buf, .len = val_len };
+	int ret = ruledb_op(write, &key, &val, 1);
+	// ENOSPC seems to be the only expectable error.
+	kr_assert(ret == 0 || ret == kr_error(ENOSPC));
+	return ret;
 }
 
 
diff --git a/lib/rules/forward.c b/lib/rules/forward.c
index 234a04ecd..f010603f8 100644
--- a/lib/rules/forward.c
+++ b/lib/rules/forward.c
@@ -143,27 +143,28 @@ int kr_rule_forward(const knot_dname_t *apex, kr_rule_fwd_flags_t flags,
 	uint8_t key_data[KEY_MAXLEN];
 	knot_db_val_t key = zla_key(apex, key_data);
 
-	const size_t targets_bytes = count * sizeof(union kr_sockaddr);
-	knot_db_val_t val = {
-		.data = NULL,
-		.len = sizeof(tags) + sizeof(ztype) + sizeof(flags) + targets_bytes,
-	};
-	int ret = ruledb_op(write, &key, &val, 1);
-	if (kr_fails_assert(ret >= 0))
-		return kr_error(ret);
-	memcpy(val.data, &tags, sizeof(tags));
-	val.data += sizeof(tags);
-	memcpy(val.data, &ztype, sizeof(ztype));
-	val.data += sizeof(ztype);
-	memcpy(val.data, &flags, sizeof(flags));
-	val.data += sizeof(flags);
+	// Prepare the data into a temporary buffer.
+	const int targets_len = count * sizeof(union kr_sockaddr);
+	const int val_len = sizeof(tags) + sizeof(ztype) + sizeof(flags) + targets_len;
+	uint8_t buf[val_len], *data = buf;
+	memcpy(data, &tags, sizeof(tags));
+	data += sizeof(tags);
+	memcpy(data, &ztype, sizeof(ztype));
+	data += sizeof(ztype);
+	memcpy(data, &flags, sizeof(flags));
+	data += sizeof(flags);
+	// targets[i] may be shorter than union kr_sockaddr, so we zero it in advance
+	memset(data, 0, targets_len);
 	for (int i = 0; i < count; ++i) {
-		// targets[i] may be shorter than union kr_sockaddr, so we zero-pad
 		// LATER: for is_auth we really drop anything but address (e.g. port!=53)
-		union kr_sockaddr a = { 0 };
-		memcpy(&a, targets[i], kr_sockaddr_len(targets[i]));
-		memcpy(val.data, &a, sizeof(a));
-		val.data += sizeof(a);
+		memcpy(data, targets[i], kr_sockaddr_len(targets[i]));
+		data += sizeof(union kr_sockaddr);
 	}
-	return kr_ok();
+	kr_require(data == buf + val_len);
+
+	knot_db_val_t val = { .data = buf, .len = val_len };
+	int ret = ruledb_op(write, &key, &val, 1);
+	// ENOSPC seems to be the only expectable error.
+	kr_assert(ret == 0 || ret == kr_error(ENOSPC));
+	return ret;
 }