From e8b9ad5cdda832ea5ff8dbe21f6078fe15badd33 Mon Sep 17 00:00:00 2001 From: Lou Berger Date: Thu, 2 May 2019 06:54:59 -0400 Subject: [PATCH] Revert "Zebra diet" --- doc/developer/lists.rst | 16 +++---- lib/prefix.c | 2 +- lib/prefix.h | 2 +- lib/table.c | 31 +++++++------ lib/table.h | 6 +-- lib/typesafe.h | 6 +-- .../scripts/cleanup_all.py | 3 +- zebra/rib.h | 43 +------------------ zebra/zebra_rib.c | 7 +-- zebra/zebra_rnh.c | 6 +-- zebra/zebra_rnh.h | 34 +++++++++++++++ zebra/zebra_vrf.c | 2 +- 12 files changed, 74 insertions(+), 84 deletions(-) diff --git a/doc/developer/lists.rst b/doc/developer/lists.rst index 987b3b7f4f..6d60420b2f 100644 --- a/doc/developer/lists.rst +++ b/doc/developer/lists.rst @@ -119,8 +119,6 @@ The common setup pattern will look like this: .. code-block:: c - #include - PREDECL_XXX(Z) struct item { int otherdata; @@ -161,26 +159,26 @@ Common iteration macros The following iteration macros work across all data structures: -.. c:function:: for_each(Z, &head, item) +.. c:function:: for_each(Z, head, item) Equivalent to: .. code-block:: c - for (item = Z_first(&head); item; item = Z_next(&head, item)) + for (item = Z_first(head); item; item = Z_next(head, item)) Note that this will fail if the list is modified while being iterated over. -.. c:function:: for_each_safe(Z, &head, item) +.. c:function:: for_each_safe(Z, head, item) Same as the previous, but the next element is pre-loaded into a "hidden" variable (named ``Z_safe``.) Equivalent to: .. code-block:: c - for (item = Z_first(&head); item; item = next) { - next = Z_next_safe(&head, item); + for (item = Z_first(head); item; item = next) { + next = Z_next_safe(head, item); ... } @@ -191,7 +189,7 @@ The following iteration macros work across all data structures: tables is resized while iterating. This will cause items to be skipped or iterated over twice. -.. c:function:: for_each_from(Z, &head, item, from) +.. c:function:: for_each_from(Z, head, item, from) Iterates over the list, starting at item ``from``. This variant is "safe" as in the previous macro. Equivalent to: @@ -199,7 +197,7 @@ The following iteration macros work across all data structures: .. code-block:: c for (item = from; item; item = from) { - from = Z_next_safe(&head, item); + from = Z_next_safe(head, item); ... } diff --git a/lib/prefix.c b/lib/prefix.c index d2a4c3a432..6b91969218 100644 --- a/lib/prefix.c +++ b/lib/prefix.c @@ -1543,7 +1543,7 @@ char *prefix_mac2str(const struct ethaddr *mac, char *buf, int size) return ptr; } -unsigned prefix_hash_key(const void *pp) +unsigned prefix_hash_key(void *pp) { struct prefix copy; diff --git a/lib/prefix.h b/lib/prefix.h index d57b43dac6..d3c387e102 100644 --- a/lib/prefix.h +++ b/lib/prefix.h @@ -466,7 +466,7 @@ extern int is_zero_mac(struct ethaddr *mac); extern int prefix_str2mac(const char *str, struct ethaddr *mac); extern char *prefix_mac2str(const struct ethaddr *mac, char *buf, int size); -extern unsigned prefix_hash_key(const void *pp); +extern unsigned prefix_hash_key(void *pp); extern int str_to_esi(const char *str, esi_t *esi); extern char *esi_to_str(const esi_t *esi, char *buf, int size); diff --git a/lib/table.c b/lib/table.c index 2d42e2d55c..edba7f1932 100644 --- a/lib/table.c +++ b/lib/table.c @@ -33,14 +33,12 @@ DEFINE_MTYPE(LIB, ROUTE_NODE, "Route node") static void route_table_free(struct route_table *); -static int route_table_hash_cmp(const void *a, const void *b) +static bool route_table_hash_cmp(const void *a, const void *b) { const struct prefix *pa = a, *pb = b; - return prefix_cmp(pa, pb); + return prefix_cmp(pa, pb) == 0; } -DECLARE_HASH(rn_hash_node, struct route_node, nodehash, route_table_hash_cmp, - prefix_hash_key) /* * route_table_init_with_delegate */ @@ -51,7 +49,8 @@ route_table_init_with_delegate(route_table_delegate_t *delegate) rt = XCALLOC(MTYPE_ROUTE_TABLE, sizeof(struct route_table)); rt->delegate = delegate; - rn_hash_node_init(&rt->hash); + rt->hash = hash_create(prefix_hash_key, route_table_hash_cmp, + "route table hash"); return rt; } @@ -70,14 +69,15 @@ static struct route_node *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; - rn_hash_node_add(&node->table->hash, node); + inserted = hash_get(node->table->hash, node, hash_alloc_intern); + assert(inserted == node); return node; } @@ -99,6 +99,9 @@ static void 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 @@ -120,7 +123,6 @@ static void route_table_free(struct route_table *rt) tmp_node->table->count--; tmp_node->lock = 0; /* to cause assert if unlocked after this */ - rn_hash_node_del(&rt->hash, tmp_node); route_node_free(rt, tmp_node); if (node != NULL) { @@ -135,7 +137,6 @@ static void route_table_free(struct route_table *rt) assert(rt->count == 0); - rn_hash_node_fini(&rt->hash); XFREE(MTYPE_ROUTE_TABLE, rt); return; } @@ -256,7 +257,7 @@ struct route_node *route_node_lookup(const struct route_table *table, prefix_copy(&p, pu.p); apply_mask(&p); - node = rn_hash_node_find(&table->hash, (void *)&p); + node = hash_get(table->hash, (void *)&p, NULL); return (node && node->info) ? route_lock_node(node) : NULL; } @@ -269,7 +270,7 @@ struct route_node *route_node_lookup_maynull(const struct route_table *table, prefix_copy(&p, pu.p); apply_mask(&p); - node = rn_hash_node_find(&table->hash, (void *)&p); + node = hash_get(table->hash, (void *)&p, NULL); return node ? route_lock_node(node) : NULL; } @@ -281,11 +282,12 @@ struct route_node *route_node_get(struct route_table *const table, struct route_node *new; struct route_node *node; struct route_node *match; + struct route_node *inserted; uint16_t prefixlen = p->prefixlen; const uint8_t *prefix = &p->u.prefix; apply_mask((struct prefix *)p); - node = rn_hash_node_find(&table->hash, (void *)p); + node = hash_get(table->hash, (void *)p, NULL); if (node && node->info) return route_lock_node(node); @@ -312,7 +314,8 @@ struct route_node *route_node_get(struct route_table *const table, new->p.family = p->family; new->table = table; set_link(new, node); - rn_hash_node_add(&table->hash, new); + inserted = hash_get(node->table->hash, new, hash_alloc_intern); + assert(inserted == new); if (match) set_link(match, new); @@ -364,7 +367,7 @@ void route_node_delete(struct route_node *node) node->table->count--; - rn_hash_node_del(&node->table->hash, node); + hash_release(node->table->hash, node); /* WARNING: FRAGILE CODE! * route_node_free may have the side effect of free'ing the entire diff --git a/lib/table.h b/lib/table.h index 3e3fb658ae..ce578e795c 100644 --- a/lib/table.h +++ b/lib/table.h @@ -25,7 +25,6 @@ #include "memory.h" #include "hash.h" #include "prefix.h" -#include "typesafe.h" #ifdef __cplusplus extern "C" { @@ -60,12 +59,10 @@ struct route_table_delegate_t_ { route_table_destroy_node_func_t destroy_node; }; -PREDECL_HASH(rn_hash_node) - /* Routing table top structure. */ struct route_table { struct route_node *top; - struct rn_hash_node_head hash; + struct hash *hash; /* * Delegate that performs certain functions for this table. @@ -132,7 +129,6 @@ struct route_table { /* Lock of this radix */ \ unsigned int table_rdonly(lock); \ \ - struct rn_hash_node_item nodehash; \ /* Each node of route. */ \ void *info; \ diff --git a/lib/typesafe.h b/lib/typesafe.h index 94002da599..bbf3ce8f1c 100644 --- a/lib/typesafe.h +++ b/lib/typesafe.h @@ -275,7 +275,7 @@ macro_pure size_t prefix ## _count(struct prefix##_head *h) \ #define DECLARE_SORTLIST_UNIQ(prefix, type, field, cmpfn) \ _DECLARE_SORTLIST(prefix, type, field, cmpfn, cmpfn) \ \ -macro_inline type *prefix ## _find(const struct prefix##_head *h, const type *item) \ +macro_inline type *prefix ## _find(struct prefix##_head *h, const type *item) \ { \ struct ssort_item *sitem = h->sh.first; \ int cmpval = 0; \ @@ -383,7 +383,7 @@ macro_inline type *prefix ## _add(struct prefix##_head *h, type *item) \ *np = &item->field.hi; \ return NULL; \ } \ -macro_inline type *prefix ## _find(const struct prefix##_head *h, const type *item) \ +macro_inline type *prefix ## _find(struct prefix##_head *h, const type *item) \ { \ if (!h->hh.tabshift) \ return NULL; \ @@ -576,7 +576,7 @@ macro_inline int prefix ## __cmp(const struct sskip_item *a, \ return cmpfn(container_of(a, type, field.si), \ container_of(b, type, field.si)); \ } \ -macro_inline type *prefix ## _find(const struct prefix##_head *h, const type *item) \ +macro_inline type *prefix ## _find(struct prefix##_head *h, const type *item) \ { \ struct sskip_item *sitem = typesafe_skiplist_find(&h->sh, \ &item->field.si, &prefix ## __cmp); \ diff --git a/tests/topotests/bgp_rfapi_basic_sanity/scripts/cleanup_all.py b/tests/topotests/bgp_rfapi_basic_sanity/scripts/cleanup_all.py index e9c1916f75..096e97fa94 100644 --- a/tests/topotests/bgp_rfapi_basic_sanity/scripts/cleanup_all.py +++ b/tests/topotests/bgp_rfapi_basic_sanity/scripts/cleanup_all.py @@ -10,8 +10,7 @@ luCommand('r3','vtysh -c "debug rfapi-dev close vn 10.0.0.2 un 2.2.2.2"','status luCommand('r4','vtysh -c "debug rfapi-dev unregister vn 10.0.0.3 un 3.3.3.3 prefix 33.33.33.0/24"','', 'none', 'Prefix removed') luCommand('r4','vtysh -c "debug rfapi-dev unregister vn 10.0.0.3 un 3.3.3.3 prefix 11.11.11.0/24"','', 'none', 'MP prefix removed') luCommand('r4','vtysh -c "show vnc registrations"','Locally: *Active: 0 ','wait','Local registration removed') -#luCommand('r4','vtysh -c "debug rfapi-dev close vn 10.0.0.3 un 3.3.3.3"','status 0', 'pass', 'Closed RFAPI') -luCommand('r4','vtysh -c "clear vnc nve *"','.', 'pass', 'Cleared NVEs') +luCommand('r4','vtysh -c "debug rfapi-dev close vn 10.0.0.3 un 3.3.3.3"','status 0', 'pass', 'Closed RFAPI') luCommand('r1','vtysh -c "show vnc registrations"','Locally: *Active: 0 .* Remotely: *Active: 0','wait','All registrations cleared') luCommand('r3','vtysh -c "show vnc registrations"','Locally: *Active: 0 .* Remotely: *Active: 0','wait','All registrations cleared') diff --git a/zebra/rib.h b/zebra/rib.h index fd2aeabf57..e26831e1a6 100644 --- a/zebra/rib.h +++ b/zebra/rib.h @@ -24,7 +24,6 @@ #include "zebra.h" #include "hook.h" -#include "typesafe.h" #include "linklist.h" #include "prefix.h" #include "table.h" @@ -40,44 +39,6 @@ extern "C" { #endif -typedef enum { RNH_NEXTHOP_TYPE, RNH_IMPORT_CHECK_TYPE } rnh_type_t; - -PREDECL_LIST(rnh_list) - -/* Nexthop structure. */ -struct rnh { - uint8_t flags; - -#define ZEBRA_NHT_CONNECTED 0x1 -#define ZEBRA_NHT_DELETED 0x2 -#define ZEBRA_NHT_EXACT_MATCH 0x4 - - /* VRF identifier. */ - vrf_id_t vrf_id; - - afi_t afi; - - rnh_type_t type; - - uint32_t seqno; - - struct route_entry *state; - struct prefix resolved_route; - struct list *client_list; - - /* pseudowires dependent on this nh */ - struct list *zebra_pseudowire_list; - - struct route_node *node; - - /* - * if this has been filtered for the client - */ - int filtered[ZEBRA_ROUTE_MAX]; - - struct rnh_list_item rnh_list_item; -}; - #define DISTANCE_INFINITY 255 #define ZEBRA_KERNEL_TABLE_MAX 252 /* support for no more than this rt tables */ @@ -190,7 +151,7 @@ typedef struct rib_dest_t_ { * the data plane we will run evaluate_rnh * on these prefixes. */ - struct rnh_list_head nht; + struct list *nht; /* * Linkage to put dest on the FPM processing queue. @@ -199,8 +160,6 @@ typedef struct rib_dest_t_ { } rib_dest_t; -DECLARE_LIST(rnh_list, struct rnh, rnh_list_item); - #define RIB_ROUTE_QUEUED(x) (1 << (x)) // If MQ_SIZE is modified this value needs to be updated. #define RIB_ROUTE_ANY_QUEUED 0x1F diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index f5ba619afa..2994911165 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -1204,6 +1204,7 @@ static int rib_can_delete_dest(rib_dest_t *dest) void zebra_rib_evaluate_rn_nexthops(struct route_node *rn, uint32_t seq) { rib_dest_t *dest = rib_dest_from_rnode(rn); + struct listnode *node, *nnode; struct rnh *rnh; /* @@ -1235,7 +1236,7 @@ void zebra_rib_evaluate_rn_nexthops(struct route_node *rn, uint32_t seq) * nht resolution and as such we need to call the * nexthop tracking evaluation code */ - for_each (rnh_list, &dest->nht, rnh) { + for (ALL_LIST_ELEMENTS(dest->nht, node, nnode, rnh)) { struct zebra_vrf *zvrf = zebra_vrf_lookup_by_id(rnh->vrf_id); struct prefix *p = &rnh->node->p; @@ -1311,7 +1312,7 @@ int rib_gc_dest(struct route_node *rn) zebra_rib_evaluate_rn_nexthops(rn, zebra_router_get_next_sequence()); dest->rnode = NULL; - rnh_list_fini(&dest->nht); + list_delete(&dest->nht); XFREE(MTYPE_RIB_DEST, dest); rn->info = NULL; @@ -2356,7 +2357,7 @@ rib_dest_t *zebra_rib_create_dest(struct route_node *rn) rib_dest_t *dest; dest = XCALLOC(MTYPE_RIB_DEST, sizeof(rib_dest_t)); - rnh_list_init(&dest->nht); + dest->nht = list_new(); route_lock_node(rn); /* rn route table reference */ rn->info = dest; dest->rnode = rn; diff --git a/zebra/zebra_rnh.c b/zebra/zebra_rnh.c index 2917d0e7a8..220a8006d0 100644 --- a/zebra/zebra_rnh.c +++ b/zebra/zebra_rnh.c @@ -119,7 +119,7 @@ static void zebra_rnh_remove_from_routing_table(struct rnh *rnh) } dest = rib_dest_from_rnode(rn); - rnh_list_del(&dest->nht, rnh); + listnode_delete(dest->nht, rnh); route_unlock_node(rn); } @@ -145,7 +145,7 @@ static void zebra_rnh_store_in_routing_table(struct rnh *rnh) } dest = rib_dest_from_rnode(rn); - rnh_list_add_tail(&dest->nht, rnh); + listnode_add(dest->nht, rnh); route_unlock_node(rn); } @@ -251,7 +251,7 @@ void zebra_free_rnh(struct rnh *rnh) route_unlock_node(rern); dest = rib_dest_from_rnode(rern); - rnh_list_del(&dest->nht, rnh); + listnode_delete(dest->nht, rnh); } } free_state(rnh->vrf_id, rnh->state, rnh->node); diff --git a/zebra/zebra_rnh.h b/zebra/zebra_rnh.h index 95a3941181..9cd9116eed 100644 --- a/zebra/zebra_rnh.h +++ b/zebra/zebra_rnh.h @@ -29,6 +29,40 @@ extern "C" { #endif +typedef enum { RNH_NEXTHOP_TYPE, RNH_IMPORT_CHECK_TYPE } rnh_type_t; + +/* Nexthop structure. */ +struct rnh { + uint8_t flags; + +#define ZEBRA_NHT_CONNECTED 0x1 +#define ZEBRA_NHT_DELETED 0x2 +#define ZEBRA_NHT_EXACT_MATCH 0x4 + + /* VRF identifier. */ + vrf_id_t vrf_id; + + afi_t afi; + + rnh_type_t type; + + uint32_t seqno; + + struct route_entry *state; + struct prefix resolved_route; + struct list *client_list; + + /* pseudowires dependent on this nh */ + struct list *zebra_pseudowire_list; + + struct route_node *node; + + /* + * if this has been filtered for the client + */ + int filtered[ZEBRA_ROUTE_MAX]; +}; + extern int zebra_rnh_ip_default_route; extern int zebra_rnh_ipv6_default_route; diff --git a/zebra/zebra_vrf.c b/zebra/zebra_vrf.c index f0b99f41fc..2d721ec8a1 100644 --- a/zebra/zebra_vrf.c +++ b/zebra/zebra_vrf.c @@ -357,7 +357,7 @@ void zebra_rtable_node_cleanup(struct route_table *table, if (node->info) { rib_dest_t *dest = node->info; - rnh_list_fini(&dest->nht); + list_delete(&dest->nht); XFREE(MTYPE_RIB_DEST, node->info); } }