From 99b4f01b334d2a6cd6ff57069f5c06d06a52726b Mon Sep 17 00:00:00 2001 From: alessio Date: Sun, 3 Nov 2024 21:25:15 +0100 Subject: [PATCH] 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 --- bin/tests/system/masterformat/ns1/compile.sh | 6 +- .../system/masterformat/ns1/named.conf.in | 12 +- .../system/masterformat/ns2/named.conf.in | 8 +- bin/tests/system/masterformat/setup.sh | 10 +- bin/tests/system/masterformat/tests.sh | 24 +-- .../masterformat/tests_sh_masterformat.py | 6 +- lib/dns/diff.c | 40 ++++- lib/dns/include/dns/diff.h | 38 ++++- lib/dns/xfrin.c | 43 +++++- lib/isc/include/isc/region.h | 2 + lib/isc/region.c | 2 + tests/dns/Makefile.am | 1 + tests/dns/diff_test.c | 144 ++++++++++++++++++ 13 files changed, 283 insertions(+), 53 deletions(-) create mode 100644 tests/dns/diff_test.c diff --git a/bin/tests/system/masterformat/ns1/compile.sh b/bin/tests/system/masterformat/ns1/compile.sh index 6e5a8b12f1..52c314023a 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 e96350fd5d..24ecdc41f3 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 790ec731b2..862b71a196 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 99556f5002..a09ab63ab1 100755 --- a/bin/tests/system/masterformat/setup.sh +++ b/bin/tests/system/masterformat/setup.sh @@ -33,23 +33,23 @@ awk 'END { }' >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; } -}' >ns1/on-limit.db -cp ns1/on-limit.db ns1/on-limit-kasp.db +}' >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; } -}' >ns1/over-limit.db +}' >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 c4ce868e34..e103e328c8 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 9bd5a815bc..d8f75dff2e 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 2ab896ab48..2778c2f4c0 100644 --- a/lib/dns/diff.c +++ b/lib/dns/diff.c @@ -15,6 +15,7 @@ #include #include +#include #include #include @@ -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 5b5cc1f0c4..95c0bce1a8 100644 --- a/lib/dns/include/dns/diff.h +++ b/lib/dns/include/dns/diff.h @@ -27,6 +27,8 @@ *** Imports ***/ +#include + #include #include @@ -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 824157fd54..d9e3a2733c 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 6f775cf722..536451142f 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 e3b1e663e2..1c53575029 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 579e034135..78b209b129 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 0000000000..66d6d9b015 --- /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 /* IWYU pragma: keep */ +#include +#include +#include +#include +#include + +#include "isc/list.h" + +#define UNIT_TESTING +#include + +#include + +#include + +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