From 4b0f1db975da34b45139f2338365a663986cac97 Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Fri, 8 Apr 2022 09:08:27 +0200 Subject: [PATCH] Proces review comments: rename variable and some words about expirig additionals in docs. --- .../docs/lua-config/additionals.rst | 20 +++++++++++++------ pdns/syncres.cc | 20 +++++++++---------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/pdns/recursordist/docs/lua-config/additionals.rst b/pdns/recursordist/docs/lua-config/additionals.rst index 1423febec6..7d1395ce01 100644 --- a/pdns/recursordist/docs/lua-config/additionals.rst +++ b/pdns/recursordist/docs/lua-config/additionals.rst @@ -41,24 +41,32 @@ If needed, the Recursor will do an active resolve to retrieve these records. The modes available are: ``pdns.AdditionalMode.Ignore`` - Do not do any additional processing for this qtype. This is equivalent to not calling :func:`addAllowedAdditionalQType` for the qtype. + Do not do any additional processing for this qtype. + This is equivalent to not calling :func:`addAllowedAdditionalQType` for the qtype. ``pdns.AdditionalMode.CacheOnly`` - Look in the record cache for available records. Allow non-authoritative (learned from additional sections received from authoritative servers) records to be added. + Look in the record cache for available records. + Allow non-authoritative (learned from additional sections received from authoritative servers) records to be added. ``pdns.AdditionalMode.CacheOnlyRequireAuth`` - Look in the record cache for available records. Only authoritative records will be added. These are records received from the nameservers for the specific domain. + Look in the record cache for available records. + Only authoritative records will be added. These are records received from the nameservers for the specific domain. ``pdns.AdditionalMode.ResolveImmediately`` - Add records from the record cache (including DNSSEC records if relevant). If no record is found in the record cache, actively try to resolve the target name/qtype. This will delay the answer to the client. + Add records from the record cache (including DNSSEC records if relevant). + If no record is found in the record cache, actively try to resolve the target name/qtype. + This will delay the answer to the client. ``pdns.AdditionalMode.ResolveDeferred`` - Add records from the record cache (including DNSSEC records if relevant). If no record is found in the record cache and the negative cache also has no entry, schedule a background task to resolve the target name/qtype. The next time the query is processed, the cache might hold the relevant information. + Add records from the record cache (including DNSSEC records if relevant). + If no record is found in the record cache and the negative cache also has no entry, schedule a task to resolve the target name/qtype. + The next time the query is processed, the cache might hold the relevant information. If a task is pushed, the answer that triggered it will be marked as variable and consequently not stored into the packet cache. - If an additional record is not available at that time the query is stored into the packet cache the answer packet stored in the packet cache will not contain the additional record. Clients repeating the same question will get an answer from the packet cache if the question is still in the packet cache. These answers do not have the additional record, even if the record cache has learned it in the meantime . Clients will only see the additional record once the packet cache entry expires and the record cache is consulted again. The ``pdns.AdditionalMode.ResolveImmediately`` mode will not have this issue, at the cost of delaying the first query to resolve the additional records needed. The ``pdns.AdditionalMode.ResolveDeferred`` mode will only store answers in the packet cache if it determines that no deferred tasks are needed, i.e. either a positive or negative answer for potential additional records is available. +If the additional records for an answer have low TTLs compared to the records in the answer section, tasks will be pushed often. +Until all tasks for the answer have completed the packet cache will not contain the answer, making the packet cache less effective for this specific answer. Configuring additional record processing ---------------------------------------- diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 01dfa2b6c5..43a52e994c 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -254,7 +254,7 @@ SyncRes::SyncRes(const struct timeval& now) : d_authzonequeries(0), d_outquerie static void allowAdditionalEntry(std::unordered_set& allowedAdditionals, const DNSRecord& rec); -void SyncRes::resolveAdditionals(const DNSName& qname, QType qtype, AdditionalMode mode, std::vector& additionals, unsigned int depth, bool& taskPushed) +void SyncRes::resolveAdditionals(const DNSName& qname, QType qtype, AdditionalMode mode, std::vector& additionals, unsigned int depth, bool& additionalsNotInCache) { vector addRecords; @@ -335,7 +335,7 @@ void SyncRes::resolveAdditionals(const DNSName& qname, QType qtype, AdditionalMo // An example is a SOA-less NODATA response. Rate limiting will kick in if those tasks are pushed too often. // We might want to fix these cases (and always either store positive or negative) some day. pushResolveTask(qname, qtype, d_now.tv_sec, d_now.tv_sec + 60); - taskPushed = true; + additionalsNotInCache = true; } break; } @@ -350,7 +350,7 @@ void SyncRes::resolveAdditionals(const DNSName& qname, QType qtype, AdditionalMo // This function uses to state sets to avoid infinite recursion and allow depulication // depth is the main recursion depth // additionaldepth is the depth for addAdditionals itself -void SyncRes::addAdditionals(QType qtype, const vector&start, vector&additionals, std::set>& uniqueCalls, std::set>& uniqueResults, unsigned int depth, unsigned additionaldepth, bool& taskPushed) +void SyncRes::addAdditionals(QType qtype, const vector&start, vector&additionals, std::set>& uniqueCalls, std::set>& uniqueResults, unsigned int depth, unsigned additionaldepth, bool& additionalsNotInCache) { if (additionaldepth >= 5 || start.empty()) { return; @@ -382,7 +382,7 @@ void SyncRes::addAdditionals(QType qtype, const vector&start, vector< std::vector records; bool inserted = uniqueCalls.emplace(addname, targettype).second; if (inserted) { - resolveAdditionals(addname, targettype, mode, records, depth, taskPushed); + resolveAdditionals(addname, targettype, mode, records, depth, additionalsNotInCache); } if (!records.empty()) { for (auto r = records.begin(); r != records.end(); ) { @@ -409,7 +409,7 @@ void SyncRes::addAdditionals(QType qtype, const vector&start, vector< } uniqueResults.emplace(r.d_name, r.d_type, covered); } - addAdditionals(targettype, records, additionals, uniqueCalls, uniqueResults, depth, additionaldepth + 1, taskPushed); + addAdditionals(targettype, records, additionals, uniqueCalls, uniqueResults, depth, additionaldepth + 1, additionalsNotInCache); } } } @@ -428,14 +428,14 @@ bool SyncRes::addAdditionals(QType qtype, vector&ret, unsigned int de // For RRSIGs, the type covered is stored in the second Qtype std::set> uniqueResults; - bool pushed = false; - addAdditionals(qtype, ret, additionals, uniqueCalls, uniqueResults, depth, 0, pushed); + bool additionalsNotInCache = false; + addAdditionals(qtype, ret, additionals, uniqueCalls, uniqueResults, depth, 0, additionalsNotInCache); for (auto& rec : additionals) { rec.d_place = DNSResourceRecord::ADDITIONAL; ret.push_back(std::move(rec)); } - return pushed; + return additionalsNotInCache; } /** everything begins here - this is the entry point just after receiving a packet */ @@ -490,8 +490,8 @@ int SyncRes::beginResolve(const DNSName &qname, const QType qtype, QClass qclass // Avoid calling addAdditionals() if we know we won't find anything auto luaLocal = g_luaconfs.getLocal(); if (res == 0 && qclass == QClass::IN && luaLocal->allowAdditionalQTypes.find(qtype) != luaLocal->allowAdditionalQTypes.end()) { - bool taskPushed = addAdditionals(qtype, ret, depth); - if (taskPushed) { + bool additionalsNotInCache = addAdditionals(qtype, ret, depth); + if (additionalsNotInCache) { d_wasVariable = true; } } -- 2.47.2