On some platforms, when a synchronizing barrier is cleared, one
thread can progress while other threads are still in the process
of releasing the barrier. If a barrier is reused by the progressing
thread during this window, it can cause a deadlock. This can occur if,
for example, we stop listening immediately after we start, because the
stop and listen functions both use socket->barrier. This has been
addressed by using separate barrier objects for stop and listen.
There were couple of redundant macros on both sides of
DNS_RBTDB_STRONG_RWLOCK_CHECK #ifdef block. Use a single set of
macros, but disable the extra REQUIRES if the #define is not set.
Extend the expire_header() to accept the node lock type as one of the
arguments and check whether the the node lock is always write locked +
fix that bug.
While doing that, it was found that expire_header() invocation in
rdataset_expire() passes `false` as a type of tree lock instead of
`isc_rwlocktype_none`.
(Un)fortunately, both values mapped to 0, so no harm was done, but it
has been fixed nevertheless.
There was a repetetive pattern:
if (NODE_TRYUPGRADE(&nodelock->lock, nlocktypep) != ISC_R_SUCCESS)
{
NODE_UNLOCK(&nodelock->lock, nlocktypep);
NODE_WRLOCK(&nodelock->lock, nlocktypep);
}
Instead of doing that over again, introduce new NODE_FORCEUPGRADE()
and TREE_FORCEUPGRADE() that does exactly this code, and simplify
the aforementioned code with just:
NODE_FORCEUPGRADE(&nodelock->lock, nlocktypep);
Previously, dns_dispatch_gettcp() could pick a TCP connection created by
different thread - this breaks our contractual promise to DNS dispatch
by using the TCP connection on a different thread than it was created.
Add .tid member to the dns_dispatch_t struct and skip the dispatches
from other threads when looking up a TCP dispatch that we can reuse in
dns_request.
NOTE: This is going to be properly refactored, but this change could be
also backported to 9.18 for better stability and thread-affinity.
Since dns_master_dump() can no longer return DNS_R_CONTINUE,
the error recovery code in synchronous calls to zone_dump() had
branches that could no longer be reached. This has been cleaned
up by using a boolean variable to determine whether the write
is asynchronous rather than depending on the result code.
The following report suggests that the 'size' parameter in the two
calls to 'isc_mem_get()' should be set to the
'count * sizeof(*tlsnames)' and 'count * sizeof(*keynames)'
respectively.
/lib/dns/remote.c: 117 in dns_remote_init()
111 }
112 } else {
113 remote->keynames = NULL;
114 }
115
116 if (tlsnames != NULL) {
>>> CID 432259: (SIZEOF_MISMATCH)
>>> Passing argument "count * 8UL /* sizeof (tlsnames) */" to
>>> function "isc__mem_get" and then casting the return value to
>>> "dns_name_t **" is suspicious. In this particular case
>>> "sizeof (dns_name_t **)" happens to be equal to
>>> "sizeof (dns_name_t *)", but this is not a portable assumption.
117 remote->tlsnames = isc_mem_get(mctx, count * sizeof(tlsnames));
118 for (i = 0; i < count; i++) {
119 remote->tlsnames[i] = NULL;
120 }
121 for (i = 0; i < count; i++) {
122 if (tlsnames[i] != NULL) {
/lib/dns/remote.c: 99 in dns_remote_init()
93 memmove(remote->dscps, dscp, count * sizeof(isc_dscp_t));
94 } else {
95 remote->dscps = NULL;
96 }
97
98 if (keynames != NULL) {
>>> CID 432259: (SIZEOF_MISMATCH)
>>> Passing argument "count * 8UL /* sizeof (keynames) */" to
>>> function "isc__mem_get" and then casting the return value to
>>> "dns_name_t **" is suspicious. In this particular case
>>> "sizeof (dns_name_t **)" happens to be equal to "sizeof
>>> (dns_name_t *)", but this is not a portable assumption.
99 remote->keynames = isc_mem_get(mctx, count * sizeof(keynames));
100 for (i = 0; i < count; i++) {
101 remote->keynames[i] = NULL;
102 }
103 for (i = 0; i < count; i++) {
104 if (keynames[i] != NULL) {
When shutting down, the cleanup path should not try to destroy
'newnodes', because it is NULL at that point.
Introduce another label for the "shuttingdown" scenario.
The clean_namehooks() function does't hold the 'adb->entries_lock'
lock, so calling maybe_expire_entry() is not thread-safe.
Instead of adding a lock/unlock, leave the expiration to later,
e.g. by the get_attached_and_locked_entry() function.
Also fix a couple of comment typos.
This commit replaces ad-hoc code for send requests buffer management
within TLS with the one based on isc_buffer_t.
Previous version of the code was trying to use pre-allocated small
buffers to avoid extra allocations. The code would allocate a larger
dynamic buffer when needed. There is no need to have ad-hoc code for
this, as isc_buffer_t now provides this functionality internally.
Additionally to the above, the old version of the code lacked any
logic to reuse the dynamically allocated buffers. Now, as we do not
manage memory buffers, but isc_buffer_t objects, we can implement this
strategy. It can be in particular helpful for longer lasting
connections, as in this case the buffer will adjust itself to the size
of the messages being transferred. That is, it is in particular useful
for XoT, as Stream DNS happen to order send requests in such a way
that the send request will get reused.
Remove parsing the configuration options 'alt-transfer-source',
'alt-transfer-source-v6', and 'use-alt-transfer-source', and remove
the corresponding code that implements the feature.
Use the configured 'source' and 'source-v6' when initiating a zone
transfer, sending a notify, or when checking for the DS. Remove the
special code for using alternate transfer sources.
Update some system tests to use the new configuration and make sure
the tests still work.
Add a new way to configure the preferred source address when talking to
remote servers such as primaries and parental-agents. This will
eventually deprecate options such as 'parental-source',
'parental-source-v6', 'transfer-source', etc.
Example of the new configuration:
parental-agents "parents" port 5353 \
source 10.10.10.10 port 5354 dscp 54 \
source-v6 2001:db8::10 port 5355 dscp 55 {
10.10.10.11;
2001:db8::11;
};
The dns_remote_t structure is intended to replace the variables in
the structure that deals with remote server communication to primaries,
parental agents, forwarders, etc.
Normally, when a 'resquery_t' object is created in fctx_query(),
we call dns_adb_beginudpfetch() (which increases the ADB quota)
only if it's a UDP query. Then, in fctx_cancelquery(), we call
dns_adb_endudpfetch() to decreases back the ADB quota, again only
if it's a UDP query.
The problem is that a UDP query can become a TCP query, preventing
the quota from adjusting back in fctx_cancelquery() later.
Call dns_adb_beginudpfetch() also when switching the query type
from UDP to TCP.
Additionally to renaming, it changes the function definition so that
it accepts a pointer to pointer instead of returning a pointer to the
new object.
It is mostly done to make it in line with other functions in the
module.
Additionally to renaming, it changes the function definition so that
it accepts a pointer to pointer instead of returning a pointer to the
new object.
It is mostly done to make it in line with other functions in the
module.
The dns_request code is very sensitive about calling the connected and
deadlocks when the timing is "right" in several places. Move the call
to the connected callback to the (udp|tcp)_connected() functions, so
they are called asynchronously instead of directly from
the (udp|tcp)_dispentry_cancel() functions.
The TCP dispatches are removed from the dispatchmgr->list in the
dispatch_destroy() and there's a brief period of time where
dns_dispatch_gettcp() can find a dispatch in connected state that's
being destroyed.
Set the dispatch state to DNS_DISPATCHSTATE_NONE in the TCP connection
callback if there are no responses waiting, and ignore TCP dispatches
with zero references in dns_dispatch_gettcp().
In tcp_connected() a typo has turned a DbC check into an assignment
breaking the state machine and making the dns_dispatch_gettcp() try to
attach to dispatch in process of destruction.
The TCP dispatches in DNS_DISPATCHSTATE_NONE could be either very
fresh or those could be dispatches that failed connecting to the
destination. Ignore them when trying to connect to an existing
TCP dispatch via dns_dispatch_gettcp().
This commit modifies the Stream DNS message so that it uses the
optimised code path (isc__nm_senddns()) for sending DNS messages over
the underlying transport. This way we avoid allocating any
intermediate memory buffers needed to render a DNS message with its
length pre-pended ahead of the contents (TCP DNS message format).
The new internal function works in the same way as isc_nm_send()
except that it sends a DNS message size ahead of the DNS message
data (the format used in DNS over TCP).
The intention is to provide a fast path for sending DNS messages over
streams protocols - that is, without allocating any intermediate
memory buffers.
This commit optimises TLS send request object allocation to enable
send request object reuse, somewhat reducing pressure on the memory
manager. It is especially helpful in the case when Stream DNS uses the
TLS implementation as the transport.
This commit unties generic TLS code (isc_nm_tlssocket) from DoH, so
that it will be available regardless of the fact if BIND was built
with DNS over HTTP support or not.
This commit ensures that Stream DNS code attempts to disable Nagle's
algorithm regardless of underlying stream transport (TCP or TLS), as
we are not interested in trading latency for throughout when dealing
with DNS messages.
This commit ensures that Nagle's algorithm is disabled by default for
TLS connections on best effort basis, just like other networking
software (e.g. NGINX) does, as, in the case of TLS, we are not
interested in trading latency for throughput, rather vice versa.
We attempt to disable it as early as we can, right after TCP
connections establishment, as an attempt to speed up handshake
handling.
This commit adds ability to turn the Nagle's algorithm on or off via
connections handle. It adds the isc_nmhandle_set_tcp_nodelay()
function as the public interface for this functionality.
This commit adds an initial implementation of isc_nm_streamdnssocket
transport: a unified transport for DNS over stream protocols messages,
which is capable of replacing both TCP DNS and TLS DNS
transports. Currently, the interface it provides is a unified set of
interfaces provided by both of the transports it attempts to replace.
The transport is built around "isc_dnsbuffer_t" and
"isc_dnsstream_assembler_t" objects and attempts to minimise both the
number of memory allocations during network transfers as well as
memory usage.
This commit adds the implementation for an "isc_dnsstream_assembler_t"
object. The object is built on top of "isc_dnsbuffer_t" and is
intended to encapsulate the state machine used for handling DNS
messages received in the format used for messages transmitted over
TCP.
The idea is that the object accepts the input data received from a
socket, tries to assemble DNS messages from the incoming data and
calls the callback which contains the status of the incoming data as
well as a pointer to the memory region referencing the data of the
assembled message. It is capable of assembling DNS messages no matter
how torn apart they are when sent over network.
The following statuses might be passed to the callback:
* ISC_R_SUCCESS - a message has been successfully assembled;
* ISC_R_NOMORE - not enough data has been processed to assemble a
message;
* ISC_R_RANGE - there was an attempt to process a zero-sized DNS
message (someone attempts to send us junk data).
One could say that the object replaces the implementation of
"isc__nm_*_processbuffer()" functions used by the old TCP DNS and TLS
DNS transports with a better defined state machine completely
decoupled from the networking code itself.
Such a design makes it trivial to write unit tests for it, leading to
better verification of its correctness.
Another important difference is directly related to the fact that it
is built on top of "isc_dnsbuffer_t", which tries to manage memory in
a smart way. In particular:
* It tries to use a static buffer for smaller messages, reducing
pressure on the memory manager (hot path);
* When allocating dynamic memory for larger messages, it tries to
allocate memory conservatively (generic path).
These characteristics is a significant upgrade over the older logic
where a 64KB(+2 bytes) buffer was allocated from dynamic memory
regardless of the fact if we need a buffer this large or not. That is,
lesser memory usage is expected in a generic case for DNS transports
built on top of "isc_dnsstream_assembler_t."
