Replace lock keyfile hashmap with lock pool

Kasp used a lock per zone origin in order to prevent concurrent access
to keyfiles. This lead to substantial memory consumption in the case of
authoritative servers with many small zones, as lots of locks need to be
allocated.

Since the number of keyfile locks taken cannot exceed the number of
helper threads, it makes more sense to use a lock pool of fixed size
keyed by the hash of the origin name, leading to memory savings.

diff --git a/lib/dns/lib.c b/lib/dns/lib.c
index 03e204f1ab73f3157b52c89bbd697b308b5e43f6..25f73684806e88e8d78ca971e5d0eb689bca5cf7 100644 (file)
--- a/lib/dns/lib.c
+++ b/lib/dns/lib.c
@@ -24,6 +24,11 @@
 #include "qp_p.h"
 #include "qpzone_p.h"
 
+void
+dns__zone_keymgmt_initialize(void);
+void
+dns__zone_keymgmt_shutdown(void);
+
 /***
  *** Functions
  ***/
@@ -48,6 +53,7 @@ dns__lib_initialize(void) {
        dns__dyndb_initialize();
        dns__qp_initialize();
        dns__qpzone_initialize();
+       dns__zone_keymgmt_initialize();
 }
 
 void
@@ -56,6 +62,7 @@ dns__lib_shutdown(void) {
                return;
        }
 
+       dns__zone_keymgmt_shutdown();
        dns__qpzone_shutdown();
        dns__qp_shutdown();
        dns__dyndb_shutdown();

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index a459d1d1b0194f341a451a64ce7dd35aea2b3a65..b524a901890ff94a13c51646644a8a2afabab0cc 100644 (file)
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -21,13 +21,13 @@
 #include <isc/atomic.h>
 #include <isc/file.h>
 #include <isc/hash.h>
-#include <isc/hashmap.h>
 #include <isc/hex.h>
 #include <isc/log.h>
 #include <isc/loop.h>
 #include <isc/md.h>
 #include <isc/mutex.h>
 #include <isc/netmgr.h>
+#include <isc/os.h>
 #include <isc/overflow.h>
 #include <isc/random.h>
 #include <isc/ratelimiter.h>
@@ -119,12 +119,6 @@
 #define IO_MAGIC          ISC_MAGIC('Z', 'm', 'I', 'O')
 #define DNS_IO_VALID(load) ISC_MAGIC_VALID(load, IO_MAGIC)
 
-#define KEYMGMT_MAGIC          ISC_MAGIC('M', 'g', 'm', 't')
-#define DNS_KEYMGMT_VALID(load) ISC_MAGIC_VALID(load, KEYMGMT_MAGIC)
-
-#define KEYFILEIO_MAGIC                  ISC_MAGIC('K', 'y', 'I', 'O')
-#define DNS_KEYFILEIO_VALID(kfio) ISC_MAGIC_VALID(kfio, KEYFILEIO_MAGIC)
-
 /*%
  * Ensure 'a' is at least 'min' but not more than 'max'.
  */
@@ -174,7 +168,6 @@ typedef struct dns_stub dns_stub_t;
 typedef struct dns_load dns_load_t;
 typedef struct dns_forward dns_forward_t;
 typedef ISC_LIST(dns_forward_t) dns_forwardlist_t;
-typedef struct dns_keymgmt dns_keymgmt_t;
 typedef struct dns_signing dns_signing_t;
 typedef ISC_LIST(dns_signing_t) dns_signinglist_t;
 typedef struct dns_nsec3chain dns_nsec3chain_t;
@@ -225,27 +218,15 @@ extern bool dns_fuzzing_resolver;
 #endif /* ifdef ENABLE_AFL */
 
 /*%
- *     Hold key file IO locks.
+ * Key file I/O lock pool.
  */
-typedef struct dns_keyfileio {
-       unsigned int magic;
+typedef struct dns_keymgmt_bucket {
        isc_mutex_t lock;
-       isc_refcount_t references;
-       dns_name_t *name;
-       dns_fixedname_t fname;
-} dns_keyfileio_t;
+       uint8_t __padding[ISC_OS_CACHELINE_SIZE -
+                         sizeof(isc_mutex_t) % ISC_OS_CACHELINE_SIZE];
+} dns_keymgmt_bucket_t;
 
