From 9dc716d64c1e2aa7dbd894f0eaa4c3363391941f Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 6 Jul 2017 14:58:49 +0200 Subject: [PATCH 01/10] lib: use "union prefixconstptr" in table code This allows passing struct prefix_{ipv4,ipv6,evpn} * in addition to struct prefix * without an extra cast (since the union uses the gcc transparent-union extension present in all compilers that we support.) Also applies some "const" while we're at it. Signed-off-by: David Lamparter --- lib/table.c | 21 +++++++++++++-------- lib/table.h | 14 +++++++------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/lib/table.c b/lib/table.c index 1461bb81a4..fdbd2913b2 100644 --- a/lib/table.c +++ b/lib/table.c @@ -208,8 +208,9 @@ route_unlock_node (struct route_node *node) /* Find matched prefix. */ struct route_node * -route_node_match (const struct route_table *table, const struct prefix *p) +route_node_match (const struct route_table *table, union prefixconstptr pu) { + const struct prefix *p = pu.p; struct route_node *node; struct route_node *matched; @@ -267,8 +268,9 @@ route_node_match_ipv6 (const struct route_table *table, /* Lookup same prefix node. Return NULL when we can't find route. */ struct route_node * -route_node_lookup (const struct route_table *table, const struct prefix *p) +route_node_lookup (const struct route_table *table, union prefixconstptr pu) { + const struct prefix *p = pu.p; struct route_node *node; u_char prefixlen = p->prefixlen; const u_char *prefix = &p->u.prefix; @@ -289,8 +291,9 @@ route_node_lookup (const struct route_table *table, const struct prefix *p) /* Lookup same prefix node. Return NULL when we can't find route. */ struct route_node * -route_node_lookup_maynull (const struct route_table *table, const struct prefix *p) +route_node_lookup_maynull (const struct route_table *table, union prefixconstptr pu) { + const struct prefix *p = pu.p; struct route_node *node; u_char prefixlen = p->prefixlen; const u_char *prefix = &p->u.prefix; @@ -311,8 +314,9 @@ route_node_lookup_maynull (const struct route_table *table, const struct prefix /* Add node to routing table. */ struct route_node * -route_node_get (struct route_table *const table, const struct prefix *p) +route_node_get (struct route_table *const table, union prefixconstptr pu) { + const struct prefix *p = pu.p; struct route_node *new; struct route_node *node; struct route_node *match; @@ -473,7 +477,7 @@ route_next (struct route_node *node) /* Unlock current node and lock next node until limit. */ struct route_node * -route_next_until (struct route_node *node, struct route_node *limit) +route_next_until (struct route_node *node, const struct route_node *limit) { struct route_node *next; struct route_node *start; @@ -578,7 +582,7 @@ route_table_init (void) * +1 if p1 occurs after p2 (p1 > p2) */ int -route_table_prefix_iter_cmp (struct prefix *p1, struct prefix *p2) +route_table_prefix_iter_cmp (const struct prefix *p1, const struct prefix *p2) { struct prefix common_space; struct prefix *common = &common_space; @@ -661,7 +665,7 @@ route_get_subtree_next (struct route_node *node) */ static struct route_node * route_table_get_next_internal (const struct route_table *table, - struct prefix *p) + const struct prefix *p) { struct route_node *node, *tmp_node; int cmp; @@ -762,8 +766,9 @@ route_table_get_next_internal (const struct route_table *table, * iteration. */ struct route_node * -route_table_get_next (const struct route_table *table, struct prefix *p) +route_table_get_next (const struct route_table *table, union prefixconstptr pu) { + const struct prefix *p = pu.p; struct route_node *node; node = route_table_get_next_internal (table, p); diff --git a/lib/table.h b/lib/table.h index 00131b29c6..6a5291487b 100644 --- a/lib/table.h +++ b/lib/table.h @@ -153,16 +153,16 @@ extern void route_unlock_node (struct route_node *node); extern struct route_node *route_top (struct route_table *); extern struct route_node *route_next (struct route_node *); extern struct route_node *route_next_until (struct route_node *, - struct route_node *); + const struct route_node *); extern struct route_node *route_node_get (struct route_table *const, - const struct prefix *); + union prefixconstptr); extern struct route_node *route_node_lookup (const struct route_table *, - const struct prefix *); + union prefixconstptr); extern struct route_node *route_node_lookup_maynull (const struct route_table *, - const struct prefix *); + union prefixconstptr); extern struct route_node *route_lock_node (struct route_node *node); extern struct route_node *route_node_match (const struct route_table *, - const struct prefix *); + union prefixconstptr); extern struct route_node *route_node_match_ipv4 (const struct route_table *, const struct in_addr *); extern struct route_node *route_node_match_ipv6 (const struct route_table *, @@ -176,9 +176,9 @@ extern void route_node_destroy (route_table_delegate_t *, struct route_table *, struct route_node *); extern struct route_node * -route_table_get_next (const struct route_table *table, struct prefix *p); +route_table_get_next (const struct route_table *table, union prefixconstptr pu); extern int -route_table_prefix_iter_cmp (struct prefix *p1, struct prefix *p2); +route_table_prefix_iter_cmp (const struct prefix *p1, const struct prefix *p2); /* * Iterator functions. From 4cb260c33e7656d35167beb3d006e6a78766716e Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 6 Jul 2017 15:30:31 +0200 Subject: [PATCH 02/10] lib: add some abstraction guards for table code route_node->parent and route_node->link shouldn't be touched by user code since that is a recipe for trouble once we have a hash table in parallel. Signed-off-by: David Lamparter --- bgpd/rfapi/rfapi_import.c | 2 ++ bgpd/rfapi/rfapi_rib.c | 2 ++ lib/table.c | 2 ++ lib/table.h | 43 +++++++++++++++++++++++++++++++++++---- ospf6d/ospf6_lsdb.c | 3 +++ 5 files changed, 48 insertions(+), 4 deletions(-) diff --git a/bgpd/rfapi/rfapi_import.c b/bgpd/rfapi/rfapi_import.c index b0c6db2a1e..b6c0e30295 100644 --- a/bgpd/rfapi/rfapi_import.c +++ b/bgpd/rfapi/rfapi_import.c @@ -1783,6 +1783,8 @@ rfapiNhlAddSubtree ( struct rfapi_ip_prefix rprefix; int rcount = 0; + /* FIXME: need to find a better way here to work without sticking our + * hands in node->link */ if (rn->l_left && rn->l_left != omit_node) { if (rn->l_left->info) diff --git a/bgpd/rfapi/rfapi_rib.c b/bgpd/rfapi/rfapi_rib.c index 5c3976a0c1..855901b5a7 100644 --- a/bgpd/rfapi/rfapi_rib.c +++ b/bgpd/rfapi/rfapi_rib.c @@ -1804,6 +1804,8 @@ rfapiRibUpdatePendingNodeSubtree ( struct route_node *omit_subtree, /* may be NULL */ uint32_t lifetime) { + /* FIXME: need to find a better way here to work without sticking our + * hands in node->link */ if (it_node->l_left && (it_node->l_left != omit_subtree)) { if (it_node->l_left->info) diff --git a/lib/table.c b/lib/table.c index fdbd2913b2..d4dbeb79cc 100644 --- a/lib/table.c +++ b/lib/table.c @@ -19,6 +19,8 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ +#define FRR_COMPILING_TABLE_C + #include #include "prefix.h" diff --git a/lib/table.h b/lib/table.h index 6a5291487b..a906eb2434 100644 --- a/lib/table.h +++ b/lib/table.h @@ -71,6 +71,41 @@ struct route_table void *info; }; +/* + * node->link is really internal to the table code and should not be + * accessed by outside code. We don't have any writers (yay), though some + * readers are left to be fixed. + * + * rationale: we need to add a hash table in parallel, to speed up + * exact-match lookups. + * + * same really applies for node->parent, though that's less of an issue. + * table->link should be - and is - NEVER written by outside code + */ +#ifdef FRR_COMPILING_TABLE_C +#define table_rdonly(x) x +#define table_internal(x) x +#else +#define table_rdonly(x) const x +#define table_internal(x) const x \ + __attribute__((deprecated("this should only be accessed by lib/table.c"))) +/* table_internal is for node->link and node->lock, once we have done + * something about remaining accesses */ +#endif + +/* so... the problem with this is that "const" doesn't mean "readonly". + * It in fact may allow the compiler to optimize based on the assumption + * that the value doesn't change. Hence, since the only purpose of this + * is to aid in development, don't put the "const" in release builds. + * + * (I haven't seen this actually break, but GCC and LLVM are getting ever + * more aggressive in optimizing...) + */ +#ifndef DEV_BUILD +#undef table_rdonly +#define table_rdonly(x) x +#endif + /* * Macro that defines all fields in a route node. */ @@ -79,12 +114,12 @@ struct route_table struct prefix p; \ \ /* Tree link. */ \ - struct route_table *table; \ - struct route_node *parent; \ - struct route_node *link[2]; \ + struct route_table * table_rdonly(table); \ + struct route_node * table_rdonly(parent); \ + struct route_node * table_rdonly(link[2]); \ \ /* Lock of this radix */ \ - unsigned int lock; \ + unsigned int table_rdonly(lock); \ \ /* Each node of route. */ \ void *info; \ diff --git a/ospf6d/ospf6_lsdb.c b/ospf6d/ospf6_lsdb.c index cb522226ac..ce31e25a2f 100644 --- a/ospf6d/ospf6_lsdb.c +++ b/ospf6d/ospf6_lsdb.c @@ -242,6 +242,9 @@ ospf6_lsdb_lookup_next (u_int16_t type, u_int32_t id, u_int32_t adv_router, zlog_debug ("lsdb_lookup_next: key: %s", buf); } + /* FIXME: need to find a better way here to work without sticking our + * hands in node->link, e.g. route_node_match_maynull() */ + node = lsdb->table->top; /* walk down tree. */ while (node && node->p.prefixlen <= p->prefixlen && From bc7a2c035aa2d8d2820503fb8b7c8328b28bdde4 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 6 Jul 2017 16:33:10 +0200 Subject: [PATCH 03/10] lib: table: maintain parallel hash for route_table See next commit for description. Signed-off-by: David Lamparter --- lib/table.c | 27 ++++++++++++++++++++++++++- lib/table.h | 2 ++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/table.c b/lib/table.c index d4dbeb79cc..1f01a29973 100644 --- a/lib/table.c +++ b/lib/table.c @@ -27,6 +27,7 @@ #include "table.h" #include "memory.h" #include "sockunion.h" +#include "jhash.h" DEFINE_MTYPE( LIB, ROUTE_TABLE, "Route table") DEFINE_MTYPE( LIB, ROUTE_NODE, "Route node") @@ -34,6 +35,16 @@ DEFINE_MTYPE( LIB, ROUTE_NODE, "Route node") static void route_node_delete (struct route_node *); static void route_table_free (struct route_table *); +static unsigned route_table_hash_key(void *pp) +{ + struct prefix copy; + + /* make sure *all* unused bits are zero, particularly including alignment / + * padding and unused prefix bytes. */ + memset (©, 0, sizeof(copy)); + prefix_copy (©, (struct prefix *)pp); + return jhash (©, sizeof(copy), 0x55aa5a5a); +} /* * route_table_init_with_delegate @@ -45,6 +56,9 @@ route_table_init_with_delegate (route_table_delegate_t *delegate) rt = XCALLOC (MTYPE_ROUTE_TABLE, sizeof (struct route_table)); rt->delegate = delegate; + rt->hash = hash_create(route_table_hash_key, + (int (*)(const void *, const void *)) prefix_same, + "route table hash"); return rt; } @@ -65,13 +79,16 @@ route_node_new (struct route_table *table) static struct route_node * route_node_set (struct route_table *table, const struct prefix *prefix) { - struct route_node *node; + struct route_node *node, *inserted; node = route_node_new (table); prefix_copy (&node->p, prefix); node->table = table; + inserted = hash_get (node->table->hash, node, hash_alloc_intern); + assert (inserted == node); + return node; } @@ -94,6 +111,9 @@ route_table_free (struct route_table *rt) if (rt == NULL) return; + hash_clean (rt->hash, NULL); + hash_free (rt->hash); + node = rt->top; /* Bulk deletion of nodes remaining in this table. This function is not @@ -322,6 +342,7 @@ route_node_get (struct route_table *const table, union prefixconstptr pu) struct route_node *new; struct route_node *node; struct route_node *match; + struct route_node *inserted; u_char prefixlen = p->prefixlen; const u_char *prefix = &p->u.prefix; @@ -352,6 +373,8 @@ route_node_get (struct route_table *const table, union prefixconstptr pu) new->p.family = p->family; new->table = table; set_link (new, node); + inserted = hash_get (node->table->hash, new, hash_alloc_intern); + assert (inserted == new); if (match) set_link (match, new); @@ -407,6 +430,8 @@ route_node_delete (struct route_node *node) node->table->count--; + hash_release (node->table->hash, node); + /* WARNING: FRAGILE CODE! * route_node_free may have the side effect of free'ing the entire table. * this is permitted only if table->count got decremented to zero above, diff --git a/lib/table.h b/lib/table.h index a906eb2434..79a32b59ab 100644 --- a/lib/table.h +++ b/lib/table.h @@ -23,6 +23,7 @@ #define _ZEBRA_TABLE_H #include "memory.h" +#include "hash.h" DECLARE_MTYPE(ROUTE_TABLE) DECLARE_MTYPE(ROUTE_NODE) @@ -56,6 +57,7 @@ struct route_table_delegate_t_ struct route_table { struct route_node *top; + struct hash *hash; /* * Delegate that performs certain functions for this table. From 736ac221d1e89f3f35703d5175057404de62b50b Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 6 Jul 2017 17:30:04 +0200 Subject: [PATCH 04/10] lib: table: use hash for exact-match lookups Most read accesses of route_table are actually exact matches where walking down the tree is wildly inefficient. Use a parallel hash structure instead. This significantly speeds up processes that are performance-bound by table accesses, e.g. BGP withdraw processing. In other locations, the improvement is not seen as strongly, e.g. when filter processing is the limiting factor. [includes fix to ignore prefix host bits in hash comparison] Signed-off-by: David Lamparter --- lib/table.c | 44 +++++++++++++++----------------------------- 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/lib/table.c b/lib/table.c index 1f01a29973..1095b03b76 100644 --- a/lib/table.c +++ b/lib/table.c @@ -46,6 +46,12 @@ static unsigned route_table_hash_key(void *pp) return jhash (©, sizeof(copy), 0x55aa5a5a); } +static int route_table_hash_cmp(const void *a, const void *b) +{ + const struct prefix *pa = a, *pb = b; + return prefix_cmp(pa, pb) == 0; +} + /* * route_table_init_with_delegate */ @@ -57,7 +63,7 @@ route_table_init_with_delegate (route_table_delegate_t *delegate) rt = XCALLOC (MTYPE_ROUTE_TABLE, sizeof (struct route_table)); rt->delegate = delegate; rt->hash = hash_create(route_table_hash_key, - (int (*)(const void *, const void *)) prefix_same, + route_table_hash_cmp, "route table hash"); return rt; } @@ -294,21 +300,9 @@ route_node_lookup (const struct route_table *table, union prefixconstptr pu) { const struct prefix *p = pu.p; struct route_node *node; - u_char prefixlen = p->prefixlen; - const u_char *prefix = &p->u.prefix; - node = table->top; - - while (node && node->p.prefixlen <= prefixlen && - prefix_match (&node->p, p)) - { - if (node->p.prefixlen == prefixlen) - return node->info ? route_lock_node (node) : NULL; - - node = node->link[prefix_bit(prefix, node->p.prefixlen)]; - } - - return NULL; + node = hash_get (table->hash, (void *)p, NULL); + return (node && node->info) ? route_lock_node (node) : NULL; } /* Lookup same prefix node. Return NULL when we can't find route. */ @@ -317,21 +311,9 @@ route_node_lookup_maynull (const struct route_table *table, union prefixconstptr { const struct prefix *p = pu.p; struct route_node *node; - u_char prefixlen = p->prefixlen; - const u_char *prefix = &p->u.prefix; - node = table->top; - - while (node && node->p.prefixlen <= prefixlen && - prefix_match (&node->p, p)) - { - if (node->p.prefixlen == prefixlen) - return route_lock_node (node); - - node = node->link[prefix_bit(prefix, node->p.prefixlen)]; - } - - return NULL; + node = hash_get (table->hash, (void *)p, NULL); + return node ? route_lock_node (node) : NULL; } /* Add node to routing table. */ @@ -346,6 +328,10 @@ route_node_get (struct route_table *const table, union prefixconstptr pu) u_char prefixlen = p->prefixlen; const u_char *prefix = &p->u.prefix; + node = hash_get (table->hash, (void *)p, NULL); + if (node && node->info) + return route_lock_node (node); + match = NULL; node = table->top; while (node && node->p.prefixlen <= prefixlen && From 49dd8e3758979254e74b8bf3480ea8a607f084b8 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 7 Jul 2017 17:23:30 +0200 Subject: [PATCH 05/10] ospf6d: use macro for LSDB walks ... to make it easier to refactor all of the iteration uses. Signed-off-by: David Lamparter --- ospf6d/ospf6_abr.c | 14 +++++------- ospf6d/ospf6_asbr.c | 9 +++----- ospf6d/ospf6_interface.c | 6 ++--- ospf6d/ospf6_intra.c | 12 ++++------ ospf6d/ospf6_lsdb.c | 13 +++++------ ospf6d/ospf6_lsdb.h | 9 ++++++++ ospf6d/ospf6_message.c | 24 +++++++------------- ospf6d/ospf6_neighbor.c | 48 ++++++++++++++-------------------------- ospf6d/ospf6_snmp.c | 22 +++++++----------- ospf6d/ospf6_spf.c | 3 +-- 10 files changed, 61 insertions(+), 99 deletions(-) diff --git a/ospf6d/ospf6_abr.c b/ospf6d/ospf6_abr.c index 7ef7fdd94a..5ef6604ccf 100644 --- a/ospf6d/ospf6_abr.c +++ b/ospf6d/ospf6_abr.c @@ -989,13 +989,11 @@ ospf6_abr_examin_brouter (u_int32_t router_id) return; type = htons (OSPF6_LSTYPE_INTER_ROUTER); - for (lsa = ospf6_lsdb_type_router_head (type, router_id, oa->lsdb); lsa; - lsa = ospf6_lsdb_type_router_next (type, router_id, lsa)) - ospf6_abr_examin_summary (lsa, oa); + for (ALL_LSDB_TYPED_ADVRTR(oa->lsdb, type, router_id, lsa)) + ospf6_abr_examin_summary (lsa, oa); type = htons (OSPF6_LSTYPE_INTER_PREFIX); - for (lsa = ospf6_lsdb_type_router_head (type, router_id, oa->lsdb); lsa; - lsa = ospf6_lsdb_type_router_next (type, router_id, lsa)) + for (ALL_LSDB_TYPED_ADVRTR(oa->lsdb, type, router_id, lsa)) ospf6_abr_examin_summary (lsa, oa); } @@ -1006,13 +1004,11 @@ ospf6_abr_reimport (struct ospf6_area *oa) u_int16_t type; type = htons (OSPF6_LSTYPE_INTER_ROUTER); - for (lsa = ospf6_lsdb_type_head (type, oa->lsdb); lsa; - lsa = ospf6_lsdb_type_next (type, lsa)) + for (ALL_LSDB_TYPED(oa->lsdb, type, lsa)) ospf6_abr_examin_summary (lsa, oa); type = htons (OSPF6_LSTYPE_INTER_PREFIX); - for (lsa = ospf6_lsdb_type_head (type, oa->lsdb); lsa; - lsa = ospf6_lsdb_type_next (type, lsa)) + for (ALL_LSDB_TYPED(oa->lsdb, type, lsa)) ospf6_abr_examin_summary (lsa, oa); } diff --git a/ospf6d/ospf6_asbr.c b/ospf6d/ospf6_asbr.c index 302d8c9865..f04c132092 100644 --- a/ospf6d/ospf6_asbr.c +++ b/ospf6d/ospf6_asbr.c @@ -343,8 +343,7 @@ ospf6_asbr_lsentry_add (struct ospf6_route *asbr_entry) type = htons (OSPF6_LSTYPE_AS_EXTERNAL); router = ospf6_linkstate_prefix_adv_router (&asbr_entry->prefix); - for (lsa = ospf6_lsdb_type_router_head (type, router, ospf6->lsdb); lsa; - lsa = ospf6_lsdb_type_router_next (type, router, lsa)) + for (ALL_LSDB_TYPED_ADVRTR(ospf6->lsdb, type, router, lsa)) { if (! OSPF6_LSA_IS_MAXAGE (lsa)) ospf6_asbr_lsa_add (lsa); @@ -360,8 +359,7 @@ ospf6_asbr_lsentry_remove (struct ospf6_route *asbr_entry) type = htons (OSPF6_LSTYPE_AS_EXTERNAL); router = ospf6_linkstate_prefix_adv_router (&asbr_entry->prefix); - for (lsa = ospf6_lsdb_type_router_head (type, router, ospf6->lsdb); - lsa; lsa = ospf6_lsdb_type_router_next (type, router, lsa)) + for (ALL_LSDB_TYPED_ADVRTR(ospf6->lsdb, type, router, lsa)) ospf6_asbr_lsa_remove (lsa); } @@ -444,8 +442,7 @@ ospf6_asbr_send_externals_to_area (struct ospf6_area *oa) { struct ospf6_lsa *lsa; - for (lsa = ospf6_lsdb_head (oa->ospf6->lsdb); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(oa->ospf6->lsdb, lsa)) { if (ntohs (lsa->header->type) == OSPF6_LSTYPE_AS_EXTERNAL) { diff --git a/ospf6d/ospf6_interface.c b/ospf6d/ospf6_interface.c index e91c249845..bff3200081 100644 --- a/ospf6d/ospf6_interface.c +++ b/ospf6d/ospf6_interface.c @@ -993,8 +993,7 @@ ospf6_interface_show (struct vty *vty, struct interface *ifp) oi->lsupdate_list->count, duration, (oi->thread_send_lsupdate ? "on" : "off"), VNL); - for (lsa = ospf6_lsdb_head (oi->lsupdate_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(oi->lsupdate_list, lsa)) vty_out (vty, " %s%s", lsa->name, VNL); timerclear (&res); @@ -1005,8 +1004,7 @@ ospf6_interface_show (struct vty *vty, struct interface *ifp) oi->lsack_list->count, duration, (oi->thread_send_lsack ? "on" : "off"), VNL); - for (lsa = ospf6_lsdb_head (oi->lsack_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(oi->lsack_list, lsa)) vty_out (vty, " %s%s", lsa->name, VNL); ospf6_bfd_show_info(vty, oi->bfd_info, 1); return 0; diff --git a/ospf6d/ospf6_intra.c b/ospf6d/ospf6_intra.c index 6acd45b7c1..4238fd224e 100644 --- a/ospf6d/ospf6_intra.c +++ b/ospf6d/ospf6_intra.c @@ -341,8 +341,7 @@ ospf6_router_lsa_originate (struct thread *thread) type = ntohs (OSPF6_LSTYPE_ROUTER); router = oa->ospf6->router_id; count = 0; - for (lsa = ospf6_lsdb_type_router_head (type, router, oa->lsdb); lsa; - lsa = ospf6_lsdb_type_router_next (type, router, lsa)) + for (ALL_LSDB_TYPED_ADVRTR(oa->lsdb, type, router, lsa)) { if (ntohl (lsa->header->id) < link_state_id) continue; @@ -495,8 +494,7 @@ ospf6_network_lsa_originate (struct thread *thread) /* Collect the interface's Link-LSAs to describe network's optional capabilities */ type = htons (OSPF6_LSTYPE_LINK); - for (lsa = ospf6_lsdb_type_head (type, oi->lsdb); lsa; - lsa = ospf6_lsdb_type_next (type, lsa)) + for (ALL_LSDB_TYPED(oi->lsdb, type, lsa)) { link_lsa = (struct ospf6_link_lsa *) ((caddr_t) lsa->header + sizeof (struct ospf6_lsa_header)); @@ -1096,8 +1094,7 @@ ospf6_intra_prefix_lsa_originate_transit (struct thread *thread) route_advertise = ospf6_route_table_create (0, 0); type = ntohs (OSPF6_LSTYPE_LINK); - for (lsa = ospf6_lsdb_type_head (type, oi->lsdb); lsa; - lsa = ospf6_lsdb_type_next (type, lsa)) + for (ALL_LSDB_TYPED(oi->lsdb, type, lsa)) { if (OSPF6_LSA_IS_MAXAGE (lsa)) continue; @@ -1429,8 +1426,7 @@ ospf6_intra_route_calculation (struct ospf6_area *oa) route->flag = OSPF6_ROUTE_REMOVE; type = htons (OSPF6_LSTYPE_INTRA_PREFIX); - for (lsa = ospf6_lsdb_type_head (type, oa->lsdb); lsa; - lsa = ospf6_lsdb_type_next (type, lsa)) + for (ALL_LSDB_TYPED(oa->lsdb, type, lsa)) ospf6_intra_prefix_lsa_add (lsa); oa->route_table->hook_add = hook_add; diff --git a/ospf6d/ospf6_lsdb.c b/ospf6d/ospf6_lsdb.c index ce31e25a2f..6879d71a16 100644 --- a/ospf6d/ospf6_lsdb.c +++ b/ospf6d/ospf6_lsdb.c @@ -80,8 +80,7 @@ _lsdb_count_assert (struct ospf6_lsdb *lsdb) { struct ospf6_lsa *debug; unsigned int num = 0; - for (debug = ospf6_lsdb_head (lsdb); debug; - debug = ospf6_lsdb_next (debug)) + for (ALL_LSDB(lsdb, debug)) num++; if (num == lsdb->count) @@ -89,8 +88,7 @@ _lsdb_count_assert (struct ospf6_lsdb *lsdb) zlog_debug ("PANIC !! lsdb[%p]->count = %d, real = %d", lsdb, lsdb->count, num); - for (debug = ospf6_lsdb_head (lsdb); debug; - debug = ospf6_lsdb_next (debug)) + for (ALL_LSDB(lsdb, debug)) zlog_debug ("%p %p %s lsdb[%p]", debug->prev, debug->next, debug->name, debug->lsdb); zlog_debug ("DUMP END"); @@ -437,7 +435,7 @@ ospf6_lsdb_remove_all (struct ospf6_lsdb *lsdb) if (lsdb == NULL) return; - for (lsa = ospf6_lsdb_head (lsdb); lsa; lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(lsdb, lsa)) ospf6_lsdb_remove (lsa, lsdb); } @@ -458,7 +456,7 @@ ospf6_lsdb_maxage_remover (struct ospf6_lsdb *lsdb) int reschedule = 0; struct ospf6_lsa *lsa; - for (lsa = ospf6_lsdb_head (lsdb); lsa; lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(lsdb, lsa)) { if (! OSPF6_LSA_IS_MAXAGE (lsa)) continue; @@ -559,8 +557,7 @@ ospf6_new_ls_id (u_int16_t type, u_int32_t adv_router, /* This routine is curently invoked only for Inter-Prefix LSAs for * non-summarized routes (no area/range). */ - for (lsa = ospf6_lsdb_type_router_head (type, adv_router, lsdb); lsa; - lsa = ospf6_lsdb_type_router_next (type, adv_router, lsa)) + for (ALL_LSDB_TYPED_ADVRTR(lsdb, type, adv_router, lsa)) { tmp_id = ntohl (lsa->header->id); if (tmp_id < id) diff --git a/ospf6d/ospf6_lsdb.h b/ospf6d/ospf6_lsdb.h index 866868080a..529fbefba6 100644 --- a/ospf6d/ospf6_lsdb.h +++ b/ospf6d/ospf6_lsdb.h @@ -50,6 +50,9 @@ extern void ospf6_lsdb_remove (struct ospf6_lsa *lsa, struct ospf6_lsdb *lsdb); extern struct ospf6_lsa *ospf6_lsdb_head (struct ospf6_lsdb *lsdb); extern struct ospf6_lsa *ospf6_lsdb_next (struct ospf6_lsa *lsa); +#define ALL_LSDB(lsdb, lsa) \ + lsa = ospf6_lsdb_head(lsdb); lsa; \ + lsa = ospf6_lsdb_next(lsa) extern struct ospf6_lsa *ospf6_lsdb_type_router_head (u_int16_t type, u_int32_t adv_router, @@ -57,11 +60,17 @@ extern struct ospf6_lsa *ospf6_lsdb_type_router_head (u_int16_t type, extern struct ospf6_lsa *ospf6_lsdb_type_router_next (u_int16_t type, u_int32_t adv_router, struct ospf6_lsa *lsa); +#define ALL_LSDB_TYPED_ADVRTR(lsdb, type, adv_router, lsa) \ + lsa = ospf6_lsdb_type_router_head(type, adv_router, lsdb); lsa; \ + lsa = ospf6_lsdb_type_router_next(type, adv_router, lsa) extern struct ospf6_lsa *ospf6_lsdb_type_head (u_int16_t type, struct ospf6_lsdb *lsdb); extern struct ospf6_lsa *ospf6_lsdb_type_next (u_int16_t type, struct ospf6_lsa *lsa); +#define ALL_LSDB_TYPED(lsdb, type, lsa) \ + lsa = ospf6_lsdb_type_head(type, lsdb); lsa; \ + lsa = ospf6_lsdb_type_next(type, lsa) extern void ospf6_lsdb_remove_all (struct ospf6_lsdb *lsdb); extern void ospf6_lsdb_lsa_unlock (struct ospf6_lsa *lsa); diff --git a/ospf6d/ospf6_message.c b/ospf6d/ospf6_message.c index 9bd337ba12..1eba306257 100644 --- a/ospf6d/ospf6_message.c +++ b/ospf6d/ospf6_message.c @@ -1826,8 +1826,7 @@ ospf6_dbdesc_send (struct thread *thread) p = (u_char *)((caddr_t) dbdesc + sizeof (struct ospf6_dbdesc)); if (! CHECK_FLAG (on->dbdesc_bits, OSPF6_DBDESC_IBIT)) { - for (lsa = ospf6_lsdb_head (on->dbdesc_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->dbdesc_list, lsa)) { ospf6_lsa_age_update_to_send (lsa, on->ospf6_if->transdelay); @@ -1870,8 +1869,7 @@ ospf6_dbdesc_send_newone (struct thread *thread) /* move LSAs from summary_list to dbdesc_list (within neighbor structure) so that ospf6_send_dbdesc () can send those LSAs */ size = sizeof (struct ospf6_lsa_header) + sizeof (struct ospf6_dbdesc); - for (lsa = ospf6_lsdb_head (on->summary_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->summary_list, lsa)) { if (size + sizeof (struct ospf6_lsa_header) > ospf6_packet_max(on->ospf6_if)) { @@ -1932,8 +1930,7 @@ ospf6_lsreq_send (struct thread *thread) /* set Request entries in lsreq */ p = (u_char *)((caddr_t) oh + sizeof (struct ospf6_header)); - for (lsa = ospf6_lsdb_head (on->request_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->request_list, lsa)) { /* MTU check */ if (p - sendbuf + sizeof (struct ospf6_lsreq_entry) > ospf6_packet_max(on->ospf6_if)) @@ -2015,8 +2012,7 @@ ospf6_lsupdate_send_neighbor (struct thread *thread) /* lsupdate_list lists those LSA which doesn't need to be retransmitted. remove those from the list */ - for (lsa = ospf6_lsdb_head (on->lsupdate_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->lsupdate_list, lsa)) { /* MTU check */ if ( (p - sendbuf + (unsigned int)OSPF6_LSA_SIZE (lsa->header)) @@ -2061,8 +2057,7 @@ ospf6_lsupdate_send_neighbor (struct thread *thread) p = (u_char *)((caddr_t) lsupdate + sizeof (struct ospf6_lsupdate)); lsa_cnt = 0; - for (lsa = ospf6_lsdb_head (on->retrans_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->retrans_list, lsa)) { /* MTU check */ if ( (p - sendbuf + (unsigned int)OSPF6_LSA_SIZE (lsa->header)) @@ -2138,8 +2133,7 @@ ospf6_lsupdate_send_interface (struct thread *thread) p = (u_char *)((caddr_t) lsupdate + sizeof (struct ospf6_lsupdate)); lsa_cnt = 0; - for (lsa = ospf6_lsdb_head (oi->lsupdate_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(oi->lsupdate_list, lsa)) { /* MTU check */ if ( (p - sendbuf + ((unsigned int)OSPF6_LSA_SIZE (lsa->header))) @@ -2213,8 +2207,7 @@ ospf6_lsack_send_neighbor (struct thread *thread) p = (u_char *)((caddr_t) oh + sizeof (struct ospf6_header)); - for (lsa = ospf6_lsdb_head (on->lsack_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->lsack_list, lsa)) { /* MTU check */ if (p - sendbuf + sizeof (struct ospf6_lsa_header) > ospf6_packet_max(on->ospf6_if)) @@ -2281,8 +2274,7 @@ ospf6_lsack_send_interface (struct thread *thread) p = (u_char *)((caddr_t) oh + sizeof (struct ospf6_header)); - for (lsa = ospf6_lsdb_head (oi->lsack_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(oi->lsack_list, lsa)) { /* MTU check */ if (p - sendbuf + sizeof (struct ospf6_lsa_header) > ospf6_packet_max(oi)) diff --git a/ospf6d/ospf6_neighbor.c b/ospf6d/ospf6_neighbor.c index c58922b0ea..9ff4e55839 100644 --- a/ospf6d/ospf6_neighbor.c +++ b/ospf6d/ospf6_neighbor.c @@ -123,8 +123,7 @@ ospf6_neighbor_delete (struct ospf6_neighbor *on) ospf6_lsdb_remove_all (on->summary_list); ospf6_lsdb_remove_all (on->request_list); - for (lsa = ospf6_lsdb_head (on->retrans_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->retrans_list, lsa)) { ospf6_decrement_retrans_count (lsa); ospf6_lsdb_remove (lsa, on->retrans_list); @@ -302,16 +301,14 @@ negotiation_done (struct thread *thread) /* clear ls-list */ ospf6_lsdb_remove_all (on->summary_list); ospf6_lsdb_remove_all (on->request_list); - for (lsa = ospf6_lsdb_head (on->retrans_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->retrans_list, lsa)) { ospf6_decrement_retrans_count (lsa); ospf6_lsdb_remove (lsa, on->retrans_list); } /* Interface scoped LSAs */ - for (lsa = ospf6_lsdb_head (on->ospf6_if->lsdb); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->ospf6_if->lsdb, lsa)) { if (OSPF6_LSA_IS_MAXAGE (lsa)) { @@ -323,8 +320,7 @@ negotiation_done (struct thread *thread) } /* Area scoped LSAs */ - for (lsa = ospf6_lsdb_head (on->ospf6_if->area->lsdb); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->ospf6_if->area->lsdb, lsa)) { if (OSPF6_LSA_IS_MAXAGE (lsa)) { @@ -336,8 +332,7 @@ negotiation_done (struct thread *thread) } /* AS scoped LSAs */ - for (lsa = ospf6_lsdb_head (on->ospf6_if->area->ospf6->lsdb); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->ospf6_if->area->ospf6->lsdb, lsa)) { if (OSPF6_LSA_IS_MAXAGE (lsa)) { @@ -472,8 +467,7 @@ adj_ok (struct thread *thread) OSPF6_NEIGHBOR_EVENT_ADJ_OK); ospf6_lsdb_remove_all (on->summary_list); ospf6_lsdb_remove_all (on->request_list); - for (lsa = ospf6_lsdb_head (on->retrans_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->retrans_list, lsa)) { ospf6_decrement_retrans_count (lsa); ospf6_lsdb_remove (lsa, on->retrans_list); @@ -506,8 +500,7 @@ seqnumber_mismatch (struct thread *thread) ospf6_lsdb_remove_all (on->summary_list); ospf6_lsdb_remove_all (on->request_list); - for (lsa = ospf6_lsdb_head (on->retrans_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->retrans_list, lsa)) { ospf6_decrement_retrans_count (lsa); ospf6_lsdb_remove (lsa, on->retrans_list); @@ -545,8 +538,7 @@ bad_lsreq (struct thread *thread) ospf6_lsdb_remove_all (on->summary_list); ospf6_lsdb_remove_all (on->request_list); - for (lsa = ospf6_lsdb_head (on->retrans_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->retrans_list, lsa)) { ospf6_decrement_retrans_count (lsa); ospf6_lsdb_remove (lsa, on->retrans_list); @@ -582,8 +574,7 @@ oneway_received (struct thread *thread) ospf6_lsdb_remove_all (on->summary_list); ospf6_lsdb_remove_all (on->request_list); - for (lsa = ospf6_lsdb_head (on->retrans_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->retrans_list, lsa)) { ospf6_decrement_retrans_count (lsa); ospf6_lsdb_remove (lsa, on->retrans_list); @@ -756,20 +747,17 @@ ospf6_neighbor_show_detail (struct vty *vty, struct ospf6_neighbor *on) vty_out (vty, " Summary-List: %d LSAs%s", on->summary_list->count, VNL); - for (lsa = ospf6_lsdb_head (on->summary_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->summary_list, lsa)) vty_out (vty, " %s%s", lsa->name, VNL); vty_out (vty, " Request-List: %d LSAs%s", on->request_list->count, VNL); - for (lsa = ospf6_lsdb_head (on->request_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->request_list, lsa)) vty_out (vty, " %s%s", lsa->name, VNL); vty_out (vty, " Retrans-List: %d LSAs%s", on->retrans_list->count, VNL); - for (lsa = ospf6_lsdb_head (on->retrans_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->retrans_list, lsa)) vty_out (vty, " %s%s", lsa->name, VNL); timerclear (&res); @@ -780,8 +768,7 @@ ospf6_neighbor_show_detail (struct vty *vty, struct ospf6_neighbor *on) on->dbdesc_list->count, duration, (on->thread_send_dbdesc ? "on" : "off"), VNL); - for (lsa = ospf6_lsdb_head (on->dbdesc_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->dbdesc_list, lsa)) vty_out (vty, " %s%s", lsa->name, VNL); timerclear (&res); @@ -792,8 +779,7 @@ ospf6_neighbor_show_detail (struct vty *vty, struct ospf6_neighbor *on) on->request_list->count, duration, (on->thread_send_lsreq ? "on" : "off"), VNL); - for (lsa = ospf6_lsdb_head (on->request_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->request_list, lsa)) vty_out (vty, " %s%s", lsa->name, VNL); timerclear (&res); @@ -804,8 +790,7 @@ ospf6_neighbor_show_detail (struct vty *vty, struct ospf6_neighbor *on) on->lsupdate_list->count, duration, (on->thread_send_lsupdate ? "on" : "off"), VNL); - for (lsa = ospf6_lsdb_head (on->lsupdate_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->lsupdate_list, lsa)) vty_out (vty, " %s%s", lsa->name, VNL); timerclear (&res); @@ -816,8 +801,7 @@ ospf6_neighbor_show_detail (struct vty *vty, struct ospf6_neighbor *on) on->lsack_list->count, duration, (on->thread_send_lsack ? "on" : "off"), VNL); - for (lsa = ospf6_lsdb_head (on->lsack_list); lsa; - lsa = ospf6_lsdb_next (lsa)) + for (ALL_LSDB(on->lsack_list, lsa)) vty_out (vty, " %s%s", lsa->name, VNL); ospf6_bfd_show_info(vty, on->bfd_info, 0); diff --git a/ospf6d/ospf6_snmp.c b/ospf6d/ospf6_snmp.c index b30d3efb06..a51fcca734 100644 --- a/ospf6d/ospf6_snmp.c +++ b/ospf6d/ospf6_snmp.c @@ -460,9 +460,8 @@ ospfv3GeneralGroup (struct variable *v, oid *name, size_t *length, case OSPFv3ASSCOPELSACHECKSUMSUM: if (ospf6) { - for (sum = 0, lsa = ospf6_lsdb_head (ospf6->lsdb); - lsa; - lsa = ospf6_lsdb_next (lsa)) + sum = 0; + for (ALL_LSDB(ospf6->lsdb, lsa)) sum += ntohs (lsa->header->checksum); return SNMP_INTEGER (sum); } @@ -474,11 +473,8 @@ ospfv3GeneralGroup (struct variable *v, oid *name, size_t *length, case OSPFv3EXTLSACOUNT: if (ospf6) { - for (count = 0, lsa = ospf6_lsdb_type_head (htons (OSPF6_LSTYPE_AS_EXTERNAL), - ospf6->lsdb); - lsa; - lsa = ospf6_lsdb_type_next (htons (OSPF6_LSTYPE_AS_EXTERNAL), - lsa)) + count = 0; + for (ALL_LSDB_TYPED(ospf6->lsdb, htons (OSPF6_LSTYPE_AS_EXTERNAL), lsa)) count += 1; return SNMP_INTEGER (count); } @@ -590,9 +586,8 @@ ospfv3AreaEntry (struct variable *v, oid *name, size_t *length, case OSPFv3AREASCOPELSACOUNT: return SNMP_INTEGER (area->lsdb->count); case OSPFv3AREASCOPELSACKSUMSUM: - for (sum = 0, lsa = ospf6_lsdb_head (area->lsdb); - lsa; - lsa = ospf6_lsdb_next (lsa)) + sum = 0; + for (ALL_LSDB(area->lsdb, lsa)) sum += ntohs (lsa->header->checksum); return SNMP_INTEGER (sum); case OSPFv3AREASUMMARY: @@ -962,9 +957,8 @@ ospfv3IfEntry (struct variable *v, oid *name, size_t *length, case OSPFv3IFLINKSCOPELSACOUNT: return SNMP_INTEGER (oi->lsdb->count); case OSPFv3IFLINKLSACKSUMSUM: - for (sum = 0, lsa = ospf6_lsdb_head (oi->lsdb); - lsa; - lsa = ospf6_lsdb_next (lsa)) + sum = 0; + for (ALL_LSDB(oi->lsdb, lsa)) sum += ntohs (lsa->header->checksum); return SNMP_INTEGER (sum); case OSPFv3IFDEMANDNBRPROBE: diff --git a/ospf6d/ospf6_spf.c b/ospf6d/ospf6_spf.c index 0b8a5e4767..93f46951b9 100644 --- a/ospf6d/ospf6_spf.c +++ b/ospf6d/ospf6_spf.c @@ -296,8 +296,7 @@ ospf6_nexthop_calc (struct ospf6_vertex *w, struct ospf6_vertex *v, ROUTER_LSDESC_GET_NBR_ROUTERID (lsdesc)); i = 0; - for (lsa = ospf6_lsdb_type_router_head (type, adv_router, oi->lsdb); lsa; - lsa = ospf6_lsdb_type_router_next (type, adv_router, lsa)) + for (ALL_LSDB_TYPED_ADVRTR(oi->lsdb, type, adv_router, lsa)) { if (VERTEX_IS_TYPE (ROUTER, v) && htonl (ROUTER_LSDESC_GET_NBR_IFID (lsdesc)) != lsa->header->id) From 2e8a2df1fbd4c1d7ee7035dff6484ec5d093c5f1 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 11 Jul 2017 14:53:31 +0200 Subject: [PATCH 06/10] tests: fix pytest API "surprise" in skipping tests pytest.mark.skipif apparently iterates through a class's methods, applying itself onto the various methods. Now, since we're deriving from a parent class, the method is actually the same object inherited from the parent, so the decorator will apply itself on the parent's testrunning method (test_refout). The result is that any TestRefout tests after "test_commands.py" will be skipped... This only became apparent after adding ospf6d/test_lsdb.py; before, test_commands.py was the last test in the list so it didn't matter... Signed-off-by: David Lamparter --- tests/lib/cli/test_commands.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/lib/cli/test_commands.py b/tests/lib/cli/test_commands.py index 85e34fa15b..bda0bbac44 100644 --- a/tests/lib/cli/test_commands.py +++ b/tests/lib/cli/test_commands.py @@ -2,7 +2,10 @@ import frrtest import pytest import os -@pytest.mark.skipif('QUAGGA_TEST_COMMANDS' not in os.environ, - reason='QUAGGA_TEST_COMMANDS not set') class TestCommands(frrtest.TestRefOut): program = './test_commands' + + @pytest.mark.skipif('QUAGGA_TEST_COMMANDS' not in os.environ, + reason='QUAGGA_TEST_COMMANDS not set') + def test_refout(self): + return super(TestCommands, self).test_refout(self) From f1c73d149564889c68fd3367fa767344090df0d2 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sat, 8 Jul 2017 15:30:34 +0200 Subject: [PATCH 07/10] tests: ospf6d: basic LSDB tests Needed these while rewriting LSDB iteration. NB: this commit fails because of a bug in ospf_lsdb_get_next, which will SEGV when the LSDB is actually empty. Whooo... (this is fixed in the following commits.) Signed-off-by: David Lamparter --- tests/.gitignore | 1 + tests/Makefile.am | 23 +++- tests/ospf6d/test_lsdb.c | 253 ++++++++++++++++++++++++++++++++++ tests/ospf6d/test_lsdb.in | 72 ++++++++++ tests/ospf6d/test_lsdb.py | 4 + tests/ospf6d/test_lsdb.refout | 192 ++++++++++++++++++++++++++ 6 files changed, 543 insertions(+), 2 deletions(-) create mode 100644 tests/ospf6d/test_lsdb.c create mode 100644 tests/ospf6d/test_lsdb.in create mode 100644 tests/ospf6d/test_lsdb.py create mode 100644 tests/ospf6d/test_lsdb.refout diff --git a/tests/.gitignore b/tests/.gitignore index 5279016b92..6d54ae155b 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -44,3 +44,4 @@ __pycache__ /lib/test_timer_correctness /lib/test_timer_performance /lib/test_ttable +/ospf6d/test_lsdb diff --git a/tests/Makefile.am b/tests/Makefile.am index 559d769702..c10c3def68 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -24,6 +24,14 @@ else TESTS_BGPD = endif +if OSPF6D +TESTS_OSPF6D = \ + ospf6d/test_lsdb \ + # end +else +TESTS_OSPF6D = +endif + if ENABLE_BGP_VNC BGP_VNC_RFP_LIB=@top_builddir@/$(LIBRFP)/librfp.a else @@ -31,6 +39,7 @@ BGP_VNC_RFP_LIB = endif lib/cli/test_cli.o: lib/cli/test_cli_clippy.c +ospf6d/test_lsdb.o: ospf6d/test_lsdb_clippy.c check_PROGRAMS = \ lib/test_buffer \ @@ -51,7 +60,9 @@ check_PROGRAMS = \ lib/test_ttable \ lib/cli/test_cli \ lib/cli/test_commands \ - $(TESTS_BGPD) + $(TESTS_BGPD) \ + $(TESTS_OSPF6D) \ + # end ../vtysh/vtysh_cmd.c: $(MAKE) -C ../vtysh vtysh_cmd.c @@ -100,8 +111,11 @@ bgpd_test_ecommunity_SOURCES = bgpd/test_ecommunity.c bgpd_test_mp_attr_SOURCES = bgpd/test_mp_attr.c bgpd_test_mpath_SOURCES = bgpd/test_mpath.c +ospf6d_test_lsdb_SOURCES = ospf6d/test_lsdb.c lib/cli/common_cli.c + ALL_TESTS_LDADD = ../lib/libfrr.la @LIBCAP@ BGP_TEST_LDADD = ../bgpd/libbgp.a $(BGP_VNC_RFP_LIB) $(ALL_TESTS_LDADD) -lm +OSPF6_TEST_LDADD = ../ospf6d/libospf6.a $(ALL_TESTS_LDADD) lib_test_buffer_LDADD = $(ALL_TESTS_LDADD) lib_test_checksum_LDADD = $(ALL_TESTS_LDADD) @@ -126,6 +140,7 @@ bgpd_test_capability_LDADD = $(BGP_TEST_LDADD) bgpd_test_ecommunity_LDADD = $(BGP_TEST_LDADD) bgpd_test_mp_attr_LDADD = $(BGP_TEST_LDADD) bgpd_test_mpath_LDADD = $(BGP_TEST_LDADD) +ospf6d_test_lsdb_LDADD = $(OSPF6_TEST_LDADD) EXTRA_DIST = \ runtests.py \ @@ -148,7 +163,11 @@ EXTRA_DIST = \ lib/test_stream.refout \ lib/test_table.py \ lib/test_timer_correctness.py \ - lib/test_ttable.py + lib/test_ttable.py \ + ospf6d/test_lsdb.py \ + ospf6d/test_lsdb.in \ + ospf6d/test_lsdb.refout \ + # end .PHONY: tests.xml tests.xml: $(check_PROGRAMS) diff --git a/tests/ospf6d/test_lsdb.c b/tests/ospf6d/test_lsdb.c new file mode 100644 index 0000000000..29d7b510ef --- /dev/null +++ b/tests/ospf6d/test_lsdb.c @@ -0,0 +1,253 @@ +/* + * CLI/command dummy handling tester + * + * Copyright (C) 2015 by David Lamparter, + * for Open Source Routing / NetDEF, Inc. + * + * Quagga is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2, or (at your option) any + * later version. + * + * Quagga is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; see the file COPYING; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include + +#include "prefix.h" +#include "vector.h" +#include "vty.h" + +#include "ospf6d/ospf6_lsa.h" +#include "ospf6d/ospf6_lsdb.h" + +#include "tests/lib/cli/common_cli.h" +#include "tests/ospf6d/test_lsdb_clippy.c" + +static struct ospf6_lsdb *lsdb; + +static struct ospf6_lsa **lsas = NULL; +static size_t lsa_count = 0; + +static void lsa_check_resize(size_t len) +{ + if (lsa_count >= len) + return; + lsas = realloc(lsas, len * sizeof(lsas[0])); + memset(lsas + lsa_count, 0, sizeof(lsas[0]) * (len - lsa_count)); + + lsa_count = len; +} + +DEFPY(lsa_set, lsa_set_cmd, + "lsa set (0-999999)$idx {type (0-65535)|id A.B.C.D|adv A.B.C.D}", + "LSA\n" + "set\n" + "LSA index in array\n" + "OSPF6 type code\n" + "OSPF6 type code\n" + "LS-ID\n" + "LS-ID\n" + "Advertising router\n" + "Advertising router\n") +{ + struct ospf6_lsa_header hdr; + memset(&hdr, 0, sizeof(hdr)); + hdr.type = htons(type); + hdr.id = id.s_addr; + hdr.adv_router = adv.s_addr; + + lsa_check_resize(idx + 1); + if (lsas[idx]) + ospf6_lsa_unlock(lsas[idx]); + lsas[idx] = ospf6_lsa_create_headeronly(&hdr); + ospf6_lsa_lock(lsas[idx]); + return CMD_SUCCESS; +} + +DEFPY(lsa_drop, lsa_drop_cmd, + "lsa drop (0-999999)$idx", + "LSA\n" + "drop reference\n" + "LSA index in array\n") +{ + if ((size_t)idx >= lsa_count) + return CMD_SUCCESS; + if (lsas[idx]->lock != 1) + vty_outln(vty, "refcount at %u", lsas[idx]->lock); + ospf6_lsa_unlock(lsas[idx]); + lsas[idx] = NULL; + return CMD_SUCCESS; +} + + +DEFPY(lsdb_add, lsdb_add_cmd, + "lsdb add (0-999999)$idx", + "LSDB\n" + "insert LSA into LSDB\n" + "LSA index in array\n") +{ + ospf6_lsdb_add(lsas[idx], lsdb); + return CMD_SUCCESS; +} + +DEFPY(lsdb_remove, lsdb_remove_cmd, + "lsdb remove (0-999999)$idx", + "LSDB\n" + "remove LSA from LSDB\n" + "LSA index in array\n") +{ + ospf6_lsdb_remove(lsas[idx], lsdb); + return CMD_SUCCESS; +} + +static void lsa_show_oneline(struct vty *vty, struct ospf6_lsa *lsa) +{ + char adv_router[64], id[64]; + + if (!lsa) { + vty_outln(vty, "lsa = NULL"); + return; + } + inet_ntop(AF_INET, &lsa->header->id, + id, sizeof (id)); + inet_ntop(AF_INET, &lsa->header->adv_router, + adv_router, sizeof (adv_router)); + vty_outln(vty, "type %u adv %s id %s", + ntohs(lsa->header->type), adv_router, id); +} + +DEFPY(lsdb_walk, lsdb_walk_cmd, + "lsdb walk", + "LSDB\n" + "walk entries\n") +{ + struct ospf6_lsa *lsa; + unsigned cnt = 0; + for (ALL_LSDB(lsdb, lsa)) { + lsa_show_oneline(vty, lsa); + cnt++; + } + vty_outln(vty, "%u entries.", cnt); + return CMD_SUCCESS; +} + +DEFPY(lsdb_walk_type, lsdb_walk_type_cmd, + "lsdb walk type (0-65535)", + "LSDB\n" + "walk entries\n" + "entry type\n" + "entry type\n") +{ + struct ospf6_lsa *lsa; + unsigned cnt = 0; + type = htons(type); + for (ALL_LSDB_TYPED(lsdb, type, lsa)) { + lsa_show_oneline(vty, lsa); + cnt++; + } + vty_outln(vty, "%u entries.", cnt); + return CMD_SUCCESS; +} + +DEFPY(lsdb_walk_type_adv, lsdb_walk_type_adv_cmd, + "lsdb walk type (0-65535) adv A.B.C.D", + "LSDB\n" + "walk entries\n" + "entry type\n" + "entry type\n" + "advertising router ID\n" + "advertising router ID\n") +{ + struct ospf6_lsa *lsa; + unsigned cnt = 0; + type = htons(type); + for (ALL_LSDB_TYPED_ADVRTR(lsdb, type, adv.s_addr, lsa)) { + lsa_show_oneline(vty, lsa); + cnt++; + } + vty_outln(vty, "%u entries.", cnt); + return CMD_SUCCESS; +} + +DEFPY(lsdb_get, lsdb_get_cmd, + "lsdb type (0-65535) adv A.B.C.D id A.B.C.D", + "LSDB\n" + "get entry's successor\n" + "entry type\n" + "entry type\n" + "advertising router ID\n" + "advertising router ID\n" + "LS-ID\n" + "LS-ID\n") +{ + struct ospf6_lsa *lsa; + type = htons(type); + if (!strcmp(argv[1]->text, "get-next")) + lsa = ospf6_lsdb_lookup_next(type, id.s_addr, adv.s_addr, lsdb); + else + lsa = ospf6_lsdb_lookup(type, id.s_addr, adv.s_addr, lsdb); + lsa_show_oneline(vty, lsa); + return CMD_SUCCESS; +} + +DEFPY(lsa_refcounts, lsa_refcounts_cmd, + "lsa refcounts", + "LSA\n" + "show reference counts\n") +{ + for (size_t i = 0; i < lsa_count; i++) + if (lsas[i]) + vty_outln(vty, "[%zu] %u", i, lsas[i]->lock); + return CMD_SUCCESS; +} + +DEFPY(lsdb_create, lsdb_create_cmd, + "lsdb create", + "LSDB\n" + "create LSDB\n") +{ + if (lsdb) + ospf6_lsdb_delete(lsdb); + lsdb = ospf6_lsdb_create(NULL); + return CMD_SUCCESS; +} + +DEFPY(lsdb_delete, lsdb_delete_cmd, + "lsdb delete", + "LSDB\n" + "delete LSDB\n") +{ + ospf6_lsdb_delete(lsdb); + lsdb = NULL; + return CMD_SUCCESS; +} + + +struct zebra_privs_t ospf6d_privs; + +void test_init(int argc, char **argv) +{ + ospf6_lsa_init(); + + install_element(ENABLE_NODE, &lsa_set_cmd); + install_element(ENABLE_NODE, &lsa_refcounts_cmd); + install_element(ENABLE_NODE, &lsa_drop_cmd); + + install_element(ENABLE_NODE, &lsdb_create_cmd); + install_element(ENABLE_NODE, &lsdb_delete_cmd); + + install_element(ENABLE_NODE, &lsdb_add_cmd); + install_element(ENABLE_NODE, &lsdb_remove_cmd); + install_element(ENABLE_NODE, &lsdb_walk_cmd); + install_element(ENABLE_NODE, &lsdb_walk_type_cmd); + install_element(ENABLE_NODE, &lsdb_walk_type_adv_cmd); + install_element(ENABLE_NODE, &lsdb_get_cmd); +} diff --git a/tests/ospf6d/test_lsdb.in b/tests/ospf6d/test_lsdb.in new file mode 100644 index 0000000000..0475f3dc5a --- /dev/null +++ b/tests/ospf6d/test_lsdb.in @@ -0,0 +1,72 @@ +lsa set 0 type 1 adv 1.2.3.4 id 0.0.0.1 +lsa set 1 type 1 adv 1.2.3.4 id 0.0.0.2 +lsa set 2 type 2 adv 1.2.3.4 id 0.0.0.3 +lsa set 3 type 2 adv 128.2.3.4 id 0.0.0.4 +lsa set 4 type 2 adv 128.2.3.4 id 0.0.0.5 +lsa set 5 type 3 adv 0.0.0.1 id 0.0.0.6 +lsa refcounts + +lsdb create + +lsdb walk +lsdb walk type 1 +lsdb walk type 2 +lsdb get type 1 adv 1.2.3.4 id 0.0.0.2 +lsdb get-next type 1 adv 1.2.3.4 id 0.0.0.2 +lsa refcounts + +lsdb add 0 +lsdb add 1 +lsa refcounts + +lsdb walk +lsdb walk type 1 +lsdb walk type 2 +lsdb get type 1 adv 1.2.3.4 id 0.0.0.2 +lsdb get-next type 1 adv 1.2.3.4 id 0.0.0.2 +lsa refcounts + +lsdb remove 0 +lsdb add 2 +lsdb add 3 +lsdb add 4 +lsa refcounts + +lsdb walk +lsdb walk type 1 +lsdb walk type 2 +lsdb get type 1 adv 1.2.3.4 id 0.0.0.2 +lsdb get-next type 1 adv 1.2.3.4 id 0.0.0.2 +lsa refcounts + +lsdb add 5 +lsa refcounts + +lsdb walk +lsdb walk type 1 +lsdb walk type 2 +lsdb get type 1 adv 1.2.3.4 id 0.0.0.2 +lsdb get-next type 1 adv 1.2.3.4 id 0.0.0.2 +lsa refcounts + +lsdb remove 1 +lsdb remove 5 +lsa refcounts + +lsdb walk +lsdb walk type 1 +lsdb walk type 2 +lsdb get type 1 adv 1.2.3.4 id 0.0.0.2 +lsdb get-next type 1 adv 1.2.3.4 id 0.0.0.2 +lsa refcounts + +lsdb delete + +lsa refcounts +lsa drop 0 +lsa drop 1 +lsa drop 2 +lsa drop 3 +lsa drop 4 +lsa drop 5 + diff --git a/tests/ospf6d/test_lsdb.py b/tests/ospf6d/test_lsdb.py new file mode 100644 index 0000000000..6a94395113 --- /dev/null +++ b/tests/ospf6d/test_lsdb.py @@ -0,0 +1,4 @@ +import frrtest + +class TestLSDB(frrtest.TestRefOut): + program = './test_lsdb' diff --git a/tests/ospf6d/test_lsdb.refout b/tests/ospf6d/test_lsdb.refout new file mode 100644 index 0000000000..8b17652266 --- /dev/null +++ b/tests/ospf6d/test_lsdb.refout @@ -0,0 +1,192 @@ +test# lsa set 0 type 1 adv 1.2.3.4 id 0.0.0.1 +test# lsa set 1 type 1 adv 1.2.3.4 id 0.0.0.2 +test# lsa set 2 type 2 adv 1.2.3.4 id 0.0.0.3 +test# lsa set 3 type 2 adv 128.2.3.4 id 0.0.0.4 +test# lsa set 4 type 2 adv 128.2.3.4 id 0.0.0.5 +test# lsa set 5 type 3 adv 0.0.0.1 id 0.0.0.6 +test# lsa refcounts +[0] 1 +[1] 1 +[2] 1 +[3] 1 +[4] 1 +[5] 1 +test# +test# lsdb create +test# +test# lsdb walk +0 entries. +test# lsdb walk type 1 +0 entries. +test# lsdb walk type 2 +0 entries. +test# lsdb get type 1 adv 1.2.3.4 id 0.0.0.2 +lsa = NULL +test# lsdb get-next type 1 adv 1.2.3.4 id 0.0.0.2 +lsa = NULL +test# lsa refcounts +[0] 1 +[1] 1 +[2] 1 +[3] 1 +[4] 1 +[5] 1 +test# +test# lsdb add 0 +test# lsdb add 1 +test# lsa refcounts +[0] 2 +[1] 2 +[2] 1 +[3] 1 +[4] 1 +[5] 1 +test# +test# lsdb walk +type 1 adv 1.2.3.4 id 0.0.0.1 +type 1 adv 1.2.3.4 id 0.0.0.2 +2 entries. +test# lsdb walk type 1 +type 1 adv 1.2.3.4 id 0.0.0.1 +type 1 adv 1.2.3.4 id 0.0.0.2 +2 entries. +test# lsdb walk type 2 +0 entries. +test# lsdb get type 1 adv 1.2.3.4 id 0.0.0.2 +type 1 adv 1.2.3.4 id 0.0.0.2 +test# lsdb get-next type 1 adv 1.2.3.4 id 0.0.0.2 +lsa = NULL +test# lsa refcounts +[0] 2 +[1] 2 +[2] 1 +[3] 1 +[4] 1 +[5] 1 +test# +test# lsdb remove 0 +test# lsdb add 2 +test# lsdb add 3 +test# lsdb add 4 +test# lsa refcounts +[0] 1 +[1] 2 +[2] 2 +[3] 2 +[4] 2 +[5] 1 +test# +test# lsdb walk +type 1 adv 1.2.3.4 id 0.0.0.2 +type 2 adv 1.2.3.4 id 0.0.0.3 +type 2 adv 128.2.3.4 id 0.0.0.4 +type 2 adv 128.2.3.4 id 0.0.0.5 +4 entries. +test# lsdb walk type 1 +type 1 adv 1.2.3.4 id 0.0.0.2 +1 entries. +test# lsdb walk type 2 +type 2 adv 1.2.3.4 id 0.0.0.3 +type 2 adv 128.2.3.4 id 0.0.0.4 +type 2 adv 128.2.3.4 id 0.0.0.5 +3 entries. +test# lsdb get type 1 adv 1.2.3.4 id 0.0.0.2 +type 1 adv 1.2.3.4 id 0.0.0.2 +test# lsdb get-next type 1 adv 1.2.3.4 id 0.0.0.2 +type 2 adv 1.2.3.4 id 0.0.0.3 +test# lsa refcounts +[0] 1 +[1] 2 +[2] 2 +[3] 2 +[4] 2 +[5] 1 +test# +test# lsdb add 5 +test# lsa refcounts +[0] 1 +[1] 2 +[2] 2 +[3] 2 +[4] 2 +[5] 2 +test# +test# lsdb walk +type 1 adv 1.2.3.4 id 0.0.0.2 +type 2 adv 1.2.3.4 id 0.0.0.3 +type 2 adv 128.2.3.4 id 0.0.0.4 +type 2 adv 128.2.3.4 id 0.0.0.5 +type 3 adv 0.0.0.1 id 0.0.0.6 +5 entries. +test# lsdb walk type 1 +type 1 adv 1.2.3.4 id 0.0.0.2 +1 entries. +test# lsdb walk type 2 +type 2 adv 1.2.3.4 id 0.0.0.3 +type 2 adv 128.2.3.4 id 0.0.0.4 +type 2 adv 128.2.3.4 id 0.0.0.5 +3 entries. +test# lsdb get type 1 adv 1.2.3.4 id 0.0.0.2 +type 1 adv 1.2.3.4 id 0.0.0.2 +test# lsdb get-next type 1 adv 1.2.3.4 id 0.0.0.2 +type 2 adv 1.2.3.4 id 0.0.0.3 +test# lsa refcounts +[0] 1 +[1] 2 +[2] 2 +[3] 2 +[4] 2 +[5] 2 +test# +test# lsdb remove 1 +test# lsdb remove 5 +test# lsa refcounts +[0] 1 +[1] 1 +[2] 2 +[3] 2 +[4] 2 +[5] 1 +test# +test# lsdb walk +type 2 adv 1.2.3.4 id 0.0.0.3 +type 2 adv 128.2.3.4 id 0.0.0.4 +type 2 adv 128.2.3.4 id 0.0.0.5 +3 entries. +test# lsdb walk type 1 +0 entries. +test# lsdb walk type 2 +type 2 adv 1.2.3.4 id 0.0.0.3 +type 2 adv 128.2.3.4 id 0.0.0.4 +type 2 adv 128.2.3.4 id 0.0.0.5 +3 entries. +test# lsdb get type 1 adv 1.2.3.4 id 0.0.0.2 +lsa = NULL +test# lsdb get-next type 1 adv 1.2.3.4 id 0.0.0.2 +type 2 adv 1.2.3.4 id 0.0.0.3 +test# lsa refcounts +[0] 1 +[1] 1 +[2] 2 +[3] 2 +[4] 2 +[5] 1 +test# +test# lsdb delete +test# +test# lsa refcounts +[0] 1 +[1] 1 +[2] 1 +[3] 1 +[4] 1 +[5] 1 +test# lsa drop 0 +test# lsa drop 1 +test# lsa drop 2 +test# lsa drop 3 +test# lsa drop 4 +test# lsa drop 5 +test# +test# +end. From 954306f70c189a124cac08438c7cc8e625957d2f Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 6 Jul 2017 15:02:31 +0200 Subject: [PATCH 08/10] ospf6d: rewrite LSDB iteration rip out this pile of open-coded goo and replace it with uses of the API that table.h provides. Signed-off-by: David Lamparter --- ospf6d/ospf6_lsdb.c | 201 +++++++++++++------------------------------- ospf6d/ospf6_lsdb.h | 41 ++++----- 2 files changed, 81 insertions(+), 161 deletions(-) diff --git a/ospf6d/ospf6_lsdb.c b/ospf6d/ospf6_lsdb.c index 6879d71a16..26b8724dea 100644 --- a/ospf6d/ospf6_lsdb.c +++ b/ospf6d/ospf6_lsdb.c @@ -64,7 +64,7 @@ ospf6_lsdb_delete (struct ospf6_lsdb *lsdb) } static void -ospf6_lsdb_set_key (struct prefix_ipv6 *key, void *value, int len) +ospf6_lsdb_set_key (struct prefix_ipv6 *key, const void *value, int len) { assert (key->prefixlen % 8 == 0); @@ -279,152 +279,77 @@ ospf6_lsdb_lookup_next (u_int16_t type, u_int32_t id, u_int32_t adv_router, return (struct ospf6_lsa *) node->info; } -/* Iteration function */ -struct ospf6_lsa * -ospf6_lsdb_head (struct ospf6_lsdb *lsdb) +const struct route_node * +ospf6_lsdb_head (struct ospf6_lsdb *lsdb, + int argmode, uint16_t type, uint32_t adv_router, + struct ospf6_lsa **lsa) { - struct route_node *node; + struct route_node *node, *end; - node = route_top (lsdb->table); - if (node == NULL) + *lsa = NULL; + + if (argmode > 0) + { + struct prefix_ipv6 key = { .family = AF_INET6, .prefixlen = 0 }; + + ospf6_lsdb_set_key (&key, &type, sizeof (type)); + if (argmode > 1) + ospf6_lsdb_set_key (&key, &adv_router, sizeof (adv_router)); + + node = route_table_get_next (lsdb->table, &key); + if (!node || !prefix_match((struct prefix *)&key, &node->p)) + return NULL; + + for (end = node; + end && end->parent && end->parent->p.prefixlen >= key.prefixlen; + end = end->parent) + ; + } + else + { + node = route_top (lsdb->table); + end = NULL; + } + + while (node && !node->info) + node = route_next_until(node, end); + + if (!node) return NULL; + if (!node->info) + { + route_unlock_node(node); + return NULL; + } - /* skip to the existing lsdb entry */ - while (node && node->info == NULL) - node = route_next (node); - if (node == NULL) - return NULL; + *lsa = node->info; + ospf6_lsa_lock (*lsa); - if (node->info) - ospf6_lsa_lock ((struct ospf6_lsa *) node->info); - return (struct ospf6_lsa *) node->info; + return end; } struct ospf6_lsa * -ospf6_lsdb_next (struct ospf6_lsa *lsa) +ospf6_lsdb_next (const struct route_node *iterend, + struct ospf6_lsa *lsa) { struct route_node *node = lsa->rn; - struct ospf6_lsa *next = NULL; - do { - node = route_next (node); - } while (node && node->info == NULL); + ospf6_lsa_unlock(lsa); - if ((node != NULL) && (node->info != NULL)) + do + node = route_next_until(node, iterend); + while (node && !node->info); + + if (node && node->info) { - next = node->info; + struct ospf6_lsa *next = node->info; ospf6_lsa_lock (next); + return next; } - ospf6_lsa_unlock (lsa); - return next; -} - -struct ospf6_lsa * -ospf6_lsdb_type_router_head (u_int16_t type, u_int32_t adv_router, - struct ospf6_lsdb *lsdb) -{ - struct route_node *node; - struct prefix_ipv6 key; - struct ospf6_lsa *lsa; - - memset (&key, 0, sizeof (key)); - ospf6_lsdb_set_key (&key, &type, sizeof (type)); - ospf6_lsdb_set_key (&key, &adv_router, sizeof (adv_router)); - - node = lsdb->table->top; - - /* Walk down tree. */ - while (node && node->p.prefixlen <= key.prefixlen && - prefix_match (&node->p, (struct prefix *) &key)) - node = node->link[prefix6_bit(&key.prefix, node->p.prefixlen)]; - if (node) - route_lock_node (node); - while (node && node->info == NULL) - node = route_next (node); - - if (node == NULL) - return NULL; - - if (! prefix_match ((struct prefix *) &key, &node->p)) - return NULL; - - lsa = node->info; - ospf6_lsa_lock (lsa); - - return lsa; -} - -struct ospf6_lsa * -ospf6_lsdb_type_router_next (u_int16_t type, u_int32_t adv_router, - struct ospf6_lsa *lsa) -{ - struct ospf6_lsa *next = ospf6_lsdb_next(lsa); - - if (next) - { - if (next->header->type != type || - next->header->adv_router != adv_router) - { - route_unlock_node (next->rn); - ospf6_lsa_unlock (next); - next = NULL; - } - } - - return next; -} - -struct ospf6_lsa * -ospf6_lsdb_type_head (u_int16_t type, struct ospf6_lsdb *lsdb) -{ - struct route_node *node; - struct prefix_ipv6 key; - struct ospf6_lsa *lsa; - - memset (&key, 0, sizeof (key)); - ospf6_lsdb_set_key (&key, &type, sizeof (type)); - - /* Walk down tree. */ - node = lsdb->table->top; - while (node && node->p.prefixlen <= key.prefixlen && - prefix_match (&node->p, (struct prefix *) &key)) - node = node->link[prefix6_bit(&key.prefix, node->p.prefixlen)]; - - if (node) - route_lock_node (node); - while (node && node->info == NULL) - node = route_next (node); - - if (node == NULL) - return NULL; - - if (! prefix_match ((struct prefix *) &key, &node->p)) - return NULL; - - lsa = node->info; - ospf6_lsa_lock (lsa); - - return lsa; -} - -struct ospf6_lsa * -ospf6_lsdb_type_next (u_int16_t type, struct ospf6_lsa *lsa) -{ - struct ospf6_lsa *next = ospf6_lsdb_next (lsa); - - if (next) - { - if (next->header->type != type) - { - route_unlock_node (next->rn); - ospf6_lsa_unlock (next); - next = NULL; - } - } - - return next; + route_unlock_node (node); + return NULL; } void @@ -492,6 +417,7 @@ ospf6_lsdb_show (struct vty *vty, enum ospf_lsdb_show_level level, struct ospf6_lsdb *lsdb) { struct ospf6_lsa *lsa; + const struct route_node *end = NULL; void (*showfunc) (struct vty *, struct ospf6_lsa *) = NULL; switch (level) @@ -526,24 +452,15 @@ ospf6_lsdb_show (struct vty *vty, enum ospf_lsdb_show_level level, if (level == OSPF6_LSDB_SHOW_LEVEL_NORMAL) ospf6_lsa_show_summary_header (vty); - if (type && adv_router) - lsa = ospf6_lsdb_type_router_head (*type, *adv_router, lsdb); - else if (type) - lsa = ospf6_lsdb_type_head (*type, lsdb); - else - lsa = ospf6_lsdb_head (lsdb); + end = ospf6_lsdb_head(lsdb, !!type + !!(type && adv_router), + *type, *adv_router, &lsa); while (lsa) { if ((! adv_router || lsa->header->adv_router == *adv_router) && (! id || lsa->header->id == *id)) (*showfunc) (vty, lsa); - if (type && adv_router) - lsa = ospf6_lsdb_type_router_next (*type, *adv_router, lsa); - else if (type) - lsa = ospf6_lsdb_type_next (*type, lsa); - else - lsa = ospf6_lsdb_next (lsa); + lsa = ospf6_lsdb_next (end, lsa); } } diff --git a/ospf6d/ospf6_lsdb.h b/ospf6d/ospf6_lsdb.h index 529fbefba6..0eb5322b41 100644 --- a/ospf6d/ospf6_lsdb.h +++ b/ospf6d/ospf6_lsdb.h @@ -48,29 +48,32 @@ extern struct ospf6_lsa *ospf6_lsdb_lookup_next (u_int16_t type, u_int32_t id, extern void ospf6_lsdb_add (struct ospf6_lsa *lsa, struct ospf6_lsdb *lsdb); extern void ospf6_lsdb_remove (struct ospf6_lsa *lsa, struct ospf6_lsdb *lsdb); -extern struct ospf6_lsa *ospf6_lsdb_head (struct ospf6_lsdb *lsdb); -extern struct ospf6_lsa *ospf6_lsdb_next (struct ospf6_lsa *lsa); -#define ALL_LSDB(lsdb, lsa) \ - lsa = ospf6_lsdb_head(lsdb); lsa; \ - lsa = ospf6_lsdb_next(lsa) +extern const struct route_node *ospf6_lsdb_head ( + struct ospf6_lsdb *lsdb, + int argmode, + uint16_t type, + uint32_t adv_router, + struct ospf6_lsa **lsa); +extern struct ospf6_lsa *ospf6_lsdb_next (const struct route_node *iterend, + struct ospf6_lsa *lsa); -extern struct ospf6_lsa *ospf6_lsdb_type_router_head (u_int16_t type, - u_int32_t adv_router, - struct ospf6_lsdb *lsdb); -extern struct ospf6_lsa *ospf6_lsdb_type_router_next (u_int16_t type, - u_int32_t adv_router, - struct ospf6_lsa *lsa); #define ALL_LSDB_TYPED_ADVRTR(lsdb, type, adv_router, lsa) \ - lsa = ospf6_lsdb_type_router_head(type, adv_router, lsdb); lsa; \ - lsa = ospf6_lsdb_type_router_next(type, adv_router, lsa) + const struct route_node *iterend = \ + ospf6_lsdb_head(lsdb, 2, type, adv_router, &lsa); \ + lsa; \ + lsa = ospf6_lsdb_next(iterend, lsa) -extern struct ospf6_lsa *ospf6_lsdb_type_head (u_int16_t type, - struct ospf6_lsdb *lsdb); -extern struct ospf6_lsa *ospf6_lsdb_type_next (u_int16_t type, - struct ospf6_lsa *lsa); #define ALL_LSDB_TYPED(lsdb, type, lsa) \ - lsa = ospf6_lsdb_type_head(type, lsdb); lsa; \ - lsa = ospf6_lsdb_type_next(type, lsa) + const struct route_node *iterend = \ + ospf6_lsdb_head(lsdb, 1, type, 0, &lsa); \ + lsa; \ + lsa = ospf6_lsdb_next(iterend, lsa) + +#define ALL_LSDB(lsdb, lsa) \ + const struct route_node *iterend = \ + ospf6_lsdb_head(lsdb, 0, 0, 0, &lsa); \ + lsa; \ + lsa = ospf6_lsdb_next(iterend, lsa) extern void ospf6_lsdb_remove_all (struct ospf6_lsdb *lsdb); extern void ospf6_lsdb_lsa_unlock (struct ospf6_lsa *lsa); From 98f65ee0b1a147b2852bf22f6d2c08d8563c124e Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 7 Jul 2017 18:26:09 +0200 Subject: [PATCH 09/10] ospf6d: rewrite ospf6_lsdb_lookup_next() Again, replace open-coded table searches with API usage. Signed-off-by: David Lamparter --- ospf6d/ospf6_lsdb.c | 34 ++++------------------------------ 1 file changed, 4 insertions(+), 30 deletions(-) diff --git a/ospf6d/ospf6_lsdb.c b/ospf6d/ospf6_lsdb.c index 26b8724dea..ed01800657 100644 --- a/ospf6d/ospf6_lsdb.c +++ b/ospf6d/ospf6_lsdb.c @@ -221,9 +221,7 @@ ospf6_lsdb_lookup_next (u_int16_t type, u_int32_t id, u_int32_t adv_router, struct ospf6_lsdb *lsdb) { struct route_node *node; - struct route_node *matched = NULL; struct prefix_ipv6 key; - struct prefix *p; if (lsdb == NULL) return NULL; @@ -232,31 +230,14 @@ ospf6_lsdb_lookup_next (u_int16_t type, u_int32_t id, u_int32_t adv_router, ospf6_lsdb_set_key (&key, &type, sizeof (type)); ospf6_lsdb_set_key (&key, &adv_router, sizeof (adv_router)); ospf6_lsdb_set_key (&key, &id, sizeof (id)); - p = (struct prefix *) &key; { char buf[PREFIX2STR_BUFFER]; - prefix2str (p, buf, sizeof (buf)); + prefix2str (&key, buf, sizeof (buf)); zlog_debug ("lsdb_lookup_next: key: %s", buf); } - /* FIXME: need to find a better way here to work without sticking our - * hands in node->link, e.g. route_node_match_maynull() */ - - node = lsdb->table->top; - /* walk down tree. */ - while (node && node->p.prefixlen <= p->prefixlen && - prefix_match (&node->p, p)) - { - matched = node; - node = node->link[prefix_bit(&p->u.prefix, node->p.prefixlen)]; - } - - if (matched) - node = matched; - else - node = lsdb->table->top; - route_lock_node (node); + node = route_table_get_next (lsdb->table, &key); /* skip to real existing entry */ while (node && node->info == NULL) @@ -265,17 +246,10 @@ ospf6_lsdb_lookup_next (u_int16_t type, u_int32_t id, u_int32_t adv_router, if (! node) return NULL; - if (prefix_same (&node->p, p)) - { - node = route_next (node); - while (node && node->info == NULL) - node = route_next (node); - } - - if (! node) + route_unlock_node (node); + if (! node->info) return NULL; - route_unlock_node (node); return (struct ospf6_lsa *) node->info; } From 53a37141f990c8a81e186559ebbeaaad5ddd4a04 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 14 Jul 2017 17:32:22 +0200 Subject: [PATCH 10/10] tests/ospf6d/test_lsdb: remove vty_outln Signed-off-by: David Lamparter --- tests/ospf6d/test_lsdb.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/ospf6d/test_lsdb.c b/tests/ospf6d/test_lsdb.c index 29d7b510ef..efacace7bc 100644 --- a/tests/ospf6d/test_lsdb.c +++ b/tests/ospf6d/test_lsdb.c @@ -81,7 +81,7 @@ DEFPY(lsa_drop, lsa_drop_cmd, if ((size_t)idx >= lsa_count) return CMD_SUCCESS; if (lsas[idx]->lock != 1) - vty_outln(vty, "refcount at %u", lsas[idx]->lock); + vty_out(vty, "refcount at %u\n", lsas[idx]->lock); ospf6_lsa_unlock(lsas[idx]); lsas[idx] = NULL; return CMD_SUCCESS; @@ -113,14 +113,14 @@ static void lsa_show_oneline(struct vty *vty, struct ospf6_lsa *lsa) char adv_router[64], id[64]; if (!lsa) { - vty_outln(vty, "lsa = NULL"); + vty_out(vty, "lsa = NULL\n"); return; } inet_ntop(AF_INET, &lsa->header->id, id, sizeof (id)); inet_ntop(AF_INET, &lsa->header->adv_router, adv_router, sizeof (adv_router)); - vty_outln(vty, "type %u adv %s id %s", + vty_out(vty, "type %u adv %s id %s\n", ntohs(lsa->header->type), adv_router, id); } @@ -135,7 +135,7 @@ DEFPY(lsdb_walk, lsdb_walk_cmd, lsa_show_oneline(vty, lsa); cnt++; } - vty_outln(vty, "%u entries.", cnt); + vty_out(vty, "%u entries.\n", cnt); return CMD_SUCCESS; } @@ -153,7 +153,7 @@ DEFPY(lsdb_walk_type, lsdb_walk_type_cmd, lsa_show_oneline(vty, lsa); cnt++; } - vty_outln(vty, "%u entries.", cnt); + vty_out(vty, "%u entries.\n", cnt); return CMD_SUCCESS; } @@ -173,7 +173,7 @@ DEFPY(lsdb_walk_type_adv, lsdb_walk_type_adv_cmd, lsa_show_oneline(vty, lsa); cnt++; } - vty_outln(vty, "%u entries.", cnt); + vty_out(vty, "%u entries.\n", cnt); return CMD_SUCCESS; } @@ -205,7 +205,7 @@ DEFPY(lsa_refcounts, lsa_refcounts_cmd, { for (size_t i = 0; i < lsa_count; i++) if (lsas[i]) - vty_outln(vty, "[%zu] %u", i, lsas[i]->lock); + vty_out(vty, "[%zu] %u\n", i, lsas[i]->lock); return CMD_SUCCESS; }