From 98b55eb44280e9204b851540f9113de73596f1a6 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Sat, 22 Feb 2020 00:37:05 -0800 Subject: [PATCH] improve calculation of database transfer size - change name of 'bytes' to 'xfrsize' in dns_db_getsize() parameter list and related variables; this is a more accurate representation of what the function is doing - change the size calculations in dns_db_getsize() to more accurately represent the space needed for a *XFR message or journal file to contain the data in the database. previously we returned the sizes of all rdataslabs, including header overhead and offset tables, which resulted in the database size being reported as much larger than the equivalent *XFR or journal. - map files caused a particular problem here: the fullname can't be determined from the node while a file is being deserialized, because the uppernode pointers aren't set yet. so we store "full name length" in the dns_rbtnode structure while serializing, and clear it after deserialization is complete. --- lib/dns/include/dns/db.h | 12 ++++--- lib/dns/include/dns/rbt.h | 24 ++++++++----- lib/dns/include/dns/rdataslab.h | 9 +++++ lib/dns/mapapi | 2 +- lib/dns/rbt.c | 52 +++++++++++++++------------ lib/dns/rbtdb.c | 64 +++++++++++++++++++++++---------- lib/dns/rdataslab.c | 28 +++++++++++++++ lib/dns/tests/rbt_test.c | 43 ++++++++++++++++++++++ lib/dns/win32/libdns.def.in | 4 +-- 9 files changed, 180 insertions(+), 58 deletions(-) diff --git a/lib/dns/include/dns/db.h b/lib/dns/include/dns/db.h index a092600274c..3227ee3f5bf 100644 --- a/lib/dns/include/dns/db.h +++ b/lib/dns/include/dns/db.h @@ -1468,16 +1468,20 @@ dns_db_getnsec3parameters(dns_db_t *db, dns_dbversion_t *version, isc_result_t dns_db_getsize(dns_db_t *db, dns_dbversion_t *version, uint64_t *records, - uint64_t *bytes); + uint64_t *xfrsize); /*%< - * Get the number of records in the given version of the database as well - * as the number bytes used to store those records. + * On success if 'records' is not NULL, it is set to the number of records + * in the given version of the database. If 'xfrisize' is not NULL, it is + * set to the approximate number of bytes needed to transfer the records, + * counting name, TTL, type, class, and rdata for each RR. (This is meant + * to be a rough approximation of the size of a full zone transfer, though + * it does not take into account DNS message overhead or name compression.) * * Requires: * \li 'db' is a valid zone database. * \li 'version' is NULL or a valid version. * \li 'records' is NULL or a pointer to return the record count in. - * \li 'bytes' is NULL or a pointer to return the byte count in. + * \li 'xfrsize' is NULL or a pointer to return the byte count in. * * Returns: * \li #ISC_R_SUCCESS diff --git a/lib/dns/include/dns/rbt.h b/lib/dns/include/dns/rbt.h index aaf5f58ca54..8c2139eb143 100644 --- a/lib/dns/include/dns/rbt.h +++ b/lib/dns/include/dns/rbt.h @@ -105,6 +105,13 @@ struct dns_rbtnode { unsigned int down_is_relative : 1; unsigned int data_is_relative : 1; + /* + * full name length; set during serialization, and used + * during deserialization to calculate database size. + * should be cleared after use. + */ + unsigned int fullnamelen : 8; /*%< range is 1..255 */ + /* node needs to be cleaned from rpz */ unsigned int rpz : 1; unsigned int : 0; /* end of bitfields c/o tree lock */ @@ -152,9 +159,8 @@ struct dns_rbtnode { uint8_t : 0; /* start of bitfields c/o node lock */ uint8_t dirty : 1; uint8_t wild : 1; - uint8_t : 0; /* end of bitfields c/o node lock */ - uint16_t locknum; /* note that this is not in the bitfield - * */ + uint8_t : 0; /* end of bitfields c/o node lock */ + uint16_t locknum; /* note that this is not in the bitfield */ isc_refcount_t references; /*@}*/ }; @@ -1029,12 +1035,12 @@ dns_rbtnodechain_nextflat(dns_rbtnodechain_t *chain, dns_name_t *name); * Find the next node at the current depth in DNSSEC order. */ -void -dns_rbtnode_nodename(dns_rbtnode_t *node, dns_name_t *name); - -dns_rbtnode_t * -dns_rbt_root(dns_rbt_t *rbt); - +unsigned int +dns__rbtnode_namelen(dns_rbtnode_t *node); +/*%< + * Returns the length of the full name of the node. Used only internally + * and in unit tests. + */ ISC_LANG_ENDDECLS #endif /* DNS_RBT_H */ diff --git a/lib/dns/include/dns/rdataslab.h b/lib/dns/include/dns/rdataslab.h index dfcf9608171..81b3494e8de 100644 --- a/lib/dns/include/dns/rdataslab.h +++ b/lib/dns/include/dns/rdataslab.h @@ -96,6 +96,15 @@ dns_rdataslab_size(unsigned char *slab, unsigned int reservelen); *\li The number of bytes in the slab, including the reservelen. */ +unsigned int +dns_rdataslab_rdatasize(unsigned char *slab, unsigned int reservelen); +/*%< + * Return the size of the rdata in an rdataslab. + * + * Requires: + *\li 'slab' points to a slab. + */ + unsigned int dns_rdataslab_count(unsigned char *slab, unsigned int reservelen); /*%< diff --git a/lib/dns/mapapi b/lib/dns/mapapi index 3a710613e9e..d7a692f9645 100644 --- a/lib/dns/mapapi +++ b/lib/dns/mapapi @@ -13,4 +13,4 @@ # Whenever releasing a new major release of BIND9, set this value # back to 1.0 when releasing the first alpha. Map files are *never* # compatible across major releases. -MAPAPI=1.0 +MAPAPI=2.0 diff --git a/lib/dns/rbt.c b/lib/dns/rbt.c index a9fdf3929f2..46967018b05 100644 --- a/lib/dns/rbt.c +++ b/lib/dns/rbt.c @@ -282,21 +282,6 @@ NODENAME(dns_rbtnode_t *node, dns_name_t *name) { name->attributes |= DNS_NAMEATTR_READONLY; } -void -dns_rbtnode_nodename(dns_rbtnode_t *node, dns_name_t *name) { - name->length = NAMELEN(node); - name->labels = OFFSETLEN(node); - name->ndata = NAME(node); - name->offsets = OFFSETS(node); - name->attributes = ATTRS(node); - name->attributes |= DNS_NAMEATTR_READONLY; -} - -dns_rbtnode_t * -dns_rbt_root(dns_rbt_t *rbt) { - return (rbt->root); -} - #ifdef DEBUG #define inline /* @@ -528,18 +513,39 @@ match_header_version(file_header_t *header) { return (true); } +unsigned int +dns__rbtnode_namelen(dns_rbtnode_t *node) { + dns_name_t current; + unsigned int len = 0; + + REQUIRE(DNS_RBTNODE_VALID(node)); + + dns_name_init(¤t, NULL); + + do { + if (node != NULL) { + NODENAME(node, ¤t); + len += current.length; + } else { + len += 1; + break; + } + + node = get_upper_node(node); + } while (!dns_name_isabsolute(¤t)); + + return (len); +} + static isc_result_t serialize_node(FILE *file, dns_rbtnode_t *node, uintptr_t left, uintptr_t right, uintptr_t down, uintptr_t parent, uintptr_t data, uint64_t *crc) { + isc_result_t result; dns_rbtnode_t temp_node; off_t file_position; - unsigned char *node_data; + unsigned char *node_data = NULL; size_t datasize; - isc_result_t result; -#ifdef DEBUG - dns_name_t nodename; -#endif /* ifdef DEBUG */ INSIST(node != NULL); @@ -582,6 +588,8 @@ serialize_node(FILE *file, dns_rbtnode_t *node, uintptr_t left, uintptr_t right, temp_node.data_is_relative = 1; } + temp_node.fullnamelen = dns__rbtnode_namelen(node); + node_data = (unsigned char *)node + sizeof(dns_rbtnode_t); datasize = NODE_SIZE(node) - sizeof(dns_rbtnode_t); @@ -590,10 +598,8 @@ serialize_node(FILE *file, dns_rbtnode_t *node, uintptr_t left, uintptr_t right, CHECK(isc_stdio_write(node_data, 1, datasize, file, NULL)); #ifdef DEBUG - dns_name_init(&nodename, NULL); - NODENAME(node, &nodename); fprintf(stderr, "serialize "); - dns_name_print(&nodename, stderr); + dns_name_print(name, stderr); fprintf(stderr, "\n"); hexdump("node header", (unsigned char *)&temp_node, sizeof(dns_rbtnode_t)); diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 7a935918ac3..9c4391a9cc1 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -392,11 +392,11 @@ typedef struct rbtdb_version { unsigned char salt[DNS_NSEC3_SALTSIZE]; /* - * records and bytes are covered by rwlock. + * records and xfrsize are covered by rwlock. */ isc_rwlock_t rwlock; uint64_t records; - uint64_t bytes; + uint64_t xfrsize; isc_rwlock_t glue_rwlock; size_t glue_table_size; @@ -1339,7 +1339,7 @@ newversion(dns_db_t *db, dns_dbversion_t **versionp) { RWLOCK(&rbtdb->current_version->rwlock, isc_rwlocktype_read); version->records = rbtdb->current_version->records; - version->bytes = rbtdb->current_version->bytes; + version->xfrsize = rbtdb->current_version->xfrsize; RWUNLOCK(&rbtdb->current_version->rwlock, isc_rwlocktype_read); rbtdb->next_serial++; @@ -5927,19 +5927,27 @@ resign_delete(dns_rbtdb_t *rbtdb, rbtdb_version_t *version, } } +static inline uint64_t +recordsize(rdatasetheader_t *header, unsigned int namelen) { + return (dns_rdataslab_rdatasize((unsigned char *)header, + sizeof(*header)) + + sizeof(dns_ttl_t) + sizeof(dns_rdatatype_t) + + sizeof(dns_rdataclass_t) + namelen); +} + static void -update_recordsandbytes(bool add, rbtdb_version_t *rbtversion, - rdatasetheader_t *header) { +update_recordsandxfrsize(bool add, rbtdb_version_t *rbtversion, + rdatasetheader_t *header, unsigned int namelen) { unsigned char *hdr = (unsigned char *)header; size_t hdrsize = sizeof(*header); RWLOCK(&rbtversion->rwlock, isc_rwlocktype_write); if (add) { rbtversion->records += dns_rdataslab_count(hdr, hdrsize); - rbtversion->bytes += dns_rdataslab_size(hdr, hdrsize); + rbtversion->xfrsize += recordsize(header, namelen); } else { rbtversion->records -= dns_rdataslab_count(hdr, hdrsize); - rbtversion->bytes -= dns_rdataslab_size(hdr, hdrsize); + rbtversion->xfrsize -= recordsize(header, namelen); } RWUNLOCK(&rbtversion->rwlock, isc_rwlocktype_write); } @@ -5950,6 +5958,8 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion, dns_rdataset_t *addedrdataset, isc_stdtime_t now) { rbtdb_changed_t *changed = NULL; rdatasetheader_t *topheader, *topheader_prev, *header, *sigheader; + dns_fixedname_t fname; + dns_name_t *nodename = dns_fixedname_initname(&fname); unsigned char *merged; isc_result_t result; bool header_nx; @@ -5968,6 +5978,8 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion, * Caller must be holding the node lock. */ + dns_rbt_fullnamefromnode(rbtnode, nodename); + if ((options & DNS_DBADD_MERGE) != 0) { REQUIRE(rbtversion != NULL); merge = true; @@ -6342,8 +6354,9 @@ find_header: } newheader->next = topheader->next; if (rbtversion != NULL && !header_nx) { - update_recordsandbytes(false, rbtversion, - header); + update_recordsandxfrsize(false, rbtversion, + header, + nodename->length); } free_rdataset(rbtdb, rbtdb->common.mctx, header); } else { @@ -6394,8 +6407,9 @@ find_header: } } if (rbtversion != NULL && !header_nx) { - update_recordsandbytes(false, rbtversion, - header); + update_recordsandxfrsize(false, rbtversion, + header, + nodename->length); } } } else { @@ -6472,7 +6486,8 @@ find_header: } if (rbtversion != NULL && !newheader_nx) { - update_recordsandbytes(true, rbtversion, newheader); + update_recordsandxfrsize(true, rbtversion, newheader, + nodename->length); } /* @@ -6836,6 +6851,8 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, dns_rbtdb_t *rbtdb = (dns_rbtdb_t *)db; dns_rbtnode_t *rbtnode = (dns_rbtnode_t *)node; rbtdb_version_t *rbtversion = version; + dns_fixedname_t fname; + dns_name_t *nodename = dns_fixedname_initname(&fname); rdatasetheader_t *topheader, *topheader_prev, *header, *newheader; unsigned char *subresult; isc_region_t region; @@ -6845,6 +6862,8 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, REQUIRE(VALID_RBTDB(rbtdb)); REQUIRE(rbtversion != NULL && rbtversion->rbtdb == rbtdb); + dns_rbt_fullnamefromnode(rbtnode, nodename); + if (rbtdb->common.methods == &zone_methods) { RWLOCK(&rbtdb->tree_lock, isc_rwlocktype_read); REQUIRE(((rbtnode->nsec == DNS_RBT_NSEC_NSEC3 && @@ -6960,7 +6979,8 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, * to additional info. We need to clear these fields * to avoid having duplicated references. */ - update_recordsandbytes(true, rbtversion, newheader); + update_recordsandxfrsize(true, rbtversion, newheader, + nodename->length); } else if (result == DNS_R_NXRRSET) { /* * This subtraction would remove all of the rdata; @@ -6995,7 +7015,8 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, * topheader. */ INSIST(rbtversion->serial >= topheader->serial); - update_recordsandbytes(false, rbtversion, header); + update_recordsandxfrsize(false, rbtversion, header, + nodename->length); if (topheader_prev != NULL) { topheader_prev->next = newheader; } else { @@ -7362,9 +7383,14 @@ rbt_datafixer(dns_rbtnode_t *rbtnode, void *base, size_t filesize, void *arg, return (ISC_R_INVALIDFILE); } } - update_recordsandbytes(true, rbtdb->current_version, header); + + update_recordsandxfrsize(true, rbtdb->current_version, header, + rbtnode->fullnamelen); } + /* We're done deserializing; clear fullnamelen */ + rbtnode->fullnamelen = 0; + return (ISC_R_SUCCESS); } @@ -7965,7 +7991,7 @@ getnsec3parameters(dns_db_t *db, dns_dbversion_t *version, dns_hash_t *hash, static isc_result_t getsize(dns_db_t *db, dns_dbversion_t *version, uint64_t *records, - uint64_t *bytes) { + uint64_t *xfrsize) { dns_rbtdb_t *rbtdb; isc_result_t result = ISC_R_SUCCESS; rbtdb_version_t *rbtversion = version; @@ -7985,8 +8011,8 @@ getsize(dns_db_t *db, dns_dbversion_t *version, uint64_t *records, *records = rbtversion->records; } - if (bytes != NULL) { - *bytes = rbtversion->bytes; + if (xfrsize != NULL) { + *xfrsize = rbtversion->xfrsize; } RWUNLOCK(&rbtversion->rwlock, isc_rwlocktype_read); RBTDB_UNLOCK(&rbtdb->lock, isc_rwlocktype_read); @@ -8575,7 +8601,7 @@ dns_rbtdb_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type, } rbtdb->current_version->records = 0; - rbtdb->current_version->bytes = 0; + rbtdb->current_version->xfrsize = 0; rbtdb->future_version = NULL; ISC_LIST_INIT(rbtdb->open_versions); /* diff --git a/lib/dns/rdataslab.c b/lib/dns/rdataslab.c index b3487de0ccc..bd7deb2fe1b 100644 --- a/lib/dns/rdataslab.c +++ b/lib/dns/rdataslab.c @@ -361,6 +361,34 @@ dns_rdataslab_size(unsigned char *slab, unsigned int reservelen) { return ((unsigned int)(current - slab)); } +unsigned int +dns_rdataslab_rdatasize(unsigned char *slab, unsigned int reservelen) { + unsigned int count, length, rdatalen = 0; + unsigned char *current; + + REQUIRE(slab != NULL); + + current = slab + reservelen; + count = *current++ * 256; + count += *current++; +#if DNS_RDATASET_FIXED + current += (4 * count); +#endif /* if DNS_RDATASET_FIXED */ + while (count > 0) { + count--; + length = *current++ * 256; + length += *current++; + rdatalen += length; +#if DNS_RDATASET_FIXED + current += length + 2; +#else /* if DNS_RDATASET_FIXED */ + current += length; +#endif /* if DNS_RDATASET_FIXED */ + } + + return (rdatalen); +} + unsigned int dns_rdataslab_count(unsigned char *slab, unsigned int reservelen) { unsigned int count; diff --git a/lib/dns/tests/rbt_test.c b/lib/dns/tests/rbt_test.c index 5f7f8344f5a..bf63730c8d8 100644 --- a/lib/dns/tests/rbt_test.c +++ b/lib/dns/tests/rbt_test.c @@ -1184,6 +1184,47 @@ rbt_nodechain(void **state) { test_context_teardown(ctx); } +/* Test addname return values */ +static void +rbtnode_namelen(void **state) { + isc_result_t result; + test_context_t *ctx = NULL; + dns_rbtnode_t *node; + unsigned int len; + + UNUSED(state); + + isc_mem_debugging = ISC_MEM_DEBUGRECORD; + + ctx = test_context_setup(); + + node = NULL; + result = insert_helper(ctx->rbt, ".", &node); + len = dns__rbtnode_namelen(node); + assert_int_equal(result, ISC_R_EXISTS); + assert_int_equal(len, 1); + node = NULL; + + result = insert_helper(ctx->rbt, "a.b.c.d.e.f.g.h.i.j.k.l.m", &node); + len = dns__rbtnode_namelen(node); + assert_int_equal(result, ISC_R_SUCCESS); + assert_int_equal(len, 27); + + node = NULL; + result = insert_helper(ctx->rbt, "isc.org", &node); + len = dns__rbtnode_namelen(node); + assert_int_equal(result, ISC_R_SUCCESS); + assert_int_equal(len, 9); + + node = NULL; + result = insert_helper(ctx->rbt, "example.com", &node); + len = dns__rbtnode_namelen(node); + assert_int_equal(result, ISC_R_SUCCESS); + assert_int_equal(len, 13); + + test_context_teardown(ctx); +} + #if defined(DNS_BENCHMARK_TESTS) && !defined(__SANITIZE_THREAD__) /* @@ -1324,6 +1365,8 @@ main(void) { _teardown), cmocka_unit_test_setup_teardown(rbt_nodechain, _setup, _teardown), + cmocka_unit_test_setup_teardown(rbtnode_namelen, _setup, + _teardown), #if defined(DNS_BENCHMARK_TESTS) && !defined(__SANITIZE_THREAD__) cmocka_unit_test_setup_teardown(benchmark, _setup, _teardown), #endif /* defined(DNS_BENCHMARK_TESTS) && !defined(__SANITIZE_THREAD__) */ diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index 3c25fec239b..71c85d9ef73 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -7,6 +7,7 @@ EXPORTS dns__rbt_checkproperties dns__rbt_getheight dns__rbtnode_getdistance +dns__rbtnode_namelen dns__zone_findkeys dns__zone_loadpending dns__zone_updatesigs @@ -765,10 +766,8 @@ dns_rbt_nodecount dns_rbt_printdot dns_rbt_printnodeinfo dns_rbt_printtext -dns_rbt_root dns_rbt_serialize_align dns_rbt_serialize_tree -dns_rbtnode_nodename dns_rbtnodechain_current dns_rbtnodechain_down dns_rbtnodechain_first @@ -870,6 +869,7 @@ dns_rdataslab_equal dns_rdataslab_equalx dns_rdataslab_fromrdataset dns_rdataslab_merge +dns_rdataslab_rdatasize dns_rdataslab_size dns_rdataslab_subtract dns_rdatatype_atcname -- 2.47.3