Use SIEVE for TSIG generated-key LRU

The generated-key cache used a strict LRU: every lookup hit promoted
the key to the tail of the list under the write lock, even when the
caller arrived holding only the read lock.  That promotion was both
a lock upgrade and a list reshuffle on the hot path.

Replace the LRU with the SIEVE eviction policy.  Lookups now do a
lock-free atomic mark of the visited bit; eviction work moves to
insertion time, which already holds the write lock.

diff --git a/lib/dns/include/dns/tsig.h b/lib/dns/include/dns/tsig.h
index cd72ed429d0a6920db0f9dee335d24c5d0d64789..66ec1ff0a5b4d2c5b91c85b73a4afce901787f64 100644 (file)
--- a/lib/dns/include/dns/tsig.h
+++ b/lib/dns/include/dns/tsig.h
@@ -20,6 +20,7 @@
 #include <isc/hashmap.h>
 #include <isc/refcount.h>
 #include <isc/rwlock.h>
+#include <isc/sieve.h>
 #include <isc/stdio.h>
 #include <isc/stdtime.h>
 
@@ -67,13 +68,11 @@ struct dns_tsigkeyring {
        unsigned int   writecount;
        isc_rwlock_t   lock;
        isc_mem_t     *mctx;
-       /*
-        * LRU list of generated key along with a count of the keys on the
-        * list and a maximum size.
-        */
-       unsigned int generated;
-       ISC_LIST(dns_tsigkey_t) lru;
+
+       unsigned int   generated;
        isc_refcount_t references;
+
+       ISC_SIEVE(dns_tsigkey_t) lrulist;
 };
 
 struct dns_tsigkey {
@@ -92,7 +91,9 @@ struct dns_tsigkey {
        isc_stdtime_t  inception;     /*%< start of validity period */
        isc_stdtime_t  expire;        /*%< end of validity period */
        isc_refcount_t references;    /*%< reference counter */
-       ISC_LINK(dns_tsigkey_t) link;
+
+       bool visited;
+       ISC_LINK(dns_tsigkey_t) lrulink; /*%< SIEVE LRU link */
 };
 
 const dns_name_t *

diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c
index cf5e41192457839a4e3c1b7561171175807f2a8e..c1b5d450c0705b626eb35f4af1066e9d608718b2 100644 (file)
--- a/lib/dns/tsig.c
+++ b/lib/dns/tsig.c
@@ -150,22 +150,6 @@ match_ptr(void *node, const void *key) {
        return node == key;
 }
 
-static void
-adjust_lru(dns_tsigkeyring_t *ring, dns_tsigkey_t *tkey) {
-       if (tkey->generated) {
-               RWLOCK(&ring->lock, isc_rwlocktype_write);
-               /*
-                * We may have been removed from the LRU list between
-                * removing the read lock and acquiring the write lock.
-                */
-               if (ISC_LINK_LINKED(tkey, link) && ring->lru.tail != tkey) {
-                       ISC_LIST_UNLINK(ring->lru, tkey, link);
-                       ISC_LIST_APPEND(ring->lru, tkey, link);
-               }
-               RWUNLOCK(&ring->lock, isc_rwlocktype_write);
-       }
-}
-
 isc_result_t
 dns_tsigkey_createfromkey(const dns_name_t *name, dst_algorithm_t algorithm,
                          dst_key_t *dstkey, bool generated, bool restored,
@@ -187,7 +171,7 @@ dns_tsigkey_createfromkey(const dns_name_t *name, dst_algorithm_t algorithm,
                .expire = expire,
                .alg = algorithm,
                .algname = DNS_NAME_INITEMPTY,
-               .link = ISC_LINK_INITIALIZER,
+               .lrulink = ISC_LINK_INITIALIZER,
        };
 
        tkey->name = dns_fixedname_initname(&tkey->fn);
@@ -252,8 +236,8 @@ cleanup_name:
 
 static void
 dns__tsigkey_deletelru(dns_tsigkeyring_t *ring, dns_tsigkey_t *tkey) {
-       if (tkey->generated && ISC_LINK_LINKED(tkey, link)) {
-               ISC_LIST_UNLINK(ring->lru, tkey, link);
+       if (tkey->generated && ISC_SIEVE_LINKED(tkey, lrulink)) {
+               ISC_SIEVE_UNLINK(ring->lrulist, tkey, lrulink);
                ring->generated--;
        }
 }
@@ -1491,7 +1475,9 @@ again:
        }
        dns_tsigkey_ref(key);
        RWUNLOCK(&ring->lock, locktype);
-       adjust_lru(ring, key);
+       if (key->generated) {
+               ISC_SIEVE_MARK(key, visited);
+       }
        *tsigkey = key;
        return ISC_R_SUCCESS;
 }
@@ -1538,14 +1524,15 @@ dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsigkeyring_t **ringp) {
 
        ring = isc_mem_get(mctx, sizeof(dns_tsigkeyring_t));
        *ring = (dns_tsigkeyring_t){
-               .lru = ISC_LIST_INITIALIZER,
+               .magic = TSIGKEYRING_MAGIC,
+               .mctx = isc_mem_ref(mctx),
+               .references = ISC_REFCOUNT_INITIALIZER(1),
        };
 
+       ISC_SIEVE_INIT(ring->lrulist);
+
        isc_hashmap_create(mctx, 12, &ring->keys);
        isc_rwlock_init(&ring->lock);
-       isc_mem_attach(mctx, &ring->mctx);
-       isc_refcount_init(&ring->references, 1);
-       ring->magic = TSIGKEYRING_MAGIC;
 
        *ringp = ring;
 }
@@ -1566,13 +1553,13 @@ dns_tsigkeyring_add(dns_tsigkeyring_t *ring, dns_tsigkey_t *tkey) {
                /*
                 * If this is a TKEY-generated key, add it to the LRU list,
                 * and if we've exceeded the quota for generated keys,
-                * remove the least recently used one from the both the
-                * list and the RBT.
+                * delete the least recently used one.
                 */
                if (tkey->generated) {
-                       ISC_LIST_APPEND(ring->lru, tkey, link);
+                       ISC_SIEVE_INSERT(ring->lrulist, tkey, lrulink);
                        if (++ring->generated > DNS_TSIG_MAXGENERATEDKEYS) {
-                               dns_tsigkey_t *key = ISC_LIST_HEAD(ring->lru);
+                               dns_tsigkey_t *key = ISC_SIEVE_NEXT(
+                                       ring->lrulist, visited, lrulink);
                                dns__tsigkey_delete(ring, key);
                        }
                }