From: Yuri Schaeffer Date: Tue, 8 Oct 2013 11:51:34 +0000 (+0000) Subject: Node are now being purged during insertion X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=796c28fbe930a77b14db5d7b7c61d263c717b5b8;p=thirdparty%2Funbound.git Node are now being purged during insertion ttl implemented extended vdg unittest git-svn-id: file:///svn/unbound/branches/edns-subnet@2973 be551aaa-1e26-0410-a405-d3ace91eadb9 --- diff --git a/edns-subnet/addrtree.c b/edns-subnet/addrtree.c index 46105cdc5..ee3e80ccb 100644 --- a/edns-subnet/addrtree.c +++ b/edns-subnet/addrtree.c @@ -43,6 +43,7 @@ edge_create(struct addrnode* node, const addrkey_t* addr, addrlen_t addrlen) * Create a new node * @param elem: Element to store at this node * @param scope: Scopemask from server reply + * @param ttl: Element is valid up to this time. Absolute, seconds * @return new addrnode or NULL on failure */ static struct addrnode* @@ -108,6 +109,11 @@ size_t addrtree_size(const struct addrtree* tree) return sizeof (struct addrtree) + tree_size(tree, tree->root); } +/** + * Scrub a node clean of elem + * @param tree: Tree the node lives in. + * @param node: Node to be cleaned + */ static void clean_node(const struct addrtree* tree, struct addrnode* node) { @@ -116,29 +122,33 @@ clean_node(const struct addrtree* tree, struct addrnode* node) node->elem = NULL; } +/** + * Purge a node from the tree. Node and parentedge are cleaned and + * free'd. + * @param tree: Tree the node lives in. + * @param node: Node to be freed + * @param parentnode: parent of node, may be NULL if node is rootnode. + * @param parentedge: edge between parentnode and node. May be NULL iff + * parentnode is NULL + */ static void purge_node(const struct addrtree* tree, struct addrnode* node, struct addrnode* parentnode, struct addredge* parentedge) { - struct addredge *child_edge; - + struct addredge *child_edge = NULL; int childcount = (node->edge[0] != NULL) + (node->edge[1] != NULL); clean_node(tree, node); if (childcount == 2 || !parentnode) return; log_assert(parentedge); /* parent node implies parent edge */ - if (childcount == 1) { - child_edge = node->edge[0]?node->edge[0]:node->edge[1]; - if (parentedge == parentnode->edge[0]) - parentnode->edge[0] = child_edge; - else - parentnode->edge[1] = child_edge; + if (childcount == 1) { /* Fix reference */ + child_edge = node->edge[!node->edge[0]]; } + parentnode->edge[parentedge != parentnode->edge[0]] = child_edge; free(parentedge->str); free(parentedge); } - /** * Free node and all nodes below * @param tree: Tree the node lives in. @@ -239,15 +249,9 @@ issub(const addrkey_t* s1, addrlen_t l1, void addrtree_insert(struct addrtree* tree, const addrkey_t* addr, - addrlen_t sourcemask, addrlen_t scope, void* elem, time_t ttl) + addrlen_t sourcemask, addrlen_t scope, void* elem, time_t ttl, + time_t now) { - /*TODO: - * check ttl of visited nodes. - * pass current time as arg - * - * purge_node(tree, node, parent_node, edge); - */ - struct addrnode* newnode, *node; struct addredge* edge, *newedge; uint8_t index; @@ -267,14 +271,21 @@ addrtree_insert(struct addrtree* tree, const addrkey_t* addr, /* Case 1: update existing node */ if (depth == sourcemask) { /* update this node's scope and data */ - if (node->elem) - tree->delfunc(tree->env, node->elem); + clean_node(tree, node); node->elem = elem; node->scope = scope; return; } index = (uint8_t)getbit(addr, sourcemask, depth); + /* Get an edge to an unexpired node */ edge = node->edge[index]; + while (edge) { + /* Purge all expired nodes on path */ + if (!edge->node->elem || edge->node->ttl >= now) + break; + purge_node(tree, edge->node, node, edge); + edge = node->edge[index]; + } /* Case 2: New leafnode */ if (!edge) { newnode = node_create(elem, scope, ttl); @@ -320,7 +331,7 @@ addrtree_insert(struct addrtree* tree, const addrkey_t* addr, } } -struct addrnode* +struct addrnode * addrtree_find(const struct addrtree* tree, const addrkey_t* addr, addrlen_t sourcemask, time_t now) { @@ -333,14 +344,12 @@ addrtree_find(const struct addrtree* tree, const addrkey_t* addr, /* Current node more specific then question. */ log_assert(depth <= sourcemask); /* does this node have data? if yes, see if we have a match */ - if (node->elem) { + if (node->elem && node->ttl >= now) { log_assert(node->scope >= depth); /* saved at wrong depth */ if (depth == node->scope || (node->scope > sourcemask && depth == sourcemask)) { /* Authority indicates it does not have a more precise - * answer or we cannot ask a more specific question. - * If expired don't clean up just now. */ - if (node->ttl < now) return NULL; + * answer or we cannot ask a more specific question. */ return node; } } diff --git a/edns-subnet/addrtree.h b/edns-subnet/addrtree.h index 5133b7894..9d65ac3cc 100644 --- a/edns-subnet/addrtree.h +++ b/edns-subnet/addrtree.h @@ -74,6 +74,8 @@ size_t addrtree_size(const struct addrtree* tree); /** * Create a new tree * @param max_depth: Tree will cap keys to this length. + * @param delfunc: f(element, env) delete element + * @param sizefunc: f(element) returning the size of element. * @param env: Module environment for alloc information * @return new addrtree or NULL on failure */ @@ -89,16 +91,21 @@ void addrtree_delete(struct addrtree* tree); /** * Insert an element in the tree. Failures are silent. Sourcemask and - * scope might be changed according to local policy. + * scope might be changed according to local policy. Caller should no + * longer access elem, it could be free'd now or later during future + * inserts. * * @param tree: Tree insert elem in * @param addr: key for element lookup * @param sourcemask: Length of addr in bits * @param scope: Number of significant bits in addr * @param elem: data to store in the tree + * @param ttl: elem is valid up to this time, seconds. + * @param now: Current time in seconds */ void addrtree_insert(struct addrtree* tree, const addrkey_t* addr, - addrlen_t sourcemask, addrlen_t scope, void* elem, time_t ttl); + addrlen_t sourcemask, addrlen_t scope, void* elem, time_t ttl, + time_t now); /** * Find a node containing an element in the tree. @@ -106,6 +113,7 @@ void addrtree_insert(struct addrtree* tree, const addrkey_t* addr, * @param tree: Tree to search * @param addr: key for element lookup * @param sourcemask: Length of addr in bits + * @param now: Current time in seconds * @return addrnode or NULL on miss */ struct addrnode* addrtree_find(const struct addrtree* tree, diff --git a/edns-subnet/subnetmod.c b/edns-subnet/subnetmod.c index 5b6dd48eb..b6808cc39 100644 --- a/edns-subnet/subnetmod.c +++ b/edns-subnet/subnetmod.c @@ -189,7 +189,8 @@ void update_cache(struct module_qstate *qstate, int id) rep->flags &= ~(BIT_AA | BIT_CD);/* a reply based on the cache */ addrtree_insert(tree, (addrkey_t*)edns->subnet_addr, edns->subnet_source_mask, - qstate->edns_server_in.subnet_scope_mask, rep, rep->ttl); + qstate->edns_server_in.subnet_scope_mask, rep, rep->ttl, + *qstate->env->now); if (acquired_lock) lock_rw_unlock(&lru_entry->lock); } diff --git a/testcode/unitvandergaast.c b/testcode/unitvandergaast.c index ff408ea0b..3db06e88b 100644 --- a/testcode/unitvandergaast.c +++ b/testcode/unitvandergaast.c @@ -53,16 +53,23 @@ { int byte; int bytes = bits/8 + ((bits%8)>0); + char msk = 0xFF; for (byte = 0; byte < bytes; byte++) { - printf("%02x ", k[byte]); + //~ if (byte+1 == bytes) + //~ msk = 0xFF<<(8-bits%8); + printf("%02x ", k[byte]&msk); } } - void print_tree(struct addrnode* node, int indent) + void print_tree(struct addrnode* node, int indent, int maxdepth) { struct addredge* edge; int i, s, byte; if (indent == 0) printf("-----Tree-----"); + if (indent > maxdepth) { + printf("\n"); + return; + } printf("[node elem:%d]\n", node->elem != NULL); for (i = 0; i<2; i++) { if (node->edge[i]) { @@ -70,7 +77,7 @@ printkey(node->edge[i]->str, node->edge[i]->len); printf("(len %d bits, %d bytes) ", node->edge[i]->len, node->edge[i]->len/8 + ((node->edge[i]->len%8)>0)); - print_tree(node->edge[i]->node, indent+1); + print_tree(node->edge[i]->node, indent+1, maxdepth); } } if (indent == 0) printf("-----Tree-----"); @@ -83,12 +90,13 @@ * child must be sub of parent * edge must be longer than parent edge * */ -static int addrtree_inconsistent_subtree(struct addredge* parent_edge) +static int addrtree_inconsistent_subtree(struct addrtree* tree, + struct addredge* parent_edge, addrlen_t depth) { struct addredge* edge; struct addrnode* node = parent_edge->node; int childcount, i, r; - + if (depth > tree->max_depth) return 15; childcount = (node->edge[0] != NULL) + (node->edge[1] != NULL); /* Only nodes with 2 children should possibly have no element. */ if (childcount < 2 && !node->elem) return 10; @@ -101,8 +109,8 @@ static int addrtree_inconsistent_subtree(struct addredge* parent_edge) if (!unittest_wrapper_addrtree_issub(parent_edge->str, parent_edge->len, edge->str, edge->len, 0)) return 14; - if ((r = addrtree_inconsistent_subtree(edge)) != 0) - return 15+r; + if ((r = addrtree_inconsistent_subtree(tree, edge, depth+1)) != 0) + return 100+r; } return 0; } @@ -110,7 +118,7 @@ static int addrtree_inconsistent_subtree(struct addredge* parent_edge) static int addrtree_inconsistent(struct addrtree* tree) { struct addredge* edge; - int i, r; + int i, r, md; if (!tree) return 0; if (!tree->root) return 1; @@ -120,7 +128,7 @@ static int addrtree_inconsistent(struct addrtree* tree) if (!edge) continue; if (!edge->node) return 3; if (!edge->str) return 4; - if ((r = addrtree_inconsistent_subtree(edge)) != 0) + if ((r = addrtree_inconsistent_subtree(tree, edge, 1)) != 0) return r; } return 0; @@ -157,14 +165,24 @@ static void consistency_test(void) srand(9195); /* just some value for reproducibility */ t = addrtree_create(100, &elemfree, NULL, &env); - for (i = 0; i < 1000; i++) { l = randomkey(&k, 128); elem = (struct reply_info *) calloc(1, sizeof(struct reply_info)); - addrtree_insert(t, k, l, 64, elem, timenow + 10); + addrtree_insert(t, k, l, 64, elem, timenow + 10, timenow); + free(k); + unit_assert( !addrtree_inconsistent(t) ); + } + addrtree_delete(t); + unit_show_func("edns-subnet/addrtree.h", "Tree consistency with purge"); + t = addrtree_create(8, &elemfree, NULL, &env); + for (i = 0; i < 1000; i++) { + l = randomkey(&k, 128); + elem = (struct reply_info *) calloc(1, sizeof(struct reply_info)); + addrtree_insert(t, k, l, 64, elem, i + 10, i); free(k); unit_assert( !addrtree_inconsistent(t) ); } + addrtree_delete(t); } static void issub_test(void)