diff --git a/isisd/isis_spf.c b/isisd/isis_spf.c index 04e0276324..3158da6bfa 100644 --- a/isisd/isis_spf.c +++ b/isisd/isis_spf.c @@ -36,6 +36,7 @@ #include "table.h" #include "spf_backoff.h" #include "jhash.h" +#include "skiplist.h" #include "isis_constants.h" #include "isis_common.h" @@ -88,14 +89,18 @@ struct isis_vertex { u_int16_t depth; /* The depth in the imaginary tree */ struct list *Adj_N; /* {Adj(N)} next hop or neighbor list */ struct list *parents; /* list of parents for ECMP */ - struct list *children; /* list of children used for tree dump */ + uint64_t insert_counter; }; /* Vertex Queue and associated functions */ struct isis_vertex_queue { - struct list *list; + union { + struct skiplist *slist; + struct list *list; + } l; struct hash *hash; + uint64_t insert_counter; }; static unsigned isis_vertex_queue_hash_key(void *vp) @@ -121,9 +126,50 @@ static int isis_vertex_queue_hash_cmp(const void *a, const void *b) return memcmp(va->N.id, vb->N.id, ISIS_SYS_ID_LEN + 1) == 0; } -static void isis_vertex_queue_init(struct isis_vertex_queue *queue, const char *name) +/* + * Compares vertizes for sorting in the TENT list. Returns true + * if candidate should be considered before current, false otherwise. + */ +static int isis_vertex_queue_tent_cmp(void *a, void *b) { - queue->list = list_new(); + struct isis_vertex *va = a; + struct isis_vertex *vb = b; + + if (va->d_N < vb->d_N) + return -1; + + if (va->d_N > vb->d_N) + return 1; + + if (va->type < vb->type) + return -1; + + if (va->type > vb->type) + return 1; + + if (va->insert_counter < vb->insert_counter) + return -1; + + if (va->insert_counter > vb->insert_counter) + return 1; + + assert(!"Vertizes should be strictly ordered"); +} + +static struct skiplist *isis_vertex_queue_skiplist(void) +{ + return skiplist_new(0, isis_vertex_queue_tent_cmp, NULL); +} + +static void isis_vertex_queue_init(struct isis_vertex_queue *queue, const char *name, bool ordered) +{ + if (ordered) { + queue->insert_counter = 1; + queue->l.slist = isis_vertex_queue_skiplist(); + } else { + queue->insert_counter = 0; + queue->l.list = list_new(); + } queue->hash = hash_create(isis_vertex_queue_hash_key, isis_vertex_queue_hash_cmp, name); @@ -135,9 +181,18 @@ static void isis_vertex_queue_clear(struct isis_vertex_queue *queue) { hash_clean(queue->hash, NULL); - queue->list->del = (void (*)(void *))isis_vertex_del; - list_delete_all_node(queue->list); - queue->list->del = NULL; + if (queue->insert_counter) { + struct isis_vertex *vertex; + while (0 == skiplist_first(queue->l.slist, NULL, (void**)&vertex)) { + isis_vertex_del(vertex); + skiplist_delete_first(queue->l.slist); + } + queue->insert_counter = 1; + } else { + queue->l.list->del = (void (*)(void *))isis_vertex_del; + list_delete_all_node(queue->l.list); + queue->l.list->del = NULL; + } } static void isis_vertex_queue_free(struct isis_vertex_queue *queue) @@ -147,19 +202,26 @@ static void isis_vertex_queue_free(struct isis_vertex_queue *queue) hash_free(queue->hash); queue->hash = NULL; - list_delete(queue->list); - queue->list = NULL; + if (queue->insert_counter) { + skiplist_free(queue->l.slist); + queue->l.slist = NULL; + } else { + list_delete(queue->l.list); + queue->l.list = NULL; + } } static unsigned int isis_vertex_queue_count(struct isis_vertex_queue *queue) { - return listcount(queue->list); + return hashcount(queue->hash); } -static void isis_vertex_queue_add(struct isis_vertex_queue *queue, - struct isis_vertex *vertex) +static void isis_vertex_queue_append(struct isis_vertex_queue *queue, + struct isis_vertex *vertex) { - listnode_add(queue->list, vertex); + assert(!queue->insert_counter); + + listnode_add(queue->l.list, vertex); struct isis_vertex *inserted; @@ -167,17 +229,30 @@ static void isis_vertex_queue_add(struct isis_vertex_queue *queue, assert(inserted == vertex); } +static void isis_vertex_queue_insert(struct isis_vertex_queue *queue, + struct isis_vertex *vertex) +{ + assert(queue->insert_counter); + vertex->insert_counter = queue->insert_counter++; + assert(queue->insert_counter != (uint64_t)-1); + + skiplist_insert(queue->l.slist, vertex, vertex); + + struct isis_vertex *inserted; + inserted = hash_get(queue->hash, vertex, hash_alloc_intern); + assert(inserted == vertex); +} + static struct isis_vertex *isis_vertex_queue_pop(struct isis_vertex_queue *queue) { - struct listnode *node; + assert(queue->insert_counter); - node = listhead(queue->list); - if (!node) + struct isis_vertex *rv; + + if (skiplist_first(queue->l.slist, NULL, (void**)&rv)) return NULL; - struct isis_vertex *rv = listgetdata(node); - - list_delete_node(queue->list, node); + skiplist_delete_first(queue->l.slist); hash_release(queue->hash, rv); return rv; @@ -186,12 +261,14 @@ static struct isis_vertex *isis_vertex_queue_pop(struct isis_vertex_queue *queue static void isis_vertex_queue_delete(struct isis_vertex_queue *queue, struct isis_vertex *vertex) { - listnode_delete(queue->list, vertex); + assert(queue->insert_counter); + + skiplist_delete(queue->l.slist, vertex, vertex); hash_release(queue->hash, vertex); } #define ALL_QUEUE_ELEMENTS_RO(queue, node, data) \ - ALL_LIST_ELEMENTS_RO((queue)->list, node, data) + ALL_LIST_ELEMENTS_RO((queue)->l.list, node, data) /* End of vertex queue definitions */ @@ -353,7 +430,6 @@ static struct isis_vertex *isis_vertex_new(void *id, enum vertextype vtype) vertex->Adj_N = list_new(); vertex->parents = list_new(); - vertex->children = list_new(); return vertex; } @@ -364,8 +440,6 @@ static void isis_vertex_del(struct isis_vertex *vertex) vertex->Adj_N = NULL; list_delete(vertex->parents); vertex->parents = NULL; - list_delete(vertex->children); - vertex->children = NULL; memset(vertex, 0, sizeof(struct isis_vertex)); XFREE(MTYPE_ISIS_VERTEX, vertex); @@ -397,8 +471,8 @@ struct isis_spftree *isis_spftree_new(struct isis_area *area) return NULL; } - isis_vertex_queue_init(&tree->tents, "IS-IS SPF tents"); - isis_vertex_queue_init(&tree->paths, "IS-IS SPF paths"); + isis_vertex_queue_init(&tree->tents, "IS-IS SPF tents", true); + isis_vertex_queue_init(&tree->paths, "IS-IS SPF paths", false); tree->area = area; tree->last_run_timestamp = 0; tree->last_run_duration = 0; @@ -422,8 +496,7 @@ static void isis_spftree_adj_del(struct isis_spftree *spftree, struct isis_vertex *v; if (!adj) return; - for (ALL_QUEUE_ELEMENTS_RO(&spftree->tents, node, v)) - isis_vertex_adj_del(v, adj); + assert(!isis_vertex_queue_count(&spftree->tents)); for (ALL_QUEUE_ELEMENTS_RO(&spftree->paths, node, v)) isis_vertex_adj_del(v, adj); return; @@ -538,7 +611,7 @@ static struct isis_vertex *isis_spf_add_root(struct isis_spftree *spftree, spftree->area->oldmetric ? VTYPE_NONPSEUDO_IS : VTYPE_NONPSEUDO_TE_IS); - isis_vertex_queue_add(&spftree->paths, vertex); + isis_vertex_queue_append(&spftree->paths, vertex); #ifdef EXTREME_DEBUG zlog_debug("ISIS-Spf: added this IS %s %s depth %d dist %d to PATHS", @@ -559,45 +632,6 @@ static struct isis_vertex *isis_find_vertex(struct isis_vertex_queue *queue, voi return hash_lookup(queue->hash, &querier); } -/* - * Compares vertizes for sorting in the TENT list. Returns true - * if candidate should be considered before current, false otherwise. - */ -static bool tent_cmp(struct isis_vertex *current, struct isis_vertex *candidate) -{ - if (current->d_N > candidate->d_N) - return true; - - if (current->d_N == candidate->d_N && current->type > candidate->type) - return true; - - return false; -} - -static void isis_vertex_queue_insert(struct isis_vertex_queue *queue, - struct isis_vertex *vertex) -{ - struct listnode *node; - struct isis_vertex *v; - - /* XXX: This cant use the standard ALL_LIST_ELEMENTS macro */ - for (node = listhead(queue->list); node; node = listnextnode(node)) { - v = listgetdata(node); - if (tent_cmp(v, vertex)) { - listnode_add_before(queue->list, node, vertex); - break; - } - } - - if (node == NULL) - listnode_add(queue->list, vertex); - - struct isis_vertex *inserted; - - inserted = hash_get(queue->hash, vertex, hash_alloc_intern); - assert(inserted == vertex); -} - /* * Add a vertex to TENT sorted by cost and by vertextype on tie break situation */ @@ -622,8 +656,6 @@ static struct isis_vertex *isis_spf_add2tent(struct isis_spftree *spftree, if (parent) { listnode_add(vertex->parents, parent); - if (listnode_lookup(parent->children, vertex) == NULL) - listnode_add(parent->children, vertex); } if (parent && parent->Adj_N && listcount(parent->Adj_N) > 0) { @@ -665,9 +697,6 @@ static void isis_spf_add_local(struct isis_spftree *spftree, if (parent && (listnode_lookup(vertex->parents, parent) == NULL)) listnode_add(vertex->parents, parent); - if (parent && (listnode_lookup(parent->children, vertex) - == NULL)) - listnode_add(parent->children, vertex); return; } else if (vertex->d_N < cost) { /* e) do nothing */ @@ -677,10 +706,6 @@ static void isis_spf_add_local(struct isis_spftree *spftree, struct listnode *pnode, *pnextnode; struct isis_vertex *pvertex; isis_vertex_queue_delete(&spftree->tents, vertex); - assert(listcount(vertex->children) == 0); - for (ALL_LIST_ELEMENTS(vertex->parents, pnode, - pnextnode, pvertex)) - listnode_delete(pvertex->children, vertex); isis_vertex_del(vertex); } } @@ -756,9 +781,6 @@ static void process_N(struct isis_spftree *spftree, enum vertextype vtype, remove_excess_adjs(vertex->Adj_N); if (listnode_lookup(vertex->parents, parent) == NULL) listnode_add(vertex->parents, parent); - if (listnode_lookup(parent->children, vertex) == NULL) - listnode_add(parent->children, vertex); - /* 3) */ return; } else if (vertex->d_N < dist) { return; @@ -767,10 +789,6 @@ static void process_N(struct isis_spftree *spftree, enum vertextype vtype, struct listnode *pnode, *pnextnode; struct isis_vertex *pvertex; isis_vertex_queue_delete(&spftree->tents, vertex); - assert(listcount(vertex->children) == 0); - for (ALL_LIST_ELEMENTS(vertex->parents, pnode, - pnextnode, pvertex)) - listnode_delete(pvertex->children, vertex); isis_vertex_del(vertex); } } @@ -1198,7 +1216,7 @@ static void add_to_paths(struct isis_spftree *spftree, if (isis_find_vertex(&spftree->paths, vertex->N.id, vertex->type)) return; - isis_vertex_queue_add(&spftree->paths, vertex); + isis_vertex_queue_append(&spftree->paths, vertex); #ifdef EXTREME_DEBUG zlog_debug("ISIS-Spf: added %s %s %s depth %d dist %d to PATHS", diff --git a/tests/.gitignore b/tests/.gitignore index 41349cce24..ab1d55b548 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -26,6 +26,7 @@ __pycache__ /bgpd/test_mpath /isisd/test_fuzz_isis_tlv /isisd/test_fuzz_isis_tlv_tests.h +/isisd/test_isis_vertex_queue /lib/cli/test_cli /lib/cli/test_commands /lib/cli/test_commands_defun.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 1c0e9ee732..fafdd73bf3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -29,7 +29,9 @@ if SOLARIS TESTS_ISISD = else TESTS_ISISD = \ - isisd/test_fuzz_isis_tlv + isisd/test_fuzz_isis_tlv \ + isisd/test_isis_vertex_queue \ + # end endif else TESTS_ISISD = @@ -135,6 +137,7 @@ bgpd_test_mp_attr_SOURCES = bgpd/test_mp_attr.c bgpd_test_mpath_SOURCES = bgpd/test_mpath.c isisd_test_fuzz_isis_tlv_SOURCES = isisd/test_fuzz_isis_tlv.c isisd_test_fuzz_isis_tlv_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_builddir)/tests/isisd +isisd_test_isis_vertex_queue_SOURCES = isisd/test_isis_vertex_queue.c ospf6d_test_lsdb_SOURCES = ospf6d/test_lsdb.c lib/cli/common_cli.c @@ -168,6 +171,7 @@ bgpd_test_ecommunity_LDADD = $(BGP_TEST_LDADD) bgpd_test_mp_attr_LDADD = $(BGP_TEST_LDADD) bgpd_test_mpath_LDADD = $(BGP_TEST_LDADD) isisd_test_fuzz_isis_tlv_LDADD = $(ISISD_TEST_LDADD) +isisd_test_isis_vertex_queue_LDADD = $(ISISD_TEST_LDADD) ospf6d_test_lsdb_LDADD = $(OSPF6_TEST_LDADD) EXTRA_DIST = \ @@ -181,6 +185,7 @@ EXTRA_DIST = \ helpers/python/frrtest.py \ isisd/test_fuzz_isis_tlv.py \ isisd/test_fuzz_isis_tlv_tests.h.gz \ + isisd/test_isis_vertex_queue.py \ lib/cli/test_commands.in \ lib/cli/test_commands.py \ lib/cli/test_commands.refout \ diff --git a/tests/isisd/test_isis_vertex_queue.c b/tests/isisd/test_isis_vertex_queue.c new file mode 100644 index 0000000000..50436387b5 --- /dev/null +++ b/tests/isisd/test_isis_vertex_queue.c @@ -0,0 +1,108 @@ +#include + +#include "isisd/isis_spf.c" + +struct thread_master *master; +int isis_sock_init(struct isis_circuit *circuit); +int isis_sock_init(struct isis_circuit *circuit) +{ + return 0; +} + +static struct isis_vertex **vertices; +static size_t vertex_count; + +static void setup_test_vertices(void) +{ + struct prefix p = { + .family = AF_UNSPEC + }; + uint8_t node_id[7]; + + vertices = XMALLOC(MTYPE_TMP, sizeof(*vertices) * 16); + + p.family = AF_INET; + p.prefixlen = 24; + inet_pton(AF_INET, "192.168.1.0", &p.u.prefix4); + vertices[vertex_count] = isis_vertex_new(&p, VTYPE_IPREACH_TE); + vertices[vertex_count]->d_N = 20; + vertex_count++; + + p.family = AF_INET; + p.prefixlen = 24; + inet_pton(AF_INET, "192.168.2.0", &p.u.prefix4); + vertices[vertex_count] = isis_vertex_new(&p, VTYPE_IPREACH_TE); + vertices[vertex_count]->d_N = 20; + vertex_count++; + + memset(node_id, 0, sizeof(node_id)); + node_id[6] = 1; + vertices[vertex_count] = isis_vertex_new(node_id, VTYPE_PSEUDO_TE_IS); + vertices[vertex_count]->d_N = 15; + vertex_count++; + + memset(node_id, 0, sizeof(node_id)); + node_id[5] = 2; + vertices[vertex_count] = isis_vertex_new(node_id, VTYPE_NONPSEUDO_TE_IS); + vertices[vertex_count]->d_N = 15; + vertex_count++; + + p.family = AF_INET; + p.prefixlen = 24; + inet_pton(AF_INET, "192.168.3.0", &p.u.prefix4); + vertices[vertex_count] = isis_vertex_new(&p, VTYPE_IPREACH_TE); + vertices[vertex_count]->d_N = 20; + vertex_count++; +}; + +static void cleanup_test_vertices(void) +{ + for (size_t i = 0; i < vertex_count; i++) + isis_vertex_del(vertices[i]); + XFREE(MTYPE_TMP, vertices); + vertex_count = 0; +} + +static void test_ordered(void) +{ + struct isis_vertex_queue q; + + isis_vertex_queue_init(&q, NULL, true); + for (size_t i = 0; i < vertex_count; i++) + isis_vertex_queue_insert(&q, vertices[i]); + + assert(isis_vertex_queue_count(&q) == vertex_count); + + for (size_t i = 0; i < vertex_count; i++) { + assert(isis_find_vertex(&q, vertices[i]->N.id, vertices[i]->type) == vertices[i]); + } + + assert(isis_vertex_queue_pop(&q) == vertices[2]); + assert(isis_find_vertex(&q, vertices[2]->N.id, vertices[2]->type) == NULL); + + assert(isis_vertex_queue_pop(&q) == vertices[3]); + assert(isis_find_vertex(&q, vertices[3]->N.id, vertices[3]->type) == NULL); + + assert(isis_vertex_queue_pop(&q) == vertices[0]); + assert(isis_find_vertex(&q, vertices[0]->N.id, vertices[0]->type) == NULL); + + assert(isis_vertex_queue_pop(&q) == vertices[1]); + assert(isis_find_vertex(&q, vertices[1]->N.id, vertices[1]->type) == NULL); + + assert(isis_vertex_queue_pop(&q) == vertices[4]); + assert(isis_find_vertex(&q, vertices[4]->N.id, vertices[4]->type) == NULL); + + assert(isis_vertex_queue_count(&q) == 0); + assert(isis_vertex_queue_pop(&q) == NULL); + + isis_vertex_queue_free(&q); +} + +int main(int argc, char **argv) +{ + setup_test_vertices(); + test_ordered(); + cleanup_test_vertices(); + + return 0; +} diff --git a/tests/isisd/test_isis_vertex_queue.py b/tests/isisd/test_isis_vertex_queue.py new file mode 100644 index 0000000000..5974edecc9 --- /dev/null +++ b/tests/isisd/test_isis_vertex_queue.py @@ -0,0 +1,6 @@ +import frrtest + +class TestIsisVertexQueue(frrtest.TestMultiOut): + program = './test_isis_vertex_queue' + +TestIsisVertexQueue.exit_cleanly()