From: Russ Combs (rucombs) Date: Wed, 31 Jul 2019 20:02:23 +0000 (-0400) Subject: Merge pull request #1695 in SNORT/snort3 from ~SMINUT/snort3:host_cache_restore to... X-Git-Tag: 3.0.0-259~18 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2fff50ce65a1866cafdded6c174067b96c4da846;p=thirdparty%2Fsnort3.git Merge pull request #1695 in SNORT/snort3 from ~SMINUT/snort3:host_cache_restore to master Squashed commit of the following: commit 2c14aae82bd89276c312d31455ec645b3e998efb Author: Silviu Minut Date: Tue Jul 30 17:14:02 2019 -0400 hash: add back size(), get_max_size() and remove() functions to lru_cache_shared. hash: add unit test for explicitly testing get / set max size. hash: fix style --- diff --git a/src/hash/lru_cache_shared.cc b/src/hash/lru_cache_shared.cc index d56b1e832..bbe7c5f13 100644 --- a/src/hash/lru_cache_shared.cc +++ b/src/hash/lru_cache_shared.cc @@ -30,6 +30,6 @@ const PegInfo lru_cache_shared_peg_names[] = { CountType::SUM, "lru_cache_prunes", "lru cache pruned entry to make space for new entry" }, { CountType::SUM, "lru_cache_find_hits", "lru cache found entry in cache" }, { CountType::SUM, "lru_cache_find_misses", "lru cache did not find entry in cache" }, + { CountType::SUM, "lru_cache_removes", "lru cache found entry and removed it" }, { CountType::END, nullptr, nullptr }, }; - diff --git a/src/hash/lru_cache_shared.h b/src/hash/lru_cache_shared.h index 6f2503e4a..d621b1bdb 100644 --- a/src/hash/lru_cache_shared.h +++ b/src/hash/lru_cache_shared.h @@ -42,6 +42,7 @@ struct LruCacheSharedStats // room for a new entry. PegCount find_hits = 0; // Found entry in cache. PegCount find_misses = 0; // Did not find entry in cache. + PegCount removes = 0; // Found entry and removed it. }; template @@ -69,10 +70,32 @@ public: // Return all data from the LruCache in order (most recently used to least) std::vector > get_all_data(); + // Get current number of elements in the LruCache. + size_t size() + { + std::lock_guard cache_lock(cache_mutex); + return current_size; + } + + size_t get_max_size() + { + std::lock_guard cache_lock(cache_mutex); + return max_size; + } + // Modify the maximum number of entries allowed in the cache. // If the size is reduced, the oldest entries are removed. bool set_max_size(size_t newsize); + // Remove entry associated with Key. + // Returns true if entry existed, false otherwise. + bool remove(const Key& key); + + // Remove entry associated with key and return removed data. + // Returns true and copy of data if entry existed. Returns false if + // entry did not exist. + bool remove(const Key& key, Data& data); + const PegInfo* get_pegs() const { return lru_cache_shared_peg_names; @@ -214,5 +237,41 @@ LruCacheShared::get_all_data() return vec; } -#endif +template +bool LruCacheShared::remove(const Key& key) +{ + LruMapIter map_iter; + std::lock_guard cache_lock(cache_mutex); + map_iter = map.find(key); + if (map_iter == map.end()) + return false; // Key is not in LruCache. + + current_size--; + list.erase(map_iter->second); + map.erase(map_iter); + stats.removes++; + return true; +} + +template +bool LruCacheShared::remove(const Key& key, std::shared_ptr& data) +{ + LruMapIter map_iter; + std::lock_guard cache_lock(cache_mutex); + + map_iter = map.find(key); + if (map_iter == map.end()) + return false; // Key is not in LruCache. + + data = map_iter->second->second; + + current_size--; + list.erase(map_iter->second); + map.erase(map_iter); + stats.removes++; + return true; +} + + +#endif diff --git a/src/hash/test/lru_cache_shared_test.cc b/src/hash/test/lru_cache_shared_test.cc index 89832a150..3df194b0b 100644 --- a/src/hash/test/lru_cache_shared_test.cc +++ b/src/hash/test/lru_cache_shared_test.cc @@ -34,7 +34,16 @@ TEST_GROUP(lru_cache_shared) { }; -// Test LruCacheShared find, operator[], and get_all_data functions. +// Test LruCacheShared constructor and member access. +TEST(lru_cache_shared, constructor_test) +{ + LruCacheShared > lru_cache(5); + + CHECK(lru_cache.get_max_size() == 5); + CHECK(lru_cache.size() == 0); +} + +// Test LruCacheShared find and get_all_data functions. TEST(lru_cache_shared, insert_test) { LruCacheShared > lru_cache(3); @@ -71,7 +80,71 @@ TEST(lru_cache_shared, insert_test) CHECK(vec[2].second->compare("two") == 0); } -// Test statistics counters and set_max_size. +// Test set / get max size. +TEST(lru_cache_shared, max_size) +{ + std::string data; + LruCacheShared > lru_cache(5); + + size_t sz = lru_cache.get_max_size(); + CHECK(sz == 5); + + for (size_t i = 0; i < sz; i++) + lru_cache[i]->assign(std::to_string(i)); + + // Check that we can't set max size to 0 + CHECK(lru_cache.set_max_size(0) == false); + CHECK(lru_cache.get_max_size() == sz); + + // this prunes and should kick out the three oldest, i.e. 0, 1 and 2 + CHECK(lru_cache.set_max_size(2) == true); + + auto vec = lru_cache.get_all_data(); + CHECK(vec.size() == 2); + CHECK(vec[0].first == 4 and *vec[0].second == "4"); + CHECK(vec[1].first == 3 and *vec[1].second == "3"); +} + +// Test the remove functions. +TEST(lru_cache_shared, remove_test) +{ + std::string data; + LruCacheShared > lru_cache(5); + + for (int i = 0; i < 5; i++) + { + lru_cache[i]->assign(std::to_string(i)); + CHECK(true == lru_cache.remove(i)); + CHECK(lru_cache.find(i) == nullptr); + } + + CHECK(0 == lru_cache.size()); + + // Test remove API that returns the removed data. + std::shared_ptr data_ptr; + lru_cache[1]->assign("one"); + CHECK(1 == lru_cache.size()); + CHECK(true == lru_cache.remove(1, data_ptr)); + CHECK(*data_ptr == "one"); + CHECK(0 == lru_cache.size()); + + lru_cache[1]->assign("one"); + lru_cache[2]->assign("two"); + CHECK(2 == lru_cache.size()); + + // Verify that removing an item that does not exist does not affect + // cache. + CHECK(false == lru_cache.remove(3)); + CHECK(false == lru_cache.remove(4, data_ptr)); + CHECK(2 == lru_cache.size()); + + auto vec = lru_cache.get_all_data(); + CHECK(2 == vec.size()); + CHECK(vec[0].first == 2 and *vec[0].second == "two"); + CHECK(vec[1].first == 1 and *vec[1].second == "one"); +} + +// Test statistics counters. TEST(lru_cache_shared, stats_test) { LruCacheShared > lru_cache(5); @@ -88,12 +161,16 @@ TEST(lru_cache_shared, stats_test) CHECK(lru_cache.set_max_size(3) == true); // change size prunes; in addition to previous 5 + lru_cache.remove(7); // Removes - hit + lru_cache.remove(10); // Removes - miss + PegCount* stats = lru_cache.get_counts(); CHECK(stats[0] == 10); // adds CHECK(stats[1] == 7); // prunes CHECK(stats[2] == 3); // find hits CHECK(stats[3] == 12); // find misses + CHECK(stats[4] == 1); // removes // Check statistics names. const PegInfo* pegs = lru_cache.get_pegs(); @@ -101,10 +178,10 @@ TEST(lru_cache_shared, stats_test) CHECK(!strcmp(pegs[1].name, "lru_cache_prunes")); CHECK(!strcmp(pegs[2].name, "lru_cache_find_hits")); CHECK(!strcmp(pegs[3].name, "lru_cache_find_misses")); + CHECK(!strcmp(pegs[4].name, "lru_cache_removes")); } int main(int argc, char** argv) { return CommandLineTestRunner::RunAllTests(argc, argv); } - diff --git a/src/host_tracker/test/host_cache_module_test.cc b/src/host_tracker/test/host_cache_module_test.cc index 5266dbb6d..7c2f6587e 100644 --- a/src/host_tracker/test/host_cache_module_test.cc +++ b/src/host_tracker/test/host_cache_module_test.cc @@ -95,12 +95,14 @@ TEST(host_cache_module, host_cache_module_test_values) CHECK(!strcmp(ht_pegs[1].name, "lru_cache_prunes")); CHECK(!strcmp(ht_pegs[2].name, "lru_cache_find_hits")); CHECK(!strcmp(ht_pegs[3].name, "lru_cache_find_misses")); - CHECK(!ht_pegs[4].name); + CHECK(!strcmp(ht_pegs[4].name, "lru_cache_removes")); + CHECK(!ht_pegs[5].name); CHECK(ht_stats[0] == 0); CHECK(ht_stats[1] == 0); CHECK(ht_stats[2] == 0); CHECK(ht_stats[3] == 0); + CHECK(ht_stats[4] == 0); size_val.set(&size_param); @@ -133,4 +135,3 @@ int main(int argc, char** argv) { return CommandLineTestRunner::RunAllTests(argc, argv); } -