277 lines
8.9 KiB
Diff
277 lines
8.9 KiB
Diff
From cedf1d41e0fb768d3a7252544be7ec1540444674 Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@sury.org>
|
|
Date: Tue, 23 Jun 2026 10:59:38 +0200
|
|
Subject: [PATCH] Make the dns_slabheaders in the cache reference counted
|
|
|
|
Instead of only reference counting the enclosing qpcnode, add the
|
|
reference counting directly to the slabheaders. The reference is
|
|
incremented when an rdataset is bound to the header and decremented when
|
|
the rdataset is disassociated, so a stale slabheader can be removed from
|
|
the node's down chain as soon as its own reference count reaches zero,
|
|
instead of waiting for the whole qpcnode to become unreferenced.
|
|
|
|
Building on that, clean up the ancient headers eagerly: mark_ancient()
|
|
is made idempotent, releases the header's own (container) reference and
|
|
reaps the stale headers from the node's down chain as soon as their
|
|
references reach zero. A header evicted over the per-name type limit is
|
|
expired only after the new rdataset has been bound, so the bind's
|
|
increment always precedes mark_ancient()'s decrement.
|
|
|
|
Because a header can now be reclaimed independently of its node, the
|
|
rdataset iterators must keep the header they are positioned on alive:
|
|
each iterator takes a reference on its current header and releases it
|
|
when it advances or is destroyed. Iteration otherwise stays lazy and
|
|
re-reads the node on every step, so it still observes records added to
|
|
the node while the iterator is live, as zone signing requires.
|
|
|
|
The slab headers are shared with the zone databases, so the matching
|
|
increment is added to every bind path. The noqname/closest proofs hand
|
|
out rdatasets backed by bare slabs that have no header, so they are
|
|
given a separate dns_rdataproof_rdatasetmethods that leaves the
|
|
reference count untouched.
|
|
|
|
(cherry picked from commit 2dabf117e1264fd13fb33096f87e78a039fd1c6c)
|
|
---
|
|
bin/tests/system/reclimit/tests.sh | 4 +-
|
|
lib/dns/include/dns/rdataslab.h | 1 +
|
|
lib/dns/rbtdb.c | 77 +++++++++++++++++++++++++-----
|
|
3 files changed, 69 insertions(+), 13 deletions(-)
|
|
|
|
diff --git a/bin/tests/system/reclimit/tests.sh b/bin/tests/system/reclimit/tests.sh
|
|
index ba574ad..112d2de 100644
|
|
--- a/bin/tests/system/reclimit/tests.sh
|
|
+++ b/bin/tests/system/reclimit/tests.sh
|
|
@@ -313,13 +313,13 @@ echo_i "checking that NXDOMAIN names over the max-types-per-name limit don't get
|
|
|
|
# Query for 10 NXDOMAIN types
|
|
for ntype in $(seq 65270 65279); do
|
|
- check_manytypes 1 manytypes.big "TYPE${ntype}" NOERROR big SOA 0 || ret=1
|
|
+ check_manytypes 1 manytypes.big "TYPE${ntype}" NOERROR big SOA 60 || ret=1
|
|
done
|
|
# Wait at least 1 second
|
|
sleep 1
|
|
# Query for 10 NXDOMAIN types again - these should not be cached
|
|
for ntype in $(seq 65270 65279); do
|
|
- check_manytypes 2 manytypes.big "TYPE${ntype}" NOERROR big SOA 0 || ret=1
|
|
+ check_manytypes 2 manytypes.big "TYPE${ntype}" NOERROR big SOA 60 || ret=1
|
|
done
|
|
|
|
if [ $ret -ne 0 ]; then echo_i "failed"; fi
|
|
diff --git a/lib/dns/include/dns/rdataslab.h b/lib/dns/include/dns/rdataslab.h
|
|
index f2f3513..6e1a06a 100644
|
|
--- a/lib/dns/include/dns/rdataslab.h
|
|
+++ b/lib/dns/include/dns/rdataslab.h
|
|
@@ -43,6 +43,7 @@
|
|
#include <stdbool.h>
|
|
|
|
#include <isc/lang.h>
|
|
+#include <isc/refcount.h>
|
|
|
|
#include <dns/types.h>
|
|
|
|
diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
|
|
index 7586e2a..5f50c79 100644
|
|
--- a/lib/dns/rbtdb.c
|
|
+++ b/lib/dns/rbtdb.c
|
|
@@ -199,6 +199,7 @@ struct noqname {
|
|
};
|
|
|
|
typedef struct rdatasetheader {
|
|
+ isc_refcount_t references;
|
|
/*%
|
|
* Locked by the owning node's lock.
|
|
*/
|
|
@@ -1514,6 +1515,7 @@ init_rdataset(dns_rbtdb_t *rbtdb, rdatasetheader_t *h) {
|
|
h->node_is_relative = 0;
|
|
atomic_init(&h->attributes, 0);
|
|
atomic_init(&h->last_refresh_fail_ts, 0);
|
|
+ isc_refcount_init(&h->references, 1);
|
|
|
|
#ifndef ISC_MUTEX_ATOMICS
|
|
STATIC_ASSERT((sizeof(h->attributes) == 2),
|
|
@@ -1646,6 +1648,9 @@ rollback_node(dns_rbtnode_t *node, rbtdb_serial_t serial) {
|
|
}
|
|
}
|
|
|
|
+static void
|
|
+clean_stale_headers(dns_rbtdb_t *rbtdb, isc_mem_t *mctx, rdatasetheader_t *top);
|
|
+
|
|
static inline void
|
|
mark_header_ancient(dns_rbtdb_t *rbtdb, rdatasetheader_t *header) {
|
|
uint_least16_t attributes = atomic_load_acquire(&header->attributes);
|
|
@@ -1671,8 +1676,12 @@ mark_header_ancient(dns_rbtdb_t *rbtdb, rdatasetheader_t *header) {
|
|
update_rrsetstats(rbtdb, header->type, attributes, false);
|
|
header->node->dirty = 1;
|
|
|
|
+ isc_refcount_decrement(&header->references);
|
|
+
|
|
/* Increment the stats counter for the ancient RRtype. */
|
|
update_rrsetstats(rbtdb, header->type, newattributes, true);
|
|
+
|
|
+ clean_stale_headers(rbtdb, rbtdb->common.mctx, header);
|
|
}
|
|
|
|
static inline void
|
|
@@ -1708,12 +1717,19 @@ static inline void
|
|
clean_stale_headers(dns_rbtdb_t *rbtdb, isc_mem_t *mctx,
|
|
rdatasetheader_t *top) {
|
|
rdatasetheader_t *d, *down_next;
|
|
+ rdatasetheader_t *down_parent = top;
|
|
|
|
for (d = top->down; d != NULL; d = down_next) {
|
|
down_next = d->down;
|
|
- free_rdataset(rbtdb, mctx, d);
|
|
+ d->next = down_parent;
|
|
+
|
|
+ if (isc_refcount_current(&d->references) == 0) {
|
|
+ free_rdataset(rbtdb, mctx, d);
|
|
+ down_parent->down = down_next;
|
|
+ } else {
|
|
+ down_parent = d;
|
|
+ }
|
|
}
|
|
- top->down = NULL;
|
|
}
|
|
|
|
static inline void
|
|
@@ -1729,6 +1745,7 @@ clean_cache_node(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node) {
|
|
for (current = node->data; current != NULL; current = top_next) {
|
|
top_next = current->next;
|
|
clean_stale_headers(rbtdb, mctx, current);
|
|
+ INSIST(current->down == NULL);
|
|
/*
|
|
* If current is nonexistent, ancient, or stale and
|
|
* we are not keeping stale, we can clean it up.
|
|
@@ -3196,6 +3213,8 @@ bind_rdataset(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node, rdatasetheader_t *header,
|
|
return;
|
|
}
|
|
|
|
+ isc_refcount_increment(&header->references);
|
|
+
|
|
new_reference(rbtdb, node, locktype);
|
|
|
|
INSIST(rdataset->methods == NULL); /* We must be disassociated. */
|
|
@@ -6258,6 +6277,7 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, const dns_name_t *nodename,
|
|
bool header_nx;
|
|
bool newheader_nx;
|
|
bool merge;
|
|
+ bool do_expireheader = false;
|
|
dns_rdatatype_t rdtype, covers;
|
|
rbtdb_rdatatype_t negtype, sigtype;
|
|
dns_trust_t trust;
|
|
@@ -6809,6 +6829,7 @@ find_header:
|
|
}
|
|
|
|
if (IS_CACHE(rbtdb) && overmaxtype(rbtdb, ntypes)) {
|
|
+ do_expireheader = true;
|
|
if (expireheader == NULL) {
|
|
expireheader = newheader;
|
|
}
|
|
@@ -6822,15 +6843,6 @@ find_header:
|
|
*/
|
|
expireheader = newheader;
|
|
}
|
|
-
|
|
- set_ttl(rbtdb, expireheader, 0);
|
|
- mark_header_ancient(rbtdb, expireheader);
|
|
- /*
|
|
- * FIXME: In theory, we should mark the RRSIG
|
|
- * and the header at the same time, but there is
|
|
- * no direct link between those two header, so
|
|
- * we would have to check the whole list again.
|
|
- */
|
|
}
|
|
}
|
|
}
|
|
@@ -6853,6 +6865,15 @@ find_header:
|
|
isc_rwlocktype_write, addedrdataset);
|
|
}
|
|
|
|
+ /*
|
|
+ * We need to delay the expiration of the header until we are bound to
|
|
+ * it to prevent decrement-then-increment on the header references.
|
|
+ */
|
|
+ if (do_expireheader) {
|
|
+ set_ttl(rbtdb, expireheader, 0);
|
|
+ mark_header_ancient(rbtdb, expireheader);
|
|
+ }
|
|
+
|
|
return (ISC_R_SUCCESS);
|
|
}
|
|
|
|
@@ -9102,6 +9123,12 @@ rdataset_disassociate(dns_rdataset_t *rdataset) {
|
|
dns_db_t *db = rdataset->private1;
|
|
dns_dbnode_t *node = rdataset->private2;
|
|
|
|
+ if (rdataset->methods == &rdataset_methods) {
|
|
+ rdatasetheader_t *header = rdataset->private3;
|
|
+ header--;
|
|
+ isc_refcount_decrement(&header->references);
|
|
+ }
|
|
+
|
|
detachnode(db, &node);
|
|
}
|
|
|
|
@@ -9215,6 +9242,11 @@ rdataset_clone(dns_rdataset_t *source, dns_rdataset_t *target) {
|
|
dns_dbnode_t *cloned_node = NULL;
|
|
|
|
attachnode(db, node, &cloned_node);
|
|
+ if (source->methods == &rdataset_methods) {
|
|
+ rdatasetheader_t *header = source->private3;
|
|
+ header--;
|
|
+ isc_refcount_increment(&header->references);
|
|
+ }
|
|
INSIST(!ISC_LINK_LINKED(target, link));
|
|
*target = *source;
|
|
ISC_LINK_INIT(target, link);
|
|
@@ -9378,6 +9410,11 @@ rdatasetiter_destroy(dns_rdatasetiter_t **iteratorp) {
|
|
|
|
rbtiterator = (rbtdb_rdatasetiter_t *)(*iteratorp);
|
|
|
|
+ if (rbtiterator->current != NULL) {
|
|
+ isc_refcount_decrement(&rbtiterator->current->references);
|
|
+ rbtiterator->current = NULL;
|
|
+ }
|
|
+
|
|
if (rbtiterator->common.version != NULL) {
|
|
closeversion(rbtiterator->common.db,
|
|
&rbtiterator->common.version, false);
|
|
@@ -9441,9 +9478,18 @@ rdatasetiter_first(dns_rdatasetiter_t *iterator) {
|
|
}
|
|
}
|
|
|
|
+ if (header != NULL) {
|
|
+ isc_refcount_increment0(&header->references);
|
|
+ }
|
|
+
|
|
NODE_UNLOCK(&rbtdb->node_locks[rbtnode->locknum].lock,
|
|
isc_rwlocktype_read);
|
|
|
|
+ if (rbtiterator->current != NULL) {
|
|
+ isc_refcount_decrement(&rbtiterator->current->references);
|
|
+ rbtiterator->current = NULL;
|
|
+ }
|
|
+
|
|
rbtiterator->current = header;
|
|
|
|
if (header == NULL) {
|
|
@@ -9525,9 +9571,18 @@ rdatasetiter_next(dns_rdatasetiter_t *iterator) {
|
|
}
|
|
}
|
|
|
|
+ if (header != NULL) {
|
|
+ isc_refcount_increment0(&header->references);
|
|
+ }
|
|
+
|
|
NODE_UNLOCK(&rbtdb->node_locks[rbtnode->locknum].lock,
|
|
isc_rwlocktype_read);
|
|
|
|
+ if (rbtiterator->current != NULL) {
|
|
+ isc_refcount_decrement(&rbtiterator->current->references);
|
|
+ rbtiterator->current = NULL;
|
|
+ }
|
|
+
|
|
rbtiterator->current = header;
|
|
|
|
if (header == NULL) {
|