From 5cfd778608315aa4b896eca2b129017ca11d9de7 Mon Sep 17 00:00:00 2001 From: Wouter Wijngaards Date: Tue, 11 Nov 2008 11:54:06 +0000 Subject: [PATCH] - unit test for negative cache, stress tests the refcounting. - fix for refcounting error that could cause fptr_wlist fatal exit in the negative cache rbtree (upcoming 1.1 feature). (Thanks to Attila Nagy for testing). - nicer comments in cachedump about failed RR to string conversion. git-svn-id: file:///svn/unbound/trunk@1342 be551aaa-1e26-0410-a405-d3ace91eadb9 --- daemon/cachedump.c | 4 +- doc/Changelog | 7 + testcode/unitmain.c | 1 + testcode/unitmain.h | 2 + testcode/unitneg.c | 527 ++++++++++++++++++++++++++++++++++++++++++++ validator/val_neg.c | 65 +++--- validator/val_neg.h | 49 ++++ 7 files changed, 614 insertions(+), 41 deletions(-) create mode 100644 testcode/unitneg.c diff --git a/daemon/cachedump.c b/daemon/cachedump.c index 483ae34fa..165f6c803 100644 --- a/daemon/cachedump.c +++ b/daemon/cachedump.c @@ -73,7 +73,7 @@ to_rr(struct ub_packed_rrset_key* k, struct packed_rrset_data* d, pos = 0; status = ldns_wire2dname(&rdf, k->rk.dname, k->rk.dname_len, &pos); if(status != LDNS_STATUS_OK) { - /* we have detailed error in status */ + /* we drop detailed error in status */ ldns_rr_free(rr); return NULL; } @@ -81,7 +81,7 @@ to_rr(struct ub_packed_rrset_key* k, struct packed_rrset_data* d, pos = 0; status = ldns_wire2rdf(rr, d->rr_data[i], d->rr_len[i], &pos); if(status != LDNS_STATUS_OK) { - /* we have detailed error in status */ + /* we drop detailed error in status */ ldns_rr_free(rr); return NULL; } diff --git a/doc/Changelog b/doc/Changelog index 37c2157db..a74175996 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,3 +1,10 @@ +11 November 2008: Wouter + - unit test for negative cache, stress tests the refcounting. + - fix for refcounting error that could cause fptr_wlist fatal exit + in the negative cache rbtree (upcoming 1.1 feature). (Thanks to + Attila Nagy for testing). + - nicer comments in cachedump about failed RR to string conversion. + 10 November 2008: Wouter - fixup the getaddrinfo compat code rename. diff --git a/testcode/unitmain.c b/testcode/unitmain.c index 7837e3836..0f007068d 100644 --- a/testcode/unitmain.c +++ b/testcode/unitmain.c @@ -413,6 +413,7 @@ main(int argc, char* argv[]) printf("Start of %s unit test.\n", PACKAGE_STRING); ERR_load_crypto_strings(); checklock_start(); + neg_test(); rnd_test(); verify_test(); net_test(); diff --git a/testcode/unitmain.h b/testcode/unitmain.h index f7bb06594..0ea212da4 100644 --- a/testcode/unitmain.h +++ b/testcode/unitmain.h @@ -63,5 +63,7 @@ void dname_test(); void anchors_test(); /** unit test for verification functions */ void verify_test(); +/** unit test for negative cache functions */ +void neg_test(); #endif /* TESTCODE_UNITMAIN_H */ diff --git a/testcode/unitneg.c b/testcode/unitneg.c new file mode 100644 index 000000000..1c9869768 --- /dev/null +++ b/testcode/unitneg.c @@ -0,0 +1,527 @@ +/* + * testcode/unitneg.c - unit test for negative cache routines. + * + * Copyright (c) 2008, NLnet Labs. All rights reserved. + * + * This software is open source. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * Neither the name of the NLNET LABS nor the names of its contributors may + * be used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * + */ +/** + * \file + * Calls negative cache unit tests. Exits with code 1 on a failure. + */ + +#include "config.h" +#include "util/log.h" +#include "util/net_help.h" +#include "util/data/packed_rrset.h" +#include "util/data/dname.h" +#include "testcode/unitmain.h" +#include "validator/val_neg.h" + +/** verbose unit test for negative cache */ +static int negverbose = 0; + +/** debug printout of neg cache */ +static void print_neg_cache(struct val_neg_cache* neg) +{ + char buf[1024]; + struct val_neg_zone* z; + struct val_neg_data* d; + printf("neg_cache print\n"); + printf("memuse %d of %d\n", (int)neg->use, (int)neg->max); + printf("maxiter %d\n", (int)neg->nsec3_max_iter); + printf("first=%x last=%x\n", (unsigned)neg->first, (unsigned)neg->last); + printf("%d zones\n", (int)neg->tree.count); + RBTREE_FOR(z, struct val_neg_zone*, &neg->tree) { + dname_str(z->name, buf); + printf("%24s", buf); + printf(" len=%2.2d labs=%d inuse=%d count=%d tree.count=%d\n", + (int)z->len, z->labs, (int)z->in_use, z->count, + (int)z->tree.count); + } + RBTREE_FOR(z, struct val_neg_zone*, &neg->tree) { + printf("\n"); + dname_print(stdout, NULL, z->name); + printf(" zone details\n"); + printf("len=%2.2d labs=%d inuse=%d count=%d tree.count=%d\n", + (int)z->len, z->labs, (int)z->in_use, z->count, + (int)z->tree.count); + if(z->parent) { + printf("parent="); + dname_print(stdout, NULL, z->parent->name); + printf("\n"); + } else { + printf("parent=NULL\n"); + } + + RBTREE_FOR(d, struct val_neg_data*, &z->tree) { + dname_str(d->name, buf); + printf("%24s", buf); + printf(" len=%2.2d labs=%d inuse=%d count=%d\n", + (int)d->len, d->labs, (int)d->in_use, d->count); + } + } +} + +/** get static pointer to random zone name */ +static char* get_random_zone() +{ + static char zname[256]; + int labels = random() % 3; + int i; + char* p = zname; + int labnum; + + for(i=0; itree.count == 0) + return; /* nothing to delete */ + + /* pick a random zone */ + walk = rbtree_first(&neg->tree); /* first highest parent, big count */ + z = (struct val_neg_zone*)walk; + n = random() % (int)(z->count); + if(negverbose) + printf("neg stress delete zone %d\n", n); + i=0; + walk = rbtree_first(&neg->tree); + z = (struct val_neg_zone*)walk; + while(i!=n+1 && walk && walk != RBTREE_NULL && !z->in_use) { + walk = rbtree_next(walk); + z = (struct val_neg_zone*)walk; + if(z->in_use) + i++; + } + if(!walk || walk == RBTREE_NULL) + return; + if(!z->in_use) + return; + if(negverbose) + log_nametypeclass(0, "delete zone", z->name, 0, 0); + + /* pick a random nsec item. - that is in use */ + walk = rbtree_first(&z->tree); /* first is highest parent */ + d = (struct val_neg_data*)walk; + n = random() % (int)(d->count); + if(negverbose) + printf("neg stress delete item %d\n", n); + i=0; + walk = rbtree_first(&z->tree); + d = (struct val_neg_data*)walk; + while(i!=n+1 && walk && walk != RBTREE_NULL && !d->in_use) { + walk = rbtree_next(walk); + d = (struct val_neg_data*)walk; + if(d->in_use) + i++; + } + if(!walk || walk == RBTREE_NULL) + return; + if(d->in_use) { + if(negverbose) + log_nametypeclass(0, "neg delete item:", d->name, 0, 0); + neg_delete_data(neg, d); + } +} + +/** sum up the zone trees */ +static size_t sumtrees_all(struct val_neg_cache* neg) +{ + size_t res = 0; + struct val_neg_zone* z; + RBTREE_FOR(z, struct val_neg_zone*, &neg->tree) { + res += z->tree.count; + } + return res; +} + +/** sum up the zone trees, in_use only */ +static size_t sumtrees_inuse(struct val_neg_cache* neg) +{ + size_t res = 0; + struct val_neg_zone* z; + struct val_neg_data* d; + RBTREE_FOR(z, struct val_neg_zone*, &neg->tree) { + /* get count of highest parent for num in use */ + d = (struct val_neg_data*)rbtree_first(&z->tree); + if(d && (rbnode_t*)d!=RBTREE_NULL) + res += d->count; + } + return res; +} + +/** check if lru is still valid */ +static void check_lru(struct val_neg_cache* neg) +{ + struct val_neg_data* p, *np; + size_t num = 0; + size_t inuse; + p = neg->first; + while(p) { + if(!p->prev) { + unit_assert(neg->first == p); + } + np = p->next; + if(np) { + unit_assert(np->prev == p); + } else { + unit_assert(neg->last == p); + } + num++; + p = np; + } + inuse = sumtrees_inuse(neg); + if(negverbose) + printf("num lru %d, inuse %d, all %d\n", + (int)num, (int)sumtrees_inuse(neg), + (int)sumtrees_all(neg)); + unit_assert( num == inuse); + unit_assert( inuse <= sumtrees_all(neg)); +} + +/** sum up number of items inuse in subtree */ +static int sum_subtree_inuse(struct val_neg_zone* zone, + struct val_neg_data* data) +{ + struct val_neg_data* d; + int num = 0; + RBTREE_FOR(d, struct val_neg_data*, &zone->tree) { + if(dname_subdomain_c(d->name, data->name)) { + if(d->in_use) + num++; + } + } + return num; +} + +/** sum up number of items inuse in subtree */ +static int sum_zone_subtree_inuse(struct val_neg_cache* neg, + struct val_neg_zone* zone) +{ + struct val_neg_zone* z; + int num = 0; + RBTREE_FOR(z, struct val_neg_zone*, &neg->tree) { + if(dname_subdomain_c(z->name, zone->name)) { + if(z->in_use) + num++; + } + } + return num; +} + +/** check point in data tree */ +static void check_data(struct val_neg_zone* zone, struct val_neg_data* data) +{ + unit_assert(data->count > 0); + if(data->parent) { + unit_assert(data->parent->count >= data->count); + if(data->parent->in_use) { + unit_assert(data->parent->count > data->count); + } + unit_assert(data->parent->labs == data->labs-1); + /* and parent must be one label shorter */ + unit_assert(data->name[0] == (data->len-data->parent->len-1)); + unit_assert(query_dname_compare(data->name + data->name[0]+1, + data->parent->name) == 0); + } else { + /* must be apex */ + unit_assert(dname_is_root(data->name)); + } + /* tree property: */ + unit_assert(data->count == sum_subtree_inuse(zone, data)); +} + +/** check if tree of data in zone is valid */ +static void checkzonetree(struct val_neg_zone* zone) +{ + struct val_neg_data* d; + + /* check all data in tree */ + RBTREE_FOR(d, struct val_neg_data*, &zone->tree) { + check_data(zone, d); + } +} + +/** check if negative cache is still valid */ +static void check_zone_invariants(struct val_neg_cache* neg, + struct val_neg_zone* zone) +{ + unit_assert(zone->nsec3_hash == 0); + unit_assert(zone->tree.cmp == &val_neg_data_compare); + unit_assert(zone->count != 0); + + if(zone->tree.count == 0) + unit_assert(!zone->in_use); + else { + if(!zone->in_use) { + /* details on error */ + log_nametypeclass(0, "zone", zone->name, 0, 0); + log_err("inuse %d count=%d tree.count=%d", + zone->in_use, zone->count, + (int)zone->tree.count); + if(negverbose) + print_neg_cache(neg); + } + unit_assert(zone->in_use); + } + + if(zone->parent) { + unit_assert(zone->parent->count >= zone->count); + if(zone->parent->in_use) { + unit_assert(zone->parent->count > zone->count); + } + unit_assert(zone->parent->labs == zone->labs-1); + /* and parent must be one label shorter */ + unit_assert(zone->name[0] == (zone->len-zone->parent->len-1)); + unit_assert(query_dname_compare(zone->name + zone->name[0]+1, + zone->parent->name) == 0); + } else { + /* must be apex */ + unit_assert(dname_is_root(zone->name)); + } + /* tree property: */ + unit_assert(zone->count == sum_zone_subtree_inuse(neg, zone)); + + /* check structure of zone data tree */ + checkzonetree(zone); +} + +/** check if negative cache is still valid */ +static void check_neg_invariants(struct val_neg_cache* neg) +{ + struct val_neg_zone* z; + /* check structure of LRU list */ + check_lru(neg); + unit_assert(neg->max == 1024*1024); + unit_assert(neg->nsec3_max_iter == 1500); + unit_assert(neg->tree.cmp == &val_neg_zone_compare); + + if(neg->tree.count == 0) { + /* empty */ + unit_assert(neg->tree.count == 0); + unit_assert(neg->first == NULL); + unit_assert(neg->last == NULL); + unit_assert(neg->use == 0); + return; + } + + unit_assert(neg->first != NULL); + unit_assert(neg->last != NULL); + + RBTREE_FOR(z, struct val_neg_zone*, &neg->tree) { + check_zone_invariants(neg, z); + } +} + +/** perform stress test on insert and delete in neg cache */ +static void stress_test(struct val_neg_cache* neg) +{ + int i; + if(negverbose) + printf("negcache test\n"); + for(i=0; i<100; i++) { + if(random() % 10 < 8) + add_item(neg); + else remove_item(neg); + check_neg_invariants(neg); + } + /* empty it */ + if(negverbose) + printf("neg stress empty\n"); + while(neg->first) { + remove_item(neg); + check_neg_invariants(neg); + } + if(negverbose) + printf("neg stress emptied\n"); + unit_assert(neg->first == NULL); + /* insert again */ + for(i=0; i<100; i++) { + if(random() % 10 < 8) + add_item(neg); + else remove_item(neg); + check_neg_invariants(neg); + } +} + +void neg_test() +{ + struct val_neg_cache* neg; + srandom(48); + + /* create with defaults */ + neg = val_neg_create(NULL, 1500); + unit_assert(neg); + + stress_test(neg); + + neg_cache_delete(neg); +} diff --git a/validator/val_neg.c b/validator/val_neg.c index 22eeef911..b6a9ca439 100644 --- a/validator/val_neg.c +++ b/validator/val_neg.c @@ -215,14 +215,7 @@ static void neg_delete_zone(struct val_neg_cache* neg, struct val_neg_zone* z) } } -/** - * Delete a data element from the negative cache. - * May delete other data elements to keep tree coherent, or - * only mark the element as 'not in use'. - * @param neg: negative cache. - * @param el: data element to delete. - */ -static void neg_delete_data(struct val_neg_cache* neg, struct val_neg_data* el) +void neg_delete_data(struct val_neg_cache* neg, struct val_neg_data* el) { struct val_neg_zone* z; struct val_neg_data* p, *np; @@ -275,15 +268,7 @@ static void neg_make_space(struct val_neg_cache* neg, size_t need) } } -/** - * Find the given zone, from the SOA owner name and class - * @param neg: negative cache - * @param nm: what to look for. - * @param len: length of nm - * @param dclass: class to look for. - * @return zone or NULL if not found. - */ -static struct val_neg_zone* neg_find_zone(struct val_neg_cache* neg, +struct val_neg_zone* neg_find_zone(struct val_neg_cache* neg, uint8_t* nm, size_t len, uint16_t dclass) { struct val_neg_zone lookfor; @@ -519,17 +504,21 @@ static struct val_neg_zone* neg_zone_chain( dname_remove_label(&nm, &nm_len); } return first; +} + +void val_neg_zone_take_inuse(struct val_neg_zone* zone) +{ + if(!zone->in_use) { + struct val_neg_zone* p; + zone->in_use = 1; + /* increase usage count of all parents */ + for(p=zone; p; p = p->parent) { + p->count++; + } + } } -/** - * Create a new zone. - * @param neg: negative cache - * @param nm: what to look for. - * @param nm_len: length of name. - * @param dclass: class of zone, host order. - * @return zone or NULL if out of memory. - */ -static struct val_neg_zone* neg_create_zone(struct val_neg_cache* neg, +struct val_neg_zone* neg_create_zone(struct val_neg_cache* neg, uint8_t* nm, size_t nm_len, uint16_t dclass) { struct val_neg_zone* zone; @@ -547,7 +536,6 @@ static struct val_neg_zone* neg_create_zone(struct val_neg_cache* neg, if(!zone) { return NULL; } - zone->in_use = 1; /* insert the list of zones into the tree */ p = zone; @@ -562,10 +550,6 @@ static struct val_neg_zone* neg_create_zone(struct val_neg_cache* neg, p->parent = parent; p = np; } - /* increase usage count of all parents */ - for(p=zone; p; p = p->parent) { - p->count++; - } return zone; } @@ -752,14 +736,7 @@ static void wipeout(struct val_neg_cache* neg, struct val_neg_zone* zone, } } - -/** - * Insert data into the data tree of a zone - * @param neg: negative cache - * @param zone: zone to insert into - * @param nsec: record to insert. - */ -static void neg_insert_data(struct val_neg_cache* neg, +void neg_insert_data(struct val_neg_cache* neg, struct val_neg_zone* zone, struct ub_packed_rrset_key* nsec) { struct packed_rrset_data* d; @@ -886,6 +863,7 @@ void val_neg_addreply(struct val_neg_cache* neg, struct reply_info* rep) return; } } + val_neg_zone_take_inuse(zone); /* insert the NSECs */ for(i=rep->an_numrrsets; i< rep->an_numrrsets+rep->ns_numrrsets; i++){ @@ -896,6 +874,10 @@ void val_neg_addreply(struct val_neg_cache* neg, struct reply_info* rep) /* insert NSEC into this zone's tree */ neg_insert_data(neg, zone, rep->rrsets[i]); } + if(zone->tree.count == 0) { + /* remove empty zone if inserts failed */ + neg_delete_zone(neg, zone); + } lock_basic_unlock(&neg->lock); } @@ -1092,6 +1074,7 @@ void val_neg_addreferral(struct val_neg_cache* neg, struct reply_info* rep, return; } } + val_neg_zone_take_inuse(zone); /* insert the NSECs */ for(i=rep->an_numrrsets; i< rep->an_numrrsets+rep->ns_numrrsets; i++){ @@ -1103,6 +1086,10 @@ void val_neg_addreferral(struct val_neg_cache* neg, struct reply_info* rep, /* insert NSEC into this zone's tree */ neg_insert_data(neg, zone, rep->rrsets[i]); } + if(zone->tree.count == 0) { + /* remove empty zone if inserts failed */ + neg_delete_zone(neg, zone); + } lock_basic_unlock(&neg->lock); } diff --git a/validator/val_neg.h b/validator/val_neg.h index 93c098a66..fa2bb8274 100644 --- a/validator/val_neg.h +++ b/validator/val_neg.h @@ -53,6 +53,7 @@ struct rrset_cache; struct regional; struct query_info; struct dns_msg; +struct ub_packed_rrset_key; /** * The negative cache. It is shared between the threads, so locked. @@ -247,4 +248,52 @@ struct dns_msg* val_neg_getmsg(struct val_neg_cache* neg, struct query_info* qinfo, struct regional* region, struct rrset_cache* rrset_cache, ldns_buffer* buf, uint32_t now); + +/**** functions exposed for unit test ****/ +/** + * Insert data into the data tree of a zone + * @param neg: negative cache + * @param zone: zone to insert into + * @param nsec: record to insert. + */ +void neg_insert_data(struct val_neg_cache* neg, + struct val_neg_zone* zone, struct ub_packed_rrset_key* nsec); + +/** + * Delete a data element from the negative cache. + * May delete other data elements to keep tree coherent, or + * only mark the element as 'not in use'. + * @param neg: negative cache. + * @param el: data element to delete. + */ +void neg_delete_data(struct val_neg_cache* neg, struct val_neg_data* el); + +/** + * Find the given zone, from the SOA owner name and class + * @param neg: negative cache + * @param nm: what to look for. + * @param len: length of nm + * @param dclass: class to look for. + * @return zone or NULL if not found. + */ +struct val_neg_zone* neg_find_zone(struct val_neg_cache* neg, + uint8_t* nm, size_t len, uint16_t dclass); + +/** + * Create a new zone. + * @param neg: negative cache + * @param nm: what to look for. + * @param nm_len: length of name. + * @param dclass: class of zone, host order. + * @return zone or NULL if out of memory. + */ +struct val_neg_zone* neg_create_zone(struct val_neg_cache* neg, + uint8_t* nm, size_t nm_len, uint16_t dclass); + +/** + * take a zone into use. increases counts of parents. + * @param zone: zone to take into use. + */ +void val_neg_zone_take_inuse(struct val_neg_zone* zone); + #endif /* VALIDATOR_VAL_NEG_H */