There were several consequtive foreach loops when adding new entry into
the cache. Merge the multiple foreach loops into a single pass loop
with some effort and a lot of comments.
The bindrdataset() already has a logic to skip the rest of the function
if the passed rdataset is NULL. Remove the external guarding for
'addedrdataset' to simplify the code flow both from the zone and cache
databases.
This is a follow-up of !10895 where the keystore pointer was removed
from the zone (as not specific to the zone) and moved to the view. But
in order to avoid adding extra lifecycle dependencies from the zone to
the view, the keystore pointer is now moved to the zonemgr, which also
makes more sense as this is a global settings, and zonemgr wraps a bunch
of other global settings to be accessibles from the zones.
Because the zonemgr lifecycle is the same of the keystores (which are
both depending on named_g_server) this should be a safe change.
Clang 20 is complaining about passing NULL to an argument with 'nonnull'
attribute. Mark these two functions with the same attribute to assure
that these two function also don't accept NULL as an argument.
libuv expects file descriptors <= STDERR_FILENO are in use. otherwise,
it may abort when closing a file descriptor it opened.
See https://github.com/libuv/libuv/pull/4559Closes#5226
In gcc 15, __builtin_stdc_rotate_{left,right} was added. Use these
builtins when available otherwise rewrite the ISC_ROTATE_LEFT and
ISC_ROTATE_RIGHT using _Generic.
The list of keystores is owned by the single server object
(named_g_server), but dns_zone_t has a pointer into it in order to
preserve encapsulation (lib/dns won't link to bin/named for good
reasons).
However, getting the keystores from the zone uses the zone lock whereas
this is not needed (as the pointer value doesn't depends on the zone,
and is initialized only with the same named_g_server->keystores value);
also storing an extra pointer per zone is not needed; also, there was a
logic based on the zone->secure property which was not needed (as there
is only one keystore).
The keystores pointer is now accessible and lock-free at view level,
it also simplifies a bit the various zone configuration APIs (server.c,
zoneconf.c).
Under certain circumstances, cache entries with equivalent rdataset
might not get replaced. Previously such entry would get preserved
regardless of the new TTL and expire time on the existing header would
get updated when the expire time was less than the expire time on the
existing header. Change the logic to preserve the existing header only
if the new expire time is larger than the existing one and replace the
existing cache entry when the new expire time is less than the existing
one.
Co-authored-by: Jinmei Tatuya <jtatuya@infoblox.com>
Previously, BIND 9 would drop the ZEROTTL attribute when updating
previously cached NS entry with ZEROTTL attribute set.
Co-authored-by: Jinmei Tatuya <jtatuya@infoblox.com>
Use C23 stdckdint.h when available and define ckd_{mul,add,sub} shims to
__builtin_{mul,add,sub}_overflow(). Require the __builtin functions
unconditionally.
Currently following __builtin functions are used:
__builtin_add_overflow
__builtin_mul_overflow
__builtin_prefetch
__builtin_sub_overflow
__builtin_unreachable
These are generally available on our supported platform, and also we use
some of these unconditionally anyway in qp.c. Thus make the support for
these functions mandatory so we fail early in the 'setup' step.
The fxhash implementation was missing a constant for 32-bit platforms.
This has been fixed. Constant for 64-bit platform was update to match
the current Rust constants.
The previous refactoring added an assertion failure when negative RRSIG
would be added to the cache database. As result, any query for RRSIG in
any unsigned zone would trigger that assertion failure.
Allow the negative RRSIG entries to be stored in the cache database
again as not caching these would trigger new remote fetch every time
such query would be received from a client.
The following check:
__builtin_types_compatible_p(size_t, uint64_t)
doesn't work with default compiler on macOS. Workaround the issue
by typing the size_t to matching unsigned int type.
Using `static inline` functions in the headers break gcov as it cannot
properly track the hits. To fix the issue, convert the expressions to
statement macros. The added static assertions will ensure integer
promotion cannot occur unlike its previous function counterpart.
By default, when named is started it may start answering to
queries before the response policy zones are completely loaded
and processed. This new feature gives an option to the users to
tell named that incoming requests should result in SERVFAIL anwser
until all the response policy zones are procesed and ready.
During the initial configuration of named after startup, 'first_time'
is true. This is needed for implementing the new 'servfail-until-ready'
configuration option, which should take into effect only during the
initial configuration.
If so we also want to tickle the apex because DNSKEY/CDNSKEY/CDS
RRsets may need to be re-signed.
Note that this may be overzealous, because if state transitions
happen just because of timing events (RUMOURED -> OMNIPRESENT,
UNRETENTIVE -> HIDDEN) this would have to cause changes in the
zone DNSSEC records.
When a key retire, key generation/introduction, or a state transition
to RUMOURED/UNRETENTIVE should happen, instead they are logged.
When those logs look good, you can run 'rndc dnssec -step' to run the
keymgr and apply those steps.
Add a new option 'manual-mode' to 'dnssec-policy'. The intended
use is that if it is enabled, it will not automatically move to the
next state transition (RUMOURED, UNRETENTIVE), only after manual
confirmation. The intended state transition should be logged.
Currently, when releasing a qpznode after a read operation, we will
check if the node is dirty due to a previous write, upgrade the lock to
a write lock and perform a cleanup.
An unintended side effect of this is that protecting a node by
increasing the reference count must also protect its parent database.
For the very common case where only one zone is configured, this is a
non-trivial source of contention, as the same refcount will be hit by
all threads.
This commit removes the opportunistic cleaning and the database
refcount, reducing contention. Cleaning will be done only on
closeversion.
Per @each, skipping cleanup of (|nsec_|nsec3_)origin nodes in
qpznode_release in qpzone.c is a residual from RBTDB, but it is
unnecessary or at most a performance optimization with QP.
Remove it to make it further changes easier to qpznode_release easier.
The `<isc/bit.h>` header is a GNU C11 compatible version of C23's
`<stdbit.h>`.
It currently uses either `<stdbit.h>` or the equivilent compiler
builtins. However, the generic `__builtin_ctzg` and `__builtin_ctlz`
builtins are not available in every compiler version and thus falls
back to manually selecting from type.
Furthermore, the ctz fallback has been removed since `__builtin_ctzll`
has been used for a while directly without any compilation issues from
users. Thus, we can also require `__builtin_ctz`.
Unlike the rest of C23's bit utilities, we avoid the stdc_rotate_*
functions since we don't need the rotation modulus precision. This adds
a couple (admittedly cheap) unwanted instructions on some architectures.
> Put a space before opening parentheses only after control statement
> keywords (for/if/while...) except this option doesn’t apply to ForEach
> and If macros. This is useful in projects where ForEach/If macros are
> treated as function calls instead of control statements.
> Insert braces after control statements (if, else, for, do, and while)
> in C++ unless the control statements are inside macro definitions or
> the braces would enclose preprocessor directives.
When first dns_db_addrdataset() succeeds in cache_rrset(), but the
second one fails with error, the added rdataset was kept associated.
This caused assertion failure down the pipe in fctx_sendevents().
The compile-time DNS__TYPEPAIR_CHECK macro (wrapping an INSIST) is a
no-op if DNS_TYPEPAIR_CHECK is off, making at least one unused variable
in DNS_TYPEPAIR_TYPE and DNS_TYPEPAIR_COVERS scopes (as in such case,
only one member of the pair is effectively needed).
In such case, having an unused variable (the other member of the pair)
is expected, this silence the warning by adding a (void) cast on the
no-op version of DNS__TYPEPAIR_CHECK.
the if statements calling iterator_active() checked the EXISTS
flag on the header and then iterator_active() checked it again.
simplify so only the caller checks it.
As the qpcache has only one active header at the time, we can move the
SIEVE-LRU members from dns_slabheader_t to dns_slabtop_t structure thus
saving a little bit of memory in each slabheader and using it only once
per type.
The code that combines the top-level hierarchy (per-typepair) and
individual slab headers (per-version) saves a little bit of memory, but
makes the code convoluted, hard to read and hard to modify. Change the
top level hierarchy to be of different type with individual slabheaders
"hanging" from the per-typepair dns_slabtop_t structure.
This change makes the future enhancements (changing the top level data
structure for faster lookups; coupling type + sig(type) into single
slabtop) much easier.
The slabheader doesn't directly attach or link to 'db' anymore. Pass
only the memory context needed to create the slab header to make the
lack of relation ship more prominent.
Also don't call dns_slabheader_reset() from dns_slabheader_new(), it has
no added value.
Previously, when the new header was NOT added into the cache, we would
increment and then decrement stat counters immediately.
Delay incrementing the stat counters until after the newheader has
been actually added into the database.
A little cleanup to accomodate the fact that qpdb->rrsetstats is always
available was also done here.
Change the add() function in the dns_qpcache to properly return
DNS_R_UNCHANGED if the newheader was not actually consumed, and move
the dns_slabheader_destroy() call outside of the add() function.
Under normal circumstances, the case bitfield in the slabheader should
be set only once. By actually (soft-)enforcing this, the read locking
can be completely removed from the rdataslab_getownercase() as we can
check whether the case has been already set or not and making everything
immutable once the case has been set.
The slabheader.c, qpzone.c and qpcache.c had couple of shared macros
that were copied and paste between the units. Move these common
attributes access macros into private header, so these can be shared
among the three compilation units.
In the dns_qpcache unit, we use EXISTS() macro, but in the dns_qpzone
there's a NONEXISTENT() macro for the same slabheader attribute. Unify
the macro to be also EXISTS() in dns_qpzone.
Previously, when a negative header was stored in the cache, it would be
stored in the dns_typepair_t as .type = 0, .covers = <negative type>.
When searching the cache internally, we would have to look for both
positive and negative typepair and the slabheader .down list could be a
mix of positive and negative types.
Remove the extra representation of the negative type and simply use the
negative attribute on the slabheader. Other units (namely dns_ncache)
can still insert the (0, type) negative rdatasets into the cache, but
internally, those will be converted into (type, 0) slabheaders, and vice
versa - when binding the rdatasets, the negative (type, 0) slabheader
will be converted to (0, type) rdataset. Simple DNS_TYPEPAIR() helper
macro was added to simplify converting single rdatatype to typepair
value.
As a side-effect, the search logic in all places can exit early if
there's a negative header for the type we are looking for, f.e. when
searching for the zone cut, we don't have to walk through all the
slabheaders, if there's a stored negative slabheader.
Use dns_rdatatype_none instead of plain '0' for dns_rdatatype_t and
dns_typepair_t manipulation. While plain '0' is technically ok, it
doesn't carry the required semantic meaning, and using the named
dns_rdatatype_none constant makes the code more readable.
When in developer's mode, make the DNS_TYPEPAIR_* macros be more
strict on the contents of the 'base' and 'covers', so we can catch
invalid use of the API.
