From: Otto Moerbeek Date: Wed, 29 Jun 2022 09:19:06 +0000 (+0200) Subject: Change main serve stale loop to catch exception X-Git-Tag: rec-4.8.0-alpha1~24^2~8 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4aedc37f6d2950577715757071a10ffe7b790bb9;p=thirdparty%2Fpdns.git Change main serve stale loop to catch exception --- diff --git a/pdns/syncres.cc b/pdns/syncres.cc index e7fc6bd34b..406a898c5a 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -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<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& servers = iter->second.d_servers; - const ComboAddress remoteIP = servers.front(); - LOG(prefix< 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<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& servers = iter->second.d_servers; + const ComboAddress remoteIP = servers.front(); + LOG(prefix< 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<> 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(), 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> 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(), false)); + fallBack.emplace(ns.first, ns.second); + } + } + } + if (fallBack.size() > 0) { + LOG(prefix<inc(subdomain); } } } - if (fallBack.size() > 0) { - LOG(prefix<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< 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) { diff --git a/pdns/syncres.hh b/pdns/syncres.hh index 527c3986e9..e4486989c1 100644 --- a/pdns/syncres.hh +++ b/pdns/syncres.hh @@ -647,8 +647,7 @@ private: bool d_queryReceivedOverTCP{false}; bool d_followCNAME{true}; bool d_refresh{false}; - bool d_exceptionOnTimeout{true}; - + LogMode d_lm; };