]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Incrementally apply AXFR transfer
authoralessio <alessio@isc.org>
Sun, 3 Nov 2024 20:25:15 +0000 (21:25 +0100)
committeralessio <alessio@isc.org>
Fri, 22 Nov 2024 14:00:55 +0000 (15:00 +0100)
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

13 files changed:
bin/tests/system/masterformat/ns1/compile.sh
bin/tests/system/masterformat/ns1/named.conf.in
bin/tests/system/masterformat/ns2/named.conf.in
bin/tests/system/masterformat/setup.sh
bin/tests/system/masterformat/tests.sh
bin/tests/system/masterformat/tests_sh_masterformat.py
lib/dns/diff.c
lib/dns/include/dns/diff.h
lib/dns/xfrin.c
lib/isc/include/isc/region.h
lib/isc/region.c
tests/dns/Makefile.am
tests/dns/diff_test.c [new file with mode: 0644]

index 6e5a8b12f10f7bedc7c700bdb8db4e58a928511b..52c314023a53b2be4cbe53cf62195a76ca914acb 100755 (executable)
@@ -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
index e96350fd5da5210d0dae360e8586fc7ccf88d2c9..24ecdc41f321b6653a4e891e099dd871b9d148b8 100644 (file)
@@ -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; };
 };
index 790ec731b21808be8093c2975ce498d4b59783bc..862b71a19636f74d3412fd96f79a72f435614561 100644 (file)
@@ -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" {
index 99556f50020c0df1536e0aebb40ae615a02e5078..a09ab63ab129df9028de625fb0f731cbfe77bc6b 100755 (executable)
@@ -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
index c4ce868e34557bc09f28693d2c3f779bb7c4e5b1..e103e328c82828aa89f745de0003d06f96093b8e 100755 (executable)
@@ -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
index 9bd5a815bca6bf2fb9d517d434051617f0d79d75..d8f75dff2eca6d3bc397319d2b51f958ffe9c8d0 100644 (file)
@@ -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-*",
index 2ab896ab48ea9cf38424ad21a016b5437adff00b..2778c2f4c0c3a720082ca4a3311955409d7bdd87 100644 (file)
@@ -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;
index 5b5cc1f0c4a890e689988f95c1f7ca4287e1d7ef..95c0bce1a853cd3c410a2a520e3a7e35c98dce60 100644 (file)
@@ -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
index 824157fd544346c9a4c84ca2935e839ccb1acf27..d9e3a2733cead60c8da79f52b7204bf5d126819d 100644 (file)
@@ -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) {
index 6f775cf7227737074838970f16419176e6a4aa89..536451142f9cee2fb856935617701c3285a679c3 100644 (file)
@@ -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
index e3b1e663e2d46e5c4f347a9073d950be2e5ffb8a..1c53575029cac0ebf23d363052a9189edcf5e600 100644 (file)
@@ -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;
 
index 579e034135bce3ff50a03c28f88fa1e5b1592477..78b209b12989e0d28b6b0ecb4cec91518747e031 100644 (file)
@@ -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 (file)
index 0000000..66d6d9b
--- /dev/null
@@ -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