> 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().
Merge branch 'ondrej/disassociate-added-on-error-in-cache_rrset' into 'main'
See merge request isc-projects/bind9!10861
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.
Merge branch 'colin/typepair-check-unused-warn' into 'main'
See merge request isc-projects/bind9!10860
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. This has been fixed.
Merge branch 'ondrej/dont-stat-count-failed-adds' into 'main'
See merge request isc-projects/bind9!10859
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.
Merge branch 'ondrej/remove-locking-from-slabheader-ownercase' into 'main'
See merge request isc-projects/bind9!10843
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.
The qpznode->name is constant - assigned when the node is created
and it is immutable, so there's no reason to have it locked at all.
Merge branch 'ondrej/nodefullname-no-read-lock' into 'main'
See merge request isc-projects/bind9!10686
Remove the extra representation of the negative type in the slabheaders
and simply use the negative attribute on the slabheader.
Merge branch 'ondrej/remove-negtype-from-qpcache' into 'main'
See merge request isc-projects/bind9!10840
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.
The RR type 0 is a reserved type for SIG[1] resource record. It should
not be ever inserted into the database nor queried. Add a special
handling to bail out quickly with DNS_R_DISALLOWED when inserting and
ISC_R_NOTFOUND when looking up TYPE0. This is also prerequisite for
stricter checks in the follow-up commit.
1. https://www.rfc-editor.org/rfc/rfc2535#section-4.1.8.1
There was an apparent typo where rdatalist->covers would be assigned the
same value as rdatalist->type. As nsupdate can't update signatures, the
covers must be dns_rdatatype_none.
The dns_typepair_t and dns_rdatatype_t variables were both named 'type'
in multiple places. Rename all dns_typepair_t variables to include word
'pair' in the variable name to make sure that the distinction between
the two types is more clear.
Instead of catching the DNS_R_UNCHANGED from dns_db_addrdataset() (via
cache_rrset() and dns_ncache_add()) individually, mask it properly as
soon as possible by moving the sigrdataset caching logic inside the
cache_rrset() and returning ISC_R_SUCCESS from cache_rrset() and
dns_ncache_add() when the database was unchanged.
Closes#5473
Merge branch '5473-fix-crash-in-validated' into 'main'
See merge request isc-projects/bind9!10850
Instead of catching the DNS_R_UNCHANGED from dns_db_addrdataset() (via
cache_rrset() and dns_ncache_add()) individually, mask it properly as
soon as possible, by moving the sigrdataset caching logic inside
cache_rrset() and returning ISC_R_SUCCESS from cache_rrset() and
dns_ncache_add() when the database was unchanged.
During a recent refactoring of `validated()`, a line was
removed, causing `result` to be left unchanged. This
caused time to be wasted continuing to try to validate when a
non-recoverable error had occurred, and also caused the wrong
reason to be logged in `add_bad()`.
Closes#5468
Merge branch '5468-fix-deadcode' into 'main'
See merge request isc-projects/bind9!10851
The logic to delete records from the cache was relying on the contents
of the validation answer. Change the logic to always delete the
contents of the cache on the broken chain result.
during a recent refactoring of validated(), a line was
removed, causing 'result' to be left unchanged. this
wasted time continuing to try to validate when a
non-recoverable error had occured, and caused the wrong
reason to be logged in add_bad().
Initialization of the common members of rdata type structures varies
across branches. Standardize it by using the `DNS_RDATACOMMON_INIT`
macro for all types, so that new types are more likely to use it,
and hence backport more cleanly.
Closes#5467
Merge branch '5467-use-dns_rdatacommon_init-to-hide-branch-differences' into 'main'
See merge request isc-projects/bind9!10831
Initialization of the common members of rdata type structures varies
across branches. Standardize it by using the DNS_RDATACOMMON_INIT
macro for all types, so that new types are more likely to use it,
and hence backport more cleanly.
We've switched to an updated dataset for shotgun jobs. The change in
underlying traffic caused the more sensitive doh-get (and partially dot)
jobs to overload the resolver, making the jobs unstable and unreliable,
due to an increased number of timeouts.
Readjust the load parameters slightly to avoid exceeding ~2 % of
timeouts in the cold cache scenario to stabilize the job results.
Merge branch 'nicki/ci-shotgun-load-new-dataset' into 'main'
See merge request isc-projects/bind9!10841
We've switched to an updated dataset for shotgun jobs. The change in
underlying traffic caused the more sensitive doh-get (and partially dot)
jobs to overload the resolver, making the jobs unstable and unreliable,
due to an increased number of timeouts.
Readjust the load parameters slightly to avoid exceeding ~2 % of
timeouts in the cold cache scenario to stabilize the job results.
All databases in the codebase follow the same structure: a database is
an associative container from DNS names to nodes, and each node is an
associative container from RR types to RR data.
Each database implementation (qpzone, qpcache, sdlz, builtin, dyndb) has
its own corresponding node type (qpznode, qpcnode, etc). However, some
code needs to work with nodes generically regardless of their specific
type - for example, to acquire locks, manage references, or
register/unregister slabs from the heap.
Before this MR, these generic node operations were implemented as methods in
a `dns_dbmethods_t` vtable. This created a coupling between the database
and node lifetimes. If a node were to outlive its parent database, the node
destructor would destroy all RR data, and each RR data destructor would
try to unregister from heaps by calling a virtual function from the
database vtable. Since the database was already freed, this would cause a
crash.
This MR breaks the coupling by standardizing the layout of all
database nodes, adding a `dns_dbnode_methods_t` vtable for node
operations, and moving node-specific methods from the database vtable to
the node vtable.
Merge branch 'alessio/dbnode-vtable' into 'main'
See merge request isc-projects/bind9!10728
All databases in the codebase follow the same structure: a database is
an associative container from DNS names to nodes, and each node is an
associative container from RR types to RR data.
Each database implementation (qpzone, qpcache, sdlz, builtin, dyndb) has
its own corresponding node type (qpznode, qpcnode, etc). However, some
code needs to work with nodes generically regardless of their specific
type - for example, to acquire locks, manage references, or
register/unregister slabs from the heap.
Currently, these generic node operations are implemented as methods in
the database vtable, which creates problematic coupling between database
and node lifetimes. If a node outlives its parent database, the node
destructor will destroy all RR data, and each RR data destructor will
try to unregister from heaps by calling a virtual function from the
database vtable. Since the database was already freed, this causes a
crash.
This commit breaks the coupling by standardizing the layout of all
database nodes, adding a dedicated vtable for node operations, and
moving node-specific methods from the database vtable to the node
vtable.
Right now dns_sdlzlookup has a slight difference from other dbnode
implementations in that it stores a pointer to a dns name instead of
the dns name itself.
This commit harmonizes dns_sdlzlookup with other dbnode
implementations, facilitating further refactoring.
Each run of `meson test` overwrites the default log file testlog.txt,
this means we lose the backtraces of previous run. This commit assigns
a different log file for each run.
The doh unit test has been timing out recently and we don't have a fix
for it yet. Mark it as warning rather than a hard failure, since it's a
known issue.
Related #5448
Merge branch '5448-allow-failure-unit-almalinux8-doh' into 'main'
See merge request isc-projects/bind9!10835
The doh unit test has been timing out recently and we don't have a fix
for it yet. Mark it as warning rather than a hard failure, since it's a
known issue.
Related #5448
