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.4.0-alpha0~18^2~4 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=1f01e20879f0668f050efb3fc0fcf2e13cefecc4;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 0be8a735e1..35bd62144b 100644 --- a/pdns/recursordist/pdns_recursor.cc +++ b/pdns/recursordist/pdns_recursor.cc @@ -372,6 +372,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; @@ -3055,9 +3057,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 c7cc489591..f10ba5b283 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -5424,45 +5424,47 @@ 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++; + } + auto match = d_eventTrace.add(RecEventTrace::AuthRequest, qname.toLogString() + '/' + qtype.toString(), true, 0); + 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! + d_eventTrace.add(RecEventTrace::AuthRequest, static_cast(lwr.d_rcode), false, match); + 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); - auto match = d_eventTrace.add(RecEventTrace::AuthRequest, qname.toLogString() + '/' + qtype.toString(), true, 0); + match = d_eventTrace.add(RecEventTrace::AuthRequest, qname.toLogString() + '/' + qtype.toString(), true, 0); resolveret = asyncresolveWrapper(remoteIP, d_doDNSSEC, qname, auth, qtype.getCode(), doTCP, sendRDQuery, &d_now, ednsmask, &lwr, &chained, nsName); // <- we go out on the wire! d_eventTrace.add(RecEventTrace::AuthRequest, static_cast(lwr.d_rcode), false, match); - 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 } } diff --git a/pdns/recursordist/syncres.hh b/pdns/recursordist/syncres.hh index bff0f85538..bba70b522e 100644 --- a/pdns/recursordist/syncres.hh +++ b/pdns/recursordist/syncres.hh @@ -645,6 +645,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);