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 af97adf535fb6a1cbbd5431b5100e748c8251ba4..1423febec670003f183e309a206bfb91cf76f5fa 100644 (file)
--- 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 e5c22cdf95bd8a32607eafe9149ed88675640c39..01dfa2b6c5bb218a3253288339981545ded6cce5 100644 (file)
--- 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<DNSName>& allowedAdditionals, const DNSRecord& rec);
 
-void SyncRes::resolveAdditionals(const DNSName& qname, QType qtype, AdditionalMode mode, std::vector<DNSRecord>& additionals, unsigned int depth)
+void SyncRes::resolveAdditionals(const DNSName& qname, QType qtype, AdditionalMode mode, std::vector<DNSRecord>& additionals, unsigned int depth, bool& taskPushed)
 {
   vector<DNSRecord> 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<DNSRecord>&start, vector<DNSRecord>&additionals, std::set<std::pair<DNSName, QType>>& uniqueCalls, std::set<std::tuple<DNSName, QType, QType>>& uniqueResults, unsigned int depth, unsigned additionaldepth)
+void SyncRes::addAdditionals(QType qtype, const vector<DNSRecord>&start, vector<DNSRecord>&additionals, std::set<std::pair<DNSName, QType>>& uniqueCalls, std::set<std::tuple<DNSName, QType, QType>>& 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<DNSRecord>&start, vector<
       std::vector<DNSRecord> 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<DNSRecord>&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<DNSRecord>&ret, unsigned int depth)
+bool SyncRes::addAdditionals(QType qtype, vector<DNSRecord>&ret, unsigned int depth)
 {
   // The additional records of interest
   std::vector<DNSRecord> additionals;
@@ -425,12 +428,14 @@ void SyncRes::addAdditionals(QType qtype, vector<DNSRecord>&ret, unsigned int de
   // For RRSIGs, the type covered is stored in the second Qtype
   std::set<std::tuple<DNSName, QType, QType>> 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<ComboAddress> 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 c9e3abc0f1d446a17ae685a3c46189060c986a1a..c681063485c2fee388c6668d081424dc9dac97f0 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -768,9 +768,9 @@ private:
   typedef std::map<DNSName,vState> zonesStates_t;
   enum StopAtDelegation { DontStop, Stop, Stopped };
 
-  void resolveAdditionals(const DNSName& qname, QType qtype, AdditionalMode, std::vector<DNSRecord>& additionals, unsigned int depth);
-  void addAdditionals(QType qtype, const vector<DNSRecord>&start, vector<DNSRecord>&addditionals, std::set<std::pair<DNSName, QType>>& uniqueCalls, std::set<std::tuple<DNSName, QType, QType>>& uniqueResults, unsigned int depth, unsigned int adddepth);
-  void addAdditionals(QType qtype, vector<DNSRecord>&ret, unsigned int depth);
+  void resolveAdditionals(const DNSName& qname, QType qtype, AdditionalMode, std::vector<DNSRecord>& additionals, unsigned int depth, bool& pushed);
+  void addAdditionals(QType qtype, const vector<DNSRecord>&start, vector<DNSRecord>&addditionals, std::set<std::pair<DNSName, QType>>& uniqueCalls, std::set<std::tuple<DNSName, QType, QType>>& uniqueResults, unsigned int depth, unsigned int adddepth, bool& pushed);
+  bool addAdditionals(QType qtype, vector<DNSRecord>&ret, unsigned int depth);
 
   bool doDoTtoAuth(const DNSName& ns) const;
   int doResolveAt(NsSet &nameservers, DNSName auth, bool flawedNSSet, const DNSName &qname, QType qtype, vector<DNSRecord>&ret,