From 50339f96638eed35dac2b673b64cc6f1eb96406c Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Thu, 15 Jan 2009 23:19:57 +0100 Subject: src: rework of the hash-cache infrastructure Currently, the caching system is implemented in a two layer architecture: hashtable (inner layer) and cache (upper layer). This patch reworks the hash-cache infrastructure to solve some initial design problems to make it more flexible, the main strong points of this patch are: * Memory handling is done in the cache layer, not in the inner hashtable layer. This removes one of the main dependencies between the hashtable and the cache classes. * Remove excessive encapsulation: the former cache used to hide a lot of details of the inner hashtable implementation. * Fix over-hashing of some operations: lookup-delete-add required three hash calculations. Similarly, the update-or-add operation required two hash calculations. Now, we calculate the hash once and re-use the value how many times as we need. This patch simplifies the caching system. As a result, we save ~130 lines of code. Small code means and less complexity means less chance to have bugs. Signed-off-by: Pablo Neira Ayuso --- src/cache.c | 273 +++++++++++++++++++++++------------------------------------- 1 file changed, 104 insertions(+), 169 deletions(-) (limited to 'src/cache.c') diff --git a/src/cache.c b/src/cache.c index 553dddf..621a3f4 100644 --- a/src/cache.c +++ b/src/cache.c @@ -1,5 +1,5 @@ /* - * (C) 2006-2007 by Pablo Neira Ayuso + * (C) 2006-2009 by Pablo Neira Ayuso * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -20,7 +20,6 @@ #include "jhash.h" #include "hash.h" #include "log.h" -#include "us-conntrack.h" #include "conntrackd.h" #include @@ -68,14 +67,14 @@ __hash6(const struct nf_conntrack *ct, const struct hashtable *table) static uint32_t hash(const void *data, const struct hashtable *table) { int ret = 0; - const struct us_conntrack *u = data; + const struct nf_conntrack *ct = data; - switch(nfct_get_attr_u8(u->ct, ATTR_L3PROTO)) { + switch(nfct_get_attr_u8(ct, ATTR_L3PROTO)) { case AF_INET: - ret = __hash4(u->ct, table); + ret = __hash4(ct, table); break; case AF_INET6: - ret = __hash6(u->ct, table); + ret = __hash6(ct, table); break; default: dlog(LOG_ERR, "unknown layer 3 proto in hash"); @@ -87,10 +86,10 @@ static uint32_t hash(const void *data, const struct hashtable *table) static int compare(const void *data1, const void *data2) { - const struct us_conntrack *u1 = data1; - const struct us_conntrack *u2 = data2; + const struct cache_object *obj = data1; + const struct nf_conntrack *ct = data2; - return nfct_cmp(u1->ct, u2->ct, NFCT_CMP_ORIG); + return nfct_cmp(obj->ct, ct, NFCT_CMP_ORIG); } struct cache_feature *cache_feature[CACHE_MAX_FEATURE] = { @@ -103,7 +102,7 @@ struct cache *cache_create(const char *name, unsigned int features, struct cache_extra *extra) { - size_t size = sizeof(struct us_conntrack); + size_t size = sizeof(struct cache_object); int i, j = 0; struct cache *c; struct cache_feature *feature_array[CACHE_MAX_FEATURE] = {}; @@ -152,7 +151,6 @@ struct cache *cache_create(const char *name, c->h = hashtable_create(CONFIG(hashsize), CONFIG(limit), - size, hash, compare); @@ -162,6 +160,7 @@ struct cache *cache_create(const char *name, free(c); return NULL; } + c->object_size = size; return c; } @@ -177,225 +176,165 @@ void cache_destroy(struct cache *c) static void __del_timeout(struct alarm_block *a, void *data); -static struct us_conntrack *__add(struct cache *c, struct nf_conntrack *ct) +struct cache_object *cache_object_new(struct cache *c, struct nf_conntrack *ct) { - unsigned i; - size_t size = c->h->datasize; - char buf[size]; - struct us_conntrack *u = (struct us_conntrack *) buf; - struct nf_conntrack *newct; + struct cache_object *obj; - memset(u, 0, size); + obj = calloc(c->object_size, 1); + if (obj == NULL) { + errno = ENOMEM; + c->stats.add_fail_enomem++; + return NULL; + } + obj->cache = c; + init_alarm(&obj->alarm, obj, __del_timeout); - u->cache = c; - if ((u->ct = newct = nfct_new()) == NULL) { + if ((obj->ct = nfct_new()) == NULL) { + free(obj); errno = ENOMEM; - return 0; + c->stats.add_fail_enomem++; + return NULL; } - memcpy(u->ct, ct, nfct_sizeof(ct)); + memcpy(obj->ct, ct, nfct_sizeof(ct)); - u = hashtable_add(c->h, u); - if (u) { - char *data = u->data; + return obj; +} - init_alarm(&u->alarm, u, __del_timeout); +void cache_object_free(struct cache_object *obj) +{ + nfct_destroy(obj->ct); + free(obj); +} - for (i = 0; i < c->num_features; i++) { - c->features[i]->add(u, data); - data += c->features[i]->size; - } +static int __add(struct cache *c, struct cache_object *obj, int id) +{ + int ret; + unsigned int i; + char *data = obj->data; - if (c->extra && c->extra->add) - c->extra->add(u, ((char *) u) + c->extra_offset); + ret = hashtable_add(c->h, &obj->hashnode, id); + if (ret == -1) + return -1; - c->stats.active++; - return u; + for (i = 0; i < c->num_features; i++) { + c->features[i]->add(obj, data); + data += c->features[i]->size; } - free(newct); - return NULL; + if (c->extra && c->extra->add) + c->extra->add(obj, ((char *) obj) + c->extra_offset); + + c->stats.active++; + return 0; } -struct us_conntrack *cache_add(struct cache *c, struct nf_conntrack *ct) +int cache_add(struct cache *c, struct cache_object *obj, int id) { - struct us_conntrack *u; + int ret; - u = __add(c, ct); - if (u) { - c->stats.add_ok++; - return u; - } - if (errno != EEXIST) { + ret = __add(c, obj, id); + if (ret == -1) { c->stats.add_fail++; if (errno == ENOSPC) c->stats.add_fail_enospc++; - if (errno == ENOMEM) - c->stats.add_fail_enomem++; + return -1; } - - return NULL; + c->stats.add_ok++; + return 0; } -static void -__cache_update(struct cache *c, struct us_conntrack *u, struct nf_conntrack *ct) +void cache_update(struct cache *c, struct cache_object *obj, int id, + struct nf_conntrack *ct) { - unsigned i; - char *data = u->data; + char *data = obj->data; + unsigned int i; - nfct_copy(u->ct, ct, NFCT_CP_META); + nfct_copy(obj->ct, ct, NFCT_CP_META); for (i = 0; i < c->num_features; i++) { - c->features[i]->update(u, data); + c->features[i]->update(obj, data); data += c->features[i]->size; } if (c->extra && c->extra->update) - c->extra->update(u, ((char *) u) + c->extra_offset); -} - -static struct us_conntrack *__update(struct cache *c, struct nf_conntrack *ct) -{ - size_t size = c->h->datasize; - char buf[size]; - struct us_conntrack *u = (struct us_conntrack *) buf; - - u->ct = ct; - - u = (struct us_conntrack *) hashtable_find(c->h, u); - if (u) { - __cache_update(c, u, ct); - return u; - } - return NULL; -} + c->extra->update(obj, ((char *) obj) + c->extra_offset); -struct us_conntrack *cache_update(struct cache *c, struct nf_conntrack *ct) -{ - struct us_conntrack *u; - - u = __update(c, ct); - if (u) { - c->stats.upd_ok++; - return u; - } - c->stats.upd_fail++; - if (errno == ENOENT) - c->stats.upd_fail_enoent++; - - return NULL; + c->stats.upd_ok++; } -static void __del(struct cache *c, struct us_conntrack *u) +static void __del(struct cache *c, struct cache_object *obj) { unsigned i; - char *data = u->data; - struct nf_conntrack *p = u->ct; + char *data = obj->data; for (i = 0; i < c->num_features; i++) { - c->features[i]->destroy(u, data); + c->features[i]->destroy(obj, data); data += c->features[i]->size; } if (c->extra && c->extra->destroy) - c->extra->destroy(u, ((char *) u) + c->extra_offset); + c->extra->destroy(obj, ((char *) obj) + c->extra_offset); - hashtable_del(c->h, u); - free(p); + hashtable_del(c->h, &obj->hashnode); } -static void __cache_del(struct cache *c, struct us_conntrack *u) +void cache_del(struct cache *c, struct cache_object *obj) { /* * Do not increase stats if we are trying to * kill an entry was previously deleted via * __cache_del_timer. */ - if (!alarm_pending(&u->alarm)) { + if (!alarm_pending(&obj->alarm)) { c->stats.del_ok++; c->stats.active--; } - del_alarm(&u->alarm); - __del(c, u); + del_alarm(&obj->alarm); + __del(c, obj); } -struct us_conntrack *cache_update_force(struct cache *c, - struct nf_conntrack *ct) +struct cache_object * +cache_update_force(struct cache *c, struct nf_conntrack *ct) { - struct us_conntrack *u; - - u = cache_find(c, ct); - if (u) { - if (!alarm_pending(&u->alarm)) { - c->stats.upd_ok++; - __cache_update(c, u, ct); - return u; + struct cache_object *obj; + int id; + + obj = cache_find(c, ct, &id); + if (obj) { + if (!alarm_pending(&obj->alarm)) { + cache_update(c, obj, id, ct); + return obj; } else { - __cache_del(c, u); + cache_del(c, obj); + cache_object_free(obj); } } - if ((u = __add(c, ct)) != NULL) { - c->stats.add_ok++; - return u; - } - c->stats.add_fail++; - if (errno == ENOSPC) - c->stats.add_fail_enospc++; - if (errno == ENOMEM) - c->stats.add_fail_enomem++; - - return NULL; -} - -int cache_test(struct cache *c, struct nf_conntrack *ct) -{ - size_t size = c->h->datasize; - char buf[size]; - struct us_conntrack *u = (struct us_conntrack *) buf; - void *ret; - - u->ct = ct; - - ret = hashtable_find(c->h, u); - - return ret != NULL; -} - -int cache_del(struct cache *c, struct nf_conntrack *ct) -{ - size_t size = c->h->datasize; - char buf[size]; - struct us_conntrack *u = (struct us_conntrack *) buf; - - u->ct = ct; + obj = cache_object_new(c, ct); + if (obj == NULL) + return NULL; - u = (struct us_conntrack *) hashtable_find(c->h, u); - if (u) { - __cache_del(c, u); - return 1; - } - c->stats.del_fail++; - if (errno == ENOENT) - c->stats.del_fail_enoent++; + if (cache_add(c, obj, id) == -1) + return NULL; - return 0; + return obj; } static void __del_timeout(struct alarm_block *a, void *data) { - struct us_conntrack *u = (struct us_conntrack *) data; - - __del(u->cache, u); + struct cache_object *obj = (struct cache_object *) data; + __del(obj->cache, obj); + cache_object_free(obj); } -int -__cache_del_timer(struct cache *c, struct us_conntrack *u, int timeout) +int cache_del_timer(struct cache *c, struct cache_object *obj, int timeout) { if (timeout <= 0) { - __cache_del(c, u); + cache_del(c, obj); + cache_object_free(obj); return 1; } - if (!alarm_pending(&u->alarm)) { - add_alarm(&u->alarm, timeout, 0); + if (!alarm_pending(&obj->alarm)) { + add_alarm(&obj->alarm, timeout, 0); /* * increase stats even if this entry was not really * removed yet. We do not want to make people think @@ -409,20 +348,16 @@ __cache_del_timer(struct cache *c, struct us_conntrack *u, int timeout) return 0; } -struct us_conntrack *cache_find(struct cache *c, struct nf_conntrack *ct) +struct cache_object * +cache_find(struct cache *c, struct nf_conntrack *ct, int *id) { - size_t size = c->h->datasize; - char buf[size]; - struct us_conntrack *u = (struct us_conntrack *) buf; - - u->ct = ct; - - return ((struct us_conntrack *) hashtable_find(c->h, u)); + *id = hashtable_hash(c->h, ct); + return ((struct cache_object *) hashtable_find(c->h, ct, *id)); } -struct us_conntrack *cache_get_conntrack(struct cache *c, void *data) +struct cache_object *cache_data_get_object(struct cache *c, void *data) { - return (struct us_conntrack *)((char*)data - c->extra_offset); + return (struct cache_object *)((char*)data - c->extra_offset); } void *cache_get_extra(struct cache *c, void *data) -- cgit v1.2.3