Async hooks are the last feature using the client->fetchhandle and
client->query.fetch pointers. Update ns_query_hookasync() and
query_hookresume() so that they use a dedicated slot in the 'recursions'
array. Note that async hooks are still not expected to initiate
recursion if one was already started by a prior ns_query_recurse() call,
so the REQUIRE assertion in ns_query_hookasync() needs to check the
RECTYPE_NORMAL slot rather than the RECTYPE_HOOK one.
With prefetch and RPZ code updated to use separate slots in the
'recursions' array, the code responsible for starting recursion in
ns_query_recurse() and resuming query handling in fetch_callback()
should follow suit, so that it does not need to explicitly cooperate
with other code paths that may initiate recursion.
Replace:
- client->fetchhandle with HANDLE_RECTYPE_NORMAL(client)
- client->query.fetch with FETCH_RECTYPE_NORMAL(client)
Also update other functions using client->fetchhandle and
client->query.fetch (ns_query_cancel(), query_usestale()) so that those
two fields can shortly be dropped altogether.
Both prefetch code and RPZ code ignore recursion results (caching the
response notwithstanding). RPZ code has been (ab)using that fact since
commit 08e36aa5a5 by employing
prefetch_done() as the fetch completion callback. This is only
seemingly a simplification as it makes the code harder to follow ("why
is prefetch code used for handling RPZ-triggered recursion?").
Turn prefetch_done() into a new function whose name clearly conveys its
purpose. Add a parameter to its prototype in order to allow callers to
specify which slot in the 'recursions' array it should use. Reintroduce
prefetch_done() as a wrapper for that function. Add rpzfetch_done(), an
RPZ-exclusive wrapper for that function (using a distinct recursion
type).
Since each slot in the 'recursions' array needs to be initialized before
getting cleaned up when recursion completes, rework fetch_and_forget()
so that it takes recursion type rather than extra fetch options as the
last parameter and make it use the requested slot in the 'recursions'
array rather than a fixed slot (RECTYPE_PREFETCH) for all callers. This
makes fetch_and_forget() a logical complement of cleanup_after_fetch().
Collectively, these changes make prefetch and RPZ code logically
separate (except for reusing client->recursionquota, which will be
refactored later).
Add a set of new helper functions for attaching to the recursion quota
in order to reduce code duplication and to ensure that the recursive
clients counter is always adjusted properly. Since some callers
(query_prefetch(), query_rpzfetch()) treat exceeding the soft quota as
an error while others (check_recursionquota()) do not, also add two
wrapper functions whose names help convey their purpose, in order to
improve code readability.
Replace:
- client->prefetchhandle with HANDLE_RECTYPE_PREFETCH(client)
- client->query.prefetch with FETCH_RECTYPE_PREFETCH(client)
This is preparatory work for separating prefetch code from RPZ code.
Add a new helper function for detaching from the recursion quota in
order to reduce code duplication and to ensure that detaching from that
quota is always accompanied by decreasing the recursive clients counter.
When a client waits for a prefetch- or RPZ-triggered recursion to
complete, its netmgr handle is attached to client->prefetchhandle and a
reference to the resolver fetch is stored in client->query.prefetch.
Both of these features use the same fields mentioned above. This makes
the code fragile and hard to follow as its logically distinct parts
become intertwined for no obvious reason.
Furthermore, storing pointers related to a specific recursion process in
two different structures makes their purpose harder to grasp than it has
to be.
To alleviate the problem, extend ns_query_t with an array of structures
containing recursion-related pointers. Each feature able to initiate
recursion is supposed to use its own slot in that array, allowing
logically unrelated code paths to be untangled. Prefetch and RPZ will
be the first users of that array.
Define helper macros for accessing specific recursion-related pointers
in order to improve code readability.
Initialize client->query using a compound literal in order to make the
ns_query_init() function shorter and more readable. This also prevents
the need to explicitly initialize any newly added fields in the future.
query_prefetch() and query_rpzfetch() contain a lot of duplicated code.
Extract the common bits into a separate function whose name clearly
suggests its purpose.
Under specific rare timing circumstances the uv_read_start() could
fail with UV_EINVAL when the connection is reset between the connect (or
accept) and the uv_read_start() call on the nmworker loop. Handle such
situation gracefully by propagating the errors from uv_read_start() into
upper layers, so the socket can be internally closed().
The statistics system test fails on Oracle Linux 7 when libxml2, Curl,
and xsltproc are present:
I:statistics:checking bind9.xsl vs xml (17)
diff: curl.out.17.xsl: No such file or directory
tests.sh: line 183: curl.out.17.xml: No such file or directory
cp: cannot stat 'curl.out.17.xml': No such file or directory
grep: xsltproc.out.17: No such file or directory
This is because the Oracle Linux 7 Curl does not know about the
--http1.1 option and silently fails with:
+ /usr/bin/curl --http1.1 http://10.53.0.3:7252
curl: option --http1.1: is unknown
curl: try 'curl --help' or 'curl --manual' for more information
The following test "checking bind9.xml socket statistics" then needs to
check for existence of stats.xml.out file which is artifact of the
previous test.
when serve-stale is enabled, NXDOMAIN cache entries are no longer
preserved after the normal negative cache TTL, in order to reduce
unnecessary cache memory consumption.
Commit 9ffb4a7ba1 causes Clang Static
Analyzer to flag a potential NULL dereference in query_nxdomain():
query.c:9394:26: warning: Dereference of null pointer [core.NullDereference]
if (!qctx->nxrewrite || qctx->rpz_st->m.rpz->addsoa) {
^~~~~~~~~~~~~~~~~~~
1 warning generated.
The warning above is for qctx->rpz_st potentially being a NULL pointer
when query_nxdomain() is called from query_resume(). This is a false
positive because none of the database lookup result codes currently
causing query_nxdomain() to be called (DNS_R_EMPTYWILD, DNS_R_NXDOMAIN)
can be returned by a database lookup following a recursive resolution
attempt. Add a NULL check nevertheless in order to future-proof the
code and silence Clang Static Analyzer.
ns_client_getnamebuf() cannot fail (i.e. return NULL) since commit
e31cc1eeb4. Remove redundant NULL checks
performed on the pointer returned by ns_client_getnamebuf().
ns_client_newname() cannot fail (i.e. return NULL) since commit
2ce0de6995 (though it was only made more
apparent by commit 33ba0057a7). Remove
redundant NULL checks performed on the pointer returned by
ns_client_newname().
ns_client_newrdataset() cannot fail (i.e. return NULL) since commit
efb385ecdc (though it was only made more
apparent by commit 33ba0057a7). Remove
redundant NULL checks performed on the pointer returned by
ns_client_newrdataset().
It turns out it is easier to regenerate Sphinx-mandated structure in
get_objects than to maintain two separate data structures. I should have
realized that before.
New directive .. statementlist:: generates table of statements in a
the given domain (named.conf or rndc.conf). The table contains link to
definition, short description, and also list of tags.
Short description and tags have to be provided by user using optional
parameters. E.g.:
.. statement:: max-cache-size
:tags: resolver, cache
:short: Short description
.. statementlist:: is currently not parametrized.
This modification is based on Sphinx "tutorial" extension "TODO".
The main trick is to use placeholder node for .. statementlist:: and
replace it with table at later stage, when all source files were
processed and all cross-references can be resolved.
Beware, some details in Sphinx docs are not up-to-date, it's better
to read Sphinx and docutil sources.
New and currently unused values can be provided using this syntax:
.. statement:: max-cache-size
:tags: resolver, cache
:short: Short description
The domain stores them in its internal structures for further use.
The extension provides a "Sphinx domain factory". Each new Sphinx domain
defines a namespace for configuration statements so named.conf and
rndc.conf do not clash. Currently the Sphinx domains are instantiated
twice and resuling domains are named "namedconf" and "rndcconf".
This commit adds a single new directive:
.. statement:: max-cache-size
It is namespaced like this:
.. namedconf:statement:: max-cache-size
This directive generates a new anchor for configuration statement and it
can be referenced like :any:`max-cache-size` (if the identifier is
unique), or more specific :namedconf:ref:`max-cache-size`.
It is based on Sphinx "tutorial" extension "recipe".
Beware, some details in Sphinx docs are not up-to-date, it's better
to read Sphinx and docutil sources.
The conversion of `DNS_R_PARTIALMATCH` into `DNS_R_NOTFOUND` is done
in the `dns_rbt_deletename()` function so there is no need to do that
in `dns_fwdtable_delete()`.
Add a possible return value of `ISC_R_NOSPACE` into the header file's
function description comment.
There is no reason for these two messages to be `ISC_LOG_INFO` while all
the other similar messages in `catz_addmodzone_taskaction()` and
`catz_delzone_taskaction()` functions are logged as `ISC_LOG_WARNING`.
When processing a catalog zone member zone make sure that there is no
configured pre-existing forward zone with that name.
Refactor the `dns_fwdtable_find()` function to not alter the
`DNS_R_PARTIALMATCH` result (coming from `dns_rbt_findname()`) into
`DNS_R_SUCCESS`, so that now the caller can differentiate partial
and exact matches. Patch the calling sites to expect and process
the new return value.
Historically, some *.py files were generated, so Python checks required
running ./configure beforehand. This is no longer the case since v9_18,
so let's run the job ASAP without the unnecessary extra dependency on
autoconf job.
