From: alessio <alessio@isc.org>
Date: Sun, 3 Nov 2024 20:25:15 +0000 (+0100)
Subject: Incrementally apply AXFR transfer
X-Git-Tag: v9.21.3~28^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=99b4f01b334d2a6cd6ff57069f5c06d06a52726b;p=thirdparty%2Fbind9.git

Incrementally apply AXFR transfer

Reintroduce logic to apply diffs when the number of pending tuples is
above 128. The previous strategy of accumulating all the tuples and
pushing them at the end leads to excessive memory consumption during
transfer.

This effectively reverts half of e3892805d6
---

diff --git a/bin/tests/system/masterformat/ns1/compile.sh b/bin/tests/system/masterformat/ns1/compile.sh
index 6e5a8b12f10..52c314023a5 100755
--- a/bin/tests/system/masterformat/ns1/compile.sh
+++ b/bin/tests/system/masterformat/ns1/compile.sh
@@ -28,9 +28,9 @@ $CHECKZONE -D -F raw -L 3333 -o example.db.serial.raw example \
   example.db >/dev/null 2>&1
 $CHECKZONE -D -F raw -o under-limit.db.raw under-limit under-limit.db >/dev/null 2>&1
 $CHECKZONE -D -F raw -o under-limit-kasp.db.raw under-limit-kasp under-limit-kasp.db >/dev/null 2>&1
-$CHECKZONE -D -F raw -o on-limit.db.raw on-limit on-limit.db >/dev/null 2>&1
-$CHECKZONE -D -F raw -o on-limit-kasp.db.raw on-limit-kasp on-limit-kasp.db >/dev/null 2>&1
-$CHECKZONE -D -F raw -o over-limit.db.raw over-limit over-limit.db >/dev/null 2>&1
+$CHECKZONE -D -F raw -o below-limit.db.raw below-limit below-limit.db >/dev/null 2>&1
+$CHECKZONE -D -F raw -o below-limit-kasp.db.raw below-limit-kasp below-limit-kasp.db >/dev/null 2>&1
+$CHECKZONE -D -F raw -o above-limit.db.raw above-limit above-limit.db >/dev/null 2>&1
 $CHECKZONE -D -F raw -o 255types.db.raw 255types 255types.db >/dev/null 2>&1
 
 $KEYGEN -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -f KSK signed >/dev/null 2>&1
diff --git a/bin/tests/system/masterformat/ns1/named.conf.in b/bin/tests/system/masterformat/ns1/named.conf.in
index e96350fd5da..24ecdc41f32 100644
--- a/bin/tests/system/masterformat/ns1/named.conf.in
+++ b/bin/tests/system/masterformat/ns1/named.conf.in
@@ -95,16 +95,16 @@ zone "under-limit-kasp" {
 	allow-transfer { any; };
 };
 
-zone "on-limit" {
+zone "below-limit" {
 	type primary;
-	file "on-limit.db.raw";
+	file "below-limit.db.raw";
 	masterfile-format raw;
 	allow-transfer { any; };
 };
 
-zone "on-limit-kasp" {
+zone "below-limit-kasp" {
 	type primary;
-	file "on-limit-kasp.db.raw";
+	file "below-limit-kasp.db.raw";
 	masterfile-format raw;
 	dnssec-policy masterformat;
 	inline-signing no;
@@ -112,9 +112,9 @@ zone "on-limit-kasp" {
 	allow-transfer { any; };
 };
 
-zone "over-limit" {
+zone "above-limit" {
 	type primary;
-	file "over-limit.db.raw";
+	file "above-limit.db.raw";
 	masterfile-format raw;
 	allow-transfer { any; };
 };
diff --git a/bin/tests/system/masterformat/ns2/named.conf.in b/bin/tests/system/masterformat/ns2/named.conf.in
index 790ec731b21..862b71a1963 100644
--- a/bin/tests/system/masterformat/ns2/named.conf.in
+++ b/bin/tests/system/masterformat/ns2/named.conf.in
@@ -72,18 +72,18 @@ zone "under-limit-kasp" {
 	file "under-limit-kasp.bk";
 };
 
-zone "on-limit" {
+zone "below-limit" {
 	type secondary;
 	primaries { 10.53.0.1; };
 	masterfile-format raw;
-	file "on-limit.bk";
+	file "below-limit.bk";
 };
 
-zone "on-limit-kasp" {
+zone "below-limit-kasp" {
 	type secondary;
 	primaries { 10.53.0.1; };
 	masterfile-format raw;
-	file "on-limit-kasp.bk";
+	file "below-limit-kasp.bk";
 };
 
 zone "255types" {
diff --git a/bin/tests/system/masterformat/setup.sh b/bin/tests/system/masterformat/setup.sh
index 99556f50020..a09ab63ab12 100755
--- a/bin/tests/system/masterformat/setup.sh
+++ b/bin/tests/system/masterformat/setup.sh
@@ -33,23 +33,23 @@ awk 'END {
 }' </dev/null >>ns1/under-limit.db
 cp ns1/under-limit.db ns1/under-limit-kasp.db
 
-cp ns1/empty.db.in ns1/on-limit.db
+cp ns1/empty.db.in ns1/below-limit.db
 awk 'END {
 	 for (i = 0; i < 500; i++ ) { print "500-txt TXT", i; }
 	 for (i = 0; i < 1000; i++ ) { print "1000-txt TXT", i; }
 	 for (i = 0; i < 2000; i++ ) { print "2000-txt TXT", i; }
 	 for (i = 0; i < 2050; i++ ) { print "2050-txt TXT", i; }
-}' </dev/null >>ns1/on-limit.db
-cp ns1/on-limit.db ns1/on-limit-kasp.db
+}' </dev/null >>ns1/below-limit.db
+cp ns1/below-limit.db ns1/below-limit-kasp.db
 
-cp ns1/empty.db.in ns1/over-limit.db
+cp ns1/empty.db.in ns1/above-limit.db
 awk 'END {
 	 for (i = 0; i < 500; i++ ) { print "500-txt TXT", i; }
 	 for (i = 0; i < 1000; i++ ) { print "1000-txt TXT", i; }
 	 for (i = 0; i < 2000; i++ ) { print "2000-txt TXT", i; }
 	 for (i = 0; i < 2050; i++ ) { print "2050-txt TXT", i; }
 	 for (i = 0; i < 2100; i++ ) { print "2100-txt TXT", i; }
-}' </dev/null >>ns1/over-limit.db
+}' </dev/null >>ns1/above-limit.db
 
 cp ns1/empty.db.in ns1/255types.db
 for ntype in $(seq 65280 65534); do
diff --git a/bin/tests/system/masterformat/tests.sh b/bin/tests/system/masterformat/tests.sh
index c4ce868e345..e103e328c82 100755
--- a/bin/tests/system/masterformat/tests.sh
+++ b/bin/tests/system/masterformat/tests.sh
@@ -244,11 +244,11 @@ n=$((n + 1))
 [ $ret -eq 0 ] || echo_i "failed"
 status=$((status + ret))
 
-echo_i "checking that on-limit rdatasets loaded ($n)"
+echo_i "checking that below-limit rdatasets loaded ($n)"
 for _attempt in 0 1 2 3 4 5 6 7 8 9; do
   ret=0
   for rrcount in 500-txt 1000-txt 2000-txt 2050-txt; do
-    $DIG +tcp txt "${rrcount}.on-limit" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
+    $DIG +tcp txt "${rrcount}.below-limit" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
     grep "status: NOERROR" "dig.out.ns1.$rrcount.test$n" >/dev/null || ret=1
   done
   [ $ret -eq 0 ] && break
@@ -258,11 +258,11 @@ n=$((n + 1))
 [ $ret -eq 0 ] || echo_i "failed"
 status=$((status + ret))
 
-echo_i "checking that on-limit rdatasets not transfered ($n)"
+echo_i "checking that below-limit rdatasets not transfered ($n)"
 for _attempt in 0 1 2 3 4 5 6 7 8 9; do
   ret=0
   for rrcount in 500-txt 1000-txt 2000-txt 2050-txt; do
-    $DIG +tcp txt "${rrcount}.on-limit" @10.53.0.2 -p "${PORT}" >"dig.out.ns2.$rrcount.test$n"
+    $DIG +tcp txt "${rrcount}.below-limit" @10.53.0.2 -p "${PORT}" >"dig.out.ns2.$rrcount.test$n"
     grep "status: SERVFAIL" "dig.out.ns2.$rrcount.test$n" >/dev/null || ret=1
   done
   [ $ret -eq 0 ] && break
@@ -272,11 +272,11 @@ n=$((n + 1))
 [ $ret -eq 0 ] || echo_i "failed"
 status=$((status + ret))
 
-echo_i "checking that on-limit-kasp rdatasets loaded ($n)"
+echo_i "checking that below-limit-kasp rdatasets loaded ($n)"
 for _attempt in 0 1 2 3 4 5 6 7 8 9; do
   ret=0
   for rrcount in 500-txt 1000-txt 2000-txt 2050-txt; do
-    $DIG +tcp +dnssec txt "${rrcount}.on-limit-kasp" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
+    $DIG +tcp +dnssec txt "${rrcount}.below-limit-kasp" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
     grep "status: NOERROR" "dig.out.ns1.$rrcount.test$n" >/dev/null || ret=1
     grep "RRSIG" "dig.out.ns1.$rrcount.test$n" >/dev/null || ret=1
   done
@@ -287,11 +287,11 @@ n=$((n + 1))
 [ $ret -eq 0 ] || echo_i "failed"
 status=$((status + ret))
 
-echo_i "checking that on-limit-kasp rdatasets not transfered ($n)"
+echo_i "checking that below-limit-kasp rdatasets not transfered ($n)"
 for _attempt in 0 1 2 3 4 5 6 7 8 9; do
   ret=0
   for rrcount in 500-txt 1000-txt 2000-txt 2050-txt; do
-    $DIG +tcp +dnssec txt "${rrcount}.on-limit-kasp" @10.53.0.2 -p "${PORT}" >"dig.out.ns2.$rrcount.test$n"
+    $DIG +tcp +dnssec txt "${rrcount}.below-limit-kasp" @10.53.0.2 -p "${PORT}" >"dig.out.ns2.$rrcount.test$n"
     grep "status: SERVFAIL" "dig.out.ns2.$rrcount.test$n" >/dev/null || ret=1
   done
   [ $ret -eq 0 ] && break
@@ -301,11 +301,11 @@ n=$((n + 1))
 [ $ret -eq 0 ] || echo_i "failed"
 status=$((status + ret))
 
-echo_i "checking that over-limit rdatasets not loaded ($n)"
+echo_i "checking that above-limit rdatasets not loaded ($n)"
 for _attempt in 0 1 2 3 4 5 6 7 8 9; do
   ret=0
   for rrcount in 500-txt 1000-txt 2000-txt 2050-txt 2100-txt; do
-    $DIG +tcp txt "${rrcount}.over-limit" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
+    $DIG +tcp txt "${rrcount}.above-limit" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
     grep "status: SERVFAIL" "dig.out.ns1.$rrcount.test$n" >/dev/null || ret=1
   done
   [ $ret -eq 0 ] && break
@@ -519,7 +519,7 @@ n=$((n + 1))
 [ $ret -eq 0 ] || echo_i "failed"
 status=$((status + ret))
 
-echo_i "checking that on-limit-kasp rdatasets loaded after re-sign and re-start ($n)"
+echo_i "checking that below-limit-kasp rdatasets loaded after re-sign and re-start ($n)"
 ret=0
 stop_server ns1
 start_server --noclean --restart --port "${PORT}" ns1
@@ -527,7 +527,7 @@ start_server --noclean --restart --port "${PORT}" ns1
 for _attempt in 0 1 2 3 4 5 6 7 8 9; do
   ret=0
   for rrcount in 500-txt 1000-txt 2000-txt 2050-txt; do
-    $DIG +tcp +dnssec txt "${rrcount}.on-limit-kasp" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
+    $DIG +tcp +dnssec txt "${rrcount}.below-limit-kasp" @10.53.0.1 -p "${PORT}" >"dig.out.ns1.$rrcount.test$n"
     grep "status: NOERROR" "dig.out.ns1.$rrcount.test$n" >/dev/null || ret=1
     grep "RRSIG" "dig.out.ns1.$rrcount.test$n" >/dev/null || ret=1
   done
diff --git a/bin/tests/system/masterformat/tests_sh_masterformat.py b/bin/tests/system/masterformat/tests_sh_masterformat.py
index 9bd5a815bca..d8f75dff2ec 100644
--- a/bin/tests/system/masterformat/tests_sh_masterformat.py
+++ b/bin/tests/system/masterformat/tests_sh_masterformat.py
@@ -26,9 +26,9 @@ pytestmark = pytest.mark.extra_artifacts(
         "ns*/K*",
         "ns1/255types.db",
         "ns1/example.db.compat",
-        "ns1/on-limit-kasp.db",
-        "ns1/on-limit.db",
-        "ns1/over-limit.db",
+        "ns1/below-limit-kasp.db",
+        "ns1/below-limit.db",
+        "ns1/above-limit.db",
         "ns1/under-limit-kasp.db",
         "ns1/under-limit.db",
         "ns2/db-*",
diff --git a/lib/dns/diff.c b/lib/dns/diff.c
index 2ab896ab48e..2778c2f4c0c 100644
--- a/lib/dns/diff.c
+++ b/lib/dns/diff.c
@@ -15,6 +15,7 @@
 
 #include <inttypes.h>
 #include <stdbool.h>
+#include <stddef.h>
 #include <stdlib.h>
 
 #include <isc/buffer.h>
@@ -123,6 +124,7 @@ dns_diff_init(isc_mem_t *mctx, dns_diff_t *diff) {
 	diff->mctx = mctx;
 	ISC_LIST_INIT(diff->tuples);
 	diff->magic = DNS_DIFF_MAGIC;
+	diff->size = 0;
 }
 
 void
@@ -133,15 +135,37 @@ dns_diff_clear(dns_diff_t *diff) {
 		ISC_LIST_UNLINK(diff->tuples, t, link);
 		dns_difftuple_free(&t);
 	}
+	diff->size = 0;
 	ENSURE(ISC_LIST_EMPTY(diff->tuples));
 }
 
 void
 dns_diff_append(dns_diff_t *diff, dns_difftuple_t **tuplep) {
+	REQUIRE(DNS_DIFF_VALID(diff));
 	ISC_LIST_APPEND(diff->tuples, *tuplep, link);
+	diff->size += 1;
 	*tuplep = NULL;
 }
 
+bool
+dns_diff_is_boundary(const dns_diff_t *diff, dns_name_t *new_name) {
+	REQUIRE(DNS_DIFF_VALID(diff));
+	REQUIRE(DNS_NAME_VALID(new_name));
+
+	if (ISC_LIST_EMPTY(diff->tuples)) {
+		return false;
+	}
+
+	dns_difftuple_t *tail = ISC_LIST_TAIL(diff->tuples);
+	return !dns_name_caseequal(&tail->name, new_name);
+}
+
+size_t
+dns_diff_size(const dns_diff_t *diff) {
+	REQUIRE(DNS_DIFF_VALID(diff));
+	return diff->size;
+}
+
 /* XXX this is O(N) */
 
 void
@@ -170,6 +194,9 @@ dns_diff_appendminimal(dns_diff_t *diff, dns_difftuple_t **tuplep) {
 		    ot->ttl == (*tuplep)->ttl)
 		{
 			ISC_LIST_UNLINK(diff->tuples, ot, link);
+			INSIST(diff->size > 0);
+			diff->size -= 1;
+
 			if ((*tuplep)->op == ot->op) {
 				UNEXPECTED_ERROR("unexpected non-minimal diff");
 			} else {
@@ -182,6 +209,7 @@ dns_diff_appendminimal(dns_diff_t *diff, dns_difftuple_t **tuplep) {
 
 	if (*tuplep != NULL) {
 		ISC_LIST_APPEND(diff->tuples, *tuplep, link);
+		diff->size += 1;
 		*tuplep = NULL;
 	}
 }
@@ -253,7 +281,8 @@ optotext(dns_diffop_t op) {
 }
 
 static isc_result_t
-diff_apply(dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver, bool warn) {
+diff_apply(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver,
+	   bool warn) {
 	dns_difftuple_t *t;
 	dns_dbnode_t *node = NULL;
 	isc_result_t result;
@@ -493,19 +522,20 @@ failure:
 }
 
 isc_result_t
-dns_diff_apply(dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver) {
+dns_diff_apply(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver) {
 	return diff_apply(diff, db, ver, true);
 }
 
 isc_result_t
-dns_diff_applysilently(dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver) {
+dns_diff_applysilently(const dns_diff_t *diff, dns_db_t *db,
+		       dns_dbversion_t *ver) {
 	return diff_apply(diff, db, ver, false);
 }
 
 /* XXX this duplicates lots of code in diff_apply(). */
 
 isc_result_t
-dns_diff_load(dns_diff_t *diff, dns_rdatacallbacks_t *callbacks) {
+dns_diff_load(const dns_diff_t *diff, dns_rdatacallbacks_t *callbacks) {
 	dns_difftuple_t *t;
 	isc_result_t result;
 
@@ -641,7 +671,7 @@ diff_tuple_tordataset(dns_difftuple_t *t, dns_rdata_t *rdata,
 }
 
 isc_result_t
-dns_diff_print(dns_diff_t *diff, FILE *file) {
+dns_diff_print(const dns_diff_t *diff, FILE *file) {
 	isc_result_t result;
 	dns_difftuple_t *t;
 	char *mem = NULL;
diff --git a/lib/dns/include/dns/diff.h b/lib/dns/include/dns/diff.h
index 5b5cc1f0c4a..95c0bce1a85 100644
--- a/lib/dns/include/dns/diff.h
+++ b/lib/dns/include/dns/diff.h
@@ -27,6 +27,8 @@
  *** Imports
  ***/
 
+#include <stddef.h>
+
 #include <isc/lang.h>
 #include <isc/magic.h>
 
@@ -97,6 +99,7 @@ struct dns_diff {
 	unsigned int	    magic;
 	isc_mem_t	   *mctx;
 	dns_difftuplelist_t tuples;
+	size_t		    size;
 };
 
 /* Type of comparison function for sorting diffs. */
@@ -186,12 +189,32 @@ dns_diff_append(dns_diff_t *diff, dns_difftuple_t **tuple);
 /*%<
  * Append a single tuple to a diff.
  *
- *\li	'diff' is a valid diff.
+ * Requires:
+ * \li	'diff' is a valid diff.
  * \li	'*tuple' is a valid tuple.
  *
  * Ensures:
- *\li	*tuple is NULL.
- *\li	The tuple has been freed, or will be freed when the diff is cleared.
+ * \li	*tuple is NULL.
+ * \li	The tuple has been freed, or will be freed when the diff is cleared.
+ */
+
+bool
+dns_diff_is_boundary(const dns_diff_t *diff, dns_name_t *name);
+/*%<
+ * Checks if 'name' is equal, up to case, to the last name of the diff.
+ *
+ * Requires:
+ * \li	'diff' is a valid diff.
+ * \li	'name' is a valid dns name.
+ */
+
+size_t
+dns_diff_size(const dns_diff_t *diff);
+/*%<
+ * Returns the number of elements in the diff.
+ *
+ * Requires:
+ * \li	'diff' is a valid diff.
  */
 
 void
@@ -218,9 +241,10 @@ dns_diff_sort(dns_diff_t *diff, dns_diff_compare_func *compare);
  */
 
 isc_result_t
-dns_diff_apply(dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver);
+dns_diff_apply(const dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver);
 isc_result_t
-dns_diff_applysilently(dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver);
+dns_diff_applysilently(const dns_diff_t *diff, dns_db_t *db,
+		       dns_dbversion_t *ver);
 /*%<
  * Apply 'diff' to the database 'db'.
  *
@@ -239,7 +263,7 @@ dns_diff_applysilently(dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *ver);
  */
 
 isc_result_t
-dns_diff_load(dns_diff_t *diff, dns_rdatacallbacks_t *callbacks);
+dns_diff_load(const dns_diff_t *diff, dns_rdatacallbacks_t *callbacks);
 /*%<
  * Like dns_diff_apply, but for use when loading a new database
  * instead of modifying an existing one.  This bypasses the
@@ -251,7 +275,7 @@ dns_diff_load(dns_diff_t *diff, dns_rdatacallbacks_t *callbacks);
  */
 
 isc_result_t
-dns_diff_print(dns_diff_t *diff, FILE *file);
+dns_diff_print(const dns_diff_t *diff, FILE *file);
 
 /*%<
  * Print the differences to 'file' or if 'file' is NULL via the
diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c
index 824157fd544..d9e3a2733ce 100644
--- a/lib/dns/xfrin.c
+++ b/lib/dns/xfrin.c
@@ -201,9 +201,13 @@ struct dns_xfrin {
 #define XFRIN_MAGIC    ISC_MAGIC('X', 'f', 'r', 'I')
 #define VALID_XFRIN(x) ISC_MAGIC_VALID(x, XFRIN_MAGIC)
 
+#define XFRIN_WORK_MAGIC    ISC_MAGIC('X', 'f', 'r', 'W')
+#define VALID_XFRIN_WORK(x) ISC_MAGIC_VALID(x, XFRIN_WORK_MAGIC)
+
 typedef struct xfrin_work {
-	dns_xfrin_t *xfr;
+	unsigned int magic;
 	isc_result_t result;
+	dns_xfrin_t *xfr;
 } xfrin_work_t;
 
 /**************************************************************************/
@@ -300,6 +304,9 @@ failure:
 	return result;
 }
 
+static void
+axfr_apply(void *arg);
+
 static isc_result_t
 axfr_putdata(dns_xfrin_t *xfr, dns_diffop_t op, dns_name_t *name, dns_ttl_t ttl,
 	     dns_rdata_t *rdata) {
@@ -312,8 +319,21 @@ axfr_putdata(dns_xfrin_t *xfr, dns_diffop_t op, dns_name_t *name, dns_ttl_t ttl,
 	}
 
 	CHECK(dns_zone_checknames(xfr->zone, name, rdata));
+	if (dns_diff_size(&xfr->diff) > 128 &&
+	    dns_diff_is_boundary(&xfr->diff, name))
+	{
+		xfrin_work_t work = (xfrin_work_t){
+			.magic = XFRIN_WORK_MAGIC,
+			.result = ISC_R_UNSET,
+			.xfr = xfr,
+		};
+		axfr_apply((void *)&work);
+		CHECK(work.result);
+	}
+
 	dns_difftuple_create(xfr->diff.mctx, op, name, ttl, rdata, &tuple);
 	dns_diff_append(&xfr->diff, &tuple);
+
 	result = ISC_R_SUCCESS;
 failure:
 	return result;
@@ -325,12 +345,14 @@ failure:
 static void
 axfr_apply(void *arg) {
 	xfrin_work_t *work = arg;
+	REQUIRE(VALID_XFRIN_WORK(work));
+
 	dns_xfrin_t *xfr = work->xfr;
+	REQUIRE(VALID_XFRIN(xfr));
+
 	isc_result_t result = ISC_R_SUCCESS;
 	uint64_t records;
 
-	REQUIRE(VALID_XFRIN(xfr));
-
 	if (atomic_load(&xfr->shuttingdown)) {
 		result = ISC_R_SHUTTINGDOWN;
 		goto failure;
@@ -357,6 +379,7 @@ axfr_apply_done(void *arg) {
 	isc_result_t result = work->result;
 
 	REQUIRE(VALID_XFRIN(xfr));
+	REQUIRE(VALID_XFRIN_WORK(work));
 
 	if (atomic_load(&xfr->shuttingdown)) {
 		result = ISC_R_SHUTTINGDOWN;
@@ -392,8 +415,9 @@ axfr_commit(dns_xfrin_t *xfr) {
 
 	xfrin_work_t *work = isc_mem_get(xfr->mctx, sizeof(*work));
 	*work = (xfrin_work_t){
-		.xfr = dns_xfrin_ref(xfr),
+		.magic = XFRIN_WORK_MAGIC,
 		.result = ISC_R_UNSET,
+		.xfr = dns_xfrin_ref(xfr),
 	};
 	xfr->diff_running = true;
 	isc_work_enqueue(xfr->loop, axfr_apply, axfr_apply_done, work);
@@ -527,6 +551,7 @@ ixfr_apply(void *arg) {
 	isc_result_t result = ISC_R_SUCCESS;
 
 	REQUIRE(VALID_XFRIN(xfr));
+	REQUIRE(VALID_XFRIN_WORK(work));
 
 	struct __cds_wfcq_head diff_head;
 	struct cds_wfcq_tail diff_tail;
@@ -564,11 +589,13 @@ ixfr_apply(void *arg) {
 static void
 ixfr_apply_done(void *arg) {
 	xfrin_work_t *work = arg;
-	dns_xfrin_t *xfr = work->xfr;
-	isc_result_t result = work->result;
+	REQUIRE(VALID_XFRIN_WORK(work));
 
+	dns_xfrin_t *xfr = work->xfr;
 	REQUIRE(VALID_XFRIN(xfr));
 
+	isc_result_t result = work->result;
+
 	if (atomic_load(&xfr->shuttingdown)) {
 		result = ISC_R_SHUTTINGDOWN;
 	}
@@ -629,8 +656,9 @@ ixfr_commit(dns_xfrin_t *xfr) {
 	if (!xfr->diff_running) {
 		xfrin_work_t *work = isc_mem_get(xfr->mctx, sizeof(*work));
 		*work = (xfrin_work_t){
-			.xfr = dns_xfrin_ref(xfr),
+			.magic = XFRIN_WORK_MAGIC,
 			.result = ISC_R_UNSET,
+			.xfr = dns_xfrin_ref(xfr),
 		};
 		xfr->diff_running = true;
 		isc_work_enqueue(xfr->loop, ixfr_apply, ixfr_apply_done, work);
@@ -1097,7 +1125,6 @@ xfrin_reset(dns_xfrin_t *xfr) {
 	}
 
 	dns_diff_clear(&xfr->diff);
-
 	xfr->ixfr.diffs = 0;
 
 	if (xfr->ixfr.journal != NULL) {
diff --git a/lib/isc/include/isc/region.h b/lib/isc/include/isc/region.h
index 6f775cf7227..536451142f9 100644
--- a/lib/isc/include/isc/region.h
+++ b/lib/isc/include/isc/region.h
@@ -85,6 +85,8 @@ isc_region_compare(isc_region_t *r1, isc_region_t *r2);
  * Requires:
  *\li	'r1' is a valid region
  *\li	'r2' is a valid region
+ *\li	'r1->base' is not null
+ *\li	'r2->base' is not null
  *
  * Returns:
  *\li	 < 0 if r1 is lexicographically less than r2
diff --git a/lib/isc/region.c b/lib/isc/region.c
index e3b1e663e2d..1c53575029c 100644
--- a/lib/isc/region.c
+++ b/lib/isc/region.c
@@ -26,6 +26,8 @@ isc_region_compare(isc_region_t *r1, isc_region_t *r2) {
 
 	REQUIRE(r1 != NULL);
 	REQUIRE(r2 != NULL);
+	REQUIRE(r1->base != NULL);
+	REQUIRE(r2->base != NULL);
 
 	l = (r1->length < r2->length) ? r1->length : r2->length;
 
diff --git a/tests/dns/Makefile.am b/tests/dns/Makefile.am
index 579e034135b..78b209b1298 100644
--- a/tests/dns/Makefile.am
+++ b/tests/dns/Makefile.am
@@ -24,6 +24,7 @@ check_PROGRAMS =		\
 	dbdiff_test		\
 	dbiterator_test		\
 	dbversion_test		\
+	diff_test		\
 	dispatch_test		\
 	dns64_test		\
 	dst_test		\
diff --git a/tests/dns/diff_test.c b/tests/dns/diff_test.c
new file mode 100644
index 00000000000..66d6d9b0154
--- /dev/null
+++ b/tests/dns/diff_test.c
@@ -0,0 +1,144 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+/* sched.h must be imported before cmocka to avoid redefinition errors */
+#include <sched.h> /* IWYU pragma: keep */
+#include <setjmp.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "isc/list.h"
+
+#define UNIT_TESTING
+#include <cmocka.h>
+
+#include <dns/diff.h>
+
+#include <tests/dns.h>
+
+unsigned char data_1[] = "\006name_1";
+unsigned char offsets_1[] = { 0, 7 };
+dns_name_t name_1 = DNS_NAME_INITABSOLUTE(data_1, offsets_1);
+
+unsigned char data_2[] = "\006name_2";
+unsigned char offsets_2[] = { 0, 7 };
+dns_name_t name_2 = DNS_NAME_INITABSOLUTE(data_2, offsets_2);
+
+unsigned char data_3[] = "\006name_3";
+unsigned char offsets_3[] = { 0, 7 };
+dns_name_t name_3 = DNS_NAME_INITABSOLUTE(data_3, offsets_3);
+
+unsigned char data_dup[] = "\006name_1";
+unsigned char offsets_dup[] = { 0, 7 };
+dns_name_t name_dup = DNS_NAME_INITABSOLUTE(data_dup, offsets_dup);
+
+unsigned char data_nodup[] = "\006name_1";
+unsigned char offsets_nodup[] = { 0, 7 };
+dns_name_t name_nodup = DNS_NAME_INITABSOLUTE(data_nodup, offsets_nodup);
+
+static size_t
+count_elements(const dns_diff_t *diff) {
+	dns_difftuple_t *ot = NULL;
+	size_t count = 0;
+
+	for (ot = ISC_LIST_HEAD(diff->tuples); ot != NULL;
+	     ot = ISC_LIST_NEXT(ot, link))
+	{
+		++count;
+	}
+
+	return count;
+}
+
+static void
+prepare_rdata(dns_rdata_t *rdata, unsigned char *dest, size_t dest_size) {
+	dns_rdataclass_t rdclass = dns_rdataclass_in;
+	dns_rdatatype_t type = dns_rdatatype_wallet;
+	const char text[] = "cid-example wid-example";
+
+	*rdata = (dns_rdata_t)DNS_RDATA_INIT;
+	isc_result_t result = dns_test_rdatafromstring(
+		rdata, rdclass, type, dest, dest_size, text, false);
+	INSIST(result == ISC_R_SUCCESS);
+}
+
+ISC_RUN_TEST_IMPL(dns_diff_size) {
+	dns_diff_t diff;
+	dns_diff_init(mctx, &diff);
+
+	assert_true(dns_diff_size(&diff) == 0);
+
+	dns_rdata_t rdatas[5] = { 0 };
+	unsigned char bufs[sizeof(rdatas) / sizeof(*rdatas)][128] = { 0 };
+	size_t buf_len = sizeof(bufs[0]);
+
+	for (size_t idx = 0; idx < sizeof(rdatas) / sizeof(*rdatas); ++idx) {
+		prepare_rdata(&rdatas[idx], bufs[idx], buf_len);
+	}
+
+	dns_difftuple_t *tup_1 = NULL, *tup_2 = NULL, *tup_3 = NULL;
+	dns_difftuple_create(mctx, DNS_DIFFOP_ADD, &name_1, 1, &rdatas[0],
+			     &tup_1);
+	dns_difftuple_create(mctx, DNS_DIFFOP_DEL, &name_2, 1, &rdatas[1],
+			     &tup_2);
+	dns_difftuple_create(mctx, DNS_DIFFOP_DEL, &name_3, 1, &rdatas[2],
+			     &tup_3);
+
+	dns_difftuple_t *tup_dup = NULL, *tup_nodup = NULL;
+	dns_difftuple_create(mctx, DNS_DIFFOP_DEL, &name_dup, 1, &rdatas[3],
+			     &tup_dup);
+	dns_difftuple_create(mctx, DNS_DIFFOP_ADD, &name_nodup, 1, &rdatas[4],
+			     &tup_nodup);
+
+	dns_diff_append(&diff, &tup_1);
+	assert_true(dns_diff_size(&diff) == 1);
+	assert_true(dns_diff_size(&diff) == count_elements(&diff));
+
+	dns_diff_append(&diff, &tup_2);
+	assert_true(dns_diff_size(&diff) == 2);
+	assert_true(dns_diff_size(&diff) == count_elements(&diff));
+
+	dns_diff_appendminimal(&diff, &tup_dup);
+	assert_true(dns_diff_size(&diff) == 1);
+	assert_true(dns_diff_size(&diff) == count_elements(&diff));
+
+	dns_diff_append(&diff, &tup_3);
+	assert_true(dns_diff_size(&diff) == 2);
+	assert_true(dns_diff_size(&diff) == count_elements(&diff));
+
+	dns_diff_appendminimal(&diff, &tup_nodup);
+	assert_true(dns_diff_size(&diff) == 3);
+	assert_true(dns_diff_size(&diff) == count_elements(&diff));
+
+	dns_diff_clear(&diff);
+	assert_true(dns_diff_size(&diff) == 0);
+	assert_true(dns_diff_size(&diff) == count_elements(&diff));
+
+	dns_difftuple_t *to_clear[] = { tup_1, tup_2, tup_3, tup_dup,
+					tup_nodup };
+	size_t to_clear_size = sizeof(to_clear) / sizeof(*to_clear);
+
+	for (size_t idx = 0; idx < to_clear_size; ++idx) {
+		if (to_clear[idx] != NULL) {
+			dns_difftuple_free(&to_clear[idx]);
+		}
+	}
+}
+
+ISC_TEST_LIST_START
+ISC_TEST_ENTRY(dns_diff_size)
+ISC_TEST_LIST_END
+
+ISC_TEST_MAIN