Ondřej Surý [Tue, 4 Jun 2024 09:21:24 +0000 (11:21 +0200)]
Remove the extra memory context with own arena for sending
The changes in this MR prevent the memory used for sending the outgoing
TCP requests to spike so much. That strictly remove the extra need for
own memory context, and thus since we generally prefer simplicity,
remove the extra memory context with own jemalloc arenas just for the
outgoing send buffers.
Ondřej Surý [Tue, 4 Jun 2024 07:12:45 +0000 (09:12 +0200)]
Limit the number of DNS message processed from a single TCP read
The single TCP read can create as much as 64k divided by the minimum
size of the DNS message. This can clog the processing thread and trash
the memory allocator because we need to do as much as ~20k allocations in
a single UV loop tick.
Limit the number of the DNS messages processed in a single UV loop tick
to just single DNS message and limit the number of the outstanding DNS
messages back to 23. This effectively limits the number of pipelined
DNS messages to that number (this is the limit we already had before).
Ondřej Surý [Tue, 4 Jun 2024 06:38:35 +0000 (08:38 +0200)]
Replace the tcp_buffers memory pool with static per-loop buffer
As a single thread can process only one TCP send at the time, we don't
really need a memory pool for the TCP buffers, but it's enough to have
a single per-loop (client manager) static buffer that's being used to
assemble the DNS message and then it gets copied into own sending
buffer.
In the future, this should get optimized by exposing the uv_try API
from the network manager, and first try to send the message directly
and allocate the sending buffer only if we need to send the data
asynchronously.
Aram Sargsyan [Tue, 12 Mar 2024 15:29:51 +0000 (15:29 +0000)]
ns_client: reuse TCP send buffers
Constantly allocating, reallocating and deallocating 64K TCP send
buffers by 'ns_client' instances takes too much CPU time.
There is an existing mechanism to reuse the ns_clent_t structure
associated with the handle using 'isc_nmhandle_getdata/_setdata'
(see ns_client_request()), but it doesn't work with TCP, because
every time ns_client_request() is called it gets a new handle even
for the same TCP connection, see the comments in
streamdns_on_complete_dnsmessage().
To solve the problem, we introduce an array of available (unused)
TCP buffers stored in ns_clientmgr_t structure so that a 'client'
working via TCP can have a chance to reuse one (if there is one)
instead of allocating a new one every time.
Ondřej Surý [Thu, 18 Jan 2024 16:24:22 +0000 (17:24 +0100)]
Throttle reading from TCP if the sends are not getting through
When TCP client would not read the DNS message sent to them, the TCP
sends inside named would accumulate and cause degradation of the
service. Throttle the reading from the TCP socket when we accumulate
enough DNS data to be sent. Currently this is limited in a way that a
single largest possible DNS message can fit into the buffer.
Artem Boldariev [Wed, 13 Mar 2024 16:04:46 +0000 (18:04 +0200)]
Keep the endpoints set reference within an HTTP/2 socket
This commit ensures that an HTTP endpoints set reference is stored in
a socket object associated with an HTTP/2 stream instead of
referencing the global set stored inside a listener.
This helps to prevent an issue like follows:
1. BIND is configured to serve DoH clients;
2. A client is connected and one or more HTTP/2 stream is
created. Internal pointers are now pointing to the data on the
associated HTTP endpoints set;
3. BIND is reconfigured - the new endpoints set object is created and
promoted to all listeners;
4. The old pointers to the HTTP endpoints set data are now invalid.
Instead referencing a global object that is updated on
re-configurations we now store a local reference which prevents the
endpoints set objects to go out of scope prematurely.
Artem Boldariev [Fri, 8 Dec 2023 12:26:46 +0000 (14:26 +0200)]
DoH: avoid potential use after free for HTTP/2 session objects
It was reported that HTTP/2 session might get closed or even deleted
before all async. processing has been completed.
This commit addresses that: now we are avoiding using the object when
we do not need it or specifically check if the pointers used are not
'NULL' and by ensuring that there is at least one reference to the
session object while we are doing incoming data processing.
This commit makes the code more resilient to such issues in the
future.
Ondřej Surý [Mon, 25 Mar 2024 11:17:42 +0000 (12:17 +0100)]
Use isc_queue to implement wait-free deadnodes queue
Replace the ISC_LIST based deadnodes implementation with isc_queue which
is wait-free and we don't have to acquire neither the tree nor node lock
to append nodes to the queue and the cleaning process can also
copy (splice) the list into a local copy without acquiring the list.
Currently, there's little benefit to this as we need to hold those
locks anyway, but in the future as we move to RCU based implementation,
this will be ready.
To align the cleaning with our event loop based model, remove the
hardcoded count for the node locks and use the number of the event loops
instead. This way, each event loop can have its own cleaning as part of
the process. Use uniform random numbers to spread the nodes evenly
between the buckets (instead of hashing the domain name).
Michał Kępień [Mon, 3 Jun 2024 11:07:21 +0000 (13:07 +0200)]
Fail for merge requests with "Affects v9.x" labels
Setting "Affects v9.x" labels on a merge request duplicates information
already present on the GitLab issue associated with that merge request.
For trivial merge requests that are not associated with any GitLab
issue, setting the "Affects v9.x" label(s) is considered unnecessary.
Trigger a failure for every merge request marked with at least one
"Affects v9.x" label.
Michał Kępień [Mon, 3 Jun 2024 11:07:21 +0000 (13:07 +0200)]
Warn about auto-generated merge request titles
Merge request titles auto-generated by GitLab are often a source of
confusion regarding the actual contents of a given merge request. Warn
for merge requests containing titles that look like auto-generated ones.
Michał Kępień [Mon, 3 Jun 2024 11:07:21 +0000 (13:07 +0200)]
Fail for branches using old-style version suffixes
Using "-v9_x" and "-v9.x" version suffixes for branch names is now
deprecated since some automation logic does not handle these. Fail for
any merge request using such old-style version suffixes.
Michał Kępień [Mon, 3 Jun 2024 11:07:21 +0000 (13:07 +0200)]
Fail for backports with "Affects v9.x" labels set
Backports are not expected to have any "Affects v9.x" labels set since
those are only meant to be set for merge requests that should have
backports created for them.
Mark Andrews [Wed, 29 Nov 2023 03:29:05 +0000 (14:29 +1100)]
Check that no primaries is logged with -4 or -6
When in -4 mode check that "IPv6 disabled and no IPv4 primaries"
is logged and when in -6 mode check that "IPv4 disabled and no IPv6
primaries" is logged.
Mark Andrews [Fri, 16 Feb 2024 00:40:26 +0000 (11:40 +1100)]
Use a new memory context when flushing the cache
When the cache's memory context was in over memory state when the
cache was flushed it resulted in LRU cleaning removing newly entered
data in the new cache straight away until the old cache had been
destroyed enough to take it out of over memory state. When flushing
the cache create a new memory context for the new db to prevent this.
Ondřej Surý [Tue, 28 May 2024 14:13:31 +0000 (16:13 +0200)]
Create the new database for AXFR from the dns_zone API
The `axfr_makedb()` didn't set the loop on the newly created database,
effectively killing delayed cleaning on such database. Move the
database creation into dns_zone API that knows all the gory details of
creating new database suitable for the zone.
Disable deadlines for hypothesis tests when running in CI
The times it takes to run tests CI vary significantly enough
that it makes hypothesis test reach their deadlines and fail randomly
marking the tests as flaky.
This commit disables the deadlines when running in CI.
Štěpán Balážik [Thu, 21 Dec 2023 19:25:20 +0000 (20:25 +0100)]
Extend isctest package with more utility functions
Check for more rcodes and various properties needed in the wildcard
test. Add a `name` module for various dns.name.Name operations (with
`prepend_label` function only now).
Expose `timeout` as a parameter of `query.tcp`/`query.udp`.
The system test from the BIND 9.19.24 release does not include the
isctest/vars/autoconf.py file from 9.19.25-dev, and therefore the job
will fail before the 9.19.25 release is published. In the meantime,
consider using the conf.sh file.
Refactor code like this into peek_uint16() and get_uint16 macros
to prevent code repetition and possible mistakes when copy and
pasting the same code over and over.
As a side note for an entertainment of a careful reader of the commit
messages: The byte manipulation was changed from multiplication and
addition to shift with or.
The difference in the assembly looks like this:
MUL and ADD:
movzx eax, BYTE PTR [rdi]
movzx edi, BYTE PTR [rdi+1]
sal eax, 8
or edi, eax
SHIFT and OR:
movzx edi, WORD PTR [rdi]
rol di, 8
movzx edi, di
If the result and/or buffer is then being used after the macro call,
there's more differences in favor of the SHIFT+OR solution.
Aydın Mercan [Fri, 17 May 2024 13:45:10 +0000 (16:45 +0300)]
fix typing mistakes in trace macros
The detach function declaration in `ISC__REFCOUNT_TRACE_DECL` had an
returned an accidental implicit int. While not allowed since C99, it
became an error by default in GCC 14.
`ISC_REFCOUNT_TRACE_IMPL` and `ISC_REFCOUNT_STATIC_TRACE_IMPL` expanded
into the wrong macros, trying to declare it again with the wrong number
of parameters.
Mark Andrews [Mon, 29 Jan 2024 18:21:37 +0000 (10:21 -0800)]
add test cases for several FORMERR code paths:
- duplicated question
- duplicated answer
- qtype as an answer
- two question types
- question names
- nsec3 bad owner name
- short record
- short question
- mismatching question class
- bad record owner name
- mismatched class in record
- mismatched KEY class
- OPT wrong owner name
- invalid RRSIG "covers" type
- UPDATE malformed delete type
- TSIG wrong class
- TSIG not the last record
Matthijs Mekking [Wed, 15 May 2024 09:35:31 +0000 (11:35 +0200)]
Rewrite qp fix_iterator()
The fix_iterator() function had a lot of bugs in it and while fixing
them, the number of corner cases and the complexity of the function
got out of hand. Rewrite the function with the following modifications:
The function now requires that the iterator is pointing to a leaf node.
This removes the cases we have to deal when the iterator was left on a
dead branch.
From the leaf node, pop up the iterator stack until we encounter the
branch where the offset point is before the point where the search key
differs. This will bring us to the right branch, or at the first
unmatched node, in which case we pop up to the parent branch. From
there it is easier to retrieve the predecessor.
Once we are at the right branch, all we have to do is find the right
twig (which is either the twig for the character at the position where
the search key differs, or the previous twig) and walk down from there
to the greatest leaf or, in case there is no good twig, get the
previous twig from the successor and get the greatest leaf from there.
If there is no previous twig to select in this branch, because every
leaf from this branch node is greater than the one we wanted, we need
to pop up the stack again and resume at the parent branch. This is
achieved by calling prevleaf().
Matthijs Mekking [Wed, 15 May 2024 08:59:07 +0000 (10:59 +0200)]
Get anyleaf when qp lookup is on a dead end branch
Move the fix_iterator out of the loop and only call it when we found
a leaf node. This leaf node may be the wrong leaf node, but fix_iterator
should correct that.
Also, when we don't need to set the iterator, just get any leaf. We
only need to have a leaf for the qpkey_compare and the end result does
not matter if compare was against an ancestor leaf or any leaf below
that point.
Mark Andrews [Tue, 9 Jan 2024 01:22:21 +0000 (12:22 +1100)]
Add regression test data for [GL #4517]
An obscured DNSKEY RRset at a delegation was incorrectly added to
the NSEC/NSEC3 type bit map leading to zone verification failures.
This adds such a RRset to the test zone.
Mark Andrews [Tue, 9 Jan 2024 06:01:07 +0000 (17:01 +1100)]
Fail if there are non apex DNSKEYs
DNSSEC only works when DNSKEYs are self signed. This only occurs
when the DNSKEY RRset is at the apex. Cause dnssec-signzone to
fail if it attempts to sign an non-apex DNSKEY RRset.