Commit f14167fd authored by Jan Kadlec's avatar Jan Kadlec

Trie iteration usage cleanup and XFR finalization simplification.

- Redundant zone walks deleted
- hattrie_apply_rev was not working as advertised, xfrin_remove_empty_nodes() had to be rewritten
- When doing that, it struck me that knot_changes_t->old_nodes and nsec3_nodes were not needed at all, because zone tree is not shallow copied anymore.
parent 3c895412
......@@ -63,8 +63,6 @@ int knot_changesets_init(knot_changesets_t **changesets)
// Init changes' allocator (storing RRs)
(*changesets)->changes->mem_ctx = (*changesets)->mmc_rr;
// Init changes' lists
init_list(&(*changesets)->changes->old_nodes);
init_list(&(*changesets)->changes->old_nsec3);
init_list(&(*changesets)->changes->new_rrsets);
init_list(&(*changesets)->changes->old_rrsets);
......@@ -226,33 +224,6 @@ int knot_changes_add_rrset(knot_changes_t *ch, knot_rrset_t *rrset,
return KNOT_EOK;
}
int knot_changes_add_node(knot_changes_t *ch, knot_node_t *kn_node,
knot_changes_part_t part)
{
if (ch == NULL || kn_node == NULL) {
return KNOT_EINVAL;
}
// Using the same allocator for node and rr's, sizes are equal.
knot_node_ln_t *list_node =
ch->mem_ctx.alloc(ch->mem_ctx.ctx, sizeof(knot_node_ln_t));
if (list_node == NULL) {
// This will not happen with mp_alloc, but allocator can change
ERR_ALLOC_FAILED;
return KNOT_ENOMEM;
}
list_node->node = kn_node;
if (part == KNOT_CHANGES_NORMAL_NODE) {
add_tail(&ch->old_nodes, (node_t *)list_node);
} else {
assert(part == KNOT_CHANGES_NSEC3_NODE);
add_tail(&ch->old_nsec3, (node_t *)list_node);
}
return KNOT_EOK;
}
static void knot_changeset_store_soa(knot_rrset_t **chg_soa,
uint32_t *chg_serial, knot_rrset_t *soa)
{
......
......@@ -65,12 +65,6 @@ typedef struct knot_rr_ln {
knot_rrset_t *rr; /*!< Actual usable data. */
} knot_rr_ln_t;
/*! \brief Wrapper for BIRD lists. Storing: Node. */
typedef struct knot_node_ln {
node_t n; /*!< List node. */
knot_node_t *node; /*!< Actual usable data. */
} knot_node_ln_t;
/*! \brief Partial changes done to zones - used for update/transfer rollback. */
typedef struct {
/*!
......@@ -86,14 +80,6 @@ typedef struct {
* Deleted after failed update.
*/
list_t new_rrsets;
/*!
* Deleted (without contents) after successful update.
*/
list_t old_nodes;
/*!
* Deleted (without contents) after successful update.
*/
list_t old_nsec3;
} knot_changes_t;
/*----------------------------------------------------------------------------*/
......@@ -122,8 +108,6 @@ typedef enum {
typedef enum {
KNOT_CHANGES_OLD,
KNOT_CHANGES_NEW,
KNOT_CHANGES_NORMAL_NODE,
KNOT_CHANGES_NSEC3_NODE
} knot_changes_part_t;
/*----------------------------------------------------------------------------*/
......@@ -278,20 +262,6 @@ void knot_changesets_free(knot_changesets_t **changesets);
int knot_changes_add_rrset(knot_changes_t *ch, knot_rrset_t *rrset,
knot_changes_part_t part);
/*!
* \brief Add Node to changes structure.
Node is either inserted to 'normal' or to 'NSEC3' list.
*
* \param chgs Change to add node into.
* \param node RRSet to be added.
* \param part Add to 'normal' or 'NSEC3'?
*
* \retval KNOT_EOK on success.
* \retval Error code on failure.
*/
int knot_changes_add_node(knot_changes_t *ch, knot_node_t *kn_node,
knot_changes_part_t part);
/*!
* \brief Merges two changesets together, second changeset's lists are kept.
*
......
......@@ -1984,17 +1984,6 @@ void xfrin_cleanup_successful_update(knot_changes_t *changes)
knot_rrset_t *rrset = rr_node->rr;
knot_rrset_deep_free_no_sig(&rrset, 1);
}
// Free old nodes
knot_node_ln_t *n_node = NULL;
WALK_LIST(n_node, changes->old_nodes) {
knot_node_free(&n_node->node);
}
// Free old NSEC3 nodes
WALK_LIST(n_node, changes->old_nsec3) {
knot_node_free(&n_node->node);
}
}
/*----------------------------------------------------------------------------*/
......@@ -2384,161 +2373,94 @@ static int xfrin_apply_changeset(knot_zone_contents_t *contents,
/*----------------------------------------------------------------------------*/
static void xfrin_mark_empty_node(knot_node_t *node, knot_changes_t *changes,
knot_changes_part_t part)
/*! \brief Wrapper for BIRD lists. Storing: Node. */
typedef struct knot_node_ln {
node_t n; /*!< List node. */
knot_node_t *node; /*!< Actual usable data. */
} knot_node_ln_t;
static int add_node_to_list(knot_node_t *node, list_t *l)
{
assert(node != NULL);
assert(changes != NULL);
assert(node && l);
knot_node_ln_t *data = malloc(sizeof(knot_node_ln_t));
if (data == NULL) {
return KNOT_ENOMEM;
}
data->node = node;
add_head(l, (node_t *)data);
return KNOT_EOK;
}
if (knot_node_rrset_count(node) == 0
&& knot_node_children(node) == 0) {
// Add node to changes.
int ret = knot_changes_add_node(changes, node, part);
static int xfrin_mark_empty(knot_node_t **node_p, void *data)
{
assert(node_p && *node_p);
knot_node_t *node = *node_p;
list_t *l = (list_t *)data;
assert(data);
if (node->rrset_count == 0 && node->children == 0 &&
!knot_node_is_empty(node)) {
/*!
* Mark this node and all parent nodes that have 0 RRSets
* for removal.
*/
int ret = add_node_to_list(node, l);
if (ret != KNOT_EOK) {
/*! \todo Stop on error? */
return;
return ret;
}
// Mark the node as empty.
knot_node_set_empty(node);
if (node->parent != NULL) {
assert(node->parent->children > 0);
--node->parent->children;
if (node->parent) {
if (node->parent->wildcard_child == node) {
node->parent->wildcard_child = NULL;
}
node->parent = NULL;
node->parent->children--;
// Recurse using the parent node
return xfrin_mark_empty(&node->parent, data);
}
}
}
/*----------------------------------------------------------------------------*/
static int xfrin_mark_empty_nsec3(knot_node_t *node, void *data)
{
xfrin_mark_empty_node(node,
(knot_changes_t *)data, KNOT_CHANGES_NSEC3_NODE);
return KNOT_EOK;
}
static int xfrin_mark_empty(knot_node_t *node, void *data)
{
xfrin_mark_empty_node(node,
(knot_changes_t *)data, KNOT_CHANGES_NORMAL_NODE);
return KNOT_EOK;
}
/*----------------------------------------------------------------------------*/
static int xfrin_remove_empty_nodes(knot_zone_contents_t *contents,
knot_changes_t *changes)
static int xfrin_remove_empty_nodes(knot_zone_contents_t *z)
{
int ret;
dbg_xfrin("Removing empty nodes from zone.\n");
list_t l;
init_list(&l);
// walk through the zone and select nodes to be removed
ret = knot_zone_contents_tree_apply_inorder_reverse(contents,
xfrin_mark_empty,
changes);
assert(ret == KNOT_EOK);
// Do the same with NSEC3 nodes.
ret = knot_zone_contents_nsec3_apply_inorder_reverse(contents,
xfrin_mark_empty_nsec3,
changes);
assert(ret == KNOT_EOK);
// Remove these nodes from zone tree.
knot_node_t *zone_node = NULL;
knot_node_ln_t *list_node = NULL;
WALK_LIST(list_node, changes->old_nodes) {
knot_node_t *node = list_node->node;
assert(node);
zone_node = NULL;
dbg_xfrin_exec_detail(
char *name = knot_dname_to_str(knot_node_owner(node));
dbg_xfrin_detail("Old node: %p, %s\n",node, name);
free(name);
);
ret = knot_zone_contents_remove_node(
contents, node, &zone_node);
if (ret == KNOT_ENONODE) {
assert(knot_node_rrset_count(node) == 1);
assert(knot_node_rrset(node,
KNOT_RRTYPE_RRSIG));
char *name = knot_dname_to_str(node->owner);
log_zone_warning("Ignoring extra RRSIG for %s!\n",
name);
free(name);
} else if (ret != KNOT_EOK) {
dbg_xfrin("Failed to remove node from zone!\n");
return ret;
}
assert(node == zone_node);
int ret = knot_zone_tree_apply(z->nodes,
xfrin_mark_empty, &l);
if (ret != KNOT_EOK) {
return ret;
}
// remove NSEC3 nodes
WALK_LIST(list_node, changes->old_nsec3) {
knot_node_t *node = list_node->node;
assert(node);
zone_node = NULL;
char *name = knot_dname_to_str(knot_node_owner(node));
dbg_xfrin_detail("Old NSEC3 node: %p, %s\n", node, name);
free(name);
ret = knot_zone_contents_remove_nsec3_node(
contents, node, &zone_node);
if (ret != KNOT_EOK) {
dbg_xfrin("Failed to remove NSEC3 node from zone!\n");
return KNOT_ENONODE;
}
assert(node == zone_node);
node_t *n = NULL;
node_t *nxt = NULL;
WALK_LIST_DELSAFE(n, nxt, l) {
knot_node_ln_t *list_node = (knot_node_ln_t *)n;
knot_zone_contents_remove_node(z, list_node->node->owner);
knot_node_free(&list_node->node);
free(n);
}
return KNOT_EOK;
}
/*----------------------------------------------------------------------------*/
static int xfrin_check_contents_copy_node(knot_node_t **node, void *data)
{
UNUSED(data);
assert(node && *node);
if (knot_node_new_node(*node) == NULL) {
return KNOT_ENONODE;
} else {
return KNOT_EOK;
}
}
/*----------------------------------------------------------------------------*/
static int xfrin_check_contents_copy(knot_zone_contents_t *old_contents)
{
int ret = knot_zone_tree_apply(old_contents->nodes,
xfrin_check_contents_copy_node, NULL);
if (ret == KNOT_EOK) {
ret = knot_zone_tree_apply(old_contents->nsec3_nodes,
xfrin_check_contents_copy_node, NULL);
init_list(&l);
// Do the same with NSEC3 nodes.
ret = knot_zone_tree_apply(z->nsec3_nodes,
xfrin_mark_empty, &l);
if (ret != KNOT_EOK) {
return ret;
}
if (knot_node_new_node(knot_zone_contents_apex(old_contents)) == NULL) {
return KNOT_ENONODE;
WALK_LIST_DELSAFE(n, nxt, l) {
knot_node_ln_t *list_node = (knot_node_ln_t *)n;
knot_zone_contents_remove_nsec3_node(z, list_node->node->owner);
knot_node_free(&list_node->node);
free(n);
}
return ret;
return KNOT_EOK;
}
/*----------------------------------------------------------------------------*/
......@@ -2574,24 +2496,13 @@ int xfrin_prepare_zone_copy(knot_zone_contents_t *old_contents,
knot_zone_contents_t *contents_copy = NULL;
dbg_xfrin("Copying zone contents.\n");
int ret = knot_zone_contents_shallow_copy2(old_contents,
&contents_copy);
int ret = knot_zone_contents_shallow_copy(old_contents, &contents_copy);
if (ret != KNOT_EOK) {
dbg_xfrin("Failed to create shallow copy of zone: %s\n",
knot_strerror(ret));
return ret;
}
/*!
* \todo Check if all nodes have their copy.
*/
ret = xfrin_check_contents_copy(old_contents);
if (ret != KNOT_EOK) {
dbg_xfrin("Contents copy check failed!\n");
xfrin_cleanup_failed_update(old_contents, &contents_copy);
return ret;
}
assert(knot_zone_contents_apex(contents_copy) != NULL);
/*
......@@ -2629,7 +2540,7 @@ int xfrin_finalize_updated_zone(knot_zone_contents_t *contents_copy,
* Select and remove empty nodes from zone trees. Do not free them right
* away as they may be referenced by some domain names.
*/
int ret = xfrin_remove_empty_nodes(contents_copy, changes);
int ret = xfrin_remove_empty_nodes(contents_copy);
if (ret != KNOT_EOK) {
dbg_xfrin("Failed to remove empty nodes: %s\n",
knot_strerror(ret));
......
......@@ -613,7 +613,7 @@ void knot_node_clear_replaced_nsec(knot_node_t *node)
/*----------------------------------------------------------------------------*/
void knot_node_free_rrsets(knot_node_t *node, int free_rdata_dnames)
void knot_node_free_rrsets(knot_node_t *node)
{
if (node == NULL) {
return;
......
......@@ -387,11 +387,8 @@ void knot_node_set_empty(knot_node_t *node);
* \brief Destroys the RRSets within the node structure.
*
* \param node Node to be destroyed.
* \param free_rdata_dnames Set to <> 0 if you want to delete ALL domain names
* present in RDATA. Set to 0 otherwise. (See
* knot_rdata_deep_free().)
*/
void knot_node_free_rrsets(knot_node_t *node, int free_rdata_dnames);
void knot_node_free_rrsets(knot_node_t *node);
/*!
* \brief Destroys the node structure.
......
......@@ -118,10 +118,10 @@ dbg_zone_exec(
static int knot_zone_contents_destroy_node_rrsets_from_tree(
knot_node_t **tnode, void *data)
{
UNUSED(data);
assert(tnode != NULL);
if (*tnode != NULL) {
int free_rdata_dnames = (int)((intptr_t)data);
knot_node_free_rrsets(*tnode, free_rdata_dnames);
knot_node_free_rrsets(*tnode);
knot_node_free(tnode);
}
......@@ -840,23 +840,19 @@ int knot_zone_contents_add_nsec3_rrset(knot_zone_contents_t *zone,
/*----------------------------------------------------------------------------*/
int knot_zone_contents_remove_node(knot_zone_contents_t *contents,
const knot_node_t *node, knot_node_t **removed_tree)
const knot_dname_t *owner)
{
if (contents == NULL || node == NULL) {
if (contents == NULL || owner == NULL) {
return KNOT_EINVAL;
}
const knot_dname_t *owner = knot_node_owner(node);
dbg_zone_exec_verb(
char *name = knot_dname_to_str(owner);
dbg_zone_verb("Removing zone node: %s\n", name);
free(name);
);
// 2) remove the node from the zone tree
*removed_tree = NULL;
int ret = knot_zone_tree_remove(contents->nodes, owner, removed_tree);
knot_node_t *removed_node = NULL;
int ret = knot_zone_tree_remove(contents->nodes, owner, &removed_node);
if (ret != KNOT_EOK) {
return KNOT_ENONODE;
}
......@@ -867,17 +863,16 @@ dbg_zone_exec_verb(
/*----------------------------------------------------------------------------*/
int knot_zone_contents_remove_nsec3_node(knot_zone_contents_t *contents,
const knot_node_t *node, knot_node_t **removed)
const knot_dname_t *owner)
{
if (contents == NULL || node == NULL) {
if (contents == NULL || owner == NULL) {
return KNOT_EINVAL;
}
const knot_dname_t *owner = knot_node_owner(node);
// remove the node from the zone tree
*removed = NULL;
int ret = knot_zone_tree_remove(contents->nsec3_nodes, owner, removed);
knot_node_t *removed_node = NULL;
int ret = knot_zone_tree_remove(contents->nsec3_nodes, owner,
&removed_node);
if (ret != KNOT_EOK) {
return KNOT_ENONODE;
}
......@@ -1398,23 +1393,6 @@ int knot_zone_contents_tree_apply_inorder(knot_zone_contents_t *zone,
/*----------------------------------------------------------------------------*/
int knot_zone_contents_tree_apply_inorder_reverse(knot_zone_contents_t *zone,
knot_zone_contents_apply_cb_t function,
void *data)
{
if (zone == NULL) {
return KNOT_EINVAL;
}
knot_zone_tree_func_t f;
f.func = function;
f.data = data;
return knot_zone_tree_apply_recursive(zone->nodes, tree_apply_cb, &f);
}
/*----------------------------------------------------------------------------*/
int knot_zone_contents_nsec3_apply_inorder(knot_zone_contents_t *zone,
knot_zone_contents_apply_cb_t function,
void *data)
......@@ -1433,24 +1411,6 @@ int knot_zone_contents_nsec3_apply_inorder(knot_zone_contents_t *zone,
/*----------------------------------------------------------------------------*/
int knot_zone_contents_nsec3_apply_inorder_reverse(knot_zone_contents_t *zone,
knot_zone_contents_apply_cb_t function,
void *data)
{
if (zone == NULL) {
return KNOT_EINVAL;
}
knot_zone_tree_func_t f;
f.func = function;
f.data = data;
return knot_zone_tree_apply_recursive(zone->nsec3_nodes,
tree_apply_cb, &f);
}
/*----------------------------------------------------------------------------*/
knot_zone_tree_t *knot_zone_contents_get_nodes(
knot_zone_contents_t *contents)
{
......@@ -1488,59 +1448,6 @@ int knot_zone_contents_shallow_copy(const knot_zone_contents_t *from,
return KNOT_ENOMEM;
}
contents->apex = from->apex;
contents->node_count = from->node_count;
contents->flags = from->flags;
contents->zone = from->zone;
/* Initialize NSEC3 params */
memcpy(&contents->nsec3_params, &from->nsec3_params,
sizeof(knot_nsec3_params_t));
if ((ret = knot_zone_tree_shallow_copy(from->nodes,
&contents->nodes)) != KNOT_EOK
|| (ret = knot_zone_tree_shallow_copy(from->nsec3_nodes,
&contents->nsec3_nodes)) != KNOT_EOK) {
goto cleanup;
}
dbg_zone("knot_zone_contents_shallow_copy: finished OK\n");
*to = contents;
return KNOT_EOK;
cleanup:
knot_zone_tree_free(&contents->nodes);
knot_zone_tree_free(&contents->nsec3_nodes);
free(contents);
return ret;
}
/*----------------------------------------------------------------------------*/
int knot_zone_contents_shallow_copy2(const knot_zone_contents_t *from,
knot_zone_contents_t **to)
{
if (from == NULL || to == NULL) {
return KNOT_EINVAL;
}
/* Copy to same destination as source. */
if (from == *to) {
return KNOT_EINVAL;
}
int ret = KNOT_EOK;
knot_zone_contents_t *contents = (knot_zone_contents_t *)calloc(
1, sizeof(knot_zone_contents_t));
if (contents == NULL) {
ERR_ALLOC_FAILED;
return KNOT_ENOMEM;
}
//contents->apex = from->apex;
contents->node_count = from->node_count;
......@@ -1600,17 +1507,14 @@ void knot_zone_contents_deep_free(knot_zone_contents_t **contents)
}
if ((*contents) != NULL) {
/* has to go through zone twice, rdata may contain references to
node owners earlier in the zone which may be already freed */
/* NSEC3 tree is deleted first as it may contain references to
the normal tree. */
knot_zone_tree_apply_recursive(
// Delete NSEC3 tree
knot_zone_tree_apply(
(*contents)->nsec3_nodes,
knot_zone_contents_destroy_node_rrsets_from_tree,
(void*)1);
knot_zone_tree_apply_recursive(
// Delete normal tree
knot_zone_tree_apply(
(*contents)->nodes,
knot_zone_contents_destroy_node_rrsets_from_tree,
(void*)1);
......
......@@ -190,10 +190,10 @@ int knot_zone_contents_add_nsec3_rrset(knot_zone_contents_t *contents,
knot_rrset_dupl_handling_t dupl);
int knot_zone_contents_remove_node(knot_zone_contents_t *contents,
const knot_node_t *node, knot_node_t **removed_tree);
const knot_dname_t *owner);
int knot_zone_contents_remove_nsec3_node(knot_zone_contents_t *contents,
const knot_node_t *node, knot_node_t **removed);
const knot_dname_t *owner);
/*!
* \brief Tries to find a node with the specified name in the zone.
......@@ -406,24 +406,6 @@ int knot_zone_contents_tree_apply_inorder(knot_zone_contents_t *zone,
knot_zone_contents_apply_cb_t function,
void *data);
/*!
* \brief Applies the given function to each regular node in the zone.
*
* This function uses in-order depth-first reverse traversal, i.e. the function
* is first recursively applied to right subtree, then to the root and then to
* the left subtree.
*
* \note This implies that the zone is stored in a binary tree. Is there a way
* to make this traversal independent on the underlying structure?
*
* \param zone Nodes of this zone will be used as parameters for the function.
* \param function Function to be applied to each node of the zone.
* \param data Arbitrary data to be passed to the function.
*/
int knot_zone_contents_tree_apply_inorder_reverse(knot_zone_contents_t *zone,
knot_zone_contents_apply_cb_t function,
void *data);
/*!
* \brief Applies the given function to each NSEC3 node in the zone.
*
......@@ -443,25 +425,6 @@ int knot_zone_contents_nsec3_apply_inorder(knot_zone_contents_t *zone,
knot_zone_contents_apply_cb_t function,
void *data);
/*!
* \brief Applies the given function to each NSEC3 node in the zone.
*
* This function uses in-order depth-first reverse traversal, i.e. the function
* is first recursively applied to right subtree, then to the root and then to
* the left subtree.
*
* \note This implies that the zone is stored in a binary tree. Is there a way
* to make this traversal independent on the underlying structure?
*
* \param zone NSEC3 nodes of this zone will be used as parameters for the
* function.
* \param function Function to be applied to each node of the zone.
* \param data Arbitrary data to be passed to the function.
*/
int knot_zone_contents_nsec3_apply_inorder_reverse(knot_zone_contents_t *zone,
knot_zone_contents_apply_cb_t function,
void *data);
knot_zone_tree_t *knot_zone_contents_get_nodes(
knot_zone_contents_t *contents);
......@@ -487,9 +450,6 @@ knot_zone_tree_t *knot_zone_contents_get_nsec3_nodes(
int knot_zone_contents_shallow_copy(const knot_zone_contents_t *from,
knot_zone_contents_t **to);
int knot_zone_contents_shallow_copy2(const knot_zone_contents_t *from,
knot_zone_contents_t **to);
void knot_zone_contents_free(knot_zone_contents_t **contents);
void knot_zone_contents_deep_free(knot_zone_contents_t **contents);
......
......@@ -243,36 +243,15 @@ int knot_zone_tree_apply_inorder(knot_zone_tree_t *tree,
/*----------------------------------------------------------------------------*/
int knot_zone_tree_apply_recursive(knot_zone_tree_t *tree,
knot_zone_tree_apply_cb_t function,
void *data)
{
if (tree == NULL || function == NULL) {
return KNOT_EINVAL;
}
return hattrie_apply_rev(tree, (int (*)(value_t*,void*))function, data);
}
/*----------------------------------------------------------------------------*/
int knot_zone_tree_apply(knot_zone_tree_t *tree,
knot_zone_tree_apply_cb_t function,
void *data)
{
int result = KNOT_EOK;
hattrie_iter_t *i = hattrie_iter_begin(tree, 0);
while(!hattrie_iter_finished(i)) {
result = function((knot_node_t **)hattrie_iter_val(i), data);
if (result != KNOT_EOK) {
break;
}
hattrie_iter_next(i);
if (tree == NULL || function == NULL) {
return KNOT_EINVAL;
}
hattrie_iter_free(i);
return result;
return hattrie_apply_rev(tree, (int (*)(value_t*,void*))function, data);
}
/*----------------------------------------------------------------------------*/
......@@ -331,7 +310,7 @@ void knot_zone_tree_deep_free(knot_zone_tree_t **tree)
return;
}
knot_zone_tree_apply_recursive(*tree, knot_zone_tree_free_node, NULL);
knot_zone_tree_apply(*tree, knot_zone_tree_free_node, NULL);
knot_zone_tree_free(tree);
}
......
......@@ -194,24 +194,8 @@ int knot_zone_tree_apply_inorder(knot_zone_tree_t *tree,
void *data);
/*!
* \brief Applies the given function to each node in the zone.
*
* This function visits leafe nodes before their parents.
* But doesn't maintain any specific ordering.
*
* \param tree Zone tree to apply the function to.
* \param function Function to be applied to each node of the zone.
* \param data Arbitrary data to be passed to the function.
*
* \retval KNOT_EOK
* \retval KNOT_EINVAL
*/
int knot_zone_tree_apply_recursive(knot_zone_tree_t *tree,
knot_zone_tree_apply_cb_t function,
void *data);
/*!