From: Otto Moerbeek Date: Fri, 1 Apr 2022 07:58:35 +0000 (+0200) Subject: Mark an answer variable if a task was push. X-Git-Tag: rec-4.7.0-beta1~10^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d7f0419371f40f559e99d9cc0eba44a4ff83b8cd;p=thirdparty%2Fpdns.git Mark an answer variable if a task was push. Plus docs and small fix in AAAA async resolve task test. --- diff --git a/pdns/recursordist/docs/lua-config/additionals.rst b/pdns/recursordist/docs/lua-config/additionals.rst index af97adf535..1423febec6 100644 --- a/pdns/recursordist/docs/lua-config/additionals.rst +++ b/pdns/recursordist/docs/lua-config/additionals.rst @@ -49,13 +49,16 @@ The modes available are: ``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. ``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. This mode is not implemented yet. + 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. + 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. Configuring additional record processing ---------------------------------------- diff --git a/pdns/syncres.cc b/pdns/syncres.cc index e5c22cdf95..01dfa2b6c5 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) +void SyncRes::resolveAdditionals(const DNSName& qname, QType qtype, AdditionalMode mode, std::vector& additionals, unsigned int depth, bool& taskPushed) { vector addRecords; @@ -331,8 +331,11 @@ void SyncRes::resolveAdditionals(const DNSName& qname, QType qtype, AdditionalMo NegCache::NegCacheEntry ne; bool inNegCache = g_negCache->get(qname, qtype, d_now, ne, false); if (!inNegCache) { + // There are a few cases where an answer is neither stored in the record cache nor in the neg cache. + // 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); - } else { + taskPushed = true; } break; } @@ -347,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) +void SyncRes::addAdditionals(QType qtype, const vector&start, vector&additionals, std::set>& uniqueCalls, std::set>& uniqueResults, unsigned int depth, unsigned additionaldepth, bool& taskPushed) { if (additionaldepth >= 5 || start.empty()) { return; @@ -379,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); + resolveAdditionals(addname, targettype, mode, records, depth, taskPushed); } if (!records.empty()) { for (auto r = records.begin(); r != records.end(); ) { @@ -406,14 +409,14 @@ 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); + addAdditionals(targettype, records, additionals, uniqueCalls, uniqueResults, depth, additionaldepth + 1, taskPushed); } } } } // The entry point for other code -void SyncRes::addAdditionals(QType qtype, vector&ret, unsigned int depth) +bool SyncRes::addAdditionals(QType qtype, vector&ret, unsigned int depth) { // The additional records of interest std::vector additionals; @@ -425,12 +428,14 @@ void SyncRes::addAdditionals(QType qtype, vector&ret, unsigned int de // For RRSIGs, the type covered is stored in the second Qtype std::set> uniqueResults; - addAdditionals(qtype, ret, additionals, uniqueCalls, uniqueResults, depth, 0); + bool pushed = false; + addAdditionals(qtype, ret, additionals, uniqueCalls, uniqueResults, depth, 0, pushed); for (auto& rec : additionals) { rec.d_place = DNSResourceRecord::ADDITIONAL; ret.push_back(std::move(rec)); } + return pushed; } /** everything begins here - this is the entry point just after receiving a packet */ @@ -485,7 +490,10 @@ 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()) { - addAdditionals(qtype, ret, depth); + bool taskPushed = addAdditionals(qtype, ret, depth); + if (taskPushed) { + d_wasVariable = true; + } } d_eventTrace.add(RecEventTrace::SyncRes, res, false); return res; @@ -1649,7 +1657,7 @@ vector SyncRes::getAddrs(const DNSName &qname, unsigned int depth, // No IPv6 records in cache, check negcache and submit async task if negache does not have the data // so that the next time the cache or the negcache will have data NegCache::NegCacheEntry ne; - bool inNegCache = g_negCache->get(qname, QType::AAAA, d_now, ne, true); + bool inNegCache = g_negCache->get(qname, QType::AAAA, d_now, ne, false); if (!inNegCache) { pushResolveTask(qname, QType::AAAA, d_now.tv_sec, d_now.tv_sec + 60); } diff --git a/pdns/syncres.hh b/pdns/syncres.hh index c9e3abc0f1..c681063485 100644 --- a/pdns/syncres.hh +++ b/pdns/syncres.hh @@ -768,9 +768,9 @@ private: typedef std::map zonesStates_t; enum StopAtDelegation { DontStop, Stop, Stopped }; - void resolveAdditionals(const DNSName& qname, QType qtype, AdditionalMode, std::vector& additionals, unsigned int depth); - void addAdditionals(QType qtype, const vector&start, vector&addditionals, std::set>& uniqueCalls, std::set>& uniqueResults, unsigned int depth, unsigned int adddepth); - void addAdditionals(QType qtype, vector&ret, unsigned int depth); + void resolveAdditionals(const DNSName& qname, QType qtype, AdditionalMode, std::vector& additionals, unsigned int depth, bool& pushed); + void addAdditionals(QType qtype, const vector&start, vector&addditionals, std::set>& uniqueCalls, std::set>& uniqueResults, unsigned int depth, unsigned int adddepth, bool& pushed); + bool addAdditionals(QType qtype, vector&ret, unsigned int depth); bool doDoTtoAuth(const DNSName& ns) const; int doResolveAt(NsSet &nameservers, DNSName auth, bool flawedNSSet, const DNSName &qname, QType qtype, vector&ret,