diff --git a/lib/dns/include/dns/lowac.h b/lib/dns/include/dns/lowac.h index e594218ab8..24b4c07d2c 100644 --- a/lib/dns/include/dns/lowac.h +++ b/lib/dns/include/dns/lowac.h @@ -13,5 +13,5 @@ isc_result_t dns_lowac_put(dns_lowac_t *lowac, dns_name_t *name, char* packet, int size); isc_result_t -dns_lowac_get(dns_lowac_t *lowac, dns_name_t *name, char* blob, int* blobsize, bool tcp); +dns_lowac_get(dns_lowac_t *lowac, dns_name_t *name, unsigned char* blob, int* blobsize, bool tcp); diff --git a/lib/dns/lowac.c b/lib/dns/lowac.c index c943489af5..7e1d0d6115 100644 --- a/lib/dns/lowac.c +++ b/lib/dns/lowac.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -17,27 +18,42 @@ #include #include +#define MAXIMUM_PKT_SIZE 1024 + struct dns_lowac_entry { - dns_name_t name; - uint16_t flags; - isc_time_t expire; - ck_ht_hash_t hash; - char blob[1024]; - uint32_t blobsize; + dns_name_t name; + isc_region_t key; + uint16_t flags; + isc_time_t expire; + ck_ht_hash_t hash; + unsigned char blob[MAXIMUM_PKT_SIZE]; + uint32_t blobsize; + isc_refcount_t refcount; + + /* + * Set to 'true' when we enqueue this entry on remq. + * Nothing serious will happen if we enqueue the entry + * on remq multiple times as we're refcounting. + * It's not locked in any way as it can only change + * from false to true during runtime. + */ + bool remq_enqueued; + + bool inht; }; struct dns_lowac { - bool running; - ck_ht_t ht; - isc_mem_t * mctx; - isc_interval_t expiry; - isc_thread_t thread; - int count; - ck_fifo_mpmc_t inq; - - ck_fifo_mpmc_t remq; + bool running; + ck_ht_t ht; + isc_mem_t * mctx; + isc_interval_t expiry; + isc_thread_t thread; + int count; + ck_ht_iterator_t htit; + ck_fifo_mpmc_t inq; + ck_fifo_mpmc_t remq; }; static void * @@ -74,96 +90,182 @@ static struct ck_malloc my_allocator = { static void free_entry(dns_lowac_t *lowac, dns_lowac_entry_t *entry) { - //fprintf(stderr, "XXX ENTRY NAME %p\n", entry); dns_name_free(&entry->name, lowac->mctx); isc_mem_put(lowac->mctx, entry, sizeof(*entry)); } +static bool +dequeue_input_entry(dns_lowac_t*lowac) { + dns_lowac_entry_t *entry = NULL; + ck_fifo_mpmc_entry_t *garbage = NULL; + if (ck_fifo_mpmc_dequeue(&lowac->inq, &entry, + &garbage) == true) { + isc_mem_put(lowac->mctx, garbage, + sizeof(*garbage)); + ck_ht_entry_t htentry; + ck_ht_entry_t oldhtentry; + + ck_ht_entry_set(&htentry, entry->hash, entry->key.base, + entry->key.length, entry); + ck_ht_entry_set(&oldhtentry, entry->hash, entry->key.base, + entry->key.length, NULL); + + if (ck_ht_get_spmc(&lowac->ht, entry->hash, + &oldhtentry) == false) { + /* We don't have this entry in hashtable */ + if (ck_ht_put_spmc(&lowac->ht, entry->hash, + &htentry) == false) { + printf("oddness but might happen\n"); + /* We can do it safely as it never was in + * hashtable */ + RUNTIME_CHECK(isc_refcount_decrement(&entry-> + refcount) == + 0); + free_entry(lowac, entry); + } else { + entry->inht = true; + lowac->count++; + } + } else { + dns_lowac_entry_t *oldentry = + ck_ht_entry_value(&oldhtentry); + /* Note: set without remove doesn't seem to work + * properly */ + RUNTIME_CHECK(ck_ht_remove_spmc(&lowac->ht, + entry->hash, + &oldhtentry) == true); + oldentry->inht = false; + RUNTIME_CHECK(ck_ht_set_spmc(&lowac->ht, entry->hash, + &htentry) == true); + + if (oldentry->remq_enqueued) { + isc_refcount_decrement(&oldentry->refcount); + } else { + /* + * We don't need to increment refcount since we + * have not decremented it + * when removing from hashtable + */ + ck_fifo_mpmc_entry_t *qentry = + isc_mem_get(lowac->mctx, + (sizeof( + ck_fifo_mpmc_entry_t))); + entry->remq_enqueued = true; + ck_fifo_mpmc_enqueue(&lowac->remq, qentry, + entry); + } + } + return (true); + } + return (false); +} + +static int +expire_entries(dns_lowac_t *lowac) { + isc_time_t now; + isc_time_now(&now); + int iterated = 0; + + ck_ht_entry_t *htitentry = NULL; + bool iterok = ck_ht_next(&lowac->ht, &lowac->htit, &htitentry); + if (!iterok) { + ck_ht_iterator_init(&lowac->htit); + iterok = ck_ht_next(&lowac->ht, &lowac->htit, &htitentry); + } + while (iterok && htitentry != NULL && iterated < 256) { + dns_lowac_entry_t *entry = ck_ht_entry_value( + htitentry); + if (isc_time_compare(&entry->expire, &now) < 0) { + RUNTIME_CHECK(ck_ht_remove_spmc(&lowac->ht, + entry->hash, + htitentry) == true); + entry->inht = false; + dns_lowac_entry_t *ent2 = ck_ht_entry_value(htitentry); + RUNTIME_CHECK(ent2 == entry); + /* + * We don't need to increment refcount since we have + * not decremented it when removing from hashtable. + */ + ck_fifo_mpmc_entry_t *qentry = + isc_mem_get(lowac->mctx, + (sizeof( + ck_fifo_mpmc_entry_t))); + entry->remq_enqueued = true; + ck_fifo_mpmc_enqueue(&lowac->remq, qentry, entry); + } + iterok = ck_ht_next(&lowac->ht, &lowac->htit, &htitentry); + iterated++; + } + + if (!iterok) { + ck_ht_iterator_init(&lowac->htit); + } + return (iterated); +} + +/* + * Iterate over remq. + * If the entry is in HT - remove it, requeue. + * If the entry is not in HT but it is referenced - requeue. + * If the entry is not referenced - free. + */ +static int +cleanup_entries(dns_lowac_t *lowac) { + ck_ht_entry_t htentry; + dns_lowac_entry_t *entry; + ck_fifo_mpmc_entry_t *qentry = NULL; + int removed = 0; + int max = 100; + + while (--max > 0 && + ck_fifo_mpmc_dequeue(&lowac->remq, &entry, &qentry) == true) + { + int rc = isc_refcount_decrement(&entry->refcount); + if (entry->inht) { + /* + * Remove entry from the hashtable, we'll remove the + * entry itself in the next iteration. + */ + ck_ht_entry_set(&htentry, entry->hash, entry->key.base, + entry->key.length, entry); + RUNTIME_CHECK(ck_ht_remove_spmc(&lowac->ht, + entry->hash, + &htentry) == true); + entry->inht = false; + isc_refcount_increment(&entry->refcount); + ck_fifo_mpmc_enqueue(&lowac->remq, qentry, entry); + } else { + if (rc > 1) { + /* Requeue if still in use */ + isc_refcount_increment(&entry->refcount); + ck_fifo_mpmc_enqueue(&lowac->remq, qentry, + entry); + } else { + isc_mem_put(lowac->mctx, qentry, + sizeof(*qentry)); + removed++; + free_entry(lowac, entry); + lowac->count--; + } + } + } + return (removed); +} static void * rthread(void *d) { dns_lowac_t *lowac = (dns_lowac_t*) d; - ck_ht_iterator_t htit = CK_HT_ITERATOR_INITIALIZER; - ck_ht_entry_t *htitentry = NULL; + ck_ht_iterator_init(&lowac->htit); while (lowac->running) { - dns_lowac_entry_t *entry = NULL; - ck_fifo_mpmc_entry_t *garbage = NULL; - if (ck_fifo_mpmc_dequeue(&lowac->inq, &entry, - &garbage) == true) { - isc_mem_put(lowac->mctx, garbage, - sizeof(*garbage)); - ck_ht_entry_t htentry; - ck_ht_entry_t oldhtentry; - isc_region_t r; - - dns_name_toregion(&entry->name, &r); - ck_ht_entry_set(&htentry, entry->hash, r.base, r.length, entry); - ck_ht_entry_set(&oldhtentry, entry->hash, r.base, r.length, NULL); - if (ck_ht_get_spmc(&lowac->ht, entry->hash, - &oldhtentry) == false) { - if (ck_ht_put_spmc(&lowac->ht, entry->hash, &htentry) == false) { - //fprintf(stderr, "XXX put false ENTRY NAME %p\n", entry); - free_entry(lowac, entry); - } else { - lowac->count++; - } - } else { - dns_lowac_entry_t *oldentry = - ck_ht_entry_value(&oldhtentry); - //fprintf(stderr, "XXX replace ENTRY NAME %p %p\n", oldentry, htentry.value); - free_entry(lowac, oldentry); - RUNTIME_CHECK(ck_ht_remove_spmc(&lowac->ht, entry->hash, &oldhtentry) == true); - RUNTIME_CHECK(ck_ht_set_spmc(&lowac->ht, entry->hash, &htentry) == true); - //fprintf(stderr, "XXX old got %p\n", ck_ht_entry_value(&htentry)); - } - isc_thread_yield(); - } else { - isc_time_t now; - isc_time_now(&now); - int removed = 0; -/* ck_ht_entry_t htentry; - dns_lowac_entry_t *entry; - ck_fifo_mpmc_entry_t *garbage = NULL; - // TODO race here! - while (ck_fifo_mpmc_dequeue(&lowac->remq, &entry, - &garbage) == true) { - removed++; - isc_mem_put(lowac->mctx, garbage, - sizeof(*garbage)); - if (ck_ht_get_spmc(&lowac->ht, entry->hash, - &htentry) == true) { - RUNTIME_CHECK(ck_ht_remove_spmc(&lowac->ht, entry->hash, &htentry) == true); - } else { - printf("Oddness\n"); - } - free_entry(lowac, entry); - lowac->count--; - } -*/ - ck_ht_iterator_init(&htit); - bool iterok = ck_ht_next(&lowac->ht, &htit, &htitentry); - while (iterok && htitentry != NULL && removed < 256) { - dns_lowac_entry_t *entry = ck_ht_entry_value( - htitentry); - if (isc_time_compare(&entry->expire, - &now) < 0) { - RUNTIME_CHECK(ck_ht_remove_spmc(&lowac->ht, entry->hash, htitentry) == true); - dns_lowac_entry_t *ent2 = ck_ht_entry_value(htitentry); - RUNTIME_CHECK(ent2 == entry); - //fprintf(stderr, "XXX free timeout ENTRY NAME %p\n", entry); - free_entry(lowac, entry); - lowac->count--; - } - removed++; - iterok = ck_ht_next(&lowac->ht, &htit, &htitentry); - } - if (!iterok) { - ck_ht_iterator_init(&htit); - } -// INSIST(ck_ht_gc(&lowac->ht, 64, -// isc_random32()) == true); + bool dequeued = dequeue_input_entry(lowac); + if (!dequeued) { + expire_entries(lowac); + int removed = cleanup_entries(lowac); if (removed < 256) { - usleep(1000); + usleep(10000); + } else { + RUNTIME_CHECK(ck_ht_gc(&lowac->ht, 128, + isc_random32())); } } } @@ -172,9 +274,11 @@ rthread(void *d) { dns_lowac_t* dns_lowac_create(isc_mem_t *mctx) { dns_lowac_t *lowac = isc_mem_get(mctx, sizeof(dns_lowac_t)); - ck_fifo_mpmc_entry_t *inq_stub = isc_mem_get(mctx, sizeof(ck_fifo_mpmc_entry_t)); - ck_fifo_mpmc_entry_t *remq_stub = isc_mem_get(mctx, sizeof(ck_fifo_mpmc_entry_t)); - + ck_fifo_mpmc_entry_t *inq_stub = + isc_mem_get(mctx, sizeof(ck_fifo_mpmc_entry_t)); + ck_fifo_mpmc_entry_t *remq_stub = + isc_mem_get(mctx, sizeof(ck_fifo_mpmc_entry_t)); + ck_fifo_mpmc_init(&lowac->inq, inq_stub); ck_fifo_mpmc_init(&lowac->remq, remq_stub); lowac->count = 0; @@ -186,7 +290,7 @@ dns_lowac_create(isc_mem_t *mctx) { abort(); } isc_mem_attach(mctx, &lowac->mctx); - isc_interval_set(&lowac->expiry, 60, 0); + isc_interval_set(&lowac->expiry, 30, 0); isc_thread_create(rthread, lowac, &lowac->thread); return (lowac); } @@ -197,33 +301,43 @@ dns_lowac_destroy(dns_lowac_t *lowac) { isc_thread_join(lowac->thread, NULL); dns_lowac_entry_t *entry; ck_fifo_mpmc_entry_t *fentry; - while (ck_ht_count(&lowac->ht)>0) { + + /* Entries in inq can only be in inq, it's safe to free them */ + while (ck_fifo_mpmc_dequeue(&lowac->inq, + &entry, + &fentry)) + { + isc_mem_put(lowac->mctx, fentry, sizeof(*fentry)); + free_entry(lowac, entry); + } + + /* + * Entries in remq can be freed only if they're no longer referenced + * by either multiple occurences in queue or HT + */ + while (ck_fifo_mpmc_dequeue(&lowac->remq, + &entry, + &fentry)) + { + if (isc_refcount_decrement(&entry->refcount) == 0) { + free_entry(lowac, entry); + } + isc_mem_put(lowac->mctx, fentry, sizeof(*fentry)); + } + + /* Finally we free rest of entries from HT */ + while (ck_ht_count(&lowac->ht) > 0) { ck_ht_gc(&lowac->ht, 0, 0); ck_ht_iterator_t htit = CK_HT_ITERATOR_INITIALIZER; ck_ht_entry_t *htitentry = NULL; while (ck_ht_next(&lowac->ht, &htit, &htitentry)) { entry = ck_ht_entry_value(htitentry); ck_ht_remove_spmc(&lowac->ht, entry->hash, htitentry); - //fprintf(stderr, "XXX cleanup %p\n", entry); free_entry(lowac, entry); } } - - while (ck_fifo_mpmc_dequeue(&lowac->inq, - &entry, - &fentry)) - { - isc_mem_put(lowac->mctx, fentry, sizeof(*fentry)); - //fprintf(stderr, "XXX cleanup2 %p\n", entry); - free_entry(lowac, entry); - } - while (ck_fifo_mpmc_dequeue(&lowac->remq, - &entry, - &fentry)) - { - isc_mem_put(lowac->mctx, fentry, sizeof(*fentry)); - } + ck_fifo_mpmc_deinit(&lowac->inq, &fentry); isc_mem_put(lowac->mctx, fentry, sizeof(*fentry)); @@ -239,7 +353,7 @@ dns_lowac_put(dns_lowac_t *lowac, dns_name_t *name, char*packet, int size) { if (!lowac->running) { return (ISC_R_SHUTTINGDOWN); } - if (size > 1024) { + if (size > MAXIMUM_PKT_SIZE) { return (ISC_R_FAILURE); } dns_lowac_entry_t *entry = isc_mem_get(lowac->mctx, sizeof(*entry)); @@ -248,9 +362,14 @@ dns_lowac_put(dns_lowac_t *lowac, dns_name_t *name, char*packet, int size) { memcpy(entry->blob, packet, size); isc_time_nowplusinterval(&entry->expire, &lowac->expiry); entry->blobsize = size; - isc_region_t r; - dns_name_toregion(&entry->name, &r); - ck_ht_hash(&entry->hash, &lowac->ht, r.base, r.length); + isc_refcount_init(&entry->refcount, 1); + entry->remq_enqueued = false; + entry->inht = false; + + dns_name_toregion(&entry->name, &entry->key); + + ck_ht_hash(&entry->hash, &lowac->ht, entry->key.base, + entry->key.length); ck_fifo_mpmc_entry_t *qentry = isc_mem_get(lowac->mctx, (sizeof(ck_fifo_mpmc_entry_t))); ck_fifo_mpmc_enqueue(&lowac->inq, qentry, entry); @@ -259,8 +378,8 @@ dns_lowac_put(dns_lowac_t *lowac, dns_name_t *name, char*packet, int size) { isc_result_t -dns_lowac_get(dns_lowac_t *lowac, dns_name_t *name, char *blob, int *blobsize, - bool tcp) { +dns_lowac_get(dns_lowac_t *lowac, dns_name_t *name, unsigned char *blob, + int *blobsize, bool tcp) { ck_ht_entry_t htentry; ck_ht_hash_t h; isc_region_t r; @@ -272,12 +391,21 @@ dns_lowac_get(dns_lowac_t *lowac, dns_name_t *name, char *blob, int *blobsize, return (ISC_R_NOTFOUND); } else { dns_lowac_entry_t *entry = ck_ht_entry_value(&htentry); + int oldrc = isc_refcount_increment(&entry->refcount); + RUNTIME_CHECK(oldrc > 0); + if (entry->remq_enqueued) { + /* This entry is being removed, bail */ + isc_refcount_decrement(&entry->refcount); + return (ISC_R_NOTFOUND); + } isc_time_t now; isc_time_now(&now); if (isc_time_compare(&entry->expire, &now) < 0) { -/* ck_fifo_mpmc_entry_t *qentry = - isc_mem_get(lowac->mctx, (sizeof(ck_fifo_mpmc_entry_t))); - ck_fifo_mpmc_enqueue(&lowac->remq, qentry, entry); */ + ck_fifo_mpmc_entry_t *qentry = + isc_mem_get(lowac->mctx, + (sizeof(ck_fifo_mpmc_entry_t))); + entry->remq_enqueued = true; + ck_fifo_mpmc_enqueue(&lowac->remq, qentry, entry); return (ISC_R_NOTFOUND); } if (tcp) { @@ -289,6 +417,7 @@ dns_lowac_get(dns_lowac_t *lowac, dns_name_t *name, char *blob, int *blobsize, memcpy(blob, entry->blob, entry->blobsize); *blobsize = entry->blobsize; } + isc_refcount_decrement(&entry->refcount); return (ISC_R_SUCCESS); } return (ISC_R_FAILURE); diff --git a/lib/dns/view.c b/lib/dns/view.c index 3c2118114e..e6313c8f7a 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -279,6 +279,7 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, view, NULL, NULL, NULL); view->viewlist = NULL; view->magic = DNS_VIEW_MAGIC; + /* TODO we might destroy the view (and LOWAC) when we still need it on shutdown! */ view->lowac = dns_lowac_create(view->mctx); *viewp = view;