From acff699b8ec6ef5c31d52bb2978f30ee5e86d262 Mon Sep 17 00:00:00 2001 From: Miod Vallat Date: Wed, 28 May 2025 14:46:16 +0200 Subject: [PATCH] Lock the topmost map (view->cache) in the packet cache. --- pdns/auth-packetcache.cc | 144 ++++++++++++++++++++++----------------- pdns/auth-packetcache.hh | 18 +++-- 2 files changed, 94 insertions(+), 68 deletions(-) diff --git a/pdns/auth-packetcache.cc b/pdns/auth-packetcache.cc index fd1605c8e9..16275dd008 100644 --- a/pdns/auth-packetcache.cc +++ b/pdns/auth-packetcache.cc @@ -43,16 +43,17 @@ AuthPacketCache::AuthPacketCache(size_t mapsCount): d_mapscount(mapsCount), d_la d_statnumentries=S.getPointer("packetcache-size"); // Create the MapCombo for the default view + auto cache = d_cache.write_lock(); std::string defaultview{}; - createViewMap(defaultview); + createViewMap(*cache, defaultview); } // Create the vector for the given view. // Assumes there is no existing data for the view. Callers are expected to // know what they are doing. -std::unordered_map>>::iterator AuthPacketCache::createViewMap(const std::string& view) +std::unordered_map>>::iterator AuthPacketCache::createViewMap(cache_t& cache, const std::string& view) { - auto iter = d_cache.emplace(view, std::make_unique>(d_mapscount)); + auto iter = cache.emplace(view, std::make_unique>(d_mapscount)); auto retval = iter.first; auto* map = retval->second.get(); // Note that this reserves more than intended, especially if multiple views @@ -85,21 +86,24 @@ bool AuthPacketCache::get(DNSPacket& pkt, DNSPacket& cached, const std::string& string value; bool haveSomething; time_t now = time(nullptr); - auto iter = d_cache.find(view); - if (iter == d_cache.end()) { - // No data for this view yet. - (*d_statnummiss)++; - return false; - } - auto& mapcombo = getMap(iter->second, pkt.qdomain); { - auto map = mapcombo.d_map.try_read_lock(); - if (!map.owns_lock()) { - S.inc("deferred-packetcache-lookup"); + auto cache = d_cache.read_lock(); + auto iter = cache->find(view); + if (iter == cache->end()) { + // No data for this view yet. + (*d_statnummiss)++; return false; } + auto& mapcombo = getMap(iter->second, pkt.qdomain); + { + auto map = mapcombo.d_map.try_read_lock(); + if (!map.owns_lock()) { + S.inc("deferred-packetcache-lookup"); + return false; + } - haveSomething = AuthPacketCache::getEntryLocked(*map, pkt.getString(), hash, pkt.qdomain, pkt.qtype.getCode(), pkt.d_tcp, now, value); + haveSomething = AuthPacketCache::getEntryLocked(*map, pkt.getString(), hash, pkt.qdomain, pkt.qtype.getCode(), pkt.d_tcp, now, value); + } } if (!haveSomething) { @@ -158,45 +162,48 @@ void AuthPacketCache::insert(DNSPacket& query, DNSPacket& response, unsigned int entry.tcp = response.d_tcp; entry.query = query.getString(); - auto iter = d_cache.find(view); - if (iter == d_cache.end()) { - // No data for this view yet, create it. - iter = createViewMap(view); - } - auto& mc = getMap(iter->second, entry.qname); // NOLINT(readability-identifier-length) { - auto map = mc.d_map.try_write_lock(); - if (!map.owns_lock()) { - S.inc("deferred-packetcache-inserts"); - return; + auto cache = d_cache.write_lock(); + auto iter = cache->find(view); + if (iter == cache->end()) { + // No data for this view yet, create it. + iter = createViewMap(*cache, view); } + auto& mc = getMap(iter->second, entry.qname); // NOLINT(readability-identifier-length) + { + auto map = mc.d_map.try_write_lock(); + if (!map.owns_lock()) { + S.inc("deferred-packetcache-inserts"); + return; + } - auto& idx = map->get(); - auto range = idx.equal_range(hash); - auto iter2 = range.first; + auto& idx = map->get(); + auto range = idx.equal_range(hash); + auto iter2 = range.first; - for( ; iter2 != range.second ; ++iter2) { - if (!entryMatches(iter2, entry.query, entry.qname, entry.qtype, entry.tcp)) { - continue; - } + for( ; iter2 != range.second ; ++iter2) { + if (!entryMatches(iter2, entry.query, entry.qname, entry.qtype, entry.tcp)) { + continue; + } - moveCacheItemToBack(*map, iter2); - iter2->value = entry.value; - iter2->ttd = now + ourttl; - iter2->created = now; - return; - } + moveCacheItemToBack(*map, iter2); + iter2->value = entry.value; + iter2->ttd = now + ourttl; + iter2->created = now; + return; + } - /* no existing entry found to refresh */ - map->insert(std::move(entry)); + /* no existing entry found to refresh */ + map->insert(std::move(entry)); - if (*d_statnumentries >= d_maxEntries) { - /* remove the least recently inserted or replaced entry */ - auto& sidx = map->get(); - sidx.pop_front(); - } - else { - ++(*d_statnumentries); + if (*d_statnumentries >= d_maxEntries) { + /* remove the least recently inserted or replaced entry */ + auto& sidx = map->get(); + sidx.pop_front(); + } + else { + ++(*d_statnumentries); + } } } } @@ -231,9 +238,12 @@ uint64_t AuthPacketCache::purge() d_statnumentries->store(0); uint64_t delcount = 0; - for (auto& iter : d_cache) { - auto* map = iter.second.get(); - delcount += purgeLockedCollectionsVector(*map); + { + auto cache = d_cache.write_lock(); + for (auto& iter : *cache) { + auto* map = iter.second.get(); + delcount += purgeLockedCollectionsVector(*map); + } } return delcount; } @@ -242,9 +252,12 @@ uint64_t AuthPacketCache::purgeExact(const DNSName& qname) { uint64_t delcount = 0; - for (auto& iter : d_cache) { - auto& mc = getMap(iter.second, qname); // NOLINT(readability-identifier-length) - delcount += purgeExactLockedCollection(mc, qname); + { + auto cache = d_cache.write_lock(); + for (auto& iter : *cache) { + auto& mc = getMap(iter.second, qname); // NOLINT(readability-identifier-length) + delcount += purgeExactLockedCollection(mc, qname); + } } *d_statnumentries -= delcount; @@ -256,9 +269,12 @@ uint64_t AuthPacketCache::purgeExact(const std::string& view, const DNSName& qna { uint64_t delcount = 0; - if (auto iter = d_cache.find(view); iter != d_cache.end()) { - auto& mc = getMap(iter->second, qname); // NOLINT(readability-identifier-length) - delcount += purgeExactLockedCollection(mc, qname); + { + auto cache = d_cache.write_lock(); + if (auto iter = cache->find(view); iter != cache->end()) { + auto& mc = getMap(iter->second, qname); // NOLINT(readability-identifier-length) + delcount += purgeExactLockedCollection(mc, qname); + } } *d_statnumentries -= delcount; @@ -276,9 +292,12 @@ uint64_t AuthPacketCache::purge(const string &match) uint64_t delcount = 0; if(boost::ends_with(match, "$")) { - for (auto& iter : d_cache) { - auto* map = iter.second.get(); - delcount += purgeLockedCollectionsVector(*map, match); + { + auto cache = d_cache.write_lock(); + for (auto& iter : *cache) { + auto* map = iter.second.get(); + delcount += purgeLockedCollectionsVector(*map, match); + } } *d_statnumentries -= delcount; } @@ -292,9 +311,12 @@ uint64_t AuthPacketCache::purge(const string &match) void AuthPacketCache::cleanup() { uint64_t totErased = 0; - for (auto& iter : d_cache) { - auto* map = iter.second.get(); - totErased += pruneLockedCollectionsVector(*map); + { + auto cache = d_cache.write_lock(); + for (auto& iter : *cache) { + auto* map = iter.second.get(); + totErased += pruneLockedCollectionsVector(*map); + } } *d_statnumentries -= totErased; diff --git a/pdns/auth-packetcache.hh b/pdns/auth-packetcache.hh index 8157ae26d9..f5cf00043a 100644 --- a/pdns/auth-packetcache.hh +++ b/pdns/auth-packetcache.hh @@ -72,11 +72,14 @@ public: void setMaxEntries(uint64_t maxEntries) { d_maxEntries = maxEntries; - for (auto& iter : d_cache) { - auto* map = iter.second.get(); + { + auto cache = d_cache.write_lock(); + for (auto& iter : *cache) { + auto* map = iter.second.get(); - for (auto& shard : *map) { - shard.reserve(maxEntries / map->size()); + for (auto& shard : *map) { + shard.reserve(maxEntries / map->size()); + } } } } @@ -129,13 +132,14 @@ private: SharedLockGuarded d_map; }; - std::unordered_map>> d_cache; - static MapCombo& getMap(std::unique_ptr>& map, const DNSName& name) + using cache_t = std::unordered_map>>; + SharedLockGuarded d_cache; + static MapCombo& getMap(const std::unique_ptr>& map, const DNSName& name) { return (*map)[name.hash() % map->size()]; } - std::unordered_map>>::iterator createViewMap(const std::string& view); + cache_t::iterator createViewMap(cache_t& cache, const std::string& view); static bool entryMatches(cmap_t::index::type::iterator& iter, const std::string& query, const DNSName& qname, uint16_t qtype, bool tcp); static bool getEntryLocked(const cmap_t& map, const std::string& query, uint32_t hash, const DNSName &qname, uint16_t qtype, bool tcp, time_t now, string& value); void cleanupIfNeeded(); -- 2.47.2