Colin Vidal [Mon, 8 Sep 2025 12:58:47 +0000 (14:58 +0200)]
cfg_aclconfctx_t object is part of named_server
`named_g_actconfctx` is a global variable holding the ACL configuration
context alive (in particular, to dynamically load zones). However, this
object is build once per configuration (early) and is used only inside
server.c `apply_configuration` flow. (Two exceptions: the shutdown flow,
still in server.c and plugin check flow, which doesn't need it, so it's
NULL in such case).
Instead of leaving this global publicly exposed, it is now part of the
`named_server_t` object. This allows us to clearly see that, when
reconfigureing the server, the new instance of the ACL context is known
only by the newly built object and not currently used by "production"
object; and will help to move move logic before the exclusive mode is
taken.
The other advantage is that the ACL configuration context can now be
built before the exclusive lock as well.
Colin Vidal [Thu, 28 Aug 2025 15:29:23 +0000 (17:29 +0200)]
apply_configuration: add configure_kasplist
The kasplist (dnssec-policy defined in the builtin and global
configuration options) was built inside apply_configuration. This
commit extracts this logic into its separate function.
In order to make the view configuration independent of the global
`server` object, the newly built kasplist is now passed as parameter.
(This eventually will help to be able to configure the views outside of
the exclusive mode by limiting its dependency to the global
`server`/`named_g_server`).
Colin Vidal [Tue, 26 Aug 2025 11:19:10 +0000 (13:19 +0200)]
apply_configuration: remove builtin_viewlist
When creating/configuring the view, the user-defined views are built and
set into the viewlist, then builtin-view inside the builtin_viewlist.
But there is no seperate logic applied to those two lists, and they are
immediately merged into viewlist right after. This commit removes this
intermediate list and add builtin-views directly into the main viewlist
instead.
Colin Vidal [Tue, 26 Aug 2025 10:35:56 +0000 (12:35 +0200)]
refactor view creation/config in apply_configuration
In order to help splitting apply_configuration, the inline loops and bit
of logic around it for views creation and configuration, each of those
are now in a dedicatated function.
chg: dev: Use lock-free hashtable for storing resolver fetch contexts
Replace the locked hashmap with the lock-free hashtable from the RCU
library and protect the fetch contexts against reuse by replacing the
libisc reference counting with urcu_ref that can soft-fail in situation
where the reference count is already zero. This allows us to easily
skip re-using the fetch context if it is already in process of being
destroyed.
Merge branch 'ondrej/use-urcu-lfht-for-resolver-tables' into 'main'
Use lock-free hashtable for storing resolver fetch contexts
Previously, the fetch contexts were stored inside rwlocked hashmap
table. This was one of the most contended places for the resolver,
especially in the cold cache situation.
Replace the locked hashmap with the lock-free hashtable from the RCU
library and protect the fetch contexts against reuse by replacing the
libisc reference counting with urcu_ref that can soft-fail in situation
where the reference count is already zero. This allows us to easily
skip re-using the fetch context if it is already in process of being
destroyed.
chg: dev: Add a circular reference between slabtops for type and RRSIG(type)
Previously, the slabtops for "type" and its signature was only loosely
coupled and the headers could expire at different time (both TTL and LRU
based expiry). Add a .related member to the slabtop that allows us to
expire the headers in both related headers and also optimize the lookups
because now both slabtops are looked up at the same time.
Closes #3396
Merge branch '3396-bind-rrsigs-to-records' into 'main'
Previously, the slabtops for "type" and its signature was only loosely
coupled and the headers could expire at different time (both TTL and LRU
based expiry). This commit expires the headers in both related
headers.
Add a circular reference between slabtops for type and RRSIG(type)
Previously, the slabtops for "type" and its signature was only loosely
coupled. Add a .related member to the slabtop that allows us to
optimize the lookups because now both slabtops are looked up at the
same time.
There was a pattern where first the header was checked for NULL
and then for being stale. In both cases the code path is the same
so it makes sense to put them in a separate function.
chg: dev: Convert slabtop and slabheader to use the cds list
This is the first MR in series that aims to reduce the node locking
by replacing the single-linked list of slabtop(s) and slabheader(s)
with CDS linked list. This commit doesn't do anything else beyond
replacing .next and .down links with the cds_list_head. The RCU
semantics will be added later.
Merge branch 'ondrej/use-rcu-list-for-slabtop' into 'main'
This is the second commit in series that aims to reduce the node locking
by replacing the single-linked list of slabheader(s) with CDS linked list.
This commit doesn't do anything else beyond replacing .next link with
the cds_list_head. RCU semantics is going to be added in the subsequent
commits.
This is the first commit in series that aims to reduce the node locking
by replacing the single-linked list of slabtop(s) with CDS linked list.
This commit doesn't do anything else beyond replacing .next link with
the cds_list_head. RCU semantics is going to be added in the subsequent
commits.
fix: dev: Fix datarace between unlocking fctx lock and shuttingdown fctx
There was a data race where new fetch response could be added to the
fetch context after we unlock the fetch context and before we shut it
down. This could cause assertion failure when fctx__done() was called
with ISC_R_SUCCESS because there was originally no fetch response, but
new fetch response without associated dataset was added before we had a
chance to shutdown the fetch context. This manifested in the
validated() callback, where cache_rrset() now returns ISC_R_SUCCESS
instead of DNS_R_UNCHANGED when cache was not changed. However the data
race was wrong on a general level.
Add new argument to fctx__done() that allows to call it with fctx->lock
already acquired to prevent these data races.
Closes #5507
Merge branch '5507-dont-release-fctx-lock-on-done' into 'main'
Fix datarace between unlocking fctx lock and shuttingdown fctx
There was a data race where new fetch response could be added to the
fetch context after we unlock the fetch context and before we shut it
down. This could cause assertion failure when fctx__done() was called
with ISC_R_SUCCESS because there was originally no fetch response, but
new fetch response without associated dataset was added before we had a
chance to shutdown the fetch context. This manifested in the
validated() callback, where cache_rrset() now returns ISC_R_SUCCESS
instead of DNS_R_UNCHANGED when cache was not changed. However the data
race was wrong on a general level.
When the fctx__done() is called with ISC_R_SUCCESS as result is expects
the fctx->lock to be already acquired to prevent these data races.
Split the fctx_done() into success and failure variants
The split will allow us to call fctx__done() with fctx->lock acquired
when it is called with ISC_R_SUCESS to prevent data races when finishing
the fetch context.
chg: ci: Only run relevant CI jobs based on the changes
Trigger selected CI jobs on MR automatically only if there are related
code changes. Otherwise, offer an option to run the jobs manually in
MRs. For other sources, like schedules, tags etc., execute the jobs as
usual.
Merge branch 'nicki/ci-restrict-rules-changes' into 'main'
Trigger selected CI jobs on MR automatically only if there are related
code changes. Otherwise, offer an option to run the jobs manually in
MRs. For other sources, like schedules, tags etc., execute the jobs as
usual.
Colin Vidal [Wed, 17 Sep 2025 15:38:54 +0000 (17:38 +0200)]
fix: usr: preserve cache when reload fails and reload the server again
Fixes an issue where failing to reconfigure/reload the server would prevent to preserved the views caches on the subsequent server reconfiguration/reload.
Colin Vidal [Tue, 16 Sep 2025 15:14:33 +0000 (17:14 +0200)]
preserve cache when reload fails
If the server is reloaded, new views are created and preexisting cache
is attached to those _but_ something goes wrong later, the previous
views are restored but the previous cache list is destroyed. This makes
the subsequent reload to drop the existing cache. This fixes it by
avoiding a mutation of the old cache list.
Colin Vidal [Tue, 16 Sep 2025 15:14:46 +0000 (17:14 +0200)]
test that cache is preserved on reconfing failure
A named bug scrap the cache on a second reload after an initial reload
failure. Adds a test checking that the cache is preserved between server
reconfiguration/reloads even if it fails at some point (after attempting
to re-use the cache) and the server is re-loaded later.
The dns_qpcache already had all the namespace changes needed to put the
normal data and auxiliary NSEC data into a single tree. Remove the
extra nsec QP trie and use the single QP trie for all the cache data.
Merge branch 'ondrej/use-qp-namespace-in-cache' into 'main'
As we removed the ability to count nodes in the auxiliary trees (because
there are no auxiliary trees), we can also cleanup the API and
associated enum type (dns_dbtree_t).
The dns_qpcache already had all the namespace changes needed to put the
normal data and auxiliary NSEC data into a single tree. Remove the
extra nsec QP trie and use the single QP trie for all the cache data.
Remove the dbiterator_{last,prev} from the qpcache
The dbiterator_{last,prev} functions are not used in the cache, and the
implementation would get quite complicated when we squash the main and
nsec trees together. It's easier to just not implement these.
Petr Špaček [Fri, 11 Jul 2025 19:10:19 +0000 (21:10 +0200)]
Add ability to load root zone into AsyncServer
We would prefer if explicit $ORIGIN is used only for root zone and
nothing else, solely to avoid zone files named "..db". For all other
zones the file name should match zone name.
Some of the API calls in `dns_db` were obsolete, and have been removed. Others were more complicated than necessary, and have been refactored to simplify.
Evan Hunt [Thu, 14 Aug 2025 23:28:11 +0000 (16:28 -0700)]
remove dns_db_{un,}locknode
remove the dns_db_locknode() and _unlocknode() calls, so that callers no
longer have the ability to directly manipulate the internal locking of
cache and zone databases.
Evan Hunt [Mon, 11 Aug 2025 19:13:53 +0000 (12:13 -0700)]
make getoriginnode implementation optional
if the dns_db_getoriginnode() call is not implemented, we can
fall back to running dns_db_findnode() on the database origin.
we now only implement getoriginnode directly in databases where
it's clearly faster than the fallback implementation would be.
Evan Hunt [Thu, 7 Aug 2025 21:24:01 +0000 (14:24 -0700)]
minor cleanup in sdlz.c
dns_db_issecure() and dns_db_nodecount() return false and 0,
respectively, if they are not implemented, so there's no need to
have implementation functions that only return false and 0.
Evan Hunt [Mon, 4 Aug 2025 23:54:08 +0000 (16:54 -0700)]
merge dns_db_find/findext and dns_db_findnode/findnodeext
the dns_db_findext and _findnodeext calls are extended versions
of dns_db_find and _findnode, which take additional arguments for
client information in order to support ECS. previously, database
implementations could support either API call, with cross-compatibility
so that, for example, dns_db_findext() could call a find implementation
if findext was not implemented, and dns_db_find() could call findext
if find was not implemented.
this has now been simplified. the find and findnodeext implementations
now support client info. all database implementations will now provide
these calls. implementations which do not support ECS will simply
ignore the clientinfo and clientinfomethods parameters.
this only affects the underlying implementation; callers will still
use the same interface. dns_db_find() and dns_db_findnode() are now
macros which pass NULL to the clientinfo parameters, so that callers
don't have to do so explicitly. dns_db_findext() and dns_db_findnodeext()
are still available for callers that do wish to pass clientinfo pointers.
Evan Hunt [Mon, 4 Aug 2025 23:24:09 +0000 (16:24 -0700)]
remove obsolete dns_db_hashsize()
this function's purpose was to populate the "CacheBuckets" statistic,
but there are no databases left that implemented it, so the return
value was always 0. "CacheBuckets" has now been removed from the
statistics, and the dns_db_hashsize() API call has been removed.
Evan Hunt [Mon, 4 Aug 2025 23:06:23 +0000 (16:06 -0700)]
dns_rdatalist functions are not for general use
the rdataset method implementation functions in dns/rdatalist.c (i.e.,
dns_rdatalist_first, _next, etc) are not meant to be called directly;
they're called via dns_rdataset_first(), dns_rdataset_next(), etc.
in dnssec-ksr.c, a list-based rdataset was iterated using these
functions. this has been fixed, and the functions have been renamed
to use the `dns__` prefix as a signal that they aren't meant to be
used outside the rdataset implementation.
fix: dev: Fix detection of whether node is active in find_wildcard()
The current code would fail during the write transaction. The first
header would not match the search->serial and the node might be
incorrectly detected as inactive.
Merge branch 'ondrej/fix-find-wildnode-active-node-detection' into 'main'
Fix detection whether node is active in find_wildcard()
The current code would fail during the write transaction. The first
header would not match the search->serial and the node might be
incorrectly detected as inactive.
chg: dev: Make the database ownercase modifiable only via addrdataset()
Simplify the implementation around the database ownercase. Remove the
dns_rdataset_setownercase() implementation for the slabheaders and only
allow setting ownercase on rdatalists and rdatasets. The ownercase in
the database can now be set only with dns_db_addrdataset() by passing
rdataset with correctly set ownercase.
Merge branch 'ondrej/make-database-ownercase-mostly-constant' into 'main'
Make the database ownercase modifiable only via addrdataset()
Simplify the implementation around the database ownercase. Remove the
dns_rdataset_setownercase() implementation for the slabheaders and only
allow setting ownercase on rdatalists and rdatasets. The ownercase in
the database can now be set only with dns_db_addrdataset() by passing
rdataset with correctly set ownercase.
Colin Vidal [Thu, 11 Sep 2025 12:21:24 +0000 (14:21 +0200)]
fix: dev: do not inline dns_zone_gethooktable
Since !10959 `dns_zone_gethooktable()` is only called once per query,
and the suspicion (from perflab analysis) that this (simple, as just
returning a pointer) call was slowing things down (perhaps because of
code locality reasons?) doesn't matter anymore. So even if !10959
inlined it, it shouldn't matter anymore.
Merge branch 'colin/minimize-hooktable-lookup-followup' into 'main'
Colin Vidal [Thu, 11 Sep 2025 09:51:06 +0000 (11:51 +0200)]
do not inline dns_zone_gethooktable
Since !10959 `dns_zone_gethooktable()` is only called once per query,
and the suspicion (from perflab analysis) that this (simple, as just
returning a pointer) call was slowing things down (perhaps because of
code locality reasons?) doesn't matter anymore. So even if !10959
inlined it, it shouldn't matter anymore.
Petr Špaček [Thu, 11 Sep 2025 09:06:21 +0000 (11:06 +0200)]
Prevent Sphinx from messing up syntax with "smartquotes" feature
Sphinx's smartquotes feature was rewriting -- to en-dash, "" to proper
English quotes etc. This was messing up syntax at unpredictable places.
Disable this feature instead of attempting to escape all the places in
the manual.
Colin Vidal [Thu, 11 Sep 2025 06:37:03 +0000 (08:37 +0200)]
fix: dev: minimize zone hooktable lookups
Merging !10483 caused a performance regression because the zone hooktable had to be looked up every time a hook point was reached, even if no zone plugins were configured. We now look up the zone hooktable when a zone is attached to the query context, and keep a pointer to it until the qctx is destroyed.
Merge branch 'each-zoneplugin-zonehook-once' into 'main'
query_reset() is called during query initialization, but the only
time the NS_QUERY_SETUP hook runs is when it's called from
query_cleanup(). it makes more sense to move the hook point to
there and rename it to NS_QUERY_CLEANUP.
this change caused a crash in the unit tests due to the view being
unnecessarily detached before ns__client_reset_cb() was called.
this has also been fixed.
Colin Vidal [Wed, 10 Sep 2025 20:40:44 +0000 (22:40 +0200)]
don't call hooks when a query hasn't started
guard the call to the NS_QUERY_RESET hook so it's called only if
the view has been set. If the view is NULL, it means the client has
been reset _before_ the query even started, and no other hook could
have been called, so it doesn't make sense to call this one.
this also enables us to avoid a NULL-check on the qctx->view in the
CALL_HOOK macros.
add a 'zhooks' member to the query_ctx structure, so that we only
need to look up the hook table for the zone once when iniitalizing
a qctx, and not once for every hook point.
chg: nil: Reduce the code duplication around getting slabheaders from slabtop
There was a lot of duplicated code around getting the first header that
exists, is active, and matches the version header from the qpzonedb.
Move the duplicate code into a helper function and unify the same
approach for the qpcache too even though the code is much simpler there.
It should come handy when top->header is something more complicated than
a pointer to first slabheader.
Merge branch 'ondrej/refactor-getting-the-first-slabheader-from-slabtop' into 'main'
Reduce the code duplication around getting slabheaders from slabtop
There was a lot of duplicated code around getting the first header that
exists, is active, and matches the version header from the qpzonedb.
Move the duplicate code into a helper function and unify the same
approach for the qpcache too even though the code is much simpler there.
It should come handy when top->header is something more complicated than
a pointer to first slabheader.
Mark Andrews [Wed, 3 Sep 2025 23:57:10 +0000 (09:57 +1000)]
Fix missing RRSIGs for "glue" lookups with CD=1
The code to test whether to store the RRSIGs on DNS_R_UNCHANGED
with CD=1 was failing because the comparison methods of the two
rdatatset instances were not compatible. Move the testing into
dns_db_addrdataset(), and request it by setting the DNS_ADD_EQUALOK
option. If the option is set and the old and new rrsets compare
as equal, dns_db_addrdataset() returns ISC_R_SUCCESS instead of
DNS_R_UNCHANGED.
When the `dnssec-signzone` tests were moved to the `dnssectools`
system test, an unused copy of the `signer` directory was left in
the `dnssec` test. This has been removed.
Merge branch 'each-cleanup-dnssec-files' into 'main'
Evan Hunt [Wed, 20 Aug 2025 18:28:32 +0000 (11:28 -0700)]
remove 'signer' files from dnssec test
when the dnssec-signzone tests were moved to the dnssectools
system test, a unused copy of the 'signer' directory was left in
the dnssec test. This has been removed.
Colin Vidal [Tue, 9 Sep 2025 19:56:03 +0000 (21:56 +0200)]
new: usr: support for zone-specific plugins
Query plugins can now be configured at the `zone` level, as well as globally or at the `view` level. A plugin's hooks are then called only while that specific zone's database is being used to answer a query.
This simplifies the implementation of plugins that are only needed for specific namespaces for which the server is authoritative. It can also enable quicker responses, since plugins will only be called when they are needed.
Colin Vidal [Mon, 8 Sep 2025 08:12:53 +0000 (10:12 +0200)]
remove query_ctx_t detach_client property
Since the removal of NS_QUERY_QCTX_DESTROYED hook, there is no need for
the `qctx->detach_client` object anymore, as this was designed to tell
the plugin whether the client object is about to be, or is already,
freed from memory. This is not needed anymore, as NS_QUERY_RESET is
called _always_ when the client object is about to be freed from memory.
Remove `detach_client` and tidy up the code a bit by including the
freeing of the qctx object (when allocated) inside the qctx_destroy
function instead of requiring extra calls.
Colin Vidal [Mon, 8 Sep 2025 08:12:29 +0000 (10:12 +0200)]
update hooks tests to use NS_QUERY_RESET
Update hooks-related unit tests since the removal of
NS_QUERY_QCTX_DESTROY and the introduction of NS_QUERY_RESET hook. This
also simplifies (a bit) the plugin usage as NS_QUERY_RESET is _always_
called when the client plugin is about to be freed from memory.
Colin Vidal [Mon, 8 Sep 2025 08:10:37 +0000 (10:10 +0200)]
replace QCTX_INIT/_DESTROY hooks with QUERY_SETUP/_RESET
The hook NS_QUERY_QCTX_DESTROY is problematic with zone plugins because
it can be called in some contexts where `qctx->client` is invalid (the
pointer is dangling); which would lead to a use-after-free (spotted by
TSAN build) as `qctx->client` is used to get the zone hooktable, to find
out whether there is an authoritive zone which would have
NS_QUERY_QCTX_DESTROY registered.
This can't easily be fixed, because there is no easy way to know from
query.c code if `client` is still a valid object: `client->reqhandle`,
representing the request from a client, is refcounted, and the `client`
object is freed from memory once its refcounter gets to 0. While
`reqhandle` is attached from query.c code, it can be attached more than
once from asynchronous code and there is no clear path where detaching
it would lead to a client free. Hence, there is no way to know for sure
when to set `qctx->client = NULL` (this is why the pointer remains
dangling).
Back to the original problem; this is why the NS_QUERY_QCTX_DESTROY hook
is incompatible with zone plugins. `qctx->detach_client`, which is used
to tell a plugin that the `client` object is either free or about to be
free can't be use either, because in some cases the client is still
there, and should be used.
Code issue aside, the `qctx` object is really just an aggregate of
various data to pass easily in the various functions and callbacks,
initially stored on the stack, but allocated in some cases (for some
asynchronous flow, when recursion is needed), so the point it gets
created/"destroyed" is really just an implementation "detail", and
providing a higher level hook for the plugin would be beneficial. Hence,
NS_QUERY_RESET and NS_QUERY_INIT are removed, and instead, the existing
NS_QUERY_SETUP can be used as well as the newly introduced
NS_QUERY_RESET (which replaces NS_QUERY_QCTX_DESTROY). The advanage is
that NS_QUERY_RESET is called _only_ when the client object is _always_
about to be freed, which avoids usage of the extra `qctx->detach_client`
usage from the plugin.
The way NS_QUERY_RESET works is that when the `client` is freed, a
callback (from `query.c`) is called. This callback creates a transient
qctx object on the stack with a pointer to the view, and uses that
to call the hook.
Colin Vidal [Tue, 19 Aug 2025 12:45:56 +0000 (14:45 +0200)]
add unit test for plugin_register source param
Update the existing test's syncplugin plugin with a new parameter
indicating whether the plugin should be loaded in a view or a zone.
If it doesn't match where the plugin is actually loaded, it fails the
initialization. This covers the correctless of the `source` parameter of
`plugin_register` API.
Colin Vidal [Tue, 19 Aug 2025 12:38:36 +0000 (14:38 +0200)]
add plugin_register param telling the source
The plugin `plugin_register` API has a new parameter `source` indicating
whether the plugin is loaded from a view or a zone.
This extra parameter enables the plugin to fail early during
initialization if a plugin written to be used in a zone exclusively
is loaded at a view level, or vice versa.
Colin Vidal [Tue, 10 Jun 2025 14:32:04 +0000 (16:32 +0200)]
filter-aaaa can be used as zone or view plugin
Update the filter-aaaa system test so the two authoritative zones
in ns4 both configure filter-aaaa as a zone plugin.
In order to work in both contexts, the plugin must register both
the `NS_QUERY_QCTX_INITIALIZED` and `NS_QUERY_AUTHZONE_ATTACHED`
hooks.
When the plugin is configured at the zone level in an authoritative
server, `NS_QUERY_QCTX_INITIALIZED` is skipped, because no zone will
have been looked up by the time it is called. When the zone is
found, calling `NS_QUERY_AUTHZONE_ATTACHED` will allow the same
initialization to occur.
Colin Vidal [Tue, 10 Jun 2025 14:27:58 +0000 (16:27 +0200)]
add NS_QUERY_AUTHZONE_ATTACHED hook
Add a new query hook called `NS_QUERY_AUTHZONE_ATTACHED`. This hook is
called whenever an authoritative zone is found and attached during a
query answer.
From code level, this hook is called when `qctx->client->query->authzone`
is attached during a query. This enables zone-specific plugins to
initialize specific states whenever a local zone is found that can
answer a query.
Colin Vidal [Thu, 5 Jun 2025 15:31:12 +0000 (17:31 +0200)]
update hook developer documentation
Attempt to add zone plugin specificities into the hook developer
documentation. In particular about the hook call order and hookpoint
which can't be called on a zone plugin.
Colin Vidal [Wed, 4 Jun 2025 11:52:30 +0000 (13:52 +0200)]
add template support for zone plugins
The zone plugin loading code now also looks into the zone template
configuration property of a zone. If it exists, it checks whether there
is a plugin sub-tree defined in the template and, if that exists, loads
the plugin definition from the template.
Colin Vidal [Wed, 28 May 2025 09:54:19 +0000 (11:54 +0200)]
add query unit test
A new query unit test covers the logic where zone hooks must be called
first, then view ones, and finally the default hooks. It also ensures
that if any hook returns NS_HOOK_RETURN the chain immediately stops.
projects / thirdparty / bind9.git / log
