Is there a time when new_qp(c|z)node() would not be followed by
assignment of the namespace? No, so let's add the assignment to the
function that creates the node.
Matthijs Mekking [Tue, 27 May 2025 08:57:11 +0000 (10:57 +0200)]
Rename dns_qp_lookup2 back to dns_qp_lookup
Now that we have to code working, rename 'dns_qp_lookup2' back to
'dns_qp_lookup' and adjust all remaining 'dns_qp_lookup' occurrences
to take a space=0 parameter.
Matthijs Mekking [Mon, 26 May 2025 09:34:16 +0000 (11:34 +0200)]
Fix the dbiterator to assume only one qp-trie
The dbiterator can take three modes: full, nsec3only and nonsec3.
Previously, in full mode the dbiterator requires special logic to jump
from one qp-trie to the other. Now everything is in one trie, other
special logic is needed.
The qp-trie is now sorted in such a way that all the normal nodes come
first, followed by NSEC nodes, and finally the NSEC3 nodes. NSEC nodes
are empty nodes and need to be skipped when iterating.
We add an additional auxiliary node to the trie, an NSEC origin, so
we can easily find the point in the trie where we need to continue
iterating.
Matthijs Mekking [Tue, 13 May 2025 10:47:02 +0000 (11:47 +0100)]
Update qp unit tests merging denial and zone data
If zone and denial data are going to be stored in the same qp storage,
the unit tests need to be updated to reflect this change. The code
changes mainly affect name to qpkey conversion, lookups, and
predecessors.
A note on predecessors: since the denial and zone data are now in the
same qp storage, the predecessor of the first name in the zone data will
consequently be the last name in the denial data.
In preparation to merge the three qp tries (tree, nsec, nsec3) into
one, add the piece of information into the qpkey. This is the most
significant bit of information, so prepend the denial type to the qpkey.
This means we need to pass on the denial type when constructing the
qpkey from a name, or doing a lookup.
Reuse the the DNS_DB_NSEC_* values. Most qp tries in the code we just
pass on 0 (nta, rpz, zt, etc.), because there is no need for denial of
existence, but for qpzone and qpcache we must pass the right value.
Change the code, so that node->nsec no longer can have the value
DNS_DB_NSEC_HAS_NSEC, instead track this in a new attribute 'havensec'.
Since we use node->nsec to convert names to keys, the value MUST be set
before inserting the node into the qp-trie.
Update the fuzzing and unit tests accordingly. This only adds a few
extra test cases, more are needed.
In the qp_test.c we can remove test code for empty keys as this is
no longer possible.
When used with the ``+keepopen`` option with a TCP connection, iscman:`dig`
could terminate unexpectedly in rare situations. Additionally, iscman:`dig`
could hang and fail to shutdown properly when interrupted during a query.
These have been fixed.
Closes #5381
Merge branch '5381-dig-keepalive-crash' into 'main'
Fix a possible hang in dig if a send is interrupted/canceled
When send_done() is called with a ISC_R_CANCELED status (e.g. because
of a signal from ctrl+c), dig can fail to shutdown because
check_if_done() is not called in the branch. Add a check_if_done()
call.
When reusing a TCP connection (because of the '+keepopen' option),
dig detaches from the query after launching it. This can cause a
crash in dig in rare cases when the "receive" callback is called
earlier than the "send" callback.
The '_cancel_lookup()' function detaches a query only if it's
found in the 'lookup->q' list. Before this commit, with one
additional detach happening before recv_done() -> _cancel_lookup()
is called, it didn't cause problems because an earlier _query_detach()
was unlinking the query from 'lookup->q' (because it was the last
reference), so the additional detach and the skipped detach were
undoing each other.
That is unless the "receive" callback was called earlier than the
"send" callback, in which case the additional detach wasn't destroying
the query (and wasn't unlinking it from 'lookup->q') because the "send"
callback's attachment was still there, and so _cancel_lookup() was
trying to "steal" the "send" callback's attachment and causing an
assertion on 'INSIST(query->sendhandle == NULL);'.
Delete the detachment which caused the described situation.
Michał Kępień [Thu, 10 Jul 2025 09:21:04 +0000 (11:21 +0200)]
fix: nil: Do not hardcode release date in man pages
The util/meson-dist-package.sh script hardcodes the date it is run on
into the man pages it creates in the dist tarball. This causes pkgdiff
to report discrepancies if the util/release-tarball-comparison.sh script
is run on a different day than the one the dist tarball was generated
on.
Fix by using the exact same solution as in BIND 9.20: generating the man
page stubs with a @RELEASE_DATE@ placeholder instead of a specific date
and only replacing that placeholder with a specific date during the
build process.
Closes #5412
Merge branch '5412-do-not-hardcode-release-date-in-man-pages' into 'main'
Michał Kępień [Thu, 10 Jul 2025 09:20:46 +0000 (11:20 +0200)]
Do not hardcode release date in man pages
The util/meson-dist-package.sh script hardcodes the date it is run on
into the man pages it creates in the dist tarball. This causes pkgdiff
to report discrepancies if the util/release-tarball-comparison.sh script
is run on a different day than the one the dist tarball was generated
on.
Fix by using the exact same solution as in BIND 9.20: generating the man
page stubs with a @RELEASE_DATE@ placeholder instead of a specific date
and only replacing that placeholder with a specific date during the
build process.
fix: usr: Log dropped or slipped responses in the query-errors category
Responses which were dropped or slipped because of RRL (Response Rate
Limiting) were logged in the ``rate-limit`` category instead of the
``query-errors`` category, as documented in ARM. This has been fixed.
Closes #5388
Merge branch '5388-rrl-log-category-fix' into 'main'
Aram Sargsyan [Mon, 30 Jun 2025 13:12:09 +0000 (13:12 +0000)]
Log dropped or slipped responses in the query-errors category
As mentioned in the comments block before the changed code block,
the dropped or slipped responses should be logged in the query
category (or rather query-errors category as done in lib/ns/client.c),
so that requests are not silently lost.
Also fix a couple of errors/typos in the code comments.
The ns_client_t struct is reset and zeroed out on every query,
but some fields (query, message, manager) are preserved.
We observe two things:
- The sendbuf field is going to be overwritten anyway, there's
no need to zero it out.
- The fields are copied out when the struct is zero-ed out, and
then copied back in. For the query field (which is 896 bytes)
this is very inefficient.
This commit makes the reset more efficient by avoiding the unnecessary
zeroing and copying.
Merge branch 'alessio/experimental-ns-client-noinit' into 'main'
Alessio Podda [Wed, 14 May 2025 13:32:53 +0000 (15:32 +0200)]
Improve efficiency of ns_client_t reset
The ns_client_t struct is reset and zero-ed out on every query,
but some fields (query, message, manager) are preserved.
We observe two things:
- The sendbuf field is going to be overwritten anyway, there's
no need to zero it out.
- The fields are copied out when the struct is zero-ed out, and
then copied back in. For the query field (which is 896 bytes)
this is very inefficient.
This commit makes the reset more efficient avoiding to unnecessary
zero-ing and copy.
This MR reduces lock contention and increases scalability in the ADB by:
a) Using SIEVE algorithm instead of classical LRU;
b) Replacing rwlocked isc_hashmap with RCU cds_lfht table;
c) Replace the single LRU table per-object with per-loop LRU tables per-object.
Merge branch 'ondrej/use-urcu-lfht-for-ADB-tables' into 'main'
Ondřej Surý [Wed, 25 Jun 2025 17:02:37 +0000 (19:02 +0200)]
Use cds_lfht for lock-free hashtables in dns_adb
Replace the read-write locked isc_hashmap with lock-free cds_lfht
hashtable and replace the singular LRU tables for ADB names and entries
with a per-thread LRU tables. These changes allowed to remove all the
read-write locking on the names and entries tables.
Use regular reference counting macro for isc_nm_t structure
Instead of having hand crafted attach/detach/destroy functions, replace
them with the standard ISC_REFCOUNT macro. This also have advantage
that delayed netmgr detach (from dns_dispatch) now doesn't cause
assertion failure. This can happen with delayed (call_rcu) shutdown of
dns_adb.
Ondřej Surý [Thu, 12 Jun 2025 09:14:51 +0000 (11:14 +0200)]
Rewrite dns_adb LRU to SIEVE
The dns_adb cleaning is little bit muddled as it mixes the "TTL"
based cleaning (.expire_v4 and .expire_v6 for adbname, .expires for
adbentry) with overmem cleaning.
Rewrite the LRU based cleaning to use SIEVE algorithm and to be overmem
cleaning only with a requirement to always cleanup at least 2-times the
size of the newly added entry.
chg: dev: Replace per-zone lock buckets with global buckets
Qpzone employs a locking strategy where rwlocks are grouped into
buckets, and each zone gets 17 buckets.
This strategy is suboptimal in two ways:
- If named is serving a single zone or a zone is the majority of the
traffic, this strategy pretty much guarantees contention when using
more than a dozen threads.
- If named is serving many small zones, it causes substantial memory
usage.
This commit switches the locking to a global table initialized at start
time. This should have three effects:
- Performance should improve in the single zone case, since now we are
selecting from a bigger pool of locks.
- Memory consumption should go down significantly in the many zone
cases.
- Performance should not degrade substantially in the many zone cases.
The reason for this is that, while we could have substantially more
zones than locks, we can query/edit only O(num threads) at the same
time. So by making the global table much bigger than the expected
number of threads, we can limit contention.
Merge branch 'alessio/global-qpzone-lock-table' into 'main'
Alessio Podda [Wed, 7 May 2025 12:52:11 +0000 (14:52 +0200)]
Replace per-zone lock buckets with global buckets
Qpzone employs a locking strategy where rwlocks are grouped into
buckets, and each zone gets 17 buckets.
This strategy is suboptimal in two ways:
- If named is serving a single zone or a zone is the majority of the
traffic, this strategy pretty much guarantees contention when using
more than a dozen threads.
- If named is serving many small zones, it causes substantial memory
usage.
This commit switches the locking to a global table initialized at start
time. This should have three effects:
- Performance should improve in the single zone case, since now we are
selecting from a bigger pool of locks.
- Memory consumption should go down significantly in the many zone
cases.
- Performance should not degrade substantially in the many zone cases.
The reason for this is that, while we could have substantially more
zones than locks, we can query/edit only O(num threads) at the same
time. So by making the global table much bigger than the expected
number of threads, we can limit contention.
chg: dev: Extract the resigning heap into a separate struct
In the current implementation, the resigning heap is part of the zone
database. This leads to a cycle, as the database has a reference to its
nodes, but each node needs a reference to the database.
This MR splits the resigning heap into its own separate struct, in order
to help breaking the cycle.
Merge branch 'alessio/split-qpzone-heap-from-qpdb' into 'main'
In the current implementation, the resigning heap is part of the zone
database. This leads to a cycle, as the database has a reference to its
nodes, but each node needs a reference to the database.
This MR splits the resigning heap into its own separate struct, in order
to help breaking the cycle.
Recovering the node lock from a pointer to the header and a pointer to
the db is a common operation. This commit abstracts it away into a
function, so that the node lock selection logic may be modified more
easily.
Mark Andrews [Wed, 9 Jul 2025 00:55:28 +0000 (10:55 +1000)]
fix: dev: Fix a possible crash when adding a zone while recursing
A query for a zone that was not yet loaded may yield an unexpected result such as a CNAME or DNAME, triggering an assertion failure. This has been fixed.
Petr Menšík [Tue, 10 Jun 2025 12:35:03 +0000 (14:35 +0200)]
Add few extra WANT_QUERYTRACE logs into resume_qmin
Print optionally a bit more details not passed to event in case
dns_view_findzonecut returns unexpected result. Result would be
visible later in foundevent, but found fname would be lost. Print it
into the log.
Petr Mensik [Tue, 3 Jun 2025 19:00:58 +0000 (21:00 +0200)]
Handle CNAME and DNAME in resume_min in a special way
When authoritative zone is loaded when query minimization query for the
same zone is already pending, it might receive unexpected result codes.
Normally DNS_R_CNAME would follow to query_cname after processing sent
events, but dns_view_findzonecut does not fill CNAME target into
event->foundevent. Usual lookup via query_lookup would always have that
filled.
Ideally we would restart the query with unmodified search name, if
unexpected change from recursing to local zone cut were detected. Until
dns_view_findzonecut is modified to export zone/cache source of the cut,
at least fail queries which went into unexpected state.
Michał Kępień [Tue, 8 Jul 2025 08:27:22 +0000 (10:27 +0200)]
Fix named-makejournal man page installation
The man page for named-makejournal is erroneously not installed when
building from a source tarball. Add that man page to the appropriate
lists in the build system so that it is installed both when building
from a Git repository and from a source tarball.
fix: usr: Clean enough memory when adding new ADB names/entries under memory pressure
The ADB memory cleaning is opportunistic even when we are under
memory pressure (in the overmem condition). Split the opportunistic
LRU cleaning and overmem cleaning and make the overmem cleaning
always cleanup double of the newly allocated adbname/adbentry to
ensure we never allocate more memory than the assigned limit.
Merge branch 'ondrej/enforce-memory-cleanup-in-ADB-when-overmem' into 'main'
Ondřej Surý [Wed, 25 Jun 2025 10:05:14 +0000 (12:05 +0200)]
When overmem, clean enough memory when adding new ADB names/entries
The purge_stale_names()/purge_stale_entries() is opportunistic even when
we are under memory pressure (overmem). Split the opportunistic LRU
cleaning and overmem cleaning. This makes the stale purging much
simpler as we don't have to try that hard and makes the overmem cleaning
always cleanup double the amount of the newly allocated ADB name/entry.
It's possible to use pytest.mark.flaky, which achieves the exact same
thing as our custom-defined isctest.mark.flaky -- attempts to rerun the
test on failure, but only is flaky package is available.
Mark secondary.kasp test case as flaky on freebsd13
The test_kasp_case[secondary.kasp] can sometimes fail on freebsd13. It
appears the test gets stuck on some operation which should be very
quick, but for some reason takes at least a few seconds, causing the
cb_ixfr_is_signed() function to time out.
In one of the cases I investigated, it wasn't a query/response that
caused a timeout, but rather some operation in between. The test
attempts to read from a keyfile/statefile, but I see no reason why that
should block.
In any case, try to increase the timeout for the verification, as that
shouldn't hurt. Also allow the test to be re-run on freebsd13, as it's
likely to be caused by some odd behaviour on that platform -- the issue
doesn't appear anywhere else.
The check "unix socket message counts" sometimes fails with "dnstap
output file smaller than expected". This only happens on freebsd13 and
can't be reproduced easily. There was an attempt to decrease the
required file size in the past, but apparently, the issue can still
occur.
The serve_stale test has some inherent instabilities affecting many
different checks. While the failure rate isn't too high (about four
failures in past three weeks of nightlies), it gets ignored, because the
test has been unstable for a very long time.
This removes a leftover check which should've been removed in a prior
change (see #5244). The softhsm2 failures when attempting to delete the
token should be ignored.
Previously, the one-second sleep was unreliable, as it didn't properly
indicate that the rndc reconfig has been processed. The "test 'rndc
reconfig' with a broken config" check would sometimes fail under TSAN
in CI, because the previous rndc reconfig was still ongoing, and the
subsequent rndc reconfig was ignored.
These tests have been unstable under TSAN in the past, but it appears
that the same failure mode can happen outside of TSAN tests as well.
These tests have produced 12 failures combined in the past three weeks
in nightlies.
The fetchlimit test has failed 8 times in the nightly CI over the past
three weeks. That makes the overall failure rate somewhere around 1 %,
which isn't a lot, but is still annoying when lots of testing is going
on.
Mark Andrews [Mon, 7 Jul 2025 02:21:58 +0000 (12:21 +1000)]
fix: test: rndc test: second 'rndc reconfig' happens too soon
Rndc test "test 'rndc reconfig' with a broken config" was failing
intermittently.
Wait for 'running' to be logged rather than just using 'sleep 1' before
calling 'rndc reconfig' a second time to get the expected error message
rather than 'reconfig request ignored: already running'.
Closes #5408
Merge branch '5408-rndc-test-second-rndc-reconfig-happens-too-soon' into 'main'
Mark Andrews [Wed, 2 Jul 2025 23:13:11 +0000 (09:13 +1000)]
rndc test: second 'rndc reconfig' happens too soon
Rndc test "test 'rndc reconfig' with a broken config" was failing
intermittently.
Wait for 'running' to be logged rather than just using 'sleep 1' before
calling 'rndc reconfig' a second time to get the expected error message
rather than 'reconfig request ignored: already running'.
new: ci: Run an additional respdiff job for merge requests and schedules
On MRs it uses the merge target as the reference.
In schedules it uses the latest released version for this branch as the reference.
This MR lays the ground work for using respdiff on non-standard configurations (like ECS) in the public repo, see https://gitlab.isc.org/isc-private/bind9/-/merge_requests/807#note_573140.
To reduce the future hassle when maintaining the -S version, most of the work (including an added job, so we know that it actually works) is done here.
Merge branch 'stepan/respdiff-against-merge-target-or-last-release' into 'main'
Mark Andrews [Sun, 6 Jul 2025 13:09:13 +0000 (23:09 +1000)]
fix: dev: Separate out adbname type flags
There are three adbname flags that are used to identify different
types of adbname lookups when hashing rather than using multiple
hash tables. Separate these to their own structure element as these
need to be able to be read without locking the adbname structure.
Closes #5404
Merge branch '5404-seperate-out-adbname-type-flags' into 'main'
Mark Andrews [Tue, 1 Jul 2025 06:45:39 +0000 (16:45 +1000)]
Separate out adbname flags that are hashed
There are three adbname flags that are used to identify different
types of adbname lookups when hashing rather than using multiple
hash tables. Separate these to their own structure element as these
need to be able to be read without locking the adbname structure.
dangerfile.py checked for new configure switches in `configure.ac`,
these were annotated with "# [pairwise:..." in a leading line. Meson
reads those from `meson_options.txt` instead.
fix: usr: Fix the default interface-interval from 60s to 60m
When the interface-interval parser was changed from uint32 parser to
duration parser, the default value stayed at plain number `60` which
now means 60 seconds instead of 60 minutes. The documentation also
incorrectly states that the value is in minutes. That has been fixed.
Closes #5246
Merge branch '5246-fix-default-interface-interval' into 'main'
Ondřej Surý [Tue, 18 Mar 2025 13:05:39 +0000 (14:05 +0100)]
Fix the default interface-interval docs and default value
When the interface-interval parser was changed from uint32 parser to
duration parser, the default value stayed at plain 60 which now means 60
seconds instead of 60 minutes. Fix the default value and the
documentation to match the reality.
Colin Vidal [Mon, 30 Jun 2025 12:51:20 +0000 (14:51 +0200)]
new: test: add startup root DNSKEY refresh system test
Root trust anchors are automatically updated as described in RFC5011.
Add a system test which ensures the root DNSKEYs are always queried by
named during startup.
Because this test uses real internet DNS root servers, it is enabled
only when `CI_ENABLE_LIVE_INTERNET_TESTS` is set.
Colin Vidal [Tue, 24 Jun 2025 09:55:42 +0000 (11:55 +0200)]
add startup root DNSKEY refresh system test
Root trust anchors are automatically updated as described in RFC5011.
Add a system test which ensures the root DNSKEYs are always queried by
named during startup.
Because this test uses real internet DNS root servers, it is enabled
only when `CI_ENABLE_LIVE_INTERNET_TESTS` is set.
Ondřej Surý [Mon, 30 Jun 2025 11:23:38 +0000 (13:23 +0200)]
fix: dev: Prevent false sharing for the .inuse member of isc_mem_t
Change the .inuse member of memory context to have a loop-local
variable, so there's no contention even when the same memory
context is shared among multiple threads.
Closes #5354
Merge branch '5354-prevent-false-sharing-in-isc_mem' into 'main'
Ondřej Surý [Wed, 4 Jun 2025 16:14:23 +0000 (18:14 +0200)]
Change the .inuse member of isc_mem to be per-thread/per-loop
The .inuse member was causing a lot of contention between threads using
the same memory context. Scather the .inuse and .overmem members of
isc_mem_t structure to be an per-tid array of variables to reduce the
contention as the writes are now independent of each other.
The array uses one tad bit nasty trick, as ISC_TID_UNKNOWN is now -1,
the array has been sized to fit the unknown tid with [-1] index into the
array accomplished with `ctx->stat = &ctx->stat_s[1];`. It will not win
a beauty contest, but it works seamlessly by just passing `isc_tid()` as
an index into the array.
The caveat here is that gathering the real inuse value requires walking
the whole array for all registered tid values (isc_tid_count()). The
gather part happens only when statistics are being gathered or when
isc_mem_isovermem() is called. As the isc_mem_isovermem() call happens
only when new data is being added to cache or ADB, it doesn't happen on
the hottest (read-only) path and according to the measurements, it
doesn't slow down neither the cold cache nor the hot cache latency.
Ondřej Surý [Thu, 5 Jun 2025 10:19:43 +0000 (12:19 +0200)]
Don't use ssize_t for storing difference between sizes
As POSIX guarantees only that the type ssize_t shall be capable of
storing values at least in the range [-1, {SSIZE_MAX}], it can't be used
to calculate the difference between two memory sizes. Change the logic
for junk filling to test whether the new size is larger than old size
and then use size_t as the result will be always positive.
Ondřej Surý [Wed, 4 Jun 2025 15:43:34 +0000 (17:43 +0200)]
Remove .hi_called member of isc_mem_t structure
The .hi_called member was dead structure member and it hasn't been used
since the overmem callback has been removed in commit 14bdd21e0a7ad5f115bb2427d4f88fe7a84e9324.
Ondřej Surý [Wed, 4 Jun 2025 08:35:57 +0000 (10:35 +0200)]
Delete jemalloc arena support from isc_mem
The jemalloc arena in isc_mem was added to solve runaway memory problem
for outgoing TCP connections. In the end, this was a red herring and
the jemalloc arena code is now unused (via e28266bf). Remove the
support for jemalloc memory arenas as we can restore this at any time if
we need it ever again, but right now it's just a dead code.
Ondřej Surý [Wed, 25 Jun 2025 06:25:41 +0000 (08:25 +0200)]
Fix implicit headers when using isc/overflow.h header
In jemalloc_shim.h, we relied on including <isc/overflow.h> implicitly
instead of explicitly and same was happening inside isc/overflow.h - the
stdbool.h (for bool type) was being included implicitly instead of
explicitly.
Aydın Mercan [Tue, 24 Jun 2025 13:30:15 +0000 (16:30 +0300)]
do not install manpages for unbuilt binaries
Building and installing from a git release installed all manpages
unconditionally even if binaries like dnstap-read were disabled and not
built.
Now the manpage configuration checks for such cases and also cleans up
remaining artifacts and unnecessary pages if the build directory is
reconfigured.
Ondřej Surý [Sat, 28 Jun 2025 12:06:05 +0000 (14:06 +0200)]
chg: dev: Change isc_tid to be isc_tid_t type (a signed integer type)
Change the internal type used for isc_tid unit to isc_tid_t to hide the
specific integer type being used for the 'tid'. Internally, the isc_tid
unit is now using signed integer type. This allows us to have negatively
indexed arrays that works both for threads with assigned tid and the
threads with unassigned tid. Additionally, limit the number of threads
(loops) to 512 (compile time default).
Ondřej Surý [Wed, 4 Jun 2025 15:54:20 +0000 (17:54 +0200)]
Convert the isc/tid.h to use own signed integer isc_tid_t type
Change the internal type used for isc_tid unit to isc_tid_t to hide the
specific integer type being used for the 'tid'. Internally, the signed
integer type is being used. This allows us to have negatively indexed
arrays that works both for threads with assigned tid and the threads
with unassigned tid. This should be used only in specific situations.
Štěpán Balážik [Sat, 28 Jun 2025 10:51:59 +0000 (10:51 +0000)]
fix: nil: Only run ci-orphaned-anchors on MR events
Now, it is also run in schedules and most annoyingly on push which means
that it is run twice on a push to a branch where a MR exists and `.gitlab-ci.yml` is changed.
This was an oversight in https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/10654
Merge branch 'stepan/remove-additional-pipeline' into 'main'
projects / thirdparty / bind9.git / log