-struct dns_keymgmt {
-       unsigned int magic;
-       isc_rwlock_t lock;
-       isc_mem_t *mctx;
-       isc_hashmap_t *table;
-};
-
-/*
- * Initial size of the keymgmt hash table.
- */
-#define DNS_KEYMGMT_HASH_BITS 12
+static dns_keymgmt_bucket_t keymgmt_buckets_g[1024];
 
 typedef struct dns_rad {
        isc_mem_t *mctx;
@@ -319,7 +300,6 @@ struct dns_zone {
        isc_stdtime_t key_expiry;
        isc_stdtime_t log_key_expired_timer;
        char *keydirectory;
-       dns_keyfileio_t *kfio;
        dns_xfrin_t *xfr;
 
        uint32_t maxrefresh;
@@ -627,8 +607,6 @@ struct dns_zonemgr {
        unsigned int startupserialqueryrate;
        dns_keystorelist_t *keystores;
 
-       dns_keymgmt_t *keymgmt;
-
        isc_tlsctx_cache_t *tlsctx_cache;
        isc_rwlock_t tlsctx_cache_rwlock;
 };
@@ -18360,121 +18338,24 @@ dns_zone_first(dns_zonemgr_t *zmgr, dns_zone_t **first) {
  ***   Zone manager.
  ***/
 
-static void
-zonemgr_keymgmt_init(dns_zonemgr_t *zmgr) {
-       dns_keymgmt_t *mgmt = isc_mem_get(zmgr->mctx, sizeof(*mgmt));
-
-       *mgmt = (dns_keymgmt_t){
-               .magic = KEYMGMT_MAGIC,
-       };
-
-       isc_mem_attach(zmgr->mctx, &mgmt->mctx);
-       isc_rwlock_init(&mgmt->lock);
-       isc_hashmap_create(mgmt->mctx, DNS_KEYMGMT_HASH_BITS, &mgmt->table);
-
-       zmgr->keymgmt = mgmt;
+static isc_mutex_t *
+zone_keymgmt_getlock(dns_zone_t *zone) {
+       uint32_t hash = dns_name_hash(&zone->origin);
+       return &keymgmt_buckets_g[hash % ARRAY_SIZE(keymgmt_buckets_g)].lock;
 }
 
-static void
-zonemgr_keymgmt_destroy(dns_zonemgr_t *zmgr) {
-       dns_keymgmt_t *mgmt = zmgr->keymgmt;
-
-       REQUIRE(DNS_KEYMGMT_VALID(mgmt));
-
-       mgmt->magic = 0;
-
-       RWLOCK(&mgmt->lock, isc_rwlocktype_write);
-       INSIST(isc_hashmap_count(mgmt->table) == 0);
-       RWUNLOCK(&mgmt->lock, isc_rwlocktype_write);
-       isc_hashmap_destroy(&mgmt->table);
-
-       isc_rwlock_destroy(&mgmt->lock);
-       isc_mem_putanddetach(&mgmt->mctx, mgmt, sizeof(dns_keymgmt_t));
-}
-
-static bool
-kfio_match(void *node, const void *key) {
-       const dns_keyfileio_t *kfio = node;
-
-       return dns_name_equal(kfio->name, key);
-}
-
-static void
-zonemgr_keymgmt_add(dns_zonemgr_t *zmgr, dns_zone_t *zone,
-                   dns_keyfileio_t **added) {
-       dns_keymgmt_t *mgmt = zmgr->keymgmt;
-       dns_keyfileio_t *kfio = NULL;
-       isc_result_t result;
-       dns_fixedname_t fname;
-       dns_name_t *name;
-
-       REQUIRE(DNS_KEYMGMT_VALID(mgmt));
-       REQUIRE(added != NULL && *added == NULL);
-
-       name = dns_fixedname_initname(&fname);
-       dns_name_downcase(&zone->origin, name);
-
-       RWLOCK(&mgmt->lock, isc_rwlocktype_write);
-
-       result = isc_hashmap_find(mgmt->table, dns_name_hash(name), kfio_match,
-                                 name, (void **)&kfio);
-       switch (result) {
-       case ISC_R_SUCCESS:
-               isc_refcount_increment(&kfio->references);
-               break;
-       case ISC_R_NOTFOUND:
-               kfio = isc_mem_get(mgmt->mctx, sizeof(*kfio));
-               *kfio = (dns_keyfileio_t){
-                       .magic = KEYFILEIO_MAGIC,
-               };
-               isc_refcount_init(&kfio->references, 1);
-               kfio->name = dns_fixedname_initname(&kfio->fname);
-               dns_name_copy(name, kfio->name);
-
-               isc_mutex_init(&kfio->lock);
-               result = isc_hashmap_add(mgmt->table, dns_name_hash(kfio->name),
-                                        kfio_match, kfio->name, kfio, NULL);
-               INSIST(result == ISC_R_SUCCESS);
-               break;
-       default:
-               UNREACHABLE();
+void
+dns__zone_keymgmt_initialize(void) {
+       for (size_t idx = 0; idx < ARRAY_SIZE(keymgmt_buckets_g); ++idx) {
+               isc_mutex_init(&keymgmt_buckets_g[idx].lock);
        }
-       *added = kfio;
-       RWUNLOCK(&mgmt->lock, isc_rwlocktype_write);
-}
-
-static bool
-match_ptr(void *node, const void *key) {
-       return node == key;
 }
 
-static void
-zonemgr_keymgmt_delete(dns_zonemgr_t *zmgr, dns_keyfileio_t **deleted) {
-       REQUIRE(DNS_KEYMGMT_VALID(zmgr->keymgmt));
-       REQUIRE(deleted != NULL && DNS_KEYFILEIO_VALID(*deleted));
-
-       dns_keymgmt_t *mgmt = zmgr->keymgmt;
-       dns_keyfileio_t *kfio = *deleted;
-       isc_result_t result;
-
-       *deleted = NULL;
-
-       RWLOCK(&mgmt->lock, isc_rwlocktype_write);
-
-       if (isc_refcount_decrement(&kfio->references) == 1) {
-               isc_refcount_destroy(&kfio->references);
-               kfio->magic = 0;
-               isc_mutex_destroy(&kfio->lock);
-
-               result = isc_hashmap_delete(mgmt->table,
-                                           dns_name_hash(kfio->name),
-                                           match_ptr, kfio);
-               INSIST(result == ISC_R_SUCCESS);
-
-               isc_mem_put(mgmt->mctx, kfio, sizeof(*kfio));
+void
+dns__zone_keymgmt_shutdown(void) {
+       for (size_t idx = 0; idx < ARRAY_SIZE(keymgmt_buckets_g); ++idx) {
+               isc_mutex_destroy(&keymgmt_buckets_g[idx].lock);
        }
-
-       RWUNLOCK(&mgmt->lock, isc_rwlocktype_write);
 }
 
 void
@@ -18513,9 +18394,6 @@ dns_zonemgr_create(isc_mem_t *mctx, dns_zonemgr_t **zmgrp) {
                isc_mem_create("zonemgr-mctxpool", &zmgr->mctxpool[i]);
        }
 
-       /* Key file I/O locks. */
-       zonemgr_keymgmt_init(zmgr);
-
        /* Default to 20 refresh queries / notifies / checkds per second. */
        setrl(zmgr->checkdsrl, &zmgr->checkdsrate, 20);
        setrl(zmgr->notifyrl, &zmgr->notifyrate, 20);
@@ -18573,9 +18451,6 @@ dns_zonemgr_managezone(dns_zonemgr_t *zmgr, dns_zone_t *zone) {
        isc_loop_t *loop = isc_loop_get(zone->tid);
        isc_loop_attach(loop, &zone->loop);
 
-       zonemgr_keymgmt_add(zmgr, zone, &zone->kfio);
-       INSIST(zone->kfio != NULL);
-
        ISC_LIST_APPEND(zmgr->zones, zone, link);
        zone->zmgr = zmgr;
 
@@ -18597,11 +18472,6 @@ dns_zonemgr_releasezone(dns_zonemgr_t *zmgr, dns_zone_t *zone) {
 
        ISC_LIST_UNLINK(zmgr->zones, zone, link);
 
-       if (zone->kfio != NULL) {
-               zonemgr_keymgmt_delete(zmgr, &zone->kfio);
-               ENSURE(zone->kfio == NULL);
-       }
-
        if (zone->timer != NULL) {
                isc_refcount_decrement(&zone->irefs);
                isc_timer_destroy(&zone->timer);
@@ -18711,8 +18581,6 @@ zonemgr_free(dns_zonemgr_t *zmgr) {
        isc_rwlock_destroy(&zmgr->rwlock);
        isc_rwlock_destroy(&zmgr->tlsctx_cache_rwlock);
 
-       zonemgr_keymgmt_destroy(zmgr);
-
        if (zmgr->tlsctx_cache != NULL) {
                isc_tlsctx_cache_detach(&zmgr->tlsctx_cache);
        }
@@ -19319,8 +19187,7 @@ dns_zone_lock_keyfiles(dns_zone_t *zone) {
                return;
        }
 
-       REQUIRE(DNS_KEYFILEIO_VALID(zone->kfio));
-       isc_mutex_lock(&zone->kfio->lock);
+       isc_mutex_lock(zone_keymgmt_getlock(zone));
 }
 
 void
@@ -19332,8 +19199,7 @@ dns_zone_unlock_keyfiles(dns_zone_t *zone) {
                return;
        }
 
-       REQUIRE(DNS_KEYFILEIO_VALID(zone->kfio));
-       isc_mutex_unlock(&zone->kfio->lock);
+       isc_mutex_unlock(zone_keymgmt_getlock(zone));
 }
 
 isc_result_t

diff --git a/lib/dns/zone_p.h b/lib/dns/zone_p.h
index 158ee0d8a8e57e9373a9b45834fdc5c1435a106f..ef891869584ddc2145ed110a34cbb2db94ffc2f6 100644 (file)
--- a/lib/dns/zone_p.h
+++ b/lib/dns/zone_p.h
@@ -207,6 +207,12 @@ dns__zone_free(dns_zone_t *zone);
 
 bool
 dns__zone_free_check(dns_zone_t *zone);
+
+void
+dns__zone_keymgmt_initialize(void);
+
+void
+dns__zone_keymgmt_shutdown(void);
 /*
  *     Check if a zone is ready to be freed.
  *