]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
Change main serve stale loop to catch exception
authorOtto Moerbeek <otto.moerbeek@open-xchange.com>
Wed, 29 Jun 2022 09:19:06 +0000 (11:19 +0200)
committerOtto Moerbeek <otto.moerbeek@open-xchange.com>
Fri, 9 Sep 2022 07:43:34 +0000 (09:43 +0200)
pdns/syncres.cc
pdns/syncres.hh

index e7fc6bd34b631d830d899e8008234cb814ac496f..406a898c5aeb719667bf10b2b9f1d4368aa82cc6 100644 (file)
@@ -1810,212 +1810,219 @@ int SyncRes::doResolveNoQNameMinimization(const DNSName &qname, const QType qtyp
     const bool serveStale = loop == 1;
 
     // When we're not on the last iteration, a timeout is not fatal
-    d_exceptionOnTimeout = loop == iterations - 1;
+    const bool exceptionOnTimeout = loop == iterations - 1;
 
-    // This is a difficult way of expressing "this is a normal query", i.e. not getRootNS.
-    if(!(d_updatingRootNS && qtype.getCode()==QType::NS && qname.isRoot())) {
-      if(d_cacheonly) { // very limited OOB support
-        LWResult lwr;
-        LOG(prefix<<qname<<": Recursion not requested for '"<<qname<<"|"<<qtype<<"', peeking at auth/forward zones"<<endl);
-        DNSName authname(qname);
-        domainmap_t::const_iterator iter=getBestAuthZone(&authname);
-        if(iter != t_sstorage.domainmap->end()) {
-          if(iter->second.isAuth()) {
-            ret.clear();
-            d_wasOutOfBand = doOOBResolve(qname, qtype, ret, depth, res);
-            if (fromCache)
-              *fromCache = d_wasOutOfBand;
-            return res;
-          }
-          else if (considerforwards) {
-            const vector<ComboAddress>& servers = iter->second.d_servers;
-            const ComboAddress remoteIP = servers.front();
-            LOG(prefix<<qname<<": forwarding query to hardcoded nameserver '"<< remoteIP.toStringWithPort()<<"' for zone '"<<authname<<"'"<<endl);
-
-            boost::optional<Netmask> nm;
-            bool chained = false;
-            // forwardes are "anonymous", so plug in an empty name; some day we might have a fancier config language...
-            auto resolveRet = asyncresolveWrapper(remoteIP, d_doDNSSEC, qname, authname, qtype.getCode(), false, false, &d_now, nm, &lwr, &chained, DNSName());
-
-            d_totUsec += lwr.d_usec;
-            accountAuthLatency(lwr.d_usec, remoteIP.sin4.sin_family);
-            if (fromCache)
-              *fromCache = true;
-
-            // filter out the good stuff from lwr.result()
-            if (resolveRet == LWResult::Result::Success) {
-              for(const auto& rec : lwr.d_records) {
-                if(rec.d_place == DNSResourceRecord::ANSWER)
-                  ret.push_back(rec);
-              }
-              return 0;
+    try {
+      // This is a difficult way of expressing "this is a normal query", i.e. not getRootNS.
+      if(!(d_updatingRootNS && qtype.getCode()==QType::NS && qname.isRoot())) {
+        if(d_cacheonly) { // very limited OOB support
+          LWResult lwr;
+          LOG(prefix<<qname<<": Recursion not requested for '"<<qname<<"|"<<qtype<<"', peeking at auth/forward zones"<<endl);
+          DNSName authname(qname);
+          domainmap_t::const_iterator iter=getBestAuthZone(&authname);
+          if(iter != t_sstorage.domainmap->end()) {
+            if(iter->second.isAuth()) {
+              ret.clear();
+              d_wasOutOfBand = doOOBResolve(qname, qtype, ret, depth, res);
+              if (fromCache)
+                *fromCache = d_wasOutOfBand;
+              return res;
             }
-            else {
-              return RCode::ServFail;
+            else if (considerforwards) {
+              const vector<ComboAddress>& servers = iter->second.d_servers;
+              const ComboAddress remoteIP = servers.front();
+              LOG(prefix<<qname<<": forwarding query to hardcoded nameserver '"<< remoteIP.toStringWithPort()<<"' for zone '"<<authname<<"'"<<endl);
+
+              boost::optional<Netmask> nm;
+              bool chained = false;
+              // forwardes are "anonymous", so plug in an empty name; some day we might have a fancier config language...
+              auto resolveRet = asyncresolveWrapper(remoteIP, d_doDNSSEC, qname, authname, qtype.getCode(), false, false, &d_now, nm, &lwr, &chained, DNSName());
+
+              d_totUsec += lwr.d_usec;
+              accountAuthLatency(lwr.d_usec, remoteIP.sin4.sin_family);
+              if (fromCache)
+                *fromCache = true;
+
+              // filter out the good stuff from lwr.result()
+              if (resolveRet == LWResult::Result::Success) {
+                for(const auto& rec : lwr.d_records) {
+                  if(rec.d_place == DNSResourceRecord::ANSWER)
+                    ret.push_back(rec);
+                }
+                return 0;
+              }
+              else {
+                return RCode::ServFail;
+              }
             }
           }
         }
-      }
 
-      DNSName authname(qname);
-      bool wasForwardedOrAuthZone = false;
-      bool wasAuthZone = false;
-      bool wasForwardRecurse = false;
-      domainmap_t::const_iterator iter = getBestAuthZone(&authname);
-      if(iter != t_sstorage.domainmap->end()) {
-        const auto& domain = iter->second;
-        wasForwardedOrAuthZone = true;
+        DNSName authname(qname);
+        bool wasForwardedOrAuthZone = false;
+        bool wasAuthZone = false;
+        bool wasForwardRecurse = false;
+        domainmap_t::const_iterator iter = getBestAuthZone(&authname);
+        if(iter != t_sstorage.domainmap->end()) {
+          const auto& domain = iter->second;
+          wasForwardedOrAuthZone = true;
 
-        if (domain.isAuth()) {
-          wasAuthZone = true;
-        } else if (domain.shouldRecurse()) {
-          wasForwardRecurse = true;
+          if (domain.isAuth()) {
+            wasAuthZone = true;
+          } else if (domain.shouldRecurse()) {
+            wasForwardRecurse = true;
+          }
         }
-      }
 
-      /* When we are looking for a DS, we want to the non-CNAME cache check first
-         because we can actually have a DS (from the parent zone) AND a CNAME (from
-         the child zone), and what we really want is the DS */
-      if (qtype != QType::DS && doCNAMECacheCheck(qname, qtype, ret, depth, res, state, wasAuthZone, wasForwardRecurse, serveStale)) { // will reroute us if needed
-        d_wasOutOfBand = wasAuthZone;
-        // Here we have an issue. If we were prevented from going out to the network (cache-only was set, possibly because we
-        // are in QM Step0) we might have a CNAME but not the corresponding target.
-        // It means that we will sometimes go to the next steps when we are in fact done, but that's fine since
-        // we will get the records from the cache, resulting in a small overhead.
-        // This might be a real problem if we had a RPZ hit, though, because we do not want the processing to continue, since
-        // RPZ rules will not be evaluated anymore (we already matched).
-        const bool stoppedByPolicyHit = d_appliedPolicy.wasHit();
-
-        if (fromCache && (!d_cacheonly || stoppedByPolicyHit)) {
-          *fromCache = true;
-        }
-        /* Apply Post filtering policies */
-
-        if (d_wantsRPZ && !stoppedByPolicyHit) {
-          auto luaLocal = g_luaconfs.getLocal();
-          if (luaLocal->dfe.getPostPolicy(ret, d_discardedPolicies, d_appliedPolicy)) {
-            mergePolicyTags(d_policyTags, d_appliedPolicy.getTags());
-            bool done = false;
-            handlePolicyHit(prefix, qname, qtype, ret, done, res, depth);
-            if (done && fromCache) {
-              *fromCache = true;
+        /* When we are looking for a DS, we want to the non-CNAME cache check first
+           because we can actually have a DS (from the parent zone) AND a CNAME (from
+           the child zone), and what we really want is the DS */
+        if (qtype != QType::DS && doCNAMECacheCheck(qname, qtype, ret, depth, res, state, wasAuthZone, wasForwardRecurse, serveStale)) { // will reroute us if needed
+          d_wasOutOfBand = wasAuthZone;
+          // Here we have an issue. If we were prevented from going out to the network (cache-only was set, possibly because we
+          // are in QM Step0) we might have a CNAME but not the corresponding target.
+          // It means that we will sometimes go to the next steps when we are in fact done, but that's fine since
+          // we will get the records from the cache, resulting in a small overhead.
+          // This might be a real problem if we had a RPZ hit, though, because we do not want the processing to continue, since
+          // RPZ rules will not be evaluated anymore (we already matched).
+          const bool stoppedByPolicyHit = d_appliedPolicy.wasHit();
+
+          if (fromCache && (!d_cacheonly || stoppedByPolicyHit)) {
+            *fromCache = true;
+          }
+          /* Apply Post filtering policies */
+
+          if (d_wantsRPZ && !stoppedByPolicyHit) {
+            auto luaLocal = g_luaconfs.getLocal();
+            if (luaLocal->dfe.getPostPolicy(ret, d_discardedPolicies, d_appliedPolicy)) {
+              mergePolicyTags(d_policyTags, d_appliedPolicy.getTags());
+              bool done = false;
+              handlePolicyHit(prefix, qname, qtype, ret, done, res, depth);
+              if (done && fromCache) {
+                *fromCache = true;
+              }
             }
           }
-        }
-        return res;
-      }
-
-      if (doCacheCheck(qname, authname, wasForwardedOrAuthZone, wasAuthZone, wasForwardRecurse, qtype, ret, depth, res, state, serveStale)) {
-        // we done
-        d_wasOutOfBand = wasAuthZone;
-        if (fromCache) {
-          *fromCache = true;
+          return res;
         }
 
-        if (d_wantsRPZ && !d_appliedPolicy.wasHit()) {
-          auto luaLocal = g_luaconfs.getLocal();
-          if (luaLocal->dfe.getPostPolicy(ret, d_discardedPolicies, d_appliedPolicy)) {
-            mergePolicyTags(d_policyTags, d_appliedPolicy.getTags());
-            bool done = false;
-            handlePolicyHit(prefix, qname, qtype, ret, done, res, depth);
+        if (doCacheCheck(qname, authname, wasForwardedOrAuthZone, wasAuthZone, wasForwardRecurse, qtype, ret, depth, res, state, serveStale)) {
+          // we done
+          d_wasOutOfBand = wasAuthZone;
+          if (fromCache) {
+            *fromCache = true;
           }
-        }
-
-        return res;
-      }
 
-      /* if we have not found a cached DS (or denial of), now is the time to look for a CNAME */
-      if (qtype == QType::DS && doCNAMECacheCheck(qname, qtype, ret, depth, res, state, wasAuthZone, wasForwardRecurse, serveStale)) { // will reroute us if needed
-        d_wasOutOfBand = wasAuthZone;
-        // Here we have an issue. If we were prevented from going out to the network (cache-only was set, possibly because we
-        // are in QM Step0) we might have a CNAME but not the corresponding target.
-        // It means that we will sometimes go to the next steps when we are in fact done, but that's fine since
-        // we will get the records from the cache, resulting in a small overhead.
-        // This might be a real problem if we had a RPZ hit, though, because we do not want the processing to continue, since
-        // RPZ rules will not be evaluated anymore (we already matched).
-        const bool stoppedByPolicyHit = d_appliedPolicy.wasHit();
+          if (d_wantsRPZ && !d_appliedPolicy.wasHit()) {
+            auto luaLocal = g_luaconfs.getLocal();
+            if (luaLocal->dfe.getPostPolicy(ret, d_discardedPolicies, d_appliedPolicy)) {
+              mergePolicyTags(d_policyTags, d_appliedPolicy.getTags());
+              bool done = false;
+              handlePolicyHit(prefix, qname, qtype, ret, done, res, depth);
+            }
+          }
 
-        if (fromCache && (!d_cacheonly || stoppedByPolicyHit)) {
-          *fromCache = true;
+          return res;
         }
-        /* Apply Post filtering policies */
-
-        if (d_wantsRPZ && !stoppedByPolicyHit) {
-          auto luaLocal = g_luaconfs.getLocal();
-          if (luaLocal->dfe.getPostPolicy(ret, d_discardedPolicies, d_appliedPolicy)) {
-            mergePolicyTags(d_policyTags, d_appliedPolicy.getTags());
-            bool done = false;
-            handlePolicyHit(prefix, qname, qtype, ret, done, res, depth);
-            if (done && fromCache) {
-              *fromCache = true;
+
+        /* if we have not found a cached DS (or denial of), now is the time to look for a CNAME */
+        if (qtype == QType::DS && doCNAMECacheCheck(qname, qtype, ret, depth, res, state, wasAuthZone, wasForwardRecurse, serveStale)) { // will reroute us if needed
+          d_wasOutOfBand = wasAuthZone;
+          // Here we have an issue. If we were prevented from going out to the network (cache-only was set, possibly because we
+          // are in QM Step0) we might have a CNAME but not the corresponding target.
+          // It means that we will sometimes go to the next steps when we are in fact done, but that's fine since
+          // we will get the records from the cache, resulting in a small overhead.
+          // This might be a real problem if we had a RPZ hit, though, because we do not want the processing to continue, since
+          // RPZ rules will not be evaluated anymore (we already matched).
+          const bool stoppedByPolicyHit = d_appliedPolicy.wasHit();
+
+          if (fromCache && (!d_cacheonly || stoppedByPolicyHit)) {
+            *fromCache = true;
+          }
+          /* Apply Post filtering policies */
+
+          if (d_wantsRPZ && !stoppedByPolicyHit) {
+            auto luaLocal = g_luaconfs.getLocal();
+            if (luaLocal->dfe.getPostPolicy(ret, d_discardedPolicies, d_appliedPolicy)) {
+              mergePolicyTags(d_policyTags, d_appliedPolicy.getTags());
+              bool done = false;
+              handlePolicyHit(prefix, qname, qtype, ret, done, res, depth);
+              if (done && fromCache) {
+                *fromCache = true;
+              }
             }
           }
+
+          return res;
         }
+      }
 
-        return res;
+      if (d_cacheonly) {
+        return 0;
       }
-    }
 
-    if (d_cacheonly) {
-      return 0;
-    }
+      LOG(prefix<<qname<<": No cache hit for '"<<qname<<"|"<<qtype<<"', trying to find an appropriate NS record"<<endl);
+
+      DNSName subdomain(qname);
+      if (qtype == QType::DS) subdomain.chopOff();
 
-    LOG(prefix<<qname<<": No cache hit for '"<<qname<<"|"<<qtype<<"', trying to find an appropriate NS record"<<endl);
+      NsSet nsset;
+      bool flawedNSSet=false;
 
-    DNSName subdomain(qname);
-    if (qtype == QType::DS) subdomain.chopOff();
+      // the two retries allow getBestNSNamesFromCache&co to reprime the root
+      // hints, in case they ever go missing
+      for(int tries=0;tries<2 && nsset.empty();++tries) {
+        subdomain=getBestNSNamesFromCache(subdomain, qtype, nsset, &flawedNSSet, depth, beenthere); //  pass beenthere to both occasions
+      }
 
-    NsSet nsset;
-    bool flawedNSSet=false;
+      res = doResolveAt(nsset, subdomain, flawedNSSet, qname, qtype, ret, depth, beenthere, state, stopAtDelegation, nullptr, serveStale);
 
-    // the two retries allow getBestNSNamesFromCache&co to reprime the root
-    // hints, in case they ever go missing
-    for(int tries=0;tries<2 && nsset.empty();++tries) {
-      subdomain=getBestNSNamesFromCache(subdomain, qtype, nsset, &flawedNSSet, depth, beenthere); //  pass beenthere to both occasions
-    }
-
-    res = doResolveAt(nsset, subdomain, flawedNSSet, qname, qtype, ret, depth, beenthere, state, stopAtDelegation, nullptr, serveStale);
-
-    if (res == -1 && s_save_parent_ns_set) {
-      // It did not work out, lets check if we have a saved parent NS set
-      map<DNSName, vector<ComboAddress>> fallBack;
-      {
-        auto lock = s_savedParentNSSet.lock();
-        auto domainData= lock->find(subdomain);
-        if (domainData != lock->end() && domainData->d_nsAddresses.size() > 0) {
-          nsset.clear();
-          // Build the nsset arg and fallBack data for the fallback doResolveAt() attempt
-          // Take a copy to be able to release the lock, NsSet is actually a map, go figure
-          for (const auto& ns : domainData->d_nsAddresses) {
-            nsset.emplace(ns.first, pair(std::vector<ComboAddress>(), false));
-            fallBack.emplace(ns.first, ns.second);
+      if (res == -1 && s_save_parent_ns_set) {
+        // It did not work out, lets check if we have a saved parent NS set
+        map<DNSName, vector<ComboAddress>> fallBack;
+        {
+          auto lock = s_savedParentNSSet.lock();
+          auto domainData= lock->find(subdomain);
+          if (domainData != lock->end() && domainData->d_nsAddresses.size() > 0) {
+            nsset.clear();
+            // Build the nsset arg and fallBack data for the fallback doResolveAt() attempt
+            // Take a copy to be able to release the lock, NsSet is actually a map, go figure
+            for (const auto& ns : domainData->d_nsAddresses) {
+              nsset.emplace(ns.first, pair(std::vector<ComboAddress>(), false));
+              fallBack.emplace(ns.first, ns.second);
+            }
+          }
+        }
+        if (fallBack.size() > 0) {
+          LOG(prefix<<qname<<": Failure, but we have a saved parent NS set, trying that one"<< endl)
+            res = doResolveAt(nsset, subdomain, flawedNSSet, qname, qtype, ret, depth, beenthere, state, stopAtDelegation, &fallBack, serveStale);
+          if (res == 0) {
+            // It did work out
+            s_savedParentNSSet.lock()->inc(subdomain);
           }
         }
       }
-      if (fallBack.size() > 0) {
-        LOG(prefix<<qname<<": Failure, but we have a saved parent NS set, trying that one"<< endl)
-          res = doResolveAt(nsset, subdomain, flawedNSSet, qname, qtype, ret, depth, beenthere, state, stopAtDelegation, &fallBack, serveStale);
-        if (res == 0) {
-          // It did work out
-          s_savedParentNSSet.lock()->inc(subdomain);
+      /* Apply Post filtering policies */
+      if (d_wantsRPZ && !d_appliedPolicy.wasHit()) {
+        auto luaLocal = g_luaconfs.getLocal();
+        if (luaLocal->dfe.getPostPolicy(ret, d_discardedPolicies, d_appliedPolicy)) {
+          mergePolicyTags(d_policyTags, d_appliedPolicy.getTags());
+          bool done = false;
+          handlePolicyHit(prefix, qname, qtype, ret, done, res, depth);
         }
       }
-    }
-    /* Apply Post filtering policies */
-    if (d_wantsRPZ && !d_appliedPolicy.wasHit()) {
-      auto luaLocal = g_luaconfs.getLocal();
-      if (luaLocal->dfe.getPostPolicy(ret, d_discardedPolicies, d_appliedPolicy)) {
-        mergePolicyTags(d_policyTags, d_appliedPolicy.getTags());
-        bool done = false;
-        handlePolicyHit(prefix, qname, qtype, ret, done, res, depth);
+
+      if (!res) {
+        return 0;
       }
-    }
 
-    if (!res) {
-      return 0;
+      LOG(prefix<<qname<<": failed (res="<<res<<")"<<endl);
+    }
+    catch (const ImmediateServFailException&) {
+      if (exceptionOnTimeout) {
+        throw;
+      }
     }
-
-    LOG(prefix<<qname<<": failed (res="<<res<<")"<<endl);
   }
   return res<0 ? RCode::ServFail : res;
 }
@@ -5117,11 +5124,7 @@ bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname,
   checkMaxQperQ(qname);
 
   if(s_maxtotusec && d_totUsec > s_maxtotusec) {
-    if (d_exceptionOnTimeout) {
-      throw ImmediateServFailException("Too much time waiting for "+qname.toLogString()+"|"+qtype.toString()+", timeouts: "+std::to_string(d_timeouts) +", throttles: "+std::to_string(d_throttledqueries) + ", queries: "+std::to_string(d_outqueries)+", "+std::to_string(d_totUsec/1000)+"msec");
-    } else {
-      return false;
-    }
+    throw ImmediateServFailException("Too much time waiting for "+qname.toLogString()+"|"+qtype.toString()+", timeouts: "+std::to_string(d_timeouts) +", throttles: "+std::to_string(d_throttledqueries) + ", queries: "+std::to_string(d_outqueries)+", "+std::to_string(d_totUsec/1000)+"msec");
   }
 
   if(doTCP) {
index 527c3986e9b37f07bab455b4ca8fe67be32220b2..e4486989c1706315b7fe58790d8c8768e2e37183 100644 (file)
@@ -647,8 +647,7 @@ private:
   bool d_queryReceivedOverTCP{false};
   bool d_followCNAME{true};
   bool d_refresh{false};
-  bool d_exceptionOnTimeout{true};
-  
+
   LogMode d_lm;
 };