From: Otto Moerbeek Date: Fri, 16 May 2025 09:01:19 +0000 (+0200) Subject: Refactor: get rid of an unneccesary loop X-Git-Tag: rec-5.1.6^2~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6939fa489232311d1b39bc3f9a4414dd310a2c52;p=thirdparty%2Fpdns.git Refactor: get rid of an unneccesary loop --- diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc index 8e29546bdf..2cc23b2554 100644 --- a/pdns/recursordist/pdns_recursor.cc +++ b/pdns/recursordist/pdns_recursor.cc @@ -375,6 +375,8 @@ LWResult::Result arecvfrom(PacketBuffer& packet, int /* flags */, const ComboAdd len = packet.size(); + // In ecs hardening mode, we consider a missing ECS in the reply as a case for retrying without ECS + // The actual logic to do that is in Syncres::doResolveAtThisIP() if (g_ECSHardening && pident->ecsSubnet && !*pident->ecsReceived) { t_Counters.at(rec::Counter::ecsMissingCount)++; return LWResult::Result::ECSMissing; @@ -3031,9 +3033,7 @@ static void handleUDPServerResponse(int fileDesc, FDMultiplexer::funcparam_t& va if (!pident->domain.empty()) { auto iter = g_multiTasker->getWaiters().find(pident); if (iter != g_multiTasker->getWaiters().end()) { - if (g_ECSHardening) { - iter->key->ecsReceived = iter->key->ecsSubnet && checkIncomingECSSource(packet, *iter->key->ecsSubnet); - } + iter->key->ecsReceived = iter->key->ecsSubnet && checkIncomingECSSource(packet, *iter->key->ecsSubnet); doResends(iter, pident, packet, iter->key->ecsReceived); } } diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index c5ceb4c95d..c27219f06c 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -5366,43 +5366,43 @@ void SyncRes::incTimeoutStats(const ComboAddress& remoteIP) } } -bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname, const QType qtype, LWResult& lwr, boost::optional& ednsmask, const DNSName& auth, bool const sendRDQuery, const bool wasForwarded, const DNSName& nsName, const ComboAddress& remoteIP, bool doTCP, bool doDoT, bool& truncated, bool& spoofed, boost::optional& extendedError, bool dontThrottle) +void SyncRes::checkTotalTime(const DNSName& qname, QType qtype, boost::optional& extendedError) const { - bool chained = false; - LWResult::Result resolveret = LWResult::Result::Success; - if (s_maxtotusec != 0 && d_totUsec > s_maxtotusec) { if (s_addExtendedResolutionDNSErrors) { extendedError = EDNSExtendedError{static_cast(EDNSExtendedError::code::NoReachableAuthority), "Timeout waiting for answer(s)"}; } 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) + " ms"); } +} + +bool SyncRes::doResolveAtThisIP(const std::string& prefix, const DNSName& qname, const QType qtype, LWResult& lwr, boost::optional& ednsmask, const DNSName& auth, bool const sendRDQuery, const bool wasForwarded, const DNSName& nsName, const ComboAddress& remoteIP, bool doTCP, bool doDoT, bool& truncated, bool& spoofed, boost::optional& extendedError, bool dontThrottle) +{ + checkTotalTime(qname, qtype, extendedError); + bool chained = false; + LWResult::Result resolveret = LWResult::Result::Success; int preOutQueryRet = RCode::NoError; + if (d_pdl && d_pdl->preoutquery(remoteIP, d_requestor, qname, qtype, doTCP, lwr.d_records, preOutQueryRet, d_eventTrace, timeval{0, 0})) { LOG(prefix << qname << ": Query handled by Lua" << endl); } else { - bool sendECSIfRelevant = true; - for (int count = 0; count < 2; ++count) { - ednsmask = sendECSIfRelevant ? getEDNSSubnetMask(qname, remoteIP) : boost::none; - if (ednsmask) { - LOG(prefix << qname << ": Adding EDNS Client Subnet Mask " << ednsmask->toString() << " to query" << endl); - s_ecsqueries++; - } + ednsmask = getEDNSSubnetMask(qname, remoteIP); + if (ednsmask) { + LOG(prefix << qname << ": Adding EDNS Client Subnet Mask " << ednsmask->toString() << " to query" << endl); + s_ecsqueries++; + } + updateQueryCounts(prefix, qname, remoteIP, doTCP, doDoT); + resolveret = asyncresolveWrapper(remoteIP, d_doDNSSEC, qname, auth, qtype.getCode(), + doTCP, sendRDQuery, &d_now, ednsmask, &lwr, &chained, nsName); // <- we go out on the wire! + ednsStats(ednsmask, qname, prefix); + if (resolveret == LWResult::Result::ECSMissing) { + ednsmask = boost::none; + LOG(prefix << qname << ": Answer has no ECS, trying again without EDNS Client Subnet Mask" << endl); updateQueryCounts(prefix, qname, remoteIP, doTCP, doDoT); resolveret = asyncresolveWrapper(remoteIP, d_doDNSSEC, qname, auth, qtype.getCode(), - doTCP, sendRDQuery, &d_now, ednsmask, &lwr, &chained, nsName); // <- we go out on the wire! - ednsStats(ednsmask, qname, prefix); - if (resolveret == LWResult::Result::ECSMissing) { - if (sendECSIfRelevant) { - sendECSIfRelevant = false; - LOG(prefix << qname << ": Answer has no ECS, trying again without EDNS Client Subnet Mask" << endl); - continue; - } - assert(0); // should not happen - } - break; // when no ECS shenanigans happened, only one pass through the loop + doTCP, sendRDQuery, &d_now, ednsmask, &lwr, &chained, nsName); } } diff --git a/pdns/recursordist/syncres.hh b/pdns/recursordist/syncres.hh index 1c1fff8db8..13a74ace9c 100644 --- a/pdns/recursordist/syncres.hh +++ b/pdns/recursordist/syncres.hh @@ -621,6 +621,7 @@ private: std::map>* fallback); void ednsStats(boost::optional& ednsmask, const DNSName& qname, const string& prefix); void incTimeoutStats(const ComboAddress& remoteIP); + void checkTotalTime(const DNSName& qname, QType qtype, boost::optional& extendedError) const; bool doResolveAtThisIP(const std::string& prefix, const DNSName& qname, QType qtype, LWResult& lwr, boost::optional& ednsmask, const DNSName& auth, bool sendRDQuery, bool wasForwarded, const DNSName& nsName, const ComboAddress& remoteIP, bool doTCP, bool doDoT, bool& truncated, bool& spoofed, boost::optional& extendedError, bool dontThrottle = false); bool processAnswer(unsigned int depth, const string& prefix, LWResult& lwr, const DNSName& qname, QType qtype, DNSName& auth, bool wasForwarded, const boost::optional& ednsmask, bool sendRDQuery, NsSet& nameservers, std::vector& ret, const DNSFilterEngine& dfe, bool* gotNewServers, int* rcode, vState& state, const ComboAddress& remoteIP);