From: Otto Moerbeek Date: Fri, 17 Apr 2020 08:08:56 +0000 (+0200) Subject: Process rgacogne's comments wrt entryMatches(). X-Git-Tag: rec-4.4.0-alpha1^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=267fa8329fb1da7287f50c589ac6a5e5dc2e956a;p=thirdparty%2Fpdns.git Process rgacogne's comments wrt entryMatches(). This simplifies the code, but has the silent assumption that entryMatches() only gets fed matching routingTags (if present). The getEntries() code does guarrantee this so we should be safe. --- diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc index 15add0a9cc..e4e1e0ea72 100644 --- a/pdns/recursor_cache.cc +++ b/pdns/recursor_cache.cc @@ -200,8 +200,10 @@ MemRecursorCache::Entries MemRecursorCache::getEntries(MapCombo& map, const DNSN return map.d_cachecache; } + bool MemRecursorCache::entryMatches(MemRecursorCache::OrderedTagIterator_t& entry, uint16_t qt, bool requireAuth, const ComboAddress& who) { + // This code assumes that if a routing tag is present, it matches // MUTEX SHOULD BE ACQUIRED if (requireAuth && !entry->d_auth) return false; @@ -213,17 +215,17 @@ bool MemRecursorCache::entryMatches(MemRecursorCache::OrderedTagIterator_t& entr return match; } -bool MemRecursorCache::entryMatches(MemRecursorCache::OrderedTagIterator_t& entry, uint16_t qt, bool requireAuth) -{ - // MUTEX SHOULD BE ACQUIRED - if (requireAuth && !entry->d_auth) - return false; +// bool MemRecursorCache::entryMatches(MemRecursorCache::OrderedTagIterator_t& entry, uint16_t qt, bool requireAuth) +// { +// // MUTEX SHOULD BE ACQUIRED +// if (requireAuth && !entry->d_auth) +// return false; - bool match = (entry->d_qtype == qt || qt == QType::ANY || - (qt == QType::ADDR && (entry->d_qtype == QType::A || entry->d_qtype == QType::AAAA))); - //cerr << match << "T " << qt << ':' << entry->d_qtype << ' ' << entry->d_rtag << endl; - return match; -} +// bool match = (entry->d_qtype == qt || qt == QType::ANY || +// (qt == QType::ADDR && (entry->d_qtype == QType::A || entry->d_qtype == QType::AAAA))); +// //cerr << match << "T " << qt << ':' << entry->d_qtype << ' ' << entry->d_rtag << endl; +// return match; +// } // returns -1 for no hits int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, bool requireAuth, vector* res, const ComboAddress& who, const OptTag& routingTag, vector>* signatures, std::vector>* authorityRecs, bool* variable, vState* state, bool* wasAuth) @@ -268,8 +270,8 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, } } - if (!routingTag) { - auto entries = getEntries(map, qname, qt, boost::none); + if (routingTag) { + auto entries = getEntries(map, qname, qt, routingTag); if (entries.first != entries.second) { for (auto i=entries.first; i != entries.second; ++i) { @@ -290,59 +292,32 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, break; } } - - // cerr<<"time left : "<size() : 0) <<"\n"; return static_cast(ttd-now); } } - else { - auto entries = getEntries(map, qname, qt, routingTag); + // Try (again) without tag + auto entries = getEntries(map, qname, qt, boost::none); - if (entries.first != entries.second) { - for (auto i=entries.first; i != entries.second; ++i) { + if (entries.first != entries.second) { + for (auto i=entries.first; i != entries.second; ++i) { - auto firstIndexIterator = map.d_map.project(i); - if (i->d_ttd <= now) { - moveCacheItemToFront(map.d_map, firstIndexIterator); - continue; - } - - if (!entryMatches(firstIndexIterator, qtype, requireAuth)) { - continue; - } - - ttd = handleHit(map, firstIndexIterator, qname, res, signatures, authorityRecs, variable, state, wasAuth); - - if (qt.getCode() != QType::ANY && qt.getCode() != QType::ADDR) { // normally if we have a hit, we are done - break; - } + auto firstIndexIterator = map.d_map.project(i); + if (i->d_ttd <= now) { + moveCacheItemToFront(map.d_map, firstIndexIterator); + continue; } - return static_cast(ttd-now); - } - // Try again without tag - entries = getEntries(map, qname, qt, boost::none); - - if (entries.first != entries.second) { - for (auto i=entries.first; i != entries.second; ++i) { - - auto firstIndexIterator = map.d_map.project(i); - if (i->d_ttd <= now) { - moveCacheItemToFront(map.d_map, firstIndexIterator); - continue; - } - if (!entryMatches(firstIndexIterator, qtype, requireAuth)) { - continue; - } + if (!entryMatches(firstIndexIterator, qtype, requireAuth, who)) { + continue; + } - ttd = handleHit(map, firstIndexIterator, qname, res, signatures, authorityRecs, variable, state, wasAuth); + ttd = handleHit(map, firstIndexIterator, qname, res, signatures, authorityRecs, variable, state, wasAuth); - if (qt.getCode() != QType::ANY && qt.getCode() != QType::ADDR) { // normally if we have a hit, we are done - break; - } + if (qt.getCode() != QType::ANY && qt.getCode() != QType::ADDR) { // normally if we have a hit, we are done + break; } - return static_cast(ttd-now); } + return static_cast(ttd-now); } return -1; } @@ -550,13 +525,8 @@ bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname, for(auto i = entries.first; i != entries.second; ++i) { auto firstIndexIterator = map.d_map.project(i); - if (routingTag) { - if (!entryMatches(firstIndexIterator, qtype, requireAuth)) { - continue; - } - } else { - if (!entryMatches(firstIndexIterator, qtype, requireAuth, who)) - continue; + if (!entryMatches(firstIndexIterator, qtype, requireAuth, who)) { + continue; } i->d_state = newState; diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh index 5ec2a6c933..f5e7b62dae 100644 --- a/pdns/recursor_cache.hh +++ b/pdns/recursor_cache.hh @@ -223,7 +223,7 @@ private: } bool entryMatches(OrderedTagIterator_t& entry, uint16_t qt, bool requireAuth, const ComboAddress& who); - bool entryMatches(OrderedTagIterator_t& entry, uint16_t qt, bool requireAuth); + //bool entryMatches(OrderedTagIterator_t& entry, uint16_t qt, bool requireAuth); Entries getEntries(MapCombo& map, const DNSName &qname, const QType& qt, const OptTag& rtag); cache_t::const_iterator getEntryUsingECSIndex(MapCombo& map, time_t now, const DNSName &qname, uint16_t qtype, bool requireAuth, const ComboAddress& who); int32_t handleHit(MapCombo& map, OrderedTagIterator_t& entry, const DNSName& qname, vector* res, vector>* signatures, std::vector>* authorityRecs, bool* variable, vState* state, bool* wasAuth);