From: Russ Combs (rucombs) Date: Wed, 1 Dec 2021 00:51:04 +0000 (+0000) Subject: Pull request #3090: Memory Update X-Git-Tag: 3.1.18.0~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5b4d82198611bec81d7f74a80fd1d1b4de7aa4f9;p=thirdparty%2Fsnort3.git Pull request #3090: Memory Update Merge in SNORT/snort3 from ~RUCOMBS/snort3:memory_update to master Squashed commit of the following: commit e73251f15db58127483e40965607a4e6979c762b Author: russ Date: Wed Oct 27 12:11:15 2021 -0400 framework: update base API version to 11 commit 062ffceeb9c4a07e489d27df0441dafa902d5264 Author: russ Date: Mon Oct 25 10:02:43 2021 -0400 dev_notes.txt: fix miscellaneous typos commit 8b260d2acd412de1c8ab81425d92d28a5a299295 Author: russ Date: Fri Sep 24 16:01:14 2021 -0400 perf_monitor: allow constraint seconds = 0 commit 28f796f0bfa37c1f7615fcec1f7b9e7ba160afc2 Author: russ Date: Wed Sep 15 15:51:39 2021 -0400 doc: remove mention of Automake commit 400f023d9b32f41da626e8395e04fd3f84b12b0a Author: russ Date: Thu Sep 16 15:38:31 2021 -0400 hyperscan: disable bogus unit test leak warnings commit 12d481d4fffa17863cf71062ada9c48a3ced20d1 Author: russ Date: Thu Sep 16 15:37:58 2021 -0400 memory: update dev notes commit 681bc7b114ca8f43b40f3fc80f765fb7d099aacc Author: russ Date: Tue Sep 28 13:16:36 2021 -0400 memory: add max rss to verbose memory output commit 6f84a31028243b06dcfbefc0bfa1148874ae5045 Author: russ Date: Sun Sep 26 09:02:21 2021 -0400 memory: add support for jemalloc commit 56dec3b93254e6e2d9418f9ee289679cf7c099f7 Author: russ Date: Fri Jul 16 09:29:57 2021 -0400 memory: refactoring commit e6831dcfd9c3ad5f84263e5e0a2880e2c700b3ee Author: russ Date: Wed Sep 15 10:15:13 2021 -0400 memory: remove explicit allocation tracking commit 368f41fcf637f6cd1a6802ea98986c1d8b78d467 Author: russ Date: Thu Jul 8 15:00:38 2021 -0400 memory: fix accounting issues 1. Ensure that all memory stats are accumulated last so stats are not skewed by later accumulations. 2. Delete the start up swappers in the main thread so packet allocation tracking is consistent. commit 371947cc47592f616705c868c33d3f4b4606c35c Author: russ Date: Thu Jul 8 15:00:15 2021 -0400 memory: refactor pruning and update unit tests commit b69c623ea64629f61f3e656b1d37f400546b5a4d Author: russ Date: Wed Jul 7 15:42:54 2021 -0400 memory: free space per DAQ message, not per allocation commit afe9ae7cb5cfd16fcf5ad16293655a8d895615bc Author: russ Date: Wed Jul 7 11:53:47 2021 -0400 memory: move mem_stats to MemoryCap commit 074a491ea51029fef7d613ff7170b1318836437a Author: russ Date: Tue Jul 6 23:31:07 2021 -0400 build: update configure options Replace --disable-memory-manager with --enable-memory-overloads. Add --enable-memory-profiler to track memory use by modules. Add --enable-rule-profiler to profile rule option as with other modules. Add --enable-deep-profiling for multi-level profile buckets. commit 06d367bc9dabbd25eb8a9f1e060aaf91256adfd6 Author: russ Date: Wed Sep 15 10:02:41 2021 -0400 memory: add original overload manager commit 327de6f23af8ada2786f9f286cee06528967e217 Author: russ Date: Thu Jul 1 12:10:25 2021 -0400 memory: expand profile report field widths --- diff --git a/CMakeLists.txt b/CMakeLists.txt index a60b2f3aa..5d76bc3c6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -48,9 +48,6 @@ string(APPEND CMAKE_C_FLAGS " ${EXTRA_C_FLAGS}") string(APPEND CMAKE_CXX_FLAGS " ${EXTRA_CXX_FLAGS}") string(APPEND CMAKE_EXE_LINKER_FLAGS " ${EXTRA_LINKER_FLAGS}") string(APPEND CMAKE_MODULE_LINKER_FLAGS " ${EXTRA_LINKER_FLAGS}") -foreach (EXTRA_LIBRARY IN LISTS EXTRA_LIBRARIES) - link_libraries(${EXTRA_LIBRARY}) -endforeach (EXTRA_LIBRARY) include_directories (${PROJECT_BINARY_DIR}) include_directories (${PROJECT_SOURCE_DIR}) @@ -183,6 +180,14 @@ else () TCMalloc: OFF") endif () +if (HAVE_JEMALLOC) + message("\ + JEMalloc: ON") +else () + message("\ + JEMalloc: OFF") +endif () + if (HAVE_UUID) message("\ UUID: ON") diff --git a/cmake/FindJEMalloc.cmake b/cmake/FindJEMalloc.cmake new file mode 100644 index 000000000..d7ad0012d --- /dev/null +++ b/cmake/FindJEMalloc.cmake @@ -0,0 +1,49 @@ +# - Try to find jemalloc +# Once done this will define +# JEMALLOC_FOUND - System has jemalloc +# JEMALLOC_INCLUDE_DIRS - The jemalloc include directories +# JEMALLOC_LIBRARIES - The libraries needed to use jemalloc + +find_package(PkgConfig QUIET) +pkg_check_modules(PC_JEMALLOC QUIET jemalloc) + +find_path(JEMALLOC_INCLUDE_DIR + NAMES jemalloc/jemalloc.h + HINTS ${PC_JEMALLOC_INCLUDE_DIRS} +) + +if ( STATIC_JEMALLOC ) + find_library(JEMALLOC_LIBRARY + NAMES libjemalloc.a jemalloc + HINTS ${PC_JEMALLOC_LIBRARY_DIRS} +) +else() + find_library(JEMALLOC_LIBRARY + NAMES jemalloc + HINTS ${PC_JEMALLOC_LIBRARY_DIRS} +) +endif() + +if(JEMALLOC_INCLUDE_DIR) + set(_version_regex "^#define[ \t]+JEMALLOC_VERSION[ \t]+\"([^\"]+)\".*") + file(STRINGS "${JEMALLOC_INCLUDE_DIR}/jemalloc/jemalloc.h" + JEMALLOC_VERSION REGEX "${_version_regex}") + string(REGEX REPLACE "${_version_regex}" "\\1" + JEMALLOC_VERSION "${JEMALLOC_VERSION}") + unset(_version_regex) +endif() + +include(FindPackageHandleStandardArgs) +# handle the QUIETLY and REQUIRED arguments and set JEMALLOC_FOUND to TRUE +# if all listed variables are TRUE and the requested version matches. +find_package_handle_standard_args(Jemalloc REQUIRED_VARS + JEMALLOC_LIBRARY JEMALLOC_INCLUDE_DIR + VERSION_VAR JEMALLOC_VERSION) + +if(JEMALLOC_FOUND) + set(JEMALLOC_LIBRARIES ${JEMALLOC_LIBRARY}) + set(JEMALLOC_INCLUDE_DIRS ${JEMALLOC_INCLUDE_DIR}) +endif() + +mark_as_advanced(JEMALLOC_INCLUDE_DIR JEMALLOC_LIBRARY) + diff --git a/cmake/configure_options.cmake b/cmake/configure_options.cmake index 8a7a3aebd..4b3cd306b 100644 --- a/cmake/configure_options.cmake +++ b/cmake/configure_options.cmake @@ -21,7 +21,9 @@ set ( USE_STDLOG ${ENABLE_STDLOG} ) set ( USE_TSC_CLOCK ${ENABLE_TSC_CLOCK} ) set ( NO_PROFILER ${DISABLE_SNORT_PROFILER} ) set ( DEEP_PROFILING ${ENABLE_DEEP_PROFILING} ) -set ( NO_MEM_MGR ${DISABLE_MEMORY_MANAGER} ) +set ( ENABLE_MEMORY_OVERLOADS ${ENABLE_MEMORY_OVERLOADS} ) +set ( ENABLE_MEMORY_PROFILER ${ENABLE_MEMORY_PROFILER} ) +set ( ENABLE_RULE_PROFILER ${ENABLE_RULE_PROFILER} ) if ( ENABLE_LARGE_PCAP ) set ( _FILE_OFFSET_BITS 64 ) @@ -170,6 +172,15 @@ if ( ENABLE_TCMALLOC ) set ( HAVE_TCMALLOC "1" ) endif ( ENABLE_TCMALLOC ) +if ( ENABLE_JEMALLOC ) + if ( ENABLE_ADDRESS_SANITIZER ) + message ( SEND_ERROR "JEMalloc cannot be used at the same time as address sanitizer!" ) + endif () + find_package ( JEMalloc REQUIRED ) + set ( JEMALLOC_C_FLAGS "-fno-builtin-malloc -fno-builtin-calloc -fno-builtin-realloc -fno-builtin-free" ) + set ( HAVE_JEMALLOC "1" ) +endif ( ENABLE_JEMALLOC ) + if ( ENABLE_CODE_COVERAGE ) include(${CMAKE_MODULE_PATH}/CodeCoverage.cmake) endif ( ENABLE_CODE_COVERAGE ) @@ -193,6 +204,6 @@ message(" set ( EXTRA_C_FLAGS "${EXTRA_C_FLAGS} ${HARDENED_CXX_FLAGS} ${DEBUGGING_C_FLAGS} ${SANITIZER_CXX_FLAGS} ${TCMALLOC_C_FLAGS} ${COVERAGE_COMPILER_FLAGS}" ) set ( EXTRA_CXX_FLAGS "${EXTRA_CXX_FLAGS} ${HARDENED_CXX_FLAGS} ${DEBUGGING_C_FLAGS} ${SANITIZER_CXX_FLAGS} ${TCMALLOC_C_FLAGS} ${COVERAGE_COMPILER_FLAGS}" ) set ( EXTRA_LINKER_FLAGS "${EXTRA_LINKER_FLAGS} ${HARDENED_LINKER_FLAGS} ${SANITIZER_LINKER_FLAGS} ${COVERAGE_LINKER_FLAGS}" ) -foreach (EXTRA_LIBRARY IN LISTS COVERAGE_LIBRARIES TCMALLOC_LIBRARIES ) +foreach (EXTRA_LIBRARY IN LISTS COVERAGE_LIBRARIES TCMALLOC_LIBRARIES JEMALLOC_LIBRARY ) list ( APPEND EXTRA_LIBRARIES ${EXTRA_LIBRARY} ) endforeach () diff --git a/cmake/create_options.cmake b/cmake/create_options.cmake index 090a76240..c5eb025a3 100644 --- a/cmake/create_options.cmake +++ b/cmake/create_options.cmake @@ -43,11 +43,14 @@ option ( ENABLE_GDB "Enable gdb debugging information" ON ) option ( ENABLE_PROFILE "Enable profiling options (developers only)" OFF ) option ( DISABLE_SNORT_PROFILER "Disable snort Profiler (developers only)" OFF ) option ( ENABLE_DEEP_PROFILING "Enable deep profiling of snort functions (developers only)" OFF ) -option ( DISABLE_MEMORY_MANAGER "Disable snort memory manager (developers only)" OFF ) +option ( ENABLE_MEMORY_OVERLOADS "Use new / delete overloads for profiling (developers only)" OFF ) +option ( ENABLE_MEMORY_PROFILER "Enable memory profiler (developers only)" OFF ) +option ( ENABLE_RULE_PROFILER "Enable rule keyword profiler (developers only)" OFF ) option ( ENABLE_ADDRESS_SANITIZER "enable address sanitizer support" OFF ) option ( ENABLE_THREAD_SANITIZER "enable thread sanitizer support" OFF ) option ( ENABLE_UB_SANITIZER "enable undefined behavior sanitizer support" OFF ) option ( ENABLE_TCMALLOC "enable using tcmalloc for dynamic memory management" OFF ) +option ( ENABLE_JEMALLOC "enable using jemalloc for dynamic memory management" OFF ) option ( ENABLE_CODE_COVERAGE "Whether to enable code coverage support" OFF ) # signals diff --git a/cmake/create_pkg_config.cmake b/cmake/create_pkg_config.cmake index 9808275e1..583990836 100644 --- a/cmake/create_pkg_config.cmake +++ b/cmake/create_pkg_config.cmake @@ -16,8 +16,16 @@ if(DAQ_INCLUDE_DIR) set(DAQ_CPPFLAGS "-I${DAQ_INCLUDE_DIR}") endif() -if(DISABLE_MEMORY_MANAGER) - set(NO_MEM_MGR_CPPFLAGS "-DNO_MEM_MGR") +if(ENABLE_MEMORY_OVERLOADS) + set(MEMORY_OVERLOADS_CPPFLAGS "-DENABLE_MEMORY_OVERLOADS") +endif() + +if(ENABLE_MEMORY_PROFILER) + set(MEMORY_PROFILER_CPPFLAGS "-DENABLE_MEMORY_PROFILER") +endif() + +if(ENABLE_RULE_PROFILER) + set(RULE_PROFILER_CPPFLAGS "-DENABLE_RULE_PROFILER") endif() if(DISABLE_SNORT_PROFILER) diff --git a/config.cmake.h.in b/config.cmake.h.in index d50c6cff1..6e1655634 100644 --- a/config.cmake.h.in +++ b/config.cmake.h.in @@ -76,8 +76,17 @@ /* disable snort profiler */ #cmakedefine NO_PROFILER 1 -/* disable snort memory manager */ -#cmakedefine NO_MEM_MGR 1 +/* enable memory profiler */ +#cmakedefine ENABLE_MEMORY_PROFILER 1 + +/* enable rule profiler */ +#cmakedefine ENABLE_RULE_PROFILER 1 + +/* enable deep profiling */ +#cmakedefine DEEP_PROFILING 1 + +/* enable new and delete overloads for profiling */ +#cmakedefine ENABLE_MEMORY_OVERLOADS 1 /* signal to dump stats */ #cmakedefine SIGNAL_SNORT_DUMP_STATS @SIGNAL_SNORT_DUMP_STATS@ @@ -135,6 +144,9 @@ /* safec available */ #cmakedefine HAVE_SAFEC 1 +/* jemalloc available */ +#cmakedefine HAVE_JEMALLOC 1 + /* uuid available */ #cmakedefine HAVE_UUID 1 diff --git a/configure_cmake.sh b/configure_cmake.sh index e2275cfcf..8c2d2b147 100755 --- a/configure_cmake.sh +++ b/configure_cmake.sh @@ -54,17 +54,22 @@ Optional Features: --enable-gprof-profile enable gprof profiling options (developers only) --disable-snort-profiler disable snort performance profiling (cpu and memory) (developers only) - --disable-memory-manager - disable snort memory manager (developers only) + --enable-memory-overloads + overload new and delete + --enable-memory-profiler + enable memory profiler + --enable-rule-profiler enable rule keyword profiler (developers only) + --enable-deep-profiling enable deep (multi-level) profiling (developers only) --disable-corefiles prevent Snort from generating core files --enable-address-sanitizer enable address sanitizer support --enable-thread-sanitizer enable thread sanitizer support - --enable-ub-sanitizer - enable undefined behavior sanitizer support - --enable-tcmalloc - enable using tcmalloc for dynamic memory management + --enable-ub-sanitizer enable undefined behavior sanitizer support + --enable-tcmalloc enable using tcmalloc for dynamic memory management + --enable-jemalloc enable using jemalloc for dynamic memory management + --enable-jemalloc-static + same as --enable-jemalloc but linked statically --enable-appid-third-party enable third party appid --enable-unit-tests build unit tests @@ -255,10 +260,19 @@ while [ $# -ne 0 ]; do append_cache_entry ENABLE_TSC_CLOCK BOOL true ;; --disable-snort-profiler) - append_cache_entry DISABLE_SNORT_PROFILER BOOL true + append_cache_entry DISABLE_SNORT_PROFILER BOOL false ;; - --disable-memory-manager) - append_cache_entry DISABLE_MEMORY_MANAGER BOOL true + --enable-memory-overloads) + append_cache_entry ENABLE_MEMORY_OVERLOADS BOOL true + ;; + --enable-memory-profiler) + append_cache_entry ENABLE_MEMORY_PROFILER BOOL true + ;; + --enable-rule-profiler) + append_cache_entry ENABLE_RULE_PROFILER BOOL true + ;; + --enable-deep-profiling) + append_cache_entry DEEP_PROFILING BOOL true ;; --disable-large-pcap) append_cache_entry ENABLE_LARGE_PCAP BOOL false @@ -317,6 +331,20 @@ while [ $# -ne 0 ]; do --disable-tcmalloc) append_cache_entry ENABLE_TCMALLOC BOOL false ;; + --enable-jemalloc) + append_cache_entry ENABLE_JEMALLOC BOOL true + append_cache_entry STATIC_JEMALLOC BOOL false + ;; + --disable-jemalloc) + append_cache_entry ENABLE_JEMALLOC BOOL false + ;; + --enable-jemalloc-static) + append_cache_entry ENABLE_JEMALLOC BOOL true + append_cache_entry STATIC_JEMALLOC BOOL true + ;; + --disable-jemalloc-static) + append_cache_entry ENABLE_JEMALLOC BOOL false + ;; --enable-appid-third-party) ;; --enable-unit-tests) diff --git a/doc/upgrade/overview.txt b/doc/upgrade/overview.txt index 732c2a824..005950f02 100644 --- a/doc/upgrade/overview.txt +++ b/doc/upgrade/overview.txt @@ -22,7 +22,6 @@ scalability, usability, and extensibility. Some of the key features of Snort 3. * New latency monitoring and enforcement * Piglets to facilitate component testing * Inspection Events -* Automake and Cmake * Autogenerate reference documentation === Efficacy diff --git a/doc/user/overview.txt b/doc/user/overview.txt index 05a5f67dc..63c5782ee 100644 --- a/doc/user/overview.txt +++ b/doc/user/overview.txt @@ -22,7 +22,6 @@ usability. Some of the key features of Snort 3.0 are: * New latency monitoring and enforcement * Piglets to facilitate component testing * Inspection Events -* Automake and Cmake * Autogenerate reference documentation Additional features are on the road map: diff --git a/snort.pc.in b/snort.pc.in index 973dcb1f5..ee35ec924 100644 --- a/snort.pc.in +++ b/snort.pc.in @@ -29,5 +29,5 @@ Description: Snort 3.0 Project URL: www.snort.org Version: @VERSION@ Libs: -L${libdir}/snort -Cflags: -I${includedir}/snort @DEEP_PROFILING_CPPFLAGS@ @NO_MEM_MGR_CPPFLAGS@ @NO_PROFILER_CPPFLAGS@ @TP_APPID_CPPFLAGS@ @TSC_CPPFLAGS@ +Cflags: -I${includedir}/snort @DEEP_PROFILING_CPPFLAGS@ @MEMORY_OVERLOADS_CPPFLAGS@ @MEMORY_PROFILER_CPPFLAGS@ @RULE_PROFILER_CPPFLAGS@ @NO_PROFILER_CPPFLAGS@ @TP_APPID_CPPFLAGS@ @TSC_CPPFLAGS@ diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index e795441dd..756be1589 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -211,6 +211,7 @@ message(" target_link_libraries( snort ${EXTERNAL_LIBRARIES} + ${EXTRA_LIBRARIES} ) # Solaris requires libnsl and libsocket for various network-related library functions diff --git a/src/actions/dev_notes.txt b/src/actions/dev_notes.txt index 8baae9c43..97212d39e 100644 --- a/src/actions/dev_notes.txt +++ b/src/actions/dev_notes.txt @@ -14,7 +14,7 @@ Reject performs active response to shutdown hostile network session by injecting TCP resets (TCP connections) or ICMP unreachable packets. React sends an HTML page to the client, a RST to the server and blocks the flow. -It is using payload_injector utilty. payload_injector should be configured when +It is using payload_injector utility. payload_injector should be configured when react is used. Rewrite enables overwrite packet contents based on "replace" option in the diff --git a/src/detection/regex_offload.cc b/src/detection/regex_offload.cc index cb57917d3..950fd3405 100644 --- a/src/detection/regex_offload.cc +++ b/src/detection/regex_offload.cc @@ -321,8 +321,8 @@ void ThreadRegexOffload::worker( } #endif } - ModuleManager::accumulate_offload("search_engine"); - ModuleManager::accumulate_offload("detection"); + ModuleManager::accumulate_module("search_engine"); + ModuleManager::accumulate_module("detection"); // FIXIT-M break this over-coupling. In reality we shouldn't be evaluating latency in offload. PacketLatency::tterm(); diff --git a/src/file_api/file_flows.h b/src/file_api/file_flows.h index 0e28cfdad..6816453c1 100644 --- a/src/file_api/file_flows.h +++ b/src/file_api/file_flows.h @@ -106,9 +106,6 @@ public: void set_file_policy(FilePolicyBase* fp) { file_policy = fp; } FilePolicyBase* get_file_policy() { return file_policy; } - size_t size_of() override - { return sizeof(*this); } - private: void init_file_context(FileDirection, FileContext*); FileContext* find_main_file_context(FilePosition, FileDirection, size_t id = 0); diff --git a/src/flow/dev_notes.txt b/src/flow/dev_notes.txt index 4abbe959a..d6ae957b8 100644 --- a/src/flow/dev_notes.txt +++ b/src/flow/dev_notes.txt @@ -38,7 +38,7 @@ flow creation when the appropriate DAQ packet message flag has been set on the initiating packet. The HA subsystem exchanges two high level message types: - - DELETE: Indicate to the partner that a session has neen removed. No + - DELETE: Indicate to the partner that a session has been removed. No additional HA client status will be exchanged. (Not used for DAQ-backed storage.) - UPDATE: Indicate all other state changes. The message always includes diff --git a/src/flow/flow.cc b/src/flow/flow.cc index 58e4e0c19..99e49ff87 100644 --- a/src/flow/flow.cc +++ b/src/flow/flow.cc @@ -40,7 +40,6 @@ using namespace snort; Flow::Flow() { - memory::MemoryCap::update_allocations(sizeof(*this) + sizeof(FlowStash)); constexpr size_t offset = offsetof(Flow, key); // FIXIT-L need a struct to zero here to make future proof memset((uint8_t*)this+offset, 0, sizeof(*this)-offset); @@ -48,7 +47,6 @@ Flow::Flow() Flow::~Flow() { - memory::MemoryCap::update_deallocations(sizeof(*this) + sizeof(FlowStash)); term(); } @@ -280,12 +278,6 @@ int Flow::set_flow_data(FlowData* fd) flow_data->prev = fd; flow_data = fd; - - // this is after actual allocation so we can't prune beforehand - // but if we are that close to the edge we are in trouble anyway - // large allocations can be accounted for directly - fd->update_allocations(fd->size_of()); - return 0; } @@ -321,7 +313,6 @@ void Flow::free_flow_data(FlowData* fd) fd->prev->next = fd->next; fd->next->prev = fd->prev; } - fd->update_deallocations(fd->size_of()); delete fd; } @@ -339,7 +330,6 @@ void Flow::free_flow_data() { FlowData* tmp = flow_data; flow_data = flow_data->next; - tmp->update_deallocations(tmp->size_of()); delete tmp; } } diff --git a/src/flow/flow_cache.cc b/src/flow/flow_cache.cc index 669405e88..f899b8939 100644 --- a/src/flow/flow_cache.cc +++ b/src/flow/flow_cache.cc @@ -162,7 +162,6 @@ Flow* FlowCache::allocate(const FlowKey* key) { Flow* new_flow = new Flow(); push(new_flow); - memory::MemoryCap::update_allocations(sizeof(HashNode) + sizeof(FlowKey)); } else if ( !prune_stale(timestamp, nullptr) ) { @@ -183,7 +182,6 @@ Flow* FlowCache::allocate(const FlowKey* key) if ( flow->session && flow->pkt_type != key->pkt_type ) flow->term(); - memory::MemoryCap::update_allocations(config.proto[to_utype(key->pkt_type)].cap_weight); flow->last_data_seen = timestamp; return flow; @@ -193,11 +191,7 @@ void FlowCache::remove(Flow* flow) { unlink_uni(flow); - // FIXIT-M This check is added for offload case where both Flow::reset - // and Flow::retire try remove the flow from hash. Flow::reset should - // just mark the flow as pending instead of trying to remove it. - if ( !hash_table->release_node(flow->key) ) - memory::MemoryCap::update_deallocations(config.proto[to_utype(flow->key->pkt_type)].cap_weight); + hash_table->release_node(flow->key); } bool FlowCache::release(Flow* flow, PruneReason reason, bool do_cleanup) @@ -462,7 +456,6 @@ unsigned FlowCache::delete_active_flows(unsigned mode, unsigned num_to_delete, u //The flow should not be removed from the hash before reset hash_table->remove(); delete flow; - memory::MemoryCap::update_deallocations(sizeof(HashNode) + sizeof(FlowKey)); --flows_allocated; ++deleted; --num_to_delete; @@ -489,7 +482,6 @@ unsigned FlowCache::delete_flows(unsigned num_to_delete) delete flow; delete_stats.update(FlowDeleteState::FREELIST); - memory::MemoryCap::update_deallocations(sizeof(HashNode) + sizeof(FlowKey)); --flows_allocated; ++deleted; @@ -526,7 +518,6 @@ unsigned FlowCache::purge() while ( Flow* flow = (Flow*)hash_table->pop() ) { delete flow; - memory::MemoryCap::update_deallocations(sizeof(HashNode) + sizeof(FlowKey)); --flows_allocated; } diff --git a/src/flow/flow_control.cc b/src/flow/flow_control.cc index 53e6961de..a09b8ee2f 100644 --- a/src/flow/flow_control.cc +++ b/src/flow/flow_control.cc @@ -27,7 +27,6 @@ #include "detection/detection_engine.h" #include "main/snort_config.h" #include "managers/inspector_manager.h" -#include "memory/memory_cap.h" #include "packet_io/active.h" #include "packet_tracer/packet_tracer.h" #include "protocols/icmp4.h" @@ -123,16 +122,6 @@ void FlowControl::timeout_flows(time_t cur_time) cache->timeout(1, cur_time); } -void FlowControl::preemptive_cleanup() -{ - // FIXIT-RC is there a possibility of this looping forever? - while ( memory::MemoryCap::over_threshold() ) - { - if ( !prune_one(PruneReason::PREEMPTIVE, true) ) - break; - } -} - Flow* FlowControl::stale_flow_cleanup(FlowCache* cache, Flow* flow, Packet* p) { if ( p->pkth->flags & DAQ_PKT_FLAG_NEW_FLOW ) @@ -432,7 +421,6 @@ unsigned FlowControl::process(Flow* flow, Packet* p) p->disable_inspect = flow->is_inspection_disabled(); last_pkt_type = p->type(); - preemptive_cleanup(); // If this code is executed on a flow in SETUP state, it will result in a packet from both // client and server on packets from 0.0.0.0 or :: diff --git a/src/flow/flow_control.h b/src/flow/flow_control.h index 006931440..28d1c4fe7 100644 --- a/src/flow/flow_control.h +++ b/src/flow/flow_control.h @@ -97,7 +97,6 @@ public: private: void set_key(snort::FlowKey*, snort::Packet*); unsigned process(snort::Flow*, snort::Packet*); - void preemptive_cleanup(); void update_stats(snort::Flow*, snort::Packet*); private: diff --git a/src/flow/flow_data.cc b/src/flow/flow_data.cc index b14d00848..1d221c1a8 100644 --- a/src/flow/flow_data.cc +++ b/src/flow/flow_data.cc @@ -48,33 +48,6 @@ FlowData::~FlowData() { if ( handler ) handler->rem_ref(); - - assert(mem_in_use == 0); - assert(net_allocation_calls == 0); -} - -void FlowData::update_allocations(size_t n) -{ - memory::MemoryCap::update_allocations(n); - - if (n > 0) - { - mem_in_use += n; - net_allocation_calls++; - } -} - -void FlowData::update_deallocations(size_t n) -{ - memory::MemoryCap::update_deallocations(n); - - if (n > 0) - { - assert(mem_in_use >= n); - mem_in_use -= n; - assert(net_allocation_calls > 0); - net_allocation_calls--; - } } RuleFlowData::RuleFlowData(unsigned u) : diff --git a/src/flow/flow_data.h b/src/flow/flow_data.h index 0b883a512..f1ac3eda5 100644 --- a/src/flow/flow_data.h +++ b/src/flow/flow_data.h @@ -39,17 +39,10 @@ public: static unsigned create_flow_data_id() { return ++flow_data_id; } - // Allocations and deallocations must balance. It is not enough that the total number of bytes - // allocated and deallocated are equal. They must be allocated and deallocated in the same - // increments or roundoffs done inside the functions may not balance. - void update_allocations(size_t); - void update_deallocations(size_t); Inspector* get_handler() { return handler; } - // return fixed size (could be an approx avg) - // this must be fixed for life of flow data instance - // track significant supplemental allocations with the above updaters - virtual size_t size_of() = 0; + // deprecated - do not implement + virtual size_t size_of() { return 0; } virtual void handle_expected(Packet*) { } virtual void handle_retransmit(Packet*) { } @@ -62,8 +55,6 @@ public: // FIXIT-L privatize private: static unsigned flow_data_id; Inspector* handler; - size_t mem_in_use = 0; - unsigned net_allocation_calls = 0; unsigned id; }; diff --git a/src/flow/prune_stats.h b/src/flow/prune_stats.h index e46c79a62..735530571 100644 --- a/src/flow/prune_stats.h +++ b/src/flow/prune_stats.h @@ -31,7 +31,6 @@ enum class PruneReason : uint8_t IDLE, EXCESS, UNI, - PREEMPTIVE, MEMCAP, HA, STALE, diff --git a/src/flow/stash_item.h b/src/flow/stash_item.h index ef36fb693..95882d680 100644 --- a/src/flow/stash_item.h +++ b/src/flow/stash_item.h @@ -38,15 +38,12 @@ class StashGenericObject { public: StashGenericObject(int type) : object_type(type) - { + { } - } virtual ~StashGenericObject() = default; + int get_object_type() const - { - return object_type; - } - virtual size_t size_of() const = 0; + { return object_type; } private: int object_type; @@ -99,7 +96,6 @@ public: { type = STASH_ITEM_TYPE_GENERIC_OBJECT; val.generic_obj_val = obj; - memory::MemoryCap::update_allocations(sizeof(*this) + obj->size_of()); } ~StashItem() @@ -110,7 +106,6 @@ public: delete val.str_val; break; case STASH_ITEM_TYPE_GENERIC_OBJECT: - memory::MemoryCap::update_deallocations(sizeof(*this) + val.generic_obj_val->size_of()); delete val.generic_obj_val; default: break; diff --git a/src/flow/test/flow_cache_test.cc b/src/flow/test/flow_cache_test.cc index 2ad995ce8..df3cf6ceb 100644 --- a/src/flow/test/flow_cache_test.cc +++ b/src/flow/test/flow_cache_test.cc @@ -30,7 +30,6 @@ #include "main/snort_config.h" #include "main/snort_debug.h" #include "managers/inspector_manager.h" -#include "memory/memory_cap.h" #include "packet_io/active.h" #include "packet_tracer/packet_tracer.h" #include "protocols/icmp4.h" @@ -99,12 +98,6 @@ SfIpRet SfIp::set(void const*, int) { return SFIP_SUCCESS; } void snort::trace_vprintf(const char*, TraceLevel, const char*, const Packet*, const char*, va_list) {} uint8_t snort::TraceApi::get_constraints_generation() { return 0; } void snort::TraceApi::filter(const Packet&) {} -namespace memory -{ -void MemoryCap::update_allocations(size_t) { } -void MemoryCap::update_deallocations(size_t) { } -bool MemoryCap::over_threshold() { return true; } -} namespace snort { diff --git a/src/flow/test/flow_control_test.cc b/src/flow/test/flow_control_test.cc index 5f3ae5ee0..4527fddee 100644 --- a/src/flow/test/flow_control_test.cc +++ b/src/flow/test/flow_control_test.cc @@ -29,7 +29,6 @@ #include "detection/detection_engine.h" #include "main/snort_config.h" #include "managers/inspector_manager.h" -#include "memory/memory_cap.h" #include "packet_io/active.h" #include "packet_tracer/packet_tracer.h" #include "protocols/icmp4.h" @@ -100,11 +99,6 @@ bool ExpectCache::check(Packet*, Flow*) { return true; } bool ExpectCache::is_expected(Packet*) { return true; } Flow* HighAvailabilityManager::import(Packet&, FlowKey&) { return nullptr; } -namespace memory -{ -bool MemoryCap::over_threshold() { return true; } -} - namespace snort { namespace layer diff --git a/src/flow/test/flow_stash_test.cc b/src/flow/test/flow_stash_test.cc index 855c7272b..2e4f11807 100644 --- a/src/flow/test/flow_stash_test.cc +++ b/src/flow/test/flow_stash_test.cc @@ -34,25 +34,8 @@ using namespace snort; using namespace std; - static DataBus* DB = nullptr; -void memory::MemoryCap::update_allocations(size_t) { } -void memory::MemoryCap::update_deallocations(size_t) { } - -class TestStashObject : public StashGenericObject -{ -public: - TestStashObject(int type) : StashGenericObject(type) - { - - } - - size_t size_of() const override - { return sizeof(*this); } -}; - - template class DBConsumer : public DataHandler { @@ -310,23 +293,23 @@ TEST(stash_tests, non_existent_item) TEST(stash_tests, new_generic_object) { FlowStash stash; - TestStashObject *test_object = new TestStashObject(111); + StashGenericObject *test_object = new StashGenericObject(111); stash.store("item_1", test_object); StashGenericObject *retrieved_object; CHECK(stash.get("item_1", retrieved_object)); POINTERS_EQUAL(test_object, retrieved_object); - CHECK_EQUAL(test_object->get_object_type(), ((TestStashObject*)retrieved_object)->get_object_type()); + CHECK_EQUAL(test_object->get_object_type(), ((StashGenericObject*)retrieved_object)->get_object_type()); } TEST(stash_tests, update_generic_object) { FlowStash stash; - TestStashObject *test_object = new TestStashObject(111); + StashGenericObject *test_object = new StashGenericObject(111); stash.store("item_1", test_object); - TestStashObject *new_test_object = new TestStashObject(111); + StashGenericObject *new_test_object = new StashGenericObject(111); stash.store("item_1", new_test_object); StashGenericObject *retrieved_object; @@ -344,7 +327,7 @@ TEST(stash_tests, non_existent_generic_object) TEST(stash_tests, mixed_items) { FlowStash stash; - TestStashObject *test_object = new TestStashObject(111); + StashGenericObject *test_object = new StashGenericObject(111); stash.store("item_1", 10); stash.store("item_2", "value_2"); @@ -364,7 +347,7 @@ TEST(stash_tests, mixed_items) StashGenericObject *retrieved_object; CHECK(stash.get("item_4", retrieved_object)); POINTERS_EQUAL(test_object, retrieved_object); - CHECK_EQUAL(test_object->get_object_type(), ((TestStashObject*)retrieved_object)->get_object_type()); + CHECK_EQUAL(test_object->get_object_type(), ((StashGenericObject*)retrieved_object)->get_object_type()); } TEST(stash_tests, store_ip) diff --git a/src/flow/test/flow_test.cc b/src/flow/test/flow_test.cc index a8e8bfb1a..872cd642d 100644 --- a/src/flow/test/flow_test.cc +++ b/src/flow/test/flow_test.cc @@ -30,7 +30,6 @@ #include "framework/inspector.h" #include "framework/data_bus.h" #include "main/snort_config.h" -#include "memory/memory_cap.h" #include "protocols/ip.h" #include "protocols/layer.h" #include "protocols/packet.h" @@ -47,12 +46,6 @@ void Inspector::rem_ref() {} void Inspector::add_ref() {} -void memory::MemoryCap::update_allocations(size_t) {} - -void memory::MemoryCap::update_deallocations(size_t) {} - -void memory::MemoryCap::free_space(size_t) { } - bool HighAvailabilityManager::active() { return false; } FlowHAState::FlowHAState() = default; diff --git a/src/framework/base_api.h b/src/framework/base_api.h index 3186b06ba..d65d337e3 100644 --- a/src/framework/base_api.h +++ b/src/framework/base_api.h @@ -29,7 +29,7 @@ // this is the current version of the base api // must be prefixed to subtype version -#define BASE_API_VERSION 10 +#define BASE_API_VERSION 11 // set options to API_OPTIONS to ensure compatibility #ifndef API_OPTIONS diff --git a/src/helpers/test/hyper_search_test.cc b/src/helpers/test/hyper_search_test.cc index 0e6b77c13..68662998f 100644 --- a/src/helpers/test/hyper_search_test.cc +++ b/src/helpers/test/hyper_search_test.cc @@ -257,6 +257,7 @@ TEST(hyper_search_test_group, not_found4) int main(int argc, char** argv) { + MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); return CommandLineTestRunner::RunAllTests(argc, argv); } diff --git a/src/ips_options/test/ips_regex_test.cc b/src/ips_options/test/ips_regex_test.cc index 76e7ef530..a6b1236b6 100644 --- a/src/ips_options/test/ips_regex_test.cc +++ b/src/ips_options/test/ips_regex_test.cc @@ -369,6 +369,7 @@ TEST(ips_regex_option_relative, no_match) int main(int argc, char** argv) { + MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); return CommandLineTestRunner::RunAllTests(argc, argv); } diff --git a/src/log/dev_notes.txt b/src/log/dev_notes.txt index 2f4366bdd..481698bac 100644 --- a/src/log/dev_notes.txt +++ b/src/log/dev_notes.txt @@ -13,7 +13,7 @@ Text output logging facilities are located here: data. class Obfuscator is implemented with a std::set to provide in order - access for pottentialy out-of-order insertions. + access for potentially out-of-order insertions. Currently does not support streaming mode. It should be possible to iterate over contiguous chunks of data, alternating between obfuscated diff --git a/src/main.cc b/src/main.cc index 8a0192dce..2530b874d 100644 --- a/src/main.cc +++ b/src/main.cc @@ -137,6 +137,13 @@ public: private: void reap_command(AnalyzerCommand* ac); + // we could just let the analyzer own this pointer and delete + // immediately after getting the data but that creates memory + // count mismatches between main and packet threads. since the + // startup swapper has no old config to delete only 32 bytes + // after held. + Swapper* swapper = nullptr; + std::thread* athread = nullptr; unsigned idx = (unsigned)-1; }; @@ -166,8 +173,8 @@ void Pig::start() assert(!athread); LogMessage("++ [%u] %s\n", idx, analyzer->get_source()); - Swapper* ps = new Swapper(SnortConfig::get_main_conf()); - athread = new std::thread(std::ref(*analyzer), ps, ++run_num); + swapper = new Swapper(SnortConfig::get_main_conf()); + athread = new std::thread(std::ref(*analyzer), swapper, ++run_num); } void Pig::stop() @@ -175,6 +182,9 @@ void Pig::stop() assert(analyzer); assert(athread); + delete swapper; + swapper = nullptr; + athread->join(); delete athread; athread = nullptr; diff --git a/src/main/analyzer.cc b/src/main/analyzer.cc index 08b810c65..8a08e503f 100644 --- a/src/main/analyzer.cc +++ b/src/main/analyzer.cc @@ -286,9 +286,6 @@ static DAQ_Verdict distill_verdict(Packet* p) void Analyzer::add_to_retry_queue(DAQ_Msg_h daq_msg) { - // Temporarily increase memcap until message is finalized in case - // DAQ makes a copy of the data buffer. - memory::MemoryCap::update_allocations(daq_msg_get_data_len(daq_msg)); retry_queue->put(daq_msg); } @@ -404,6 +401,8 @@ void Analyzer::process_daq_pkt_msg(DAQ_Msg_h msg, bool retry) void Analyzer::process_daq_msg(DAQ_Msg_h msg, bool retry) { oops_handler->set_current_message(msg); + memory::MemoryCap::free_space(); + DAQ_Verdict verdict = DAQ_VERDICT_PASS; switch (daq_msg_get_type(msg)) { @@ -437,13 +436,11 @@ void Analyzer::process_retry_queue() struct timeval now; packet_gettimeofday(&now); DAQ_Msg_h msg; + while ((msg = retry_queue->get(&now)) != nullptr) { process_daq_msg(msg, true); daq_stats.retries_processed++; - - // Decrease memcap now that msg has been finalized. - memory::MemoryCap::update_deallocations(daq_msg_get_data_len(msg)); } } } @@ -678,6 +675,8 @@ void Analyzer::term() RateFilter_Cleanup(); TraceApi::thread_term(); + + ModuleManager::accumulate_module("memory"); } Analyzer::Analyzer(SFDAQInstance* instance, unsigned i, const char* s, uint64_t msg_cnt) @@ -708,7 +707,6 @@ void Analyzer::operator()(Swapper* ps, uint16_t run_num) local_analyzer = this; ps->apply(*this); - delete ps; if (SnortConfig::get_conf()->pcap_show()) show_source(); diff --git a/src/main/snort.cc b/src/main/snort.cc index 2fdea0fbe..10e5daac6 100644 --- a/src/main/snort.cc +++ b/src/main/snort.cc @@ -322,8 +322,6 @@ void Snort::term() const SnortConfig* sc = SnortConfig::get_conf(); - memory::MemoryCap::print(); - IpsManager::global_term(sc); HostAttributesManager::term(); @@ -370,6 +368,8 @@ void Snort::term() ModuleManager::term(); PluginManager::release_plugins(); ScriptManager::release_scripts(); + memory::MemoryCap::cleanup(); + term_signals(); } @@ -414,8 +414,9 @@ void Snort::setup(int argc, char* argv[]) set_quick_exit(false); - memory::MemoryCap::calculate(); - memory::MemoryCap::print(); + memory::MemoryCap::setup(*sc->memory, sc->thread_config->get_instance_max()); + memory::MemoryCap::print(SnortConfig::log_verbose()); + host_cache.print_config(); TimeStart(); diff --git a/src/main/swapper.cc b/src/main/swapper.cc index 9325445f6..db69b8e05 100644 --- a/src/main/swapper.cc +++ b/src/main/swapper.cc @@ -51,8 +51,10 @@ Swapper::Swapper() Swapper::~Swapper() { - if ( new_conf ) + if ( new_conf and old_conf ) + // don't do this to startup configs InspectorManager::clear_removed_inspectors(new_conf); + if ( old_conf ) delete old_conf; } diff --git a/src/main/test/distill_verdict_test.cc b/src/main/test/distill_verdict_test.cc index cf57a8796..377bf90eb 100644 --- a/src/main/test/distill_verdict_test.cc +++ b/src/main/test/distill_verdict_test.cc @@ -51,9 +51,6 @@ void Flow::trust() { } SFDAQInstance* SFDAQ::get_local_instance() { return nullptr; } } -void memory::MemoryCap::update_allocations(size_t) { } -void memory::MemoryCap::update_deallocations(size_t) { } - using namespace snort; //-------------------------------------------------------------------------- diff --git a/src/managers/module_manager.cc b/src/managers/module_manager.cc index 069d3dd4d..b0efb5b0a 100644 --- a/src/managers/module_manager.cc +++ b/src/managers/module_manager.cc @@ -1389,13 +1389,16 @@ void ModuleManager::accumulate() for ( auto* mh : mod_hooks ) { + if ( !strcmp(mh->mod->name, "memory") ) + continue; + lock_guard lock(stats_mutex); mh->mod->prep_counts(); mh->mod->sum_stats(true); } } -void ModuleManager::accumulate_offload(const char* name) +void ModuleManager::accumulate_module(const char* name) { ModHook* mh = get_hook(name); if ( mh ) diff --git a/src/managers/module_manager.h b/src/managers/module_manager.h index 4bb78b325..9c4386c50 100644 --- a/src/managers/module_manager.h +++ b/src/managers/module_manager.h @@ -86,7 +86,7 @@ public: static void dump_stats(const char* skip = nullptr, bool dynamic = false); static void accumulate(); - static void accumulate_offload(const char* name); + static void accumulate_module(const char* name); static void reset_stats(SnortConfig*); static void reset_stats(clear_counter_type_t); diff --git a/src/memory/CMakeLists.txt b/src/memory/CMakeLists.txt index 3b60f8142..4d2c563c5 100644 --- a/src/memory/CMakeLists.txt +++ b/src/memory/CMakeLists.txt @@ -4,10 +4,13 @@ set (MEMCAP_INCLUDES set ( MEMORY_SOURCES ${MEMCAP_INCLUDES} + memory_allocator.cc + memory_allocator.h memory_cap.cc + memory_config.h + memory_manager.cc memory_module.cc memory_module.h - memory_config.h prune_handler.cc prune_handler.h ) diff --git a/src/memory/dev_notes.txt b/src/memory/dev_notes.txt index 740f87968..7bb872eff 100644 --- a/src/memory/dev_notes.txt +++ b/src/memory/dev_notes.txt @@ -1,12 +1,79 @@ -This directory provides a simple mechanism for implementing a memory cap. -Modules can use the MemoryCap::update_allocations() and -update_deallocations() calls to self-report when they allocate or free -heap memory. If the total memory allocations exceed the configured memory -cap, flow pruning is done to free up additional memory. +Snort memory management monitors memory usage and prunes flows as needed to keep the total process +usage below the configured limit, if any. There are two ways to build memory management: build with +jemalloc (--enable-jemalloc) or enable the new / delete overloads (--enable-memory-overloads). The +latter option is required to support memory profiling (--enable-memory-profiler). Profiling is not +enabled by default due to performance concerns and is viewed as a developer tool: apart from cache +memcaps, users should not have to care about how Snort allocates memory, only that it doesn't +exceed the configured limit if any. -This mechanism is approximate and does not directly reflect the activities -of the memory allocator or the OOM killer. +tcmalloc builds (--enable-tcmalloc) do not support memory management. A process total is available +from the tcmalloc extensions but it is too expensive to call per packet. Checking every N packets +would also mean potentially freeing K > 1 flows after each exceeded event. Also, a process total +does not allow pruning only the threads that are over limit. tcmalloc does provide a performance +advantage over glibc so that may be preferred for deployments that don't need memory management. -TODO: +jemalloc is preferred because it is quicker and uses less memory since the implementation does not +require memory tracking. jemalloc provides access to the current thread allocated total (which is +between the number that Snort requests and what the system footprint is). + +memory_module.* - provides parameters and peg counts. The key parameters are the process_cap, +thread_cap, and threshold. The caps are in bytes, and the threshold is a percentage of the caps +specified. + +memory_manager.cc - when enabled with --enable-memory-overloads, overloads new and delete operators +to provide support memory tracking. Metadata is allocated in front of the requested memory to store +the sized allocated so the deallocation can be tracked. Due to the drag on performance, this is +disabled by default. + +memory_allocator.* - implements the malloc and free calls used by the operator new and delete +overloads. + +memory_config.h - provides MemoryConfig used by MemoryCap and stored in SnortConfig. + +memory_cap.* - provides the logic to enforce the thread cap by calling the prune handler. Tracks +thread usage in pegs and updates the memory profiler if built. To avoid confusion, thread_cap +refers to the configured maximums, while thread_limit refer to the configured percentage of the +caps (memory.cap * memory.threshold / 100). The jemalloc specific code is here. + +prune_handler.* - implements the call to stream to prune. + +The current iteration of the memory manager is exclusively preemptive. MemoryCap::free_space is +called by the analyzer before each DAQ message is processed. If thread_usage > thread_limit, a +single flow will be pruned. Demand-based pruning, ie enforcing that each allocation stays below +limit, is both not necessary and bug prone (due to reentrancy when pruning causes flushing causes +more allocations). + +This implementation has the following limitations: + +* If the overload manager is built, it only tracks memory allocated with C++ new. Specifically, + direct calls to malloc or calloc which may be made by libraries are not tracked. + +* Packet thread tracking does not include heap overhead, which can be substantial. + +* Non-packet threads are assumed to have bounded memory usage, eg via a cache. + +* Heap managers tend to acquire memory from the system quickly and release back much more slowly, + if ever. It is also relatively expensive to force a release. + +* Therefore, pruning a single flow almost certainly won't release memory back to the system. + +For these reasons, the goal of the memory manager is to prevent allocation past the limit rather +than try to reclaim memory allocated past the limit. This means that the configured limit must be +well enough below the actual hard limit, for example the limit enforced by cgroups, such that the +processing of a single packet will not push us over. It must also allow for additional heap +overhead. + +Future work: + +* Support simplified configuration of a process cap instead of a thread cap. Implement a MemoryCap + method that can be called to inform the memory module of various cache related memcaps. Deduct + the startup ru_maxrss and the sum of memcaps from the configured process cap and then divide by + --max-packet-threads to get the effective thread cap. + +* Compensate for heap fragmentation and other overhead by using the current process footprint + (process_total below) as feedback to adjust the current packet thread limits: + + thread_limit = [(cap - (process_total - sum_thread_usage)) / num_threads] * threshold + +* Recognize when a memory leak drives excessive pruning. -- possibly add eventing diff --git a/src/memory/memory_allocator.cc b/src/memory/memory_allocator.cc new file mode 100644 index 000000000..502962054 --- /dev/null +++ b/src/memory/memory_allocator.cc @@ -0,0 +1,39 @@ +//-------------------------------------------------------------------------- +// Copyright (C) 2016-2021 Cisco and/or its affiliates. All rights reserved. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License Version 2 as published +// by the Free Software Foundation. You may not use, modify or distribute +// this program under any other version of the GNU General Public License. +// +// This program is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this program; if not, write to the Free Software Foundation, Inc., +// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +//-------------------------------------------------------------------------- + +// memory_allocator.cc author Joel Cornett + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include "memory_allocator.h" + +#include + +namespace memory +{ + +// FIXIT-L (de)allocate() could be inlined if defined in memory_manager.cc +void* MemoryAllocator::allocate(size_t n) +{ return malloc(n); } + +void MemoryAllocator::deallocate(void* p) +{ free(p); } + +} // namespace memory diff --git a/src/memory/memory_allocator.h b/src/memory/memory_allocator.h new file mode 100644 index 000000000..10c4a85dc --- /dev/null +++ b/src/memory/memory_allocator.h @@ -0,0 +1,37 @@ +//-------------------------------------------------------------------------- +// Copyright (C) 2016-2021 Cisco and/or its affiliates. All rights reserved. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License Version 2 as published +// by the Free Software Foundation. You may not use, modify or distribute +// this program under any other version of the GNU General Public License. +// +// This program is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this program; if not, write to the Free Software Foundation, Inc., +// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +//-------------------------------------------------------------------------- + +// memory_allocator.h author Joel Cornett + +#ifndef MEMORY_ALLOCATOR_H +#define MEMORY_ALLOCATOR_H + +#include + +namespace memory +{ + +struct MemoryAllocator +{ + static void* allocate(size_t); + static void deallocate(void*); +}; + +} // namespace memory + +#endif diff --git a/src/memory/memory_cap.cc b/src/memory/memory_cap.cc index 63b43a447..684f90a05 100644 --- a/src/memory/memory_cap.cc +++ b/src/memory/memory_cap.cc @@ -22,13 +22,22 @@ #include "config.h" #endif +#include +#include + #include +#include + +#ifdef HAVE_JEMALLOC +#include +#endif #include "memory_cap.h" #include "log/messages.h" #include "main/snort_config.h" #include "main/snort_types.h" +#include "main/thread.h" #include "profiler/memory_profiler_active_context.h" #include "utils/stats.h" @@ -36,76 +45,77 @@ #include "memory_module.h" #include "prune_handler.h" -#ifdef UNIT_TEST -#include "catch/snort_catch.h" -#endif - using namespace snort; namespace memory { +static MemoryCounts ctl_mem_stats; +static std::vector pkt_mem_stats; + namespace { -struct Tracker +// ----------------------------------------------------------------------------- +// helpers +// ----------------------------------------------------------------------------- + +#ifdef HAVE_JEMALLOC +static size_t get_usage(MemoryCounts& mc) { - void allocate(size_t n) - { mem_stats.allocated += n; ++mem_stats.allocations; } + static THREAD_LOCAL uint64_t* alloc_ptr = nullptr, * dealloc_ptr = nullptr; - void deallocate(size_t n) + if ( !alloc_ptr ) { - mem_stats.deallocated += n; ++mem_stats.deallocations; - assert(mem_stats.deallocated <= mem_stats.allocated); - assert(mem_stats.deallocations <= mem_stats.allocations); - assert(mem_stats.allocated or !mem_stats.allocations); + size_t sz = sizeof(alloc_ptr); + // __STRDUMP_DISABLE__ + mallctl("thread.allocatedp", (void*)&alloc_ptr, &sz, nullptr, 0); + mallctl("thread.deallocatedp", (void*)&dealloc_ptr, &sz, nullptr, 0); + // __STRDUMP_ENABLE__ } + mc.allocated = *alloc_ptr; + mc.deallocated = *dealloc_ptr; - size_t used() const + if ( mc.allocated > mc.deallocated ) { - if ( mem_stats.allocated < mem_stats.deallocated ) - { - assert(false); - return 0; - } - return mem_stats.allocated - mem_stats.deallocated; - } -}; + size_t usage = mc.allocated - mc.deallocated; -static Tracker s_tracker; + if ( usage > mc.max_in_use ) + mc.max_in_use = usage; -// ----------------------------------------------------------------------------- -// helpers -// ----------------------------------------------------------------------------- + return usage; + } + return 0; +} +#else +static size_t get_usage(const MemoryCounts& mc) +{ +#ifdef ENABLE_MEMORY_OVERLOADS + assert(mc.allocated >= mc.deallocated); + return mc.allocated - mc.deallocated; -template -inline bool free_space(size_t requested, size_t cap, Tracker& trk, Handler& handler) +#else + UNUSED(mc); + return 0; +#endif +} +#endif + +template +inline void free_space(size_t cap, Handler& handler) { - if ( requested > cap ) - { - return false; - } + MemoryCounts& mc = memory::MemoryCap::get_mem_stats(); + size_t usage = get_usage(mc); - auto used = trk.used(); + if ( usage < cap ) + return; - if ( used + requested <= cap ) - return true; + ++mc.reap_attempts; - ++mem_stats.reap_attempts; + if ( handler() ) + return; - while ( used + requested > cap ) - { - handler(); - auto tmp = trk.used(); - - if ( tmp >= used ) - { - ++mem_stats.reap_failures; - return false; - } - used = tmp; - } - return true; + ++mc.reap_failures; } inline size_t calculate_threshold(size_t cap, size_t threshold) @@ -117,190 +127,111 @@ inline size_t calculate_threshold(size_t cap, size_t threshold) // per-thread configuration // ----------------------------------------------------------------------------- -size_t MemoryCap::thread_cap = 0; -size_t MemoryCap::preemptive_threshold = 0; +size_t MemoryCap::limit = 0; // ----------------------------------------------------------------------------- // public interface // ----------------------------------------------------------------------------- -void MemoryCap::free_space(size_t n) +void MemoryCap::setup(const MemoryConfig& config, unsigned n) { - if ( !is_packet_thread() ) - return; - - if ( !thread_cap ) - return; - - static THREAD_LOCAL bool entered = false; - - if ( entered ) - return; - - entered = true; - memory::free_space(n, thread_cap, s_tracker, prune_handler); - entered = false; + assert(!is_packet_thread()); + limit = memory::calculate_threshold(config.cap, config.threshold); + pkt_mem_stats.resize(n); } -static size_t fudge_it(size_t n) +void MemoryCap::cleanup() { - return ((n >> 7) + 1) << 7; + pkt_mem_stats.resize(0); } -void MemoryCap::update_allocations(size_t n) +MemoryCounts& MemoryCap::get_mem_stats() { - if (n == 0) - return; - - size_t k = n; - n = fudge_it(n); - free_space(n); - mem_stats.total_fudge += (n - k); - s_tracker.allocate(n); - auto in_use = s_tracker.used(); - if ( in_use > mem_stats.max_in_use ) - mem_stats.max_in_use = in_use; - mp_active_context.update_allocs(n); -} - -void MemoryCap::update_deallocations(size_t n) -{ - if (n == 0) - return; + if ( !is_packet_thread() ) + return ctl_mem_stats; - n = fudge_it(n); - s_tracker.deallocate(n); - mp_active_context.update_deallocs(n); + auto id = get_instance_id(); + return pkt_mem_stats[id]; } -bool MemoryCap::over_threshold() +void MemoryCap::free_space() { - if ( !preemptive_threshold ) - return false; - - return s_tracker.used() >= preemptive_threshold; -} + assert(is_packet_thread()); -// FIXIT-L this should not be called while the packet threads are running. -// once reload is implemented for the memory manager, the configuration -// model will need to be updated - -void MemoryCap::calculate() -{ - assert(!is_packet_thread()); - const MemoryConfig& config = *SnortConfig::get_conf()->memory; + if ( !limit ) + return; - thread_cap = config.cap; - preemptive_threshold = memory::calculate_threshold(thread_cap, config.threshold); + memory::free_space(limit, prune_handler); } -void MemoryCap::print() +#ifdef ENABLE_MEMORY_OVERLOADS +void MemoryCap::allocate(size_t n) { - if ( !MemoryModule::is_active() ) - return; + MemoryCounts& mc = memory::MemoryCap::get_mem_stats(); - bool verbose = SnortConfig::log_verbose(); + mc.allocated += n; + ++mc.allocations; - if ( verbose or mem_stats.allocations ) - LogLabel("memory (heap)"); + assert(mc.allocated >= mc.deallocated); + auto in_use = mc.allocated - mc.deallocated; - if ( verbose ) - { - LogMessage(" thread cap: %zu\n", thread_cap); - LogMessage(" thread preemptive threshold: %zu\n", preemptive_threshold); - } + if ( in_use > mc.max_in_use ) + mc.max_in_use = in_use; - if ( mem_stats.allocations ) - { - LogMessage(" main thread usage: %zu\n", s_tracker.used()); - LogMessage(" allocations: %" PRIu64 "\n", mem_stats.allocations); - LogMessage(" deallocations: %" PRIu64 "\n", mem_stats.deallocations); - } +#ifdef ENABLE_MEMORY_PROFILER + mp_active_context.update_allocs(n); +#endif } -} // namespace memory - -#ifdef UNIT_TEST - -namespace t_memory_cap +void MemoryCap::deallocate(size_t n) { + MemoryCounts& mc = memory::MemoryCap::get_mem_stats(); -struct MockTracker -{ - size_t result = 0; - size_t used() const - { return result; } - - MockTracker(size_t r) : result { r } { } - MockTracker() = default; -}; - -struct HandlerSpy -{ - size_t calls = 0; - ssize_t modifier; - MockTracker* tracker; - - void operator()() + // std::thread causes an extra deallocation in packet + // threads so the below asserts don't hold + if ( mc.allocated >= mc.deallocated + n ) { - ++calls; - if ( modifier && tracker ) - tracker->result += modifier; + mc.deallocated += n; + ++mc.deallocations; } - HandlerSpy(ssize_t n, MockTracker& trk) : - modifier(n), tracker(&trk) { } -}; +#if 0 + assert(mc.deallocated <= mc.allocated); + assert(mc.deallocations <= mc.allocations); + assert(mc.allocated or !mc.allocations); +#endif -} // namespace t_memory_cap +#ifdef ENABLE_MEMORY_PROFILER + mp_active_context.update_deallocs(n); +#endif +} +#endif -TEST_CASE( "memory cap free space", "[memory]" ) +void MemoryCap::print(bool verbose, bool print_all) { - using namespace t_memory_cap; - - SECTION( "no handler call required" ) - { - MockTracker tracker; - HandlerSpy handler { 0, tracker }; - - CHECK( memory::free_space(1, 1024, tracker, handler) ); - CHECK( handler.calls == 0 ); - } - - SECTION( "handler frees enough space the first time" ) - { - MockTracker tracker { 1024 }; - HandlerSpy handler { -5, tracker }; - - CHECK( memory::free_space(1, 1024, tracker, handler) ); - CHECK( handler.calls == 1 ); - } + if ( !MemoryModule::is_active() ) + return; - SECTION( "handler needs to be called multiple times to free up space" ) - { - MockTracker tracker { 1024 }; - HandlerSpy handler { -1, tracker }; + MemoryCounts& mc = get_mem_stats(); + uint64_t usage = get_usage(mc); - CHECK( memory::free_space(2, 1024, tracker, handler) ); - CHECK( (handler.calls == 2) ); - } + if ( verbose or usage ) + LogLabel("memory (heap)"); - SECTION( "handler fails to free enough space" ) - { - MockTracker tracker { 1024 }; - HandlerSpy handler { 0, tracker }; + if ( verbose and print_all ) + LogCount("pruning threshold", limit); - CHECK_FALSE( memory::free_space(1, 1024, tracker, handler) ); - CHECK( handler.calls == 1 ); - } + LogCount("main thread usage", usage); + LogCount("allocations", mc.allocations); + LogCount("deallocations", mc.deallocations); - SECTION( "handler actually uses more space" ) + if ( verbose ) { - MockTracker tracker { 1024 }; - HandlerSpy handler { 5, tracker }; - CHECK_FALSE( memory::free_space(1, 1024, tracker, handler) ); - CHECK( handler.calls == 1 ); + struct rusage ru; + getrusage(RUSAGE_SELF, &ru); + LogCount("max_rss", ru.ru_maxrss * 1024); } } -#endif +} // namespace memory + diff --git a/src/memory/memory_cap.h b/src/memory/memory_cap.h index b28bb47e7..1f7ece093 100644 --- a/src/memory/memory_cap.h +++ b/src/memory/memory_cap.h @@ -23,31 +23,45 @@ #include -#include "main/thread.h" +#include "framework/counts.h" +#include "main/snort_types.h" + +struct MemoryConfig; namespace memory { +struct MemoryCounts +{ + PegCount allocations; + PegCount deallocations; + PegCount allocated; + PegCount deallocated; + PegCount reap_attempts; + PegCount reap_failures; + PegCount max_in_use; +}; + class SO_PUBLIC MemoryCap { public: - static void free_space(size_t); - // The following functions perform internal rounding. Allocations and deallocations must be - // performed in identical increments or leakage may occur. - static void update_allocations(size_t); - static void update_deallocations(size_t); + static void setup(const MemoryConfig&, unsigned); + static void cleanup(); - static bool over_threshold(); + static void free_space(); // call from main thread - static void calculate(); + static void print(bool verbose, bool print_all = true); - // call from main thread - static void print(); + static MemoryCounts& get_mem_stats(); + +#ifdef ENABLE_MEMORY_OVERLOADS + static void allocate(size_t); + static void deallocate(size_t); +#endif private: - static size_t thread_cap; - static size_t preemptive_threshold; + static size_t limit; }; } // namespace memory diff --git a/src/memory/memory_manager.cc b/src/memory/memory_manager.cc new file mode 100644 index 000000000..070c9fb67 --- /dev/null +++ b/src/memory/memory_manager.cc @@ -0,0 +1,414 @@ +//-------------------------------------------------------------------------- +// Copyright (C) 2016-2021 Cisco and/or its affiliates. All rights reserved. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License Version 2 as published +// by the Free Software Foundation. You may not use, modify or distribute +// this program under any other version of the GNU General Public License. +// +// This program is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this program; if not, write to the Free Software Foundation, Inc., +// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +//-------------------------------------------------------------------------- + +// memory_manager.cc author Joel Cornett + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include +#include + +#include "main/thread.h" + +#include "memory_allocator.h" +#include "memory_cap.h" + +#ifdef UNIT_TEST +#include "catch/snort_catch.h" +#endif + +namespace memory +{ + +// ----------------------------------------------------------------------------- +// metadata +// ----------------------------------------------------------------------------- + +// This structure must be aligned to max_align_t as long as we are prefixing +// it to memory allocations so that the returned memory is also aligned. +struct alignas(max_align_t) Metadata +{ +#if defined(REG_TEST) || defined(UNIT_TEST) + static constexpr size_t SANITY_CHECK_VALUE = 0xabcdef; + size_t sanity; +#endif + + // number of requested bytes + size_t payload_size; + + // total number of bytes allocated, including Metadata header + size_t total_size() const; + void* payload_offset(); + +#if defined(REG_TEST) || defined(UNIT_TEST) + bool valid() const + { return sanity == SANITY_CHECK_VALUE; } +#endif + + Metadata(size_t = 0); + + static size_t calculate_total_size(size_t); + + template + static Metadata* create(size_t); + + static Metadata* extract(void*); +}; + +inline size_t Metadata::total_size() const +{ return calculate_total_size(payload_size); } + +inline void* Metadata::payload_offset() +{ return this + 1; } + +inline Metadata::Metadata(size_t n) : +#if defined(REG_TEST) || defined(UNIT_TEST) + sanity(SANITY_CHECK_VALUE), +#endif + payload_size(n) +{ } + +inline size_t Metadata::calculate_total_size(size_t n) +{ return sizeof(Metadata) + n; } + +template +Metadata* Metadata::create(size_t n) +{ + auto meta = + static_cast(Allocator::allocate(calculate_total_size(n))); + + if ( !meta ) + return nullptr; + + // Trigger metadata ctor + *meta = Metadata(n); + +#if defined(REG_TEST) || defined(UNIT_TEST) + assert(meta->valid()); +#endif + + return meta; +} + +Metadata* Metadata::extract(void* p) +{ + assert(p); + + auto meta = static_cast(p) - 1; + +#if defined(REG_TEST) || defined(UNIT_TEST) + assert(meta->valid()); +#endif + + return meta; +} + +// ----------------------------------------------------------------------------- +// the meat +// ----------------------------------------------------------------------------- + +class ReentryContext +{ +public: + ReentryContext(bool& flag) : + already_entered(flag), flag(flag) + { flag = true; } + + ~ReentryContext() + { flag = false; } + + bool is_reentry() const + { return already_entered; } + +private: + const bool already_entered; + bool& flag; +}; + +template +struct Interface +{ + static void* allocate(size_t); + static void deallocate(void*); + + static THREAD_LOCAL bool in_allocation_call; +}; + +template +void* Interface::allocate(size_t n) +{ + // prevent allocation reentry + ReentryContext reentry_context(in_allocation_call); + assert(!reentry_context.is_reentry()); + + auto meta = Metadata::create(n); + + if ( !meta ) + return nullptr; + + Cap::allocate(meta->total_size()); + return meta->payload_offset(); +} + +template +void Interface::deallocate(void* p) +{ + if ( !p ) + return; + + auto meta = Metadata::extract(p); + assert(meta); + + Cap::deallocate(meta->total_size()); + Allocator::deallocate(meta); +} + +template +THREAD_LOCAL bool Interface::in_allocation_call = false; + +} //namespace memory + +// ----------------------------------------------------------------------------- +// new /delete replacements +// ----------------------------------------------------------------------------- + +// these don't have to be visible to operate as replacements + +#ifdef ENABLE_MEMORY_OVERLOADS +void* operator new(size_t n) +{ + auto p = memory::Interface<>::allocate(n); + if ( !p ) + throw std::bad_alloc(); + + return p; +} + +void* operator new[](size_t n) +{ return ::operator new(n); } + +void* operator new(size_t n, const std::nothrow_t&) noexcept +{ return memory::Interface<>::allocate(n); } + +void* operator new[](size_t n, const std::nothrow_t&) noexcept +{ return memory::Interface<>::allocate(n); } + +void operator delete(void* p) noexcept +{ memory::Interface<>::deallocate(p); } + +void operator delete[](void* p) noexcept +{ ::operator delete(p); } + +void operator delete(void* p, const std::nothrow_t&) noexcept +{ ::operator delete(p); } + +void operator delete[](void* p, const std::nothrow_t&) noexcept +{ ::operator delete[](p); } + +void operator delete(void* p, size_t) noexcept +{ ::operator delete(p); } + +void operator delete[](void* p, size_t) noexcept +{ ::operator delete[](p); } +#endif + +// ----------------------------------------------------------------------------- +// unit tests +// ----------------------------------------------------------------------------- + +#ifdef UNIT_TEST + +namespace t_memory +{ + +struct AllocatorSpy +{ + static void* allocate(size_t n) + { allocate_called = true; allocate_arg = n; return pool; } + + static void deallocate(void* p) + { deallocate_called = true; deallocate_arg = p; } + + static void reset() + { + pool = nullptr; + allocate_called = false; + allocate_arg = 0; + deallocate_called = false; + deallocate_arg = nullptr; + } + + static void* pool; + static bool allocate_called; + static size_t allocate_arg; + static bool deallocate_called; + static void* deallocate_arg; +}; + +void* AllocatorSpy::pool = nullptr; +bool AllocatorSpy::allocate_called = false; +size_t AllocatorSpy::allocate_arg = 0; +bool AllocatorSpy::deallocate_called = false; +void* AllocatorSpy::deallocate_arg = nullptr; + +struct CapSpy +{ + static void allocate(size_t n) + { + update_allocations_called = true; + update_allocations_arg = n; + } + + static void deallocate(size_t n) + { + update_deallocations_called = true; + update_deallocations_arg = n; + } + + static void reset() + { + update_allocations_called = false; + update_allocations_arg = 0; + + update_deallocations_called = false; + update_deallocations_arg = 0; + } + + static bool update_allocations_called; + static size_t update_allocations_arg; + + static bool update_deallocations_called; + static size_t update_deallocations_arg; +}; + +bool CapSpy::update_allocations_called = false; +size_t CapSpy::update_allocations_arg = 0; + +bool CapSpy::update_deallocations_called = false; +size_t CapSpy::update_deallocations_arg = 0; + +} // namespace t_memory + +TEST_CASE( "memory metadata", "[memory]" ) +{ + using namespace t_memory; + + AllocatorSpy::reset(); + constexpr size_t n = 1; + char pool[sizeof(memory::Metadata) + n]; + + SECTION( "create" ) + { + AllocatorSpy::pool = pool; + + auto meta = memory::Metadata::create(n); + + CHECK( (void*)meta == (void*)pool ); + CHECK( meta->valid() ); + CHECK( meta->payload_size == n ); + } + + SECTION( "extract" ) + { + auto meta_pool = reinterpret_cast(pool); + meta_pool[0] = memory::Metadata(n); + + void* p = &meta_pool[1]; + + auto meta = memory::Metadata::extract(p); + + CHECK( (void*)meta == (void*)pool ); + CHECK( meta->payload_offset() == p ); + } +} + +TEST_CASE( "memory manager interface", "[memory]" ) +{ + using namespace t_memory; + + AllocatorSpy::reset(); + CapSpy::reset(); + + constexpr size_t n = 1; + char pool[sizeof(memory::Metadata) + n]; + + using Interface = memory::Interface; + + SECTION( "allocation" ) + { + SECTION( "allocation failure" ) + { + auto p = Interface::allocate(n); + + CHECK( p == nullptr ); + + CHECK( AllocatorSpy::allocate_called ); + CHECK( AllocatorSpy::allocate_arg == memory::Metadata::calculate_total_size(n) ); + + CHECK_FALSE( CapSpy::update_allocations_called ); + } + + SECTION( "success" ) + { + AllocatorSpy::pool = pool; + + auto p = Interface::allocate(n); + + CHECK( p > (void*)pool ); + + CHECK( AllocatorSpy::allocate_called ); + CHECK( AllocatorSpy::allocate_arg == memory::Metadata::calculate_total_size(n) ); + + CHECK( CapSpy::update_allocations_called ); + CHECK( CapSpy::update_allocations_arg == memory::Metadata::calculate_total_size(n) ); + } + } + + SECTION( "deallocation" ) + { + SECTION( "nullptr" ) + { + Interface::deallocate(nullptr); + + CHECK_FALSE( AllocatorSpy::deallocate_called ); + CHECK_FALSE( CapSpy::update_deallocations_called ); + } + + SECTION( "success" ) + { + auto meta_pool = reinterpret_cast(pool); + meta_pool[0] = memory::Metadata(n); + + auto p = meta_pool[0].payload_offset(); + + Interface::deallocate(p); + + CHECK( AllocatorSpy::deallocate_called ); + CHECK( AllocatorSpy::deallocate_arg == (void*)pool ); + CHECK( CapSpy::update_deallocations_called ); + CHECK( CapSpy::update_deallocations_arg == memory::Metadata::calculate_total_size(n) ); + } + } + AllocatorSpy::pool = nullptr; + AllocatorSpy::deallocate_arg = nullptr; +} + +#endif + diff --git a/src/memory/memory_module.cc b/src/memory/memory_module.cc index b1a3da9c7..31152168a 100644 --- a/src/memory/memory_module.cc +++ b/src/memory/memory_module.cc @@ -26,6 +26,7 @@ #include "main/snort_config.h" +#include "memory_cap.h" #include "memory_config.h" using namespace snort; @@ -43,15 +44,13 @@ static const Parameter s_params[] = { "cap", Parameter::PT_INT, "0:maxSZ", "0", "set the per-packet-thread cap on memory (bytes, 0 to disable)" }, - { "threshold", Parameter::PT_INT, "0:100", "0", - "set the per-packet-thread threshold for preemptive cleanup actions " - "(percent, 0 to disable)" }, + { "threshold", Parameter::PT_INT, "1:100", "100", + "scale cap to account for heap overhead" }, { nullptr, Parameter::PT_MAX, nullptr, nullptr, nullptr } }; -THREAD_LOCAL MemoryCounts mem_stats; -static MemoryCounts zero_stats = { }; +static memory::MemoryCounts zero_stats = { }; const PegInfo mem_pegs[] = { @@ -62,7 +61,6 @@ const PegInfo mem_pegs[] = { CountType::NOW, "reap_attempts", "attempts to reclaim memory" }, { CountType::NOW, "reap_failures", "failures to reclaim memory" }, { CountType::MAX, "max_in_use", "highest allocated - deallocated" }, - { CountType::NOW, "total_fudge", "sum of all adjustments" }, { CountType::END, nullptr, nullptr } }; @@ -100,5 +98,10 @@ const PegInfo* MemoryModule::get_pegs() const { return mem_pegs; } PegCount* MemoryModule::get_counts() const -{ return is_active() ? (PegCount*)&mem_stats : (PegCount*)&zero_stats; } +{ + if ( !is_active() ) + return (PegCount*)&zero_stats; + + return (PegCount*)&memory::MemoryCap::get_mem_stats(); +} diff --git a/src/memory/memory_module.h b/src/memory/memory_module.h index 4563e9c53..456f23241 100644 --- a/src/memory/memory_module.h +++ b/src/memory/memory_module.h @@ -23,20 +23,6 @@ #include "framework/module.h" -struct MemoryCounts -{ - PegCount allocations; - PegCount deallocations; - PegCount allocated; - PegCount deallocated; - PegCount reap_attempts; - PegCount reap_failures; - PegCount max_in_use; - PegCount total_fudge; -}; - -extern THREAD_LOCAL MemoryCounts mem_stats; - class MemoryModule : public snort::Module { public: diff --git a/src/memory/prune_handler.cc b/src/memory/prune_handler.cc index 61a8ad78f..c8b949a87 100644 --- a/src/memory/prune_handler.cc +++ b/src/memory/prune_handler.cc @@ -31,9 +31,9 @@ using namespace snort; namespace memory { -void prune_handler() +bool prune_handler() { - Stream::prune_flows(); + return Stream::prune_flows(); } } // namespace memory diff --git a/src/memory/prune_handler.h b/src/memory/prune_handler.h index 6e1b418ac..2bde83b3c 100644 --- a/src/memory/prune_handler.h +++ b/src/memory/prune_handler.h @@ -24,7 +24,7 @@ namespace memory { -void prune_handler(); +bool prune_handler(); } diff --git a/src/mime/file_mime_log.h b/src/mime/file_mime_log.h index 019a36ff8..f514a4776 100644 --- a/src/mime/file_mime_log.h +++ b/src/mime/file_mime_log.h @@ -68,8 +68,6 @@ public: bool is_email_hdrs_present() const; bool is_email_from_present() const; bool is_email_to_present() const; - size_t size_of() const override - { return sizeof(*this); } private: int log_flags = 0; diff --git a/src/mime/file_mime_process.cc b/src/mime/file_mime_process.cc index a2cd6df19..5db1b7120 100644 --- a/src/mime/file_mime_process.cc +++ b/src/mime/file_mime_process.cc @@ -815,12 +815,10 @@ MimeSession::MimeSession(Packet* p, DecodeConfig* dconf, MailLogConfig* lconf, u { p->flow->stash->store(STASH_EXTRADATA_MIME, log_state); reset_mime_paf_state(&mime_boundary); - memory::MemoryCap::update_allocations(sizeof(*this)); } MimeSession::~MimeSession() { - memory::MemoryCap::update_deallocations(sizeof(*this)); if ( decode_state ) delete(decode_state); } diff --git a/src/network_inspectors/appid/appid_discovery.cc b/src/network_inspectors/appid/appid_discovery.cc index fdb5ab51e..110e99a3d 100644 --- a/src/network_inspectors/appid/appid_discovery.cc +++ b/src/network_inspectors/appid/appid_discovery.cc @@ -601,7 +601,6 @@ bool AppIdDiscovery::do_discovery(Packet* p, AppIdSession& asd, IpProtocol proto if (asd.tpsession and asd.tpsession->get_ctxt_version() != tp_appid_ctxt->get_version()) { bool is_tp_done = asd.is_tp_processing_done(); - memory::MemoryCap::update_deallocations(asd.tpsession->size_of()); delete asd.tpsession; asd.tpsession = nullptr; if (!is_tp_done) diff --git a/src/network_inspectors/appid/appid_dns_session.h b/src/network_inspectors/appid/appid_dns_session.h index 5b71f300a..512b38bcf 100644 --- a/src/network_inspectors/appid/appid_dns_session.h +++ b/src/network_inspectors/appid/appid_dns_session.h @@ -31,15 +31,8 @@ class AppIdDnsSession { public: - AppIdDnsSession() - { - memory::MemoryCap::update_allocations(sizeof(*this)); - } - - ~AppIdDnsSession() - { - memory::MemoryCap::update_deallocations(sizeof(*this)); - } + AppIdDnsSession() { } + ~AppIdDnsSession() { } void reset() { diff --git a/src/network_inspectors/appid/appid_ha.cc b/src/network_inspectors/appid/appid_ha.cc index dcf69d750..9f2fbd14d 100644 --- a/src/network_inspectors/appid/appid_ha.cc +++ b/src/network_inspectors/appid/appid_ha.cc @@ -113,7 +113,6 @@ bool AppIdHAAppsClient::consume(Flow*& flow, const FlowKey* key, HAMessage& msg, ErrorMessage("appid: Could not allocate asd.tpsession data in consume"); else { - memory::MemoryCap::update_allocations(asd->tpsession->size_of()); asd->tpsession->set_state(TP_STATE_HA); } } diff --git a/src/network_inspectors/appid/appid_http_session.cc b/src/network_inspectors/appid/appid_http_session.cc index 92a4083e9..dcbc15576 100644 --- a/src/network_inspectors/appid/appid_http_session.cc +++ b/src/network_inspectors/appid/appid_http_session.cc @@ -41,9 +41,7 @@ using namespace snort; AppIdHttpSession::AppIdHttpSession(AppIdSession& asd, uint32_t http2_stream_id) : asd(asd), http2_stream_id(http2_stream_id) -{ - memory::MemoryCap::update_allocations(sizeof(AppIdHttpSession)); -} +{ } AppIdHttpSession::~AppIdHttpSession() { @@ -51,7 +49,6 @@ AppIdHttpSession::~AppIdHttpSession() delete meta_data[i]; if (tun_dest) delete tun_dest; - memory::MemoryCap::update_deallocations(sizeof(AppIdHttpSession)); } void AppIdHttpSession::free_chp_matches(ChpMatchDescriptor& cmd, unsigned num_matches) diff --git a/src/network_inspectors/appid/appid_session.cc b/src/network_inspectors/appid/appid_session.cc index 904b9ee45..fb3e752ce 100644 --- a/src/network_inspectors/appid/appid_session.cc +++ b/src/network_inspectors/appid/appid_session.cc @@ -165,7 +165,6 @@ AppIdSession::~AppIdSession() if (tpsession) { - memory::MemoryCap::update_deallocations(tpsession->size_of()); if (pkt_thread_tp_appid_ctxt and ((tpsession->get_ctxt_version() == pkt_thread_tp_appid_ctxt->get_version()) and !ThirdPartyAppIdContext::get_tp_reload_in_progress())) diff --git a/src/network_inspectors/appid/appid_session.h b/src/network_inspectors/appid/appid_session.h index eca17b2b3..a0494e8fc 100644 --- a/src/network_inspectors/appid/appid_session.h +++ b/src/network_inspectors/appid/appid_session.h @@ -106,13 +106,10 @@ class TlsSession { public: TlsSession() - { - memory::MemoryCap::update_allocations(sizeof(*this)); - } + { } ~TlsSession() { - memory::MemoryCap::update_deallocations(sizeof(*this)); if (tls_host) snort_free(tls_host); if (tls_first_alt_name) @@ -239,9 +236,6 @@ public: bool bidirectional=false); void initialize_future_session(AppIdSession&, uint64_t); - size_t size_of() override - { return sizeof(*this); } - snort::Flow* flow = nullptr; AppIdConfig& config; std::unordered_map flow_data; diff --git a/src/network_inspectors/appid/appid_session_api.h b/src/network_inspectors/appid/appid_session_api.h index 159d3dbaa..c5a96cdf1 100644 --- a/src/network_inspectors/appid/appid_session_api.h +++ b/src/network_inspectors/appid/appid_session_api.h @@ -153,9 +153,6 @@ public: void clear_user_logged_in() { user_logged_in = false; } - size_t size_of() const override - { return sizeof(*this); } - protected: AppIdSessionApi(const AppIdSession* asd, const SfIp& ip); diff --git a/src/network_inspectors/appid/detector_plugins/test/detector_sip_test.cc b/src/network_inspectors/appid/detector_plugins/test/detector_sip_test.cc index 77cafe1c6..fe7359693 100644 --- a/src/network_inspectors/appid/detector_plugins/test/detector_sip_test.cc +++ b/src/network_inspectors/appid/detector_plugins/test/detector_sip_test.cc @@ -168,7 +168,6 @@ void AppIdDetector::add_user(AppIdSession&, char const*, int, bool, AppidChangeB void AppIdDetector::add_payload(AppIdSession&, int) { } void AppIdDetector::add_app(snort::Packet const&, AppIdSession&, AppidSessionDirection, int, int, char const*, AppidChangeBits&) { } -void memory::MemoryCap::update_deallocations(size_t) { } // LCOV_EXCL_STOP SipEvent::SipEvent(snort::Packet const* p, SIPMsg const*, SIP_DialogData const*) { this->p = p; } diff --git a/src/network_inspectors/appid/detector_plugins/test/http_url_patterns_test.cc b/src/network_inspectors/appid/detector_plugins/test/http_url_patterns_test.cc index cc18cbdc6..ac3cfce5d 100644 --- a/src/network_inspectors/appid/detector_plugins/test/http_url_patterns_test.cc +++ b/src/network_inspectors/appid/detector_plugins/test/http_url_patterns_test.cc @@ -56,9 +56,6 @@ static AppId service_id = APP_ID_NONE; static AppId client_id = APP_ID_NONE; static DetectorHTTPPattern mpattern; -void memory::MemoryCap::update_allocations(size_t) { } -void memory::MemoryCap::update_deallocations(size_t) { } - namespace snort { AppIdSessionApi::AppIdSessionApi(const AppIdSession*, const SfIp&) : diff --git a/src/network_inspectors/appid/dev_notes.txt b/src/network_inspectors/appid/dev_notes.txt index 89e6a2d96..7b64cca79 100644 --- a/src/network_inspectors/appid/dev_notes.txt +++ b/src/network_inspectors/appid/dev_notes.txt @@ -14,7 +14,7 @@ provide the logic to plug this inspector into the snort framework. AppIdModule configuration for AppId and AppIdInspector provides the packet processing context. An AppId inspector is instantiated for each packet thread created by the framework. -AppId registers to recieve any IP packet, it does not process rebuilt packets. +AppId registers to receive any IP packet, it does not process rebuilt packets. AppIdModule contains all the logic to process the AppId inspector Lua configuration which is identified by the 'appid' keyword. This configuration includes settings for logging, statistics, etc. and also diff --git a/src/network_inspectors/appid/test/appid_api_test.cc b/src/network_inspectors/appid/test/appid_api_test.cc index f16d04b2e..fb610d982 100644 --- a/src/network_inspectors/appid/test/appid_api_test.cc +++ b/src/network_inspectors/appid/test/appid_api_test.cc @@ -48,9 +48,6 @@ using namespace snort; static SnortProtocolId dummy_http2_protocol_id = 1; -void memory::MemoryCap::update_allocations(size_t) { } -void memory::MemoryCap::update_deallocations(size_t) { } - namespace snort { diff --git a/src/network_inspectors/appid/test/appid_debug_test.cc b/src/network_inspectors/appid/test/appid_debug_test.cc index c5024f00b..cef3da949 100644 --- a/src/network_inspectors/appid/test/appid_debug_test.cc +++ b/src/network_inspectors/appid/test/appid_debug_test.cc @@ -37,9 +37,6 @@ // Mocks -void memory::MemoryCap::update_allocations(size_t) { } -void memory::MemoryCap::update_deallocations(size_t) { } - namespace snort { unsigned get_instance_id() { return 3; } diff --git a/src/network_inspectors/appid/test/appid_detector_test.cc b/src/network_inspectors/appid/test/appid_detector_test.cc index 85aaffffa..9a9edfe7d 100644 --- a/src/network_inspectors/appid/test/appid_detector_test.cc +++ b/src/network_inspectors/appid/test/appid_detector_test.cc @@ -36,9 +36,6 @@ #include #include -void memory::MemoryCap::update_allocations(size_t) { } -void memory::MemoryCap::update_deallocations(size_t) { } - namespace snort { AppIdSessionApi::AppIdSessionApi(const AppIdSession*, const SfIp&) : diff --git a/src/network_inspectors/appid/test/appid_discovery_test.cc b/src/network_inspectors/appid/test/appid_discovery_test.cc index 3cc3eb3cf..a5ce3b432 100644 --- a/src/network_inspectors/appid/test/appid_discovery_test.cc +++ b/src/network_inspectors/appid/test/appid_discovery_test.cc @@ -43,9 +43,6 @@ uint32_t ThirdPartyAppIdContext::next_version = 0; -void memory::MemoryCap::update_allocations(size_t) { } -void memory::MemoryCap::update_deallocations(size_t) { } - namespace snort { // Stubs for appid api diff --git a/src/network_inspectors/appid/test/appid_efp_process_event_handler_test.cc b/src/network_inspectors/appid/test/appid_efp_process_event_handler_test.cc index 6cb7db5dc..63ed44503 100644 --- a/src/network_inspectors/appid/test/appid_efp_process_event_handler_test.cc +++ b/src/network_inspectors/appid/test/appid_efp_process_event_handler_test.cc @@ -54,9 +54,6 @@ Packet::Packet(bool) { } Packet::~Packet() = default; } -void memory::MemoryCap::update_allocations(size_t) { } -void memory::MemoryCap::update_deallocations(size_t) { } - void ApplicationDescriptor::set_id(const Packet&, AppIdSession&, AppidSessionDirection, AppId, AppidChangeBits&) { } void AppIdModule::reset_stats() { } diff --git a/src/network_inspectors/appid/test/appid_http_event_test.cc b/src/network_inspectors/appid/test/appid_http_event_test.cc index cdb1de740..220c75fe9 100644 --- a/src/network_inspectors/appid/test/appid_http_event_test.cc +++ b/src/network_inspectors/appid/test/appid_http_event_test.cc @@ -49,9 +49,6 @@ void ApplicationDescriptor::set_id(const Packet&, AppIdSession&, AppidSessionDir using namespace snort; -void memory::MemoryCap::update_allocations(size_t) { } -void memory::MemoryCap::update_deallocations(size_t) { } - namespace snort { AppIdApi appid_api; diff --git a/src/network_inspectors/appid/test/appid_http_session_test.cc b/src/network_inspectors/appid/test/appid_http_session_test.cc index 0be59c7c8..525b08170 100644 --- a/src/network_inspectors/appid/test/appid_http_session_test.cc +++ b/src/network_inspectors/appid/test/appid_http_session_test.cc @@ -165,9 +165,6 @@ void Profiler::consolidate_stats() { } void Profiler::reset_stats() { } void Profiler::show_stats() { } -void memory::MemoryCap::update_allocations(size_t) { } -void memory::MemoryCap::update_deallocations(size_t) { } - OdpContext::OdpContext(const AppIdConfig&, snort::SnortConfig*) { } AppIdConfig::~AppIdConfig() = default; diff --git a/src/network_inspectors/appid/test/appid_session_api_test.cc b/src/network_inspectors/appid/test/appid_session_api_test.cc index b00534595..a82f96346 100644 --- a/src/network_inspectors/appid/test/appid_session_api_test.cc +++ b/src/network_inspectors/appid/test/appid_session_api_test.cc @@ -36,9 +36,6 @@ static AppIdConfig config; static OdpContext odpctxt(config, nullptr); static Flow flow; -void memory::MemoryCap::update_allocations(size_t) { } -void memory::MemoryCap::update_deallocations(size_t) { } - void ApplicationDescriptor::set_id(const Packet&, AppIdSession&, AppidSessionDirection, AppId, AppidChangeBits&) { } void AppIdModule::reset_stats() {} diff --git a/src/network_inspectors/appid/test/service_state_test.cc b/src/network_inspectors/appid/test/service_state_test.cc index e1d9ce23a..eb0ffcf63 100644 --- a/src/network_inspectors/appid/test/service_state_test.cc +++ b/src/network_inspectors/appid/test/service_state_test.cc @@ -28,9 +28,6 @@ #include -void memory::MemoryCap::update_allocations(size_t) { } -void memory::MemoryCap::update_deallocations(size_t) { } - namespace snort { // Stubs for logs diff --git a/src/network_inspectors/appid/test/tp_mock.cc b/src/network_inspectors/appid/test/tp_mock.cc index 4427d362d..29db0b692 100644 --- a/src/network_inspectors/appid/test/tp_mock.cc +++ b/src/network_inspectors/appid/test/tp_mock.cc @@ -80,7 +80,6 @@ public: void clear_attr(TPSessionAttr attr) override { flags &= ~attr; } void set_attr(TPSessionAttr attr) override { flags |= attr; } unsigned get_attr(TPSessionAttr attr) override { return flags & attr; } - size_t size_of() const override { return sizeof(*this); } private: unsigned flags = 0; diff --git a/src/network_inspectors/appid/tp_appid_session_api.h b/src/network_inspectors/appid/tp_appid_session_api.h index f7a3c9443..1c12e5b45 100644 --- a/src/network_inspectors/appid/tp_appid_session_api.h +++ b/src/network_inspectors/appid/tp_appid_session_api.h @@ -58,7 +58,6 @@ public: virtual void clear_attr(TPSessionAttr) = 0; virtual void set_attr(TPSessionAttr) = 0; virtual unsigned get_attr(TPSessionAttr) = 0; - virtual size_t size_of() const = 0; virtual AppId get_appid(int& conf) { conf=confidence; return appid; } virtual const ThirdPartyAppIdContext& get_ctxt() const { return ctxt; } diff --git a/src/network_inspectors/appid/tp_appid_utils.cc b/src/network_inspectors/appid/tp_appid_utils.cc index 3b3158ea3..debf211ff 100644 --- a/src/network_inspectors/appid/tp_appid_utils.cc +++ b/src/network_inspectors/appid/tp_appid_utils.cc @@ -556,7 +556,6 @@ bool do_tp_discovery(ThirdPartyAppIdContext& tp_appid_ctxt, AppIdSession& asd, I ErrorMessage("Could not allocate asd.tpsession data"); return false; } - memory::MemoryCap::update_allocations(asd.tpsession->size_of()); } int tp_confidence; diff --git a/src/network_inspectors/arp_spoof/dev_notes.txt b/src/network_inspectors/arp_spoof/dev_notes.txt index 17f31481d..e53deb98e 100644 --- a/src/network_inspectors/arp_spoof/dev_notes.txt +++ b/src/network_inspectors/arp_spoof/dev_notes.txt @@ -3,7 +3,7 @@ protocol violations. This network inspector looks at all ARP ethernet frames and attempts to locate ARP spoofing attacks. It alerts on source or destination address mismatch. It also alerts on an -ARP request ocuring on a uni-cast frame (needs to be multi-cast). +ARP request occurring on a uni-cast frame (needs to be multi-cast). A network inspector module as it needs to examine all ethernet frames with packet type of ARP. diff --git a/src/network_inspectors/perf_monitor/perf_module.cc b/src/network_inspectors/perf_monitor/perf_module.cc index f3286daa7..4c685c58b 100644 --- a/src/network_inspectors/perf_monitor/perf_module.cc +++ b/src/network_inspectors/perf_monitor/perf_module.cc @@ -76,7 +76,7 @@ static const Parameter s_params[] = { "packets", Parameter::PT_INT, "0:max32", "10000", "minimum packets to report" }, - { "seconds", Parameter::PT_INT, "1:max32", "60", + { "seconds", Parameter::PT_INT, "0:max32", "60", "report interval" }, { "flow_ip_memcap", Parameter::PT_INT, "236:maxSZ", "52428800", @@ -274,8 +274,6 @@ bool PerfMonModule::set(const char*, Value& v, SnortConfig*) else if ( v.is("seconds") ) { config->sample_interval = v.get_uint32(); - if ( config->sample_interval == 0 ) - config->perf_flags |= PERF_SUMMARY; } else if ( v.is("flow_ip_memcap") ) { diff --git a/src/network_inspectors/rna/dev_notes.txt b/src/network_inspectors/rna/dev_notes.txt index de7c2cbe2..7a58d64c1 100644 --- a/src/network_inspectors/rna/dev_notes.txt +++ b/src/network_inspectors/rna/dev_notes.txt @@ -179,12 +179,12 @@ mss: maximum segment size FpElementType::SYN_MATCH, FpElementType::SYNTS Examples: - fptype = 2, mss = "12-34" -- client traffic (fptype = 2) with mss beween 12 and 34 + fptype = 2, mss = "12-34" -- client traffic (fptype = 2) with mss between 12 and 34 fptype = 2, mss = "X" -- don't use mss for matching, so match anything from cient fptype = 2, mss = "SYN" -- error: client traffic (fptype = 2) but mss of type SYN_MATCH only accepted for server traffic (fptyp1 = 1 or 10) fptype = 1, mss = "TS" -- OK: server traffic (fptype = 1) and mss of type SYNTS - mss = "+5" -- eror: mss cannot be an INCREMENT type + mss = "+5" -- error: mss cannot be an INCREMENT type id: ip id FpElementType::RANGE @@ -199,7 +199,7 @@ topts: tcp options FpElementType::RANGE These are defined by TcpOptCode in src/protocols/tcp_options.h. - The ones we use for fingerprint matcing are + The ones we use for fingerprint matching are - MAXSEG (2) - WSCALE (3) diff --git a/src/network_inspectors/rna/rna_fingerprint_tcp.cc b/src/network_inspectors/rna/rna_fingerprint_tcp.cc index b0e4a636b..49d7fa0fa 100644 --- a/src/network_inspectors/rna/rna_fingerprint_tcp.cc +++ b/src/network_inspectors/rna/rna_fingerprint_tcp.cc @@ -454,11 +454,6 @@ void RNAFlow::init() inspector_id = snort::FlowData::create_flow_data_id(); } -size_t RNAFlow::size_of() -{ - return sizeof(*this); -} - bool FpFingerprintState::set(const Packet* p) { int pos = 0; diff --git a/src/network_inspectors/rna/rna_flow.h b/src/network_inspectors/rna/rna_flow.h index 8af1e8d8a..7c7d3b119 100644 --- a/src/network_inspectors/rna/rna_flow.h +++ b/src/network_inspectors/rna/rna_flow.h @@ -48,7 +48,6 @@ public: ~RNAFlow() override; static void init(); - size_t size_of() override; void clear_ht(snort::HostTracker& ht); diff --git a/src/payload_injector/test/payload_injector_test.cc b/src/payload_injector/test/payload_injector_test.cc index 626ccf644..4e60e1cee 100644 --- a/src/payload_injector/test/payload_injector_test.cc +++ b/src/payload_injector/test/payload_injector_test.cc @@ -132,8 +132,6 @@ InjectionReturnStatus PayloadInjector::get_http2_payload(InjectionControl, unsigned Http2FlowData::inspector_id = 0; Http2Stream::~Http2Stream() = default; -HpackDynamicTable::HpackDynamicTable(Http2FlowData* flow_data) : - session_data(flow_data) {} HpackDynamicTable::~HpackDynamicTable() = default; Http2DataCutter::Http2DataCutter(Http2FlowData* _session_data, HttpCommon::SourceId src_id) : session_data(_session_data), source_id(src_id) { } @@ -149,7 +147,6 @@ Http2FlowData::Http2FlowData(snort::Flow*) : data_cutter {Http2DataCutter(this, SRC_CLIENT), Http2DataCutter(this, SRC_SERVER)} { } Http2FlowData::~Http2FlowData() = default; -size_t Http2FlowData::size_of() { return 1; } Http2FlowData http2_flow_data(nullptr); void Http2FlowData::set_mid_frame(bool val) { continuation_expected[SRC_SERVER] = val; } bool Http2FlowData::is_mid_frame() const { return continuation_expected[SRC_SERVER]; } diff --git a/src/profiler/memory_profiler.cc b/src/profiler/memory_profiler.cc index 0d9034b79..ef45b555b 100644 --- a/src/profiler/memory_profiler.cc +++ b/src/profiler/memory_profiler.cc @@ -46,13 +46,13 @@ namespace memory_stats static const StatsTable::Field fields[] = { { "#", 5, ' ', 0, std::ios_base::left }, - { "module", 20, ' ', 0, std::ios_base::fmtflags() }, + { "module", 24, ' ', 0, std::ios_base::fmtflags() }, { "layer", 6, ' ', 0, std::ios_base::fmtflags() }, - { "allocs", 9, ' ', 0, std::ios_base::fmtflags() }, - { "used (kb)", 12, ' ', 2, std::ios_base::fmtflags() }, - { "avg/allocation", 15, ' ', 1, std::ios_base::fmtflags() }, + { "allocs", 12, ' ', 0, std::ios_base::fmtflags() }, + { "used (kb)", 15, ' ', 2, std::ios_base::fmtflags() }, + { "avg/alloc", 12, ' ', 1, std::ios_base::fmtflags() }, { "%/caller", 10, ' ', 2, std::ios_base::fmtflags() }, - { "%/total", 9, ' ', 2, std::ios_base::fmtflags() }, + { "%/total", 10, ' ', 2, std::ios_base::fmtflags() }, { nullptr, 0, '\0', 0, std::ios_base::fmtflags() } }; diff --git a/src/profiler/profiler.cc b/src/profiler/profiler.cc index 792f11365..6cc8edd39 100644 --- a/src/profiler/profiler.cc +++ b/src/profiler/profiler.cc @@ -120,7 +120,9 @@ void Profiler::show_stats() assert(config); show_time_profiler_stats(s_profiler_nodes, config->time); +#ifdef ENABLE_MEMORY_PROFILER show_memory_profiler_stats(s_profiler_nodes, config->memory); +#endif show_rule_profiler_stats(config->rule); } diff --git a/src/profiler/profiler_defs.h b/src/profiler/profiler_defs.h index 6871894fd..f5318ce09 100644 --- a/src/profiler/profiler_defs.h +++ b/src/profiler/profiler_defs.h @@ -120,16 +120,18 @@ public: #ifdef NO_PROFILER using Profile = ProfileDisabled; #else -#ifdef NO_MEM_MGR +#ifndef ENABLE_MEMORY_PROFILER using Profile = NoMemContext; #else using Profile = ProfileContext; #endif #endif -// developer enable for profiling rule options -//using RuleProfile = ProfileContext; +#ifndef ENABLE_RULE_PROFILER using RuleProfile = ProfileDisabled; +#else +using RuleProfile = ProfileContext; +#endif } #endif diff --git a/src/service_inspectors/cip/cip.h b/src/service_inspectors/cip/cip.h index 514b8163d..5c87741d7 100644 --- a/src/service_inspectors/cip/cip.h +++ b/src/service_inspectors/cip/cip.h @@ -90,9 +90,6 @@ public: static void init() { inspector_id = snort::FlowData::create_flow_data_id(); } - size_t size_of() override - { return sizeof(*this); } - public: static unsigned inspector_id; CipSessionData session; diff --git a/src/service_inspectors/cip/dev_notes.txt b/src/service_inspectors/cip/dev_notes.txt index c314b674c..d27915e68 100644 --- a/src/service_inspectors/cip/dev_notes.txt +++ b/src/service_inspectors/cip/dev_notes.txt @@ -6,7 +6,7 @@ access certain protocol fields. This allows a user to write rules for CIP packets without decoding the protocol with a series of ”content” and ”byte test” options. -The preprocessor only evaluates PAF-flushed PDUs. If the rule options don't +The inspector only evaluates PAF-flushed PDUs. If the rule options don't check for this, they'll fire on stale session data when the original packet goes through before flushing. diff --git a/src/service_inspectors/dce_rpc/dce_smb1.cc b/src/service_inspectors/dce_rpc/dce_smb1.cc index 1265cf8f8..623405f95 100644 --- a/src/service_inspectors/dce_rpc/dce_smb1.cc +++ b/src/service_inspectors/dce_rpc/dce_smb1.cc @@ -314,13 +314,11 @@ Dce2Smb1SessionData::Dce2Smb1SessionData(const Packet* p, ssd.sd = sd; ssd.policy = policy; SMB_DEBUG(dce_smb_trace, DEFAULT_TRACE_OPTION_ID, TRACE_DEBUG_LEVEL, p, "smb1 session created\n"); - memory::MemoryCap::update_allocations(sizeof(*this)); } Dce2Smb1SessionData::~Dce2Smb1SessionData() { DCE2_SmbDataFree(&ssd); - memory::MemoryCap::update_deallocations(sizeof(*this)); } void Dce2Smb1SessionData::process() diff --git a/src/service_inspectors/dce_rpc/dce_smb2.cc b/src/service_inspectors/dce_rpc/dce_smb2.cc index 05ae281c9..3306bf8db 100644 --- a/src/service_inspectors/dce_rpc/dce_smb2.cc +++ b/src/service_inspectors/dce_rpc/dce_smb2.cc @@ -106,7 +106,6 @@ Dce2Smb2SessionData::Dce2Smb2SessionData(const Packet* p, tcp_file_tracker = nullptr; flow_key = get_smb2_flow_key(tcp_flow->key); SMB_DEBUG(dce_smb_trace, DEFAULT_TRACE_OPTION_ID, TRACE_DEBUG_LEVEL, p, "smb2 session created\n"); - memory::MemoryCap::update_allocations(sizeof(*this)); } Dce2Smb2SessionData::~Dce2Smb2SessionData() @@ -117,7 +116,6 @@ Dce2Smb2SessionData::~Dce2Smb2SessionData() it_session.second->detach_flow(flow_key); } session_data_mutex.unlock(); - memory::MemoryCap::update_deallocations(sizeof(*this)); } void Dce2Smb2SessionData::reset_matching_tcp_file_tracker( diff --git a/src/service_inspectors/dce_rpc/dce_smb_common.h b/src/service_inspectors/dce_rpc/dce_smb_common.h index f0a8bdb32..905c9df78 100644 --- a/src/service_inspectors/dce_rpc/dce_smb_common.h +++ b/src/service_inspectors/dce_rpc/dce_smb_common.h @@ -283,9 +283,6 @@ public: static void init() { inspector_id = snort::FlowData::create_flow_data_id(); } - size_t size_of() override - { return sizeof(*this); } - Dce2SmbSessionData* get_smb_session_data() { return ssd; } diff --git a/src/service_inspectors/dce_rpc/dce_tcp.h b/src/service_inspectors/dce_rpc/dce_tcp.h index 3c4735476..527ac4eb5 100644 --- a/src/service_inspectors/dce_rpc/dce_tcp.h +++ b/src/service_inspectors/dce_rpc/dce_tcp.h @@ -125,9 +125,6 @@ public: static void init() { inspector_id = snort::FlowData::create_flow_data_id(); } - size_t size_of() override - { return sizeof(*this); } - public: static unsigned inspector_id; DCE2_TcpSsnData dce2_tcp_session; diff --git a/src/service_inspectors/dce_rpc/dce_udp.h b/src/service_inspectors/dce_rpc/dce_udp.h index fd642124e..53a7d454f 100644 --- a/src/service_inspectors/dce_rpc/dce_udp.h +++ b/src/service_inspectors/dce_rpc/dce_udp.h @@ -197,9 +197,6 @@ public: static unsigned inspector_id; DCE2_UdpSsnData dce2_udp_session; - - size_t size_of() override - { return sizeof(*this); } }; DCE2_UdpSsnData* get_dce2_udp_session_data(snort::Flow*); diff --git a/src/service_inspectors/dev_notes.txt b/src/service_inspectors/dev_notes.txt index 07ff45bf3..bcba8aefd 100644 --- a/src/service_inspectors/dev_notes.txt +++ b/src/service_inspectors/dev_notes.txt @@ -9,7 +9,3 @@ PDUs. The wizard is a special service inspector that examines the beginning of a data stream and decides what application protocol is present. -http_inspect is the legacy Snort HTTP preprocessor ported to Snort\++ . -nhttp_inspect is the complete rewrite being developed specifically for -Snort++. - diff --git a/src/service_inspectors/dnp3/dnp3.h b/src/service_inspectors/dnp3/dnp3.h index 09c715762..b641c3c52 100644 --- a/src/service_inspectors/dnp3/dnp3.h +++ b/src/service_inspectors/dnp3/dnp3.h @@ -176,9 +176,6 @@ public: static void init() { inspector_id = snort::FlowData::create_flow_data_id(); } - size_t size_of() override - { return sizeof(*this); } - public: static unsigned inspector_id; dnp3_session_data_t dnp3_session; diff --git a/src/service_inspectors/dns/dns.h b/src/service_inspectors/dns/dns.h index 3beebf251..03cf5e9e7 100644 --- a/src/service_inspectors/dns/dns.h +++ b/src/service_inspectors/dns/dns.h @@ -175,9 +175,6 @@ public: static void init() { inspector_id = snort::FlowData::create_flow_data_id(); } - size_t size_of() override - { return sizeof(*this); } - public: static unsigned inspector_id; DNSData session; diff --git a/src/service_inspectors/ftp_telnet/ftpp_si.h b/src/service_inspectors/ftp_telnet/ftpp_si.h index 9b10ba100..9df76aa29 100644 --- a/src/service_inspectors/ftp_telnet/ftpp_si.h +++ b/src/service_inspectors/ftp_telnet/ftpp_si.h @@ -105,9 +105,6 @@ public: static void init() { inspector_id = snort::FlowData::create_flow_data_id(); } - size_t size_of() override - { return sizeof(*this); } - public: static unsigned inspector_id; TELNET_SESSION session; diff --git a/src/service_inspectors/gtp/gtp_inspect.h b/src/service_inspectors/gtp/gtp_inspect.h index 57f95fdea..4a94a61b4 100644 --- a/src/service_inspectors/gtp/gtp_inspect.h +++ b/src/service_inspectors/gtp/gtp_inspect.h @@ -43,9 +43,6 @@ public: static void init(); - size_t size_of() override - { return sizeof(*this); } - public: static unsigned inspector_id; GTP_Roptions ropts; diff --git a/src/service_inspectors/http2_inspect/http2_flow_data.cc b/src/service_inspectors/http2_inspect/http2_flow_data.cc index ad2bf3c99..7da830704 100644 --- a/src/service_inspectors/http2_inspect/http2_flow_data.cc +++ b/src/service_inspectors/http2_inspect/http2_flow_data.cc @@ -43,12 +43,6 @@ unsigned Http2FlowData::inspector_id = 0; uint64_t Http2FlowData::instance_count = 0; #endif -// Each stream will have class Http2Stream allocated and a node in streams list -const size_t Http2FlowData::stream_memory_size = sizeof(class Http2Stream) + - stream_extra_memory; -const size_t Http2FlowData::stream_increment_memory_size = stream_memory_size * - STREAM_MEMORY_TRACKING_INCREMENT; - Http2FlowData::Http2FlowData(Flow* flow_) : FlowData(inspector_id), flow(flow_), @@ -103,10 +97,6 @@ Http2FlowData::~Http2FlowData() for (Http2Stream* stream : streams) delete stream; - // Since stream memory is allocated in blocks of 25, must also deallocate in blocks of 25 to - // ensure consistent rounding. - while (stream_memory_allocations_tracked > STREAM_MEMORY_TRACKING_INCREMENT) - update_stream_memory_deallocations(); } HttpFlowData* Http2FlowData::get_hi_flow_data() const @@ -123,28 +113,6 @@ void Http2FlowData::set_hi_flow_data(HttpFlowData* flow) stream->set_hi_flow_data(flow); } -size_t Http2FlowData::size_of() -{ - // Account for memory for 25 concurrent streams up front, plus 1 stream for stream id 0. - return sizeof(*this) + stream_increment_memory_size + stream_memory_size + - (2 * sizeof(Http2EventGen)) + (2 * sizeof(Http2Infractions)); -} - -void Http2FlowData::update_stream_memory_allocations() -{ - assert(concurrent_streams > stream_memory_allocations_tracked); - assert(concurrent_streams % stream_memory_allocations_tracked == 1); - update_allocations(stream_increment_memory_size); - stream_memory_allocations_tracked += STREAM_MEMORY_TRACKING_INCREMENT; -} - -void Http2FlowData::update_stream_memory_deallocations() -{ - assert(stream_memory_allocations_tracked >= STREAM_MEMORY_TRACKING_INCREMENT); - update_deallocations(stream_increment_memory_size); - stream_memory_allocations_tracked -= STREAM_MEMORY_TRACKING_INCREMENT; -} - Http2Stream* Http2FlowData::find_stream(const uint32_t key) const { for (Http2Stream* stream : streams) @@ -215,8 +183,6 @@ Http2Stream* Http2FlowData::get_processing_stream(const SourceId source_id, uint concurrent_streams += 1; if (concurrent_streams > Http2Module::get_peg_counts(PEG_MAX_CONCURRENT_STREAMS)) Http2Module::increment_peg_counts(PEG_MAX_CONCURRENT_STREAMS); - if (concurrent_streams > stream_memory_allocations_tracked) - update_stream_memory_allocations(); } } return stream; @@ -278,16 +244,6 @@ uint32_t Http2FlowData::get_current_stream_id(const SourceId source_id) const return current_stream[source_id]; } -void Http2FlowData::allocate_hi_memory(HttpFlowData* hi_flow_data) -{ - update_allocations(hi_flow_data->size_of()); -} - -void Http2FlowData::deallocate_hi_memory(HttpFlowData* hi_flow_data) -{ - update_deallocations(hi_flow_data->size_of()); -} - bool Http2FlowData::is_mid_frame() const { return (header_octets_seen[SRC_SERVER] != 0) || (remaining_data_padding[SRC_SERVER] != 0) || diff --git a/src/service_inspectors/http2_inspect/http2_flow_data.h b/src/service_inspectors/http2_inspect/http2_flow_data.h index fee0d104f..2fc6f244d 100644 --- a/src/service_inspectors/http2_inspect/http2_flow_data.h +++ b/src/service_inspectors/http2_inspect/http2_flow_data.h @@ -84,8 +84,6 @@ public: friend class Http2WindowUpdateFrame; friend void finish_msg_body(Http2FlowData* session_data, HttpCommon::SourceId source_id); - size_t size_of() override; - Http2Stream* find_current_stream(const HttpCommon::SourceId source_id) const; uint32_t get_current_stream_id(const HttpCommon::SourceId source_id) const; Http2Stream* get_processing_stream(const HttpCommon::SourceId source_id, uint32_t concurrent_streams_limit); @@ -202,19 +200,6 @@ private: Http2Stream* get_hi_stream() const; Http2Stream* find_stream(const uint32_t key) const; void delete_processing_stream(); - - // When H2I allocates http_inspect flows, it bypasses the usual FlowData memory allocation - // bookkeeping. So H2I needs to update memory allocations and deallocations itself. - void allocate_hi_memory(HttpFlowData* hi_flow_data); - void deallocate_hi_memory(HttpFlowData* hi_flow_data); - // Memory for streams is tracked in increments of 25 to minimize tracking overhead - void update_stream_memory_allocations(); - void update_stream_memory_deallocations(); - static const size_t stream_memory_size; - static const size_t stream_increment_memory_size; - // Per-stream extra memory estimate to account for the std::list streams. Actual memory usage - // is implementation dependent - static const size_t stream_extra_memory = 24; }; #endif diff --git a/src/service_inspectors/http2_inspect/http2_hpack_dynamic_table.cc b/src/service_inspectors/http2_inspect/http2_hpack_dynamic_table.cc index dc291995f..ec441ff3c 100644 --- a/src/service_inspectors/http2_inspect/http2_hpack_dynamic_table.cc +++ b/src/service_inspectors/http2_inspect/http2_hpack_dynamic_table.cc @@ -22,38 +22,20 @@ #endif #include "http2_hpack_dynamic_table.h" +#include "http2_module.h" #include -#include "http2_flow_data.h" #include "http2_hpack_table.h" -#include "http2_module.h" using namespace Http2Enums; -HpackDynamicTable::HpackDynamicTable(Http2FlowData* flow_data) : - session_data(flow_data) -{ - session_data->update_allocations( ARRAY_CAPACITY * sizeof(HpackTableEntry*) + - TABLE_MEMORY_TRACKING_INCREMENT); - table_memory_allocated = TABLE_MEMORY_TRACKING_INCREMENT; -} - - HpackDynamicTable::~HpackDynamicTable() { - for (uint32_t i = 0, indx = start; i < num_entries; i++) + for (std::vector::iterator it = circular_buf.begin(); + it != circular_buf.end(); ++it) { - delete circular_buf[indx]; - indx = (indx + 1) % ARRAY_CAPACITY; - } - session_data->update_deallocations( ARRAY_CAPACITY * sizeof(HpackTableEntry*) + - TABLE_MEMORY_TRACKING_INCREMENT ); - - while (table_memory_allocated > TABLE_MEMORY_TRACKING_INCREMENT) - { - session_data->update_deallocations(TABLE_MEMORY_TRACKING_INCREMENT); - table_memory_allocated -= TABLE_MEMORY_TRACKING_INCREMENT; + delete *it; } } @@ -89,12 +71,6 @@ bool HpackDynamicTable::add_entry(const Field& name, const Field& value) Http2Module::increment_peg_counts(PEG_MAX_TABLE_ENTRIES); rfc_table_size += new_entry_size; - while (rfc_table_size > table_memory_allocated) - { - session_data->update_allocations(TABLE_MEMORY_TRACKING_INCREMENT); - table_memory_allocated += TABLE_MEMORY_TRACKING_INCREMENT; - } - return true; } diff --git a/src/service_inspectors/http2_inspect/http2_hpack_dynamic_table.h b/src/service_inspectors/http2_inspect/http2_hpack_dynamic_table.h index 43e13178c..2636a2d9a 100644 --- a/src/service_inspectors/http2_inspect/http2_hpack_dynamic_table.h +++ b/src/service_inspectors/http2_inspect/http2_hpack_dynamic_table.h @@ -34,7 +34,7 @@ class HpackDynamicTable { public: // FIXIT-P This array can be optimized to start smaller and grow on demand - HpackDynamicTable(Http2FlowData* flow_data); + HpackDynamicTable() : circular_buf(ARRAY_CAPACITY, nullptr) {} ~HpackDynamicTable(); const HpackTableEntry* get_entry(uint32_t index) const; bool add_entry(const Field& name, const Field& value); @@ -46,15 +46,12 @@ private: const static uint32_t DEFAULT_MAX_SIZE = 4096; const static uint32_t ARRAY_CAPACITY = 512; - const static uint32_t TABLE_MEMORY_TRACKING_INCREMENT = 500; uint32_t max_size = DEFAULT_MAX_SIZE; uint32_t start = 0; uint32_t num_entries = 0; uint32_t rfc_table_size = 0; - HpackTableEntry* circular_buf[ARRAY_CAPACITY] = {0}; - Http2FlowData* const session_data; - uint32_t table_memory_allocated; + std::vector circular_buf; void prune_to_size(uint32_t new_max_size); }; diff --git a/src/service_inspectors/http2_inspect/http2_hpack_table.h b/src/service_inspectors/http2_inspect/http2_hpack_table.h index 579253ef2..44b57c53f 100644 --- a/src/service_inspectors/http2_inspect/http2_hpack_table.h +++ b/src/service_inspectors/http2_inspect/http2_hpack_table.h @@ -40,7 +40,7 @@ struct HpackTableEntry class HpackIndexTable { public: - HpackIndexTable(Http2FlowData* flow_data) : dynamic_table(flow_data) { } + HpackIndexTable(Http2FlowData*) { } const HpackTableEntry* lookup(uint64_t index) const; bool add_index(const Field& name, const Field& value); HpackDynamicTable& get_dynamic_table() { return dynamic_table; } diff --git a/src/service_inspectors/http2_inspect/http2_stream.cc b/src/service_inspectors/http2_inspect/http2_stream.cc index 3ab143105..1aeab5241 100644 --- a/src/service_inspectors/http2_inspect/http2_stream.cc +++ b/src/service_inspectors/http2_inspect/http2_stream.cc @@ -45,8 +45,6 @@ Http2Stream::Http2Stream(uint32_t stream_id_, Http2FlowData* session_data_) : Http2Stream::~Http2Stream() { delete current_frame; - if (hi_flow_data) - session_data->deallocate_hi_memory(hi_flow_data); delete hi_flow_data; } @@ -77,7 +75,6 @@ void Http2Stream::check_and_cleanup_completed() { if (hi_flow_data != nullptr) { - session_data->deallocate_hi_memory(hi_flow_data); delete hi_flow_data; hi_flow_data = nullptr; } @@ -108,7 +105,6 @@ void Http2Stream::set_hi_flow_data(HttpFlowData* flow_data) { assert(hi_flow_data == nullptr); hi_flow_data = flow_data; - session_data->allocate_hi_memory(hi_flow_data); } const Field& Http2Stream::get_buf(unsigned id) diff --git a/src/service_inspectors/http_inspect/http_cutter.cc b/src/service_inspectors/http_inspect/http_cutter.cc index 4455dcf70..5922d340c 100644 --- a/src/service_inspectors/http_inspect/http_cutter.cc +++ b/src/service_inspectors/http_inspect/http_cutter.cc @@ -280,9 +280,8 @@ ScanResult HttpHeaderCutter::cut(const uint8_t* buffer, uint32_t length, } HttpBodyCutter::HttpBodyCutter(bool accelerated_blocking_, ScriptFinder* finder_, - CompressId compression_, HttpFlowData* ssn_data) - : accelerated_blocking(accelerated_blocking_), compression(compression_), finder(finder_), - session_data(ssn_data) + CompressId compression_) + : accelerated_blocking(accelerated_blocking_), compression(compression_), finder(finder_) { if (accelerated_blocking) { @@ -302,9 +301,6 @@ HttpBodyCutter::HttpBodyCutter(bool accelerated_blocking_, ScriptFinder* finder_ delete compress_stream; compress_stream = nullptr; } - else - session_data->update_allocations(session_data->zlib_inflate_memory); - } static const uint8_t inspect_string[] = { '<', '/', 's', 'c', 'r', 'i', 'p', 't', '>' }; @@ -322,7 +318,6 @@ HttpBodyCutter::~HttpBodyCutter() { inflateEnd(compress_stream); delete compress_stream; - session_data->update_deallocations(session_data->zlib_inflate_memory); } } diff --git a/src/service_inspectors/http_inspect/http_cutter.h b/src/service_inspectors/http_inspect/http_cutter.h index 2a6fe1f08..04e33305f 100644 --- a/src/service_inspectors/http_inspect/http_cutter.h +++ b/src/service_inspectors/http_inspect/http_cutter.h @@ -104,7 +104,7 @@ class HttpBodyCutter : public HttpCutter { public: HttpBodyCutter(bool accelerated_blocking_, ScriptFinder* finder, - HttpEnums::CompressId compression_, HttpFlowData* ssn_data); + HttpEnums::CompressId compression_); ~HttpBodyCutter() override; void soft_reset() override { octets_seen = 0; } @@ -124,7 +124,6 @@ private: const uint8_t* match_string; const uint8_t* match_string_upper; uint8_t string_length; - HttpFlowData* const session_data; }; class HttpBodyClCutter : public HttpBodyCutter @@ -133,9 +132,8 @@ public: HttpBodyClCutter(int64_t expected_length, bool accelerated_blocking, ScriptFinder* finder, - HttpEnums::CompressId compression, - HttpFlowData* ssn_data) : - HttpBodyCutter(accelerated_blocking, finder, compression, ssn_data), + HttpEnums::CompressId compression) : + HttpBodyCutter(accelerated_blocking, finder, compression), remaining(expected_length) { assert(remaining > 0); } HttpEnums::ScanResult cut(const uint8_t*, uint32_t length, HttpInfractions*, HttpEventGen*, @@ -149,8 +147,8 @@ class HttpBodyOldCutter : public HttpBodyCutter { public: HttpBodyOldCutter(bool accelerated_blocking, ScriptFinder* finder, - HttpEnums::CompressId compression, HttpFlowData* ssn_data) : - HttpBodyCutter(accelerated_blocking, finder, compression, ssn_data) + HttpEnums::CompressId compression) : + HttpBodyCutter(accelerated_blocking, finder, compression) {} HttpEnums::ScanResult cut(const uint8_t*, uint32_t, HttpInfractions*, HttpEventGen*, uint32_t flow_target, bool stretch, HttpEnums::H2BodyState) override; @@ -160,8 +158,8 @@ class HttpBodyChunkCutter : public HttpBodyCutter { public: HttpBodyChunkCutter(int64_t maximum_chunk_length_, bool accelerated_blocking, - ScriptFinder* finder, HttpEnums::CompressId compression, HttpFlowData* ssn_data) : - HttpBodyCutter(accelerated_blocking, finder, compression, ssn_data), + ScriptFinder* finder, HttpEnums::CompressId compression) : + HttpBodyCutter(accelerated_blocking, finder, compression), maximum_chunk_length(maximum_chunk_length_) {} HttpEnums::ScanResult cut(const uint8_t* buffer, uint32_t length, @@ -190,8 +188,8 @@ class HttpBodyH2Cutter : public HttpBodyCutter { public: HttpBodyH2Cutter(int64_t expected_length, bool accelerated_blocking, ScriptFinder* finder, - HttpEnums::CompressId compression, HttpFlowData* ssn_data) : - HttpBodyCutter(accelerated_blocking, finder, compression, ssn_data), + HttpEnums::CompressId compression) : + HttpBodyCutter(accelerated_blocking, finder, compression), expected_body_length(expected_length) {} HttpEnums::ScanResult cut(const uint8_t* buffer, uint32_t length, HttpInfractions*, diff --git a/src/service_inspectors/http_inspect/http_field.cc b/src/service_inspectors/http_inspect/http_field.cc index c9761bd82..cf2a34982 100644 --- a/src/service_inspectors/http_inspect/http_field.cc +++ b/src/service_inspectors/http_inspect/http_field.cc @@ -23,7 +23,6 @@ #include "http_field.h" -#include "flow/flow_data.h" #include "http_common.h" #include "http_enum.h" #include "http_test_manager.h" @@ -78,18 +77,6 @@ void Field::set(const Field& f) // Both Fields cannot be responsible for deleting the buffer so do not copy own_the_buffer } -void Field::update_allocations(snort::FlowData* flow_data) -{ - if (own_the_buffer && (len > 0)) - flow_data->update_allocations(len); -} - -void Field::update_deallocations(snort::FlowData* flow_data) -{ - if (own_the_buffer && (len > 0)) - flow_data->update_deallocations(len); -} - #ifdef REG_TEST void Field::print(FILE* output, const char* name) const { diff --git a/src/service_inspectors/http_inspect/http_field.h b/src/service_inspectors/http_inspect/http_field.h index 2356bc2ae..0246ee331 100644 --- a/src/service_inspectors/http_inspect/http_field.h +++ b/src/service_inspectors/http_inspect/http_field.h @@ -27,11 +27,6 @@ #include "http_common.h" #include "http_enum.h" -namespace snort -{ -class FlowData; -} - // Individual pieces of the message found during parsing. // Length values <= 0 are StatusCode values and imply that the start pointer is meaningless. // Never use the start pointer without verifying that length > 0. @@ -51,8 +46,6 @@ public: void set(const Field& f); void set(HttpCommon::StatusCode stat_code); void set(int32_t length) { set(static_cast(length)); } - void update_allocations(snort::FlowData* flow_data); - void update_deallocations(snort::FlowData* flow_data); #ifdef REG_TEST void print(FILE* output, const char* name) const; diff --git a/src/service_inspectors/http_inspect/http_flow_data.cc b/src/service_inspectors/http_inspect/http_flow_data.cc index d058c56e1..ce3875c85 100644 --- a/src/service_inspectors/http_inspect/http_flow_data.cc +++ b/src/service_inspectors/http_inspect/http_flow_data.cc @@ -95,7 +95,6 @@ HttpFlowData::~HttpFlowData() #ifndef UNIT_TEST_BUILD if (js_ident_ctx) { - update_deallocations(js_ident_ctx->size()); delete js_ident_ctx; debug_log(4, http_trace, TRACE_JS_PROC, nullptr, @@ -103,7 +102,6 @@ HttpFlowData::~HttpFlowData() } if (js_normalizer) { - update_deallocations(JSNormalizer::size()); delete js_normalizer; debug_log(4, http_trace, TRACE_JS_PROC, nullptr, @@ -117,16 +115,13 @@ HttpFlowData::~HttpFlowData() delete events[k]; delete[] section_buffer[k]; delete[] partial_buffer[k]; - update_deallocations(partial_buffer_length[k]); delete[] partial_detect_buffer[k]; - update_deallocations(partial_detect_length[k]); HttpTransaction::delete_transaction(transaction[k], nullptr); delete cutter[k]; if (compress_stream[k] != nullptr) { inflateEnd(compress_stream[k]); delete compress_stream[k]; - update_deallocations(zlib_inflate_memory); } if (mime_state[k] != nullptr) { @@ -147,11 +142,6 @@ HttpFlowData::~HttpFlowData() } } -size_t HttpFlowData::size_of() -{ - return sizeof(HttpFlowData) + (2 * sizeof(HttpEventGen)); -} - void HttpFlowData::half_reset(SourceId source_id) { assert((source_id == SRC_CLIENT) || (source_id == SRC_SERVER)); @@ -176,7 +166,6 @@ void HttpFlowData::half_reset(SourceId source_id) inflateEnd(compress_stream[source_id]); delete compress_stream[source_id]; compress_stream[source_id] = nullptr; - update_deallocations(zlib_inflate_memory); } if (mime_state[source_id] != nullptr) { @@ -220,7 +209,6 @@ void HttpFlowData::trailer_prep(SourceId source_id) inflateEnd(compress_stream[source_id]); delete compress_stream[source_id]; compress_stream[source_id] = nullptr; - update_deallocations(zlib_inflate_memory); } detection_status[source_id] = DET_REACTIVATING; } @@ -268,7 +256,6 @@ snort::JSNormalizer& HttpFlowData::acquire_js_ctx(int32_t ident_depth, size_t no if (!js_ident_ctx) { js_ident_ctx = new JSIdentifierCtx(ident_depth, max_scope_depth, built_in_ident); - update_allocations(js_ident_ctx->size()); debug_logf(4, http_trace, TRACE_JS_PROC, nullptr, "js_ident_ctx created (ident_depth %d)\n", ident_depth); @@ -276,7 +263,6 @@ snort::JSNormalizer& HttpFlowData::acquire_js_ctx(int32_t ident_depth, size_t no js_normalizer = new JSNormalizer(*js_ident_ctx, norm_depth, max_template_nesting, max_bracket_depth); - update_allocations(JSNormalizer::size()); debug_logf(4, http_trace, TRACE_JS_PROC, nullptr, "js_normalizer created (norm_depth %zd, max_template_nesting %d)\n", @@ -299,7 +285,6 @@ void HttpFlowData::release_js_ctx() if (!js_normalizer) return; - update_deallocations(JSNormalizer::size()); delete js_normalizer; js_normalizer = nullptr; @@ -320,7 +305,6 @@ bool HttpFlowData::add_to_pipeline(HttpTransaction* latest) { pipeline = new HttpTransaction*[MAX_PIPELINE]; HttpModule::increment_peg_counts(PEG_PIPELINED_FLOWS); - update_allocations(sizeof(HttpTransaction*) * MAX_PIPELINE); } assert(!pipeline_overflow && !pipeline_underflow); int new_back = (pipeline_back+1) % MAX_PIPELINE; @@ -353,8 +337,6 @@ void HttpFlowData::delete_pipeline() { delete pipeline[k]; } - if (pipeline != nullptr) - update_deallocations(sizeof(HttpTransaction*) * MAX_PIPELINE); delete[] pipeline; } @@ -378,12 +360,10 @@ void HttpFlowData::finish_h2_body(HttpCommon::SourceId source_id, HttpEnums::H2B { // We've already sent all data through detection so no need to reinspect. Just need to // prep for trailers - update_deallocations(partial_buffer_length[source_id]); partial_buffer_length[source_id] = 0; delete[] partial_buffer[source_id]; partial_buffer[source_id] = nullptr; - update_deallocations(partial_detect_length[source_id]); partial_detect_length[source_id] = 0; delete[] partial_detect_buffer[source_id]; partial_detect_buffer[source_id] = nullptr; diff --git a/src/service_inspectors/http_inspect/http_flow_data.h b/src/service_inspectors/http_inspect/http_flow_data.h index 7adef8158..2b39678f1 100644 --- a/src/service_inspectors/http_inspect/http_flow_data.h +++ b/src/service_inspectors/http_inspect/http_flow_data.h @@ -52,7 +52,6 @@ public: ~HttpFlowData() override; static unsigned inspector_id; static void init() { inspector_id = snort::FlowData::create_flow_data_id(); } - size_t size_of() override; friend class HttpBodyCutter; friend class HttpInspect; diff --git a/src/service_inspectors/http_inspect/http_msg_body.cc b/src/service_inspectors/http_inspect/http_msg_body.cc index a86830c9c..f2df2ae97 100644 --- a/src/service_inspectors/http_inspect/http_msg_body.cc +++ b/src/service_inspectors/http_inspect/http_msg_body.cc @@ -100,7 +100,6 @@ void HttpMsgBody::clean_partial(uint32_t& partial_inspected_octets, uint32_t& pa if (session_data->detect_depth_remaining[source_id] > 0) { delete[] partial_detect_buffer; - session_data->update_deallocations(partial_detect_length); assert(detect_length <= session_data->detect_depth_remaining[source_id]); bookkeeping_regular_flush(partial_detect_length, partial_detect_buffer, partial_js_detect_length, detect_length); @@ -190,7 +189,6 @@ void HttpMsgBody::analyze() detect_data.set(detect_length, js_norm_body.start()); delete[] partial_detect_buffer; - session_data->update_deallocations(partial_detect_length); if (!session_data->partial_flush[source_id]) { @@ -206,7 +204,6 @@ void HttpMsgBody::analyze() partial_detect_buffer = save_partial; partial_detect_length = decompressed->length(); partial_js_detect_length = js_norm_body.length(); - session_data->update_allocations(partial_detect_length); } set_file_data(const_cast(detect_data.start()), @@ -367,7 +364,6 @@ void HttpMsgBody::do_enhanced_js_normalization(const Field& input, Field& output { *infractions += INF_JS_PDU_MISS; session_data->events[HttpCommon::SRC_SERVER]->create_event(EVENT_JS_PDU_MISS); - session_data->js_data_lost_once = true; return; } diff --git a/src/service_inspectors/http_inspect/http_msg_head_shared.cc b/src/service_inspectors/http_inspect/http_msg_head_shared.cc index 65293e50e..167a4c09f 100755 --- a/src/service_inspectors/http_inspect/http_msg_head_shared.cc +++ b/src/service_inspectors/http_inspect/http_msg_head_shared.cc @@ -34,35 +34,11 @@ using namespace HttpCommon; using namespace HttpEnums; using namespace snort; -// Memory calculation: -// Approximations based on assumptions: -// - there will be a cookie -// - http_header and http_cookie rule options will be required -// - classic normalization on the headers will not be trivial -// - individual normalized headers won't be a lot and 500 bytes will cover them -// -// Header infrastructure (header_line, header_name, header_name_id, header_value): -// session_data_->num_head_lines[source_id_] * (3 * sizeof(Field) + sizeof(HeaderId)) -// -// The entire headers consist of http_raw_header and http_raw_cookie. Because there is -// a cookie it will be necessary to write out the full msg_text into these two buffers, -// resulting in allocations totaling approximately msg_text.length(). These raw buffers -// in turn will need to be normalized, requiring another msg_text.length(). -// Total cost: 2 * msg_text.length(). -// -// Plus 500 bytes for normalized headers. HttpMsgHeadShared::HttpMsgHeadShared(const uint8_t* buffer, const uint16_t buf_size, HttpFlowData* session_data_, HttpCommon::SourceId source_id_, bool buf_owner, snort::Flow* flow_, const HttpParaList* params_): HttpMsgSection(buffer, buf_size, session_data_, source_id_, - buf_owner, flow_, params_), own_msg_buffer(buf_owner), - extra_memory_allocations(session_data_->num_head_lines[source_id_] * - (3 * sizeof(Field) + sizeof(HeaderId)) + 2 * msg_text.length() + 500) -{ - if (own_msg_buffer) - session_data->update_allocations(msg_text.length()); - - session_data->update_allocations(extra_memory_allocations); -} + buf_owner, flow_, params_), own_msg_buffer(buf_owner) +{ } HttpMsgHeadShared::~HttpMsgHeadShared() { @@ -77,11 +53,6 @@ HttpMsgHeadShared::~HttpMsgHeadShared() list_ptr = list_ptr->next; delete temp_ptr; } - - if (own_msg_buffer) - session_data->update_deallocations(msg_text.length()); - - session_data->update_deallocations(extra_memory_allocations); } bool HttpMsgHeadShared::is_external_js() diff --git a/src/service_inspectors/http_inspect/http_msg_head_shared.h b/src/service_inspectors/http_inspect/http_msg_head_shared.h index f0f791a68..4bdcfb61a 100755 --- a/src/service_inspectors/http_inspect/http_msg_head_shared.h +++ b/src/service_inspectors/http_inspect/http_msg_head_shared.h @@ -109,7 +109,6 @@ private: bool file_cache_index_computed = false; bool own_msg_buffer; - const uint32_t extra_memory_allocations; int js_external = HttpCommon::STAT_NOT_COMPUTE; }; diff --git a/src/service_inspectors/http_inspect/http_msg_header.cc b/src/service_inspectors/http_inspect/http_msg_header.cc index e896de26b..c8234c9de 100755 --- a/src/service_inspectors/http_inspect/http_msg_header.cc +++ b/src/service_inspectors/http_inspect/http_msg_header.cc @@ -605,8 +605,6 @@ void HttpMsgHeader::setup_encoding_decompression() delete session_data->compress_stream[source_id]; session_data->compress_stream[source_id] = nullptr; } - else - session_data->update_allocations(session_data->zlib_inflate_memory); } void HttpMsgHeader::setup_utf_decoding() diff --git a/src/service_inspectors/http_inspect/http_msg_request.cc b/src/service_inspectors/http_inspect/http_msg_request.cc index bc1a57d4c..95d1e03fb 100644 --- a/src/service_inspectors/http_inspect/http_msg_request.cc +++ b/src/service_inspectors/http_inspect/http_msg_request.cc @@ -120,7 +120,7 @@ void HttpMsgRequest::parse_start_line() { uri = new HttpUri(start_line.start() + first_end + 1, last_begin - first_end - 1, method_id, params->uri_param, transaction->get_infractions(source_id), - session_data->events[source_id], session_data); + session_data->events[source_id]); } else { @@ -165,7 +165,7 @@ bool HttpMsgRequest::handle_zero_nine() uri_end--); uri = new HttpUri(start_line.start() + uri_begin, uri_end - uri_begin + 1, method_id, params->uri_param, transaction->get_infractions(source_id), - session_data->events[source_id], session_data); + session_data->events[source_id]); } else { diff --git a/src/service_inspectors/http_inspect/http_msg_start.cc b/src/service_inspectors/http_inspect/http_msg_start.cc index af60e9d13..685221863 100644 --- a/src/service_inspectors/http_inspect/http_msg_start.cc +++ b/src/service_inspectors/http_inspect/http_msg_start.cc @@ -31,16 +31,10 @@ HttpMsgStart::HttpMsgStart(const uint8_t* buffer, const uint16_t buf_size, HttpF HttpCommon::SourceId source_id_, bool buf_owner, snort::Flow* flow_, const HttpParaList* params_) : HttpMsgSection(buffer, buf_size, session_data_, source_id_, buf_owner, flow_, params_), own_msg_buffer(buf_owner) -{ - if (own_msg_buffer) - session_data->update_allocations(msg_text.length()); -} +{ } HttpMsgStart::~HttpMsgStart() -{ - if (own_msg_buffer) - session_data->update_deallocations(msg_text.length()); -} +{ } void HttpMsgStart::analyze() { diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc index 21e75fcac..238db9471 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc @@ -179,7 +179,6 @@ void HttpStreamSplitter::decompress_copy(uint8_t* buffer, uint32_t& offset, cons inflateEnd(compress_stream); delete compress_stream; compress_stream = nullptr; - session_data->update_deallocations(session_data->zlib_inflate_memory); } return; } @@ -207,7 +206,6 @@ void HttpStreamSplitter::decompress_copy(uint8_t* buffer, uint32_t& offset, cons inflateEnd(compress_stream); delete compress_stream; compress_stream = nullptr; - session_data->update_deallocations(session_data->zlib_inflate_memory); // Since we failed to uncompress the data, fall through } } @@ -384,7 +382,6 @@ const StreamBuffer HttpStreamSplitter::reassemble(Flow* flow, unsigned total, memcpy(buffer, partial_buffer, partial_buffer_length); session_data->section_offset[source_id] = partial_buffer_length; delete[] partial_buffer; - session_data->update_deallocations(partial_buffer_length); partial_buffer_length = 0; partial_buffer = nullptr; } @@ -428,7 +425,6 @@ const StreamBuffer HttpStreamSplitter::reassemble(Flow* flow, unsigned total, partial_buffer = new uint8_t[buf_size]; memcpy(partial_buffer, buffer, buf_size); partial_buffer_length = buf_size; - session_data->update_allocations(partial_buffer_length); } partial_raw_bytes += total; } diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc b/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc index b5623e6bc..dd4c28f41 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc @@ -77,24 +77,24 @@ HttpCutter* HttpStreamSplitter::get_cutter(SectionType type, session_data->data_length[source_id], session_data->accelerated_blocking[source_id], my_inspector->script_finder, - session_data->compression[source_id], session_data); + session_data->compression[source_id]); case SEC_BODY_CHUNK: return (HttpCutter*)new HttpBodyChunkCutter( my_inspector->params->maximum_chunk_length, session_data->accelerated_blocking[source_id], my_inspector->script_finder, - session_data->compression[source_id], session_data); + session_data->compression[source_id]); case SEC_BODY_OLD: return (HttpCutter*)new HttpBodyOldCutter( session_data->accelerated_blocking[source_id], my_inspector->script_finder, - session_data->compression[source_id], session_data); + session_data->compression[source_id]); case SEC_BODY_H2: return (HttpCutter*)new HttpBodyH2Cutter( session_data->data_length[source_id], session_data->accelerated_blocking[source_id], my_inspector->script_finder, - session_data->compression[source_id], session_data); + session_data->compression[source_id]); default: assert(false); return nullptr; diff --git a/src/service_inspectors/http_inspect/http_transaction.cc b/src/service_inspectors/http_inspect/http_transaction.cc index ab940a0dd..84ff55776 100644 --- a/src/service_inspectors/http_inspect/http_transaction.cc +++ b/src/service_inspectors/http_inspect/http_transaction.cc @@ -54,7 +54,6 @@ HttpTransaction::HttpTransaction(HttpFlowData* session_data_): session_data(sess { infractions[0] = nullptr; infractions[1] = nullptr; - session_data->update_allocations(transaction_memory_usage_estimate); } HttpTransaction::~HttpTransaction() @@ -69,7 +68,6 @@ HttpTransaction::~HttpTransaction() } delete_section_list(body_list); delete_section_list(discard_list); - session_data->update_deallocations(transaction_memory_usage_estimate); } HttpTransaction* HttpTransaction::attach_my_transaction(HttpFlowData* session_data, SourceId diff --git a/src/service_inspectors/http_inspect/http_uri.cc b/src/service_inspectors/http_inspect/http_uri.cc index 8b929d4c9..79fb0f835 100644 --- a/src/service_inspectors/http_inspect/http_uri.cc +++ b/src/service_inspectors/http_inspect/http_uri.cc @@ -25,28 +25,12 @@ #include "http_common.h" #include "http_enum.h" -#include "http_flow_data.h" #include "hash/hash_key_operations.h" using namespace HttpCommon; using namespace HttpEnums; using namespace snort; -HttpUri::HttpUri(const uint8_t* start, int32_t length, HttpEnums::MethodId method_id_, - const HttpParaList::UriParam& uri_param_, HttpInfractions* infractions_, - HttpEventGen* events_, HttpFlowData* session_data_) : - uri(length, start), infractions(infractions_), events(events_), method_id(method_id_), - uri_param(uri_param_), session_data(session_data_) -{ - normalize(); - classic_norm.update_allocations(session_data); -} - -HttpUri::~HttpUri() -{ - classic_norm.update_deallocations(session_data); -} - void HttpUri::parse_uri() { // Four basic types of HTTP URI @@ -426,4 +410,3 @@ const Field& HttpUri::get_norm_host() return host_norm; } - diff --git a/src/service_inspectors/http_inspect/http_uri.h b/src/service_inspectors/http_inspect/http_uri.h index 5c1bd91df..5f5e26c81 100644 --- a/src/service_inspectors/http_inspect/http_uri.h +++ b/src/service_inspectors/http_inspect/http_uri.h @@ -20,13 +20,11 @@ #ifndef HTTP_URI_H #define HTTP_URI_H -#include "http_event.h" -#include "http_field.h" -#include "http_module.h" #include "http_str_to_code.h" +#include "http_module.h" #include "http_uri_norm.h" - -class HttpFlowData; +#include "http_field.h" +#include "http_event.h" static const int MAX_SCHEME_LENGTH = 36; // schemes longer than 36 characters are malformed static const int LONG_SCHEME_LENGTH = 10; // schemes longer than 10 characters will alert @@ -40,8 +38,10 @@ class HttpUri public: HttpUri(const uint8_t* start, int32_t length, HttpEnums::MethodId method_id_, const HttpParaList::UriParam& uri_param_, HttpInfractions* infractions_, - HttpEventGen* events_, HttpFlowData* session_data_); - ~HttpUri(); + HttpEventGen* events_) : + uri(length, start), infractions(infractions_), events(events_), method_id(method_id_), + uri_param(uri_param_) + { normalize(); } const Field& get_uri() const { return uri; } HttpEnums::UriType get_uri_type() { return uri_type; } const Field& get_scheme() { return scheme; } @@ -85,7 +85,6 @@ private: HttpEnums::UriType uri_type = HttpEnums::URI__NOT_COMPUTE; const HttpEnums::MethodId method_id; const HttpParaList::UriParam& uri_param; - HttpFlowData* const session_data; void normalize(); void parse_uri(); diff --git a/src/service_inspectors/http_inspect/test/http_module_test.cc b/src/service_inspectors/http_inspect/test/http_module_test.cc index 587bf9ce5..175fea15f 100755 --- a/src/service_inspectors/http_inspect/test/http_module_test.cc +++ b/src/service_inspectors/http_inspect/test/http_module_test.cc @@ -74,8 +74,6 @@ HttpJsNorm::HttpJsNorm(const HttpParaList::UriParam& uri_param_, int64_t normali HttpJsNorm::~HttpJsNorm() = default; void HttpJsNorm::configure(){} int64_t Parameter::get_int(char const*) { return 0; } -void FlowData::update_allocations(size_t) {} -void FlowData::update_deallocations(size_t) {} TEST_GROUP(http_peg_count_test) { diff --git a/src/service_inspectors/http_inspect/test/http_msg_head_shared_util_test.cc b/src/service_inspectors/http_inspect/test/http_msg_head_shared_util_test.cc index edc2e8379..e57e5957e 100644 --- a/src/service_inspectors/http_inspect/test/http_msg_head_shared_util_test.cc +++ b/src/service_inspectors/http_inspect/test/http_msg_head_shared_util_test.cc @@ -37,8 +37,6 @@ using namespace snort; // Stubs whose sole purpose is to make the test code link long HttpTestManager::print_amount {}; bool HttpTestManager::print_hex {}; -void FlowData::update_allocations(size_t) {} -void FlowData::update_deallocations(size_t) {} // Tests for get_next_code() TEST_GROUP(get_next_code) diff --git a/src/service_inspectors/http_inspect/test/http_normalizers_test.cc b/src/service_inspectors/http_inspect/test/http_normalizers_test.cc index 57258726d..9dfb54b79 100644 --- a/src/service_inspectors/http_inspect/test/http_normalizers_test.cc +++ b/src/service_inspectors/http_inspect/test/http_normalizers_test.cc @@ -40,8 +40,6 @@ const bool HttpEnums::is_sp_tab[256] {}; const bool HttpEnums::is_sp_tab_quote_dquote[256] {}; long HttpTestManager::print_amount {}; bool HttpTestManager::print_hex {}; -void FlowData::update_allocations(size_t) {} -void FlowData::update_deallocations(size_t) {} TEST_GROUP(norm_decimal_integer_test) {}; diff --git a/src/service_inspectors/http_inspect/test/http_transaction_test.cc b/src/service_inspectors/http_inspect/test/http_transaction_test.cc index a9b8acc57..b235ad4f3 100644 --- a/src/service_inspectors/http_inspect/test/http_transaction_test.cc +++ b/src/service_inspectors/http_inspect/test/http_transaction_test.cc @@ -47,8 +47,6 @@ FlowData::~FlowData() = default; int DetectionEngine::queue_event(unsigned int, unsigned int) { return 0; } fd_status_t File_Decomp_StopFree(fd_session_t*) { return File_Decomp_OK; } uint32_t str_to_hash(const uint8_t *, size_t) { return 0; } -void FlowData::update_allocations(size_t) {} -void FlowData::update_deallocations(size_t) {} FlowData* Flow::get_flow_data(uint32_t) const { return nullptr; } } diff --git a/src/service_inspectors/http_inspect/test/http_uri_norm_test.cc b/src/service_inspectors/http_inspect/test/http_uri_norm_test.cc index 0e58086bf..219330fe5 100755 --- a/src/service_inspectors/http_inspect/test/http_uri_norm_test.cc +++ b/src/service_inspectors/http_inspect/test/http_uri_norm_test.cc @@ -63,8 +63,6 @@ HttpJsNorm::HttpJsNorm(const HttpParaList::UriParam& uri_param_, int64_t normali HttpJsNorm::~HttpJsNorm() = default; void HttpJsNorm::configure() {} int64_t Parameter::get_int(char const*) { return 0; } -void FlowData::update_allocations(size_t) {} -void FlowData::update_deallocations(size_t) {} TEST_GROUP(http_inspect_uri_norm) { diff --git a/src/service_inspectors/iec104/iec104.h b/src/service_inspectors/iec104/iec104.h index cce2fb34a..38cb35849 100644 --- a/src/service_inspectors/iec104/iec104.h +++ b/src/service_inspectors/iec104/iec104.h @@ -57,14 +57,7 @@ public: static void init(); void reset() - { - ssn_data.session_data_reset(); - } - - size_t size_of() override - { - return sizeof(*this); - } + { ssn_data.session_data_reset(); } public: static unsigned inspector_id; diff --git a/src/service_inspectors/imap/imap.h b/src/service_inspectors/imap/imap.h index b99403e7e..c1d1324b5 100644 --- a/src/service_inspectors/imap/imap.h +++ b/src/service_inspectors/imap/imap.h @@ -175,9 +175,6 @@ public: static void init() { inspector_id = snort::FlowData::create_flow_data_id(); } - size_t size_of() override - { return sizeof(*this); } - public: static unsigned inspector_id; IMAPData session; diff --git a/src/service_inspectors/modbus/dev_notes.txt b/src/service_inspectors/modbus/dev_notes.txt index c6ee14d4c..a4355aa60 100644 --- a/src/service_inspectors/modbus/dev_notes.txt +++ b/src/service_inspectors/modbus/dev_notes.txt @@ -6,7 +6,7 @@ to access certain protocol fields. This allows a user to write rules for Modbus packets without decoding the protocol with a series of ”content” and ”byte test” options. -The preprocessor only evaluates PAF-flushed PDUs. If the rule options don't +The inspector only evaluates PAF-flushed PDUs. If the rule options don't check for this, they'll fire on stale session data when the original packet goes through before flushing. diff --git a/src/service_inspectors/modbus/modbus.h b/src/service_inspectors/modbus/modbus.h index 6f18a1bd1..542a550b3 100644 --- a/src/service_inspectors/modbus/modbus.h +++ b/src/service_inspectors/modbus/modbus.h @@ -54,9 +54,6 @@ public: ssn_data.flags = 0; } - size_t size_of() override - { return sizeof(*this); } - public: static unsigned inspector_id; modbus_session_data_t ssn_data; diff --git a/src/service_inspectors/pop/pop.h b/src/service_inspectors/pop/pop.h index 10b6f59a5..f07fb92ab 100644 --- a/src/service_inspectors/pop/pop.h +++ b/src/service_inspectors/pop/pop.h @@ -129,9 +129,6 @@ public: static void init() { inspector_id = snort::FlowData::create_flow_data_id(); } - size_t size_of() override - { return sizeof(*this); } - public: static unsigned inspector_id; POPData session; diff --git a/src/service_inspectors/rpc_decode/rpc_decode.cc b/src/service_inspectors/rpc_decode/rpc_decode.cc index 7d1ffca44..458bc102f 100644 --- a/src/service_inspectors/rpc_decode/rpc_decode.cc +++ b/src/service_inspectors/rpc_decode/rpc_decode.cc @@ -84,9 +84,6 @@ public: static void init() { inspector_id = FlowData::create_flow_data_id(); } - size_t size_of() override - { return sizeof(*this); } - public: static unsigned inspector_id; RpcSsnData session; diff --git a/src/service_inspectors/s7commplus/s7comm.h b/src/service_inspectors/s7commplus/s7comm.h index 762be6e73..e6ce59884 100644 --- a/src/service_inspectors/s7commplus/s7comm.h +++ b/src/service_inspectors/s7commplus/s7comm.h @@ -64,9 +64,6 @@ public: ssn_data.session_data_reset(); } - size_t size_of() override - { return sizeof(*this); } - public: static unsigned inspector_id; S7commplusSessionData ssn_data; diff --git a/src/service_inspectors/sip/sip.cc b/src/service_inspectors/sip/sip.cc index cd995c090..c4e23768d 100644 --- a/src/service_inspectors/sip/sip.cc +++ b/src/service_inspectors/sip/sip.cc @@ -59,13 +59,11 @@ SipFlowData::~SipFlowData() FreeSipData(&session); assert(sip_stats.concurrent_sessions > 0); sip_stats.concurrent_sessions--; - memory::MemoryCap::update_deallocations(sizeof(SipFlowData)); } static SIPData* SetNewSIPData(Packet* p) { SipFlowData* fd = new SipFlowData; - memory::MemoryCap::update_allocations(sizeof(SipFlowData)); p->flow->set_flow_data(fd); return &fd->session; } diff --git a/src/service_inspectors/sip/sip.h b/src/service_inspectors/sip/sip.h index 68b6c8947..255e3e8c4 100644 --- a/src/service_inspectors/sip/sip.h +++ b/src/service_inspectors/sip/sip.h @@ -45,9 +45,6 @@ public: static void init() { inspector_id = snort::FlowData::create_flow_data_id(); } - size_t size_of() override - { return sizeof(*this); } - public: static unsigned inspector_id; SIPData session; diff --git a/src/service_inspectors/sip/sip_dialog.cc b/src/service_inspectors/sip/sip_dialog.cc index fa65fc4bd..594b8fdae 100644 --- a/src/service_inspectors/sip/sip_dialog.cc +++ b/src/service_inspectors/sip/sip_dialog.cc @@ -410,7 +410,6 @@ static int SIP_ignoreChannels(SIP_DialogData* dialog, Packet* p, SIP_PROTO_CONF* { Stream::ignore_flow(p, p->flow->pkt_type, p->get_ip_proto_next(), &mdataA->maddress, mdataA->mport, &mdataB->maddress, mdataB->mport, SSN_DIR_BOTH, (new SipFlowData)); - memory::MemoryCap::update_allocations(sizeof(SipFlowData)); } sip_stats.ignoreChannels++; mdataA = mdataA->nextM; @@ -529,7 +528,6 @@ static SIP_DialogData* SIP_addDialog(SIPMsg* sipMsg, SIP_DialogData* currDialog, sip_stats.dialogs++; dialog = (SIP_DialogData*)snort_calloc(sizeof(SIP_DialogData)); - memory::MemoryCap::update_allocations(sizeof(SIP_DialogData)); // Add to the head dialog->nextD = currDialog; @@ -589,9 +587,10 @@ static int SIP_deleteDialog(SIP_DialogData* currDialog, SIP_DialogList* dList) } sip_freeMediaList(currDialog->mediaSessions); snort_free(currDialog); - memory::MemoryCap::update_deallocations(sizeof(SIP_DialogData)); + if ( dList->num_dialogs > 0) dList->num_dialogs--; + return true; } @@ -686,7 +685,6 @@ void sip_freeDialogs(SIP_DialogList* list) nextNode = curNode->nextD; sip_freeMediaList(curNode->mediaSessions); snort_free(curNode); - memory::MemoryCap::update_deallocations(sizeof(SIP_DialogData)); curNode = nextNode; } } diff --git a/src/service_inspectors/sip/sip_parser.cc b/src/service_inspectors/sip/sip_parser.cc index 4c3309406..a78f4f42a 100644 --- a/src/service_inspectors/sip/sip_parser.cc +++ b/src/service_inspectors/sip/sip_parser.cc @@ -505,7 +505,6 @@ static bool sip_body_parse(SIPMsg* msg, const char* buff, const char* end, const // Create a media session msg->mediaSession = (SIP_MediaSession*)snort_calloc(sizeof(SIP_MediaSession)); - memory::MemoryCap::update_allocations(sizeof(SIP_MediaSession)); const char* start = buff; /* @@ -1130,7 +1129,6 @@ static int sip_parse_sdp_m(SIPMsg* msg, const char* start, const char* end) return SIP_PARSE_ERROR; mdata = (SIP_MediaData*)snort_calloc(sizeof(SIP_MediaData)); - memory::MemoryCap::update_allocations(sizeof(SIP_MediaData)); mdata->mport = (uint16_t)SnortStrtoul(spaceIndex + 1, &next, 10); if ((nullptr != next)&&('/'==next[0])) @@ -1256,13 +1254,11 @@ void sip_freeMediaSession(SIP_MediaSession* mediaSession) { nextNode = curNode->nextM; snort_free(curNode); - memory::MemoryCap::update_deallocations(sizeof(SIP_MediaData)); curNode = nextNode; } if (nullptr != mediaSession) { snort_free(mediaSession); - memory::MemoryCap::update_deallocations(sizeof(SIP_MediaSession)); } } diff --git a/src/service_inspectors/smtp/smtp.h b/src/service_inspectors/smtp/smtp.h index c032f34d9..d5d419503 100644 --- a/src/service_inspectors/smtp/smtp.h +++ b/src/service_inspectors/smtp/smtp.h @@ -174,9 +174,6 @@ public: static void init() { inspector_id = snort::FlowData::create_flow_data_id(); } - size_t size_of() override - { return sizeof(*this); } - public: static unsigned inspector_id; SMTPData session; diff --git a/src/service_inspectors/ssh/ssh.h b/src/service_inspectors/ssh/ssh.h index ff3e0b895..6753d0ef1 100644 --- a/src/service_inspectors/ssh/ssh.h +++ b/src/service_inspectors/ssh/ssh.h @@ -106,9 +106,6 @@ public: static void init() { inspector_id = snort::FlowData::create_flow_data_id(); } - size_t size_of() override - { return sizeof(*this); } - public: static unsigned inspector_id; SSHData session; diff --git a/src/service_inspectors/ssl/ssl_inspector.h b/src/service_inspectors/ssl/ssl_inspector.h index 5e5699615..3207e422b 100644 --- a/src/service_inspectors/ssl/ssl_inspector.h +++ b/src/service_inspectors/ssl/ssl_inspector.h @@ -33,9 +33,6 @@ public: static void init() { assign_ssl_inspector_id(snort::FlowData::create_flow_data_id()); } - size_t size_of() override - { return sizeof(*this); } - SSLData& get_session() override { return session; } diff --git a/src/stream/base/stream_base.cc b/src/stream/base/stream_base.cc index 14a9c55a0..6e5340baa 100644 --- a/src/stream/base/stream_base.cc +++ b/src/stream/base/stream_base.cc @@ -60,7 +60,7 @@ const PegInfo base_pegs[] = { CountType::SUM, "idle_prunes", " sessions pruned due to timeout" }, { CountType::SUM, "excess_prunes", "sessions pruned due to excess" }, { CountType::SUM, "uni_prunes", "uni sessions pruned" }, - { CountType::SUM, "preemptive_prunes", "sessions pruned during preemptive pruning" }, + { CountType::SUM, "preemptive_prunes", "sessions pruned during preemptive pruning (deprecated)" }, { CountType::SUM, "memcap_prunes", "sessions pruned due to memcap" }, { CountType::SUM, "ha_prunes", "sessions pruned by high availability sync" }, { CountType::SUM, "stale_prunes", "sessions pruned due to stale connection" }, @@ -90,7 +90,7 @@ void base_prep() stream_base_stats.timeout_prunes = flow_con->get_prunes(PruneReason::IDLE); stream_base_stats.excess_prunes = flow_con->get_prunes(PruneReason::EXCESS); stream_base_stats.uni_prunes = flow_con->get_prunes(PruneReason::UNI); - stream_base_stats.preemptive_prunes = flow_con->get_prunes(PruneReason::PREEMPTIVE); + stream_base_stats.preemptive_prunes = flow_con->get_prunes(PruneReason::MEMCAP); stream_base_stats.memcap_prunes = flow_con->get_prunes(PruneReason::MEMCAP); stream_base_stats.ha_prunes = flow_con->get_prunes(PruneReason::HA); stream_base_stats.stale_prunes = flow_con->get_prunes(PruneReason::STALE); diff --git a/src/stream/file/file_session.cc b/src/stream/file/file_session.cc index 14a909dbf..1cfd81c7d 100644 --- a/src/stream/file/file_session.cc +++ b/src/stream/file/file_session.cc @@ -44,10 +44,10 @@ static THREAD_LOCAL ProfileStats file_ssn_stats; //------------------------------------------------------------------------- FileSession::FileSession(Flow* f) : Session(f) -{ memory::MemoryCap::update_allocations(sizeof(*this)); } +{ } FileSession::~FileSession() -{ memory::MemoryCap::update_deallocations(sizeof(*this)); } +{ } bool FileSession::setup(Packet*) { diff --git a/src/stream/icmp/icmp_session.cc b/src/stream/icmp/icmp_session.cc index 235fcf02c..8900b3cfd 100644 --- a/src/stream/icmp/icmp_session.cc +++ b/src/stream/icmp/icmp_session.cc @@ -185,10 +185,10 @@ static int ProcessIcmpUnreach(Packet* p) //------------------------------------------------------------------------- IcmpSession::IcmpSession(Flow* f) : Session(f) -{ memory::MemoryCap::update_allocations(sizeof(*this)); } +{ } IcmpSession::~IcmpSession() -{ memory::MemoryCap::update_deallocations(sizeof(*this)); } +{ } bool IcmpSession::setup(Packet*) { diff --git a/src/stream/ip/ip_defrag.cc b/src/stream/ip/ip_defrag.cc index b5c2eb5de..e2322e7c3 100644 --- a/src/stream/ip/ip_defrag.cc +++ b/src/stream/ip/ip_defrag.cc @@ -137,7 +137,6 @@ struct Fragment { delete[] fptr; ip_stats.nodes_released++; - memory::MemoryCap::update_deallocations(sizeof(*this) + flen); } uint8_t* data = nullptr; /* ptr to adjusted start position */ @@ -157,7 +156,6 @@ private: inline void init(uint16_t flen, const uint8_t* fptr, int ord) { assert(flen > 0); - memory::MemoryCap::update_allocations(sizeof(*this) + flen); this->flen = flen; this->fptr = new uint8_t[flen]; diff --git a/src/stream/ip/ip_session.cc b/src/stream/ip/ip_session.cc index 61a203987..988c25d52 100644 --- a/src/stream/ip/ip_session.cc +++ b/src/stream/ip/ip_session.cc @@ -130,10 +130,10 @@ static inline void update_session(Packet* p, Flow* lws) //------------------------------------------------------------------------- IpSession::IpSession(Flow* f) : Session(f) -{ memory::MemoryCap::update_allocations(sizeof(*this)); } +{ } IpSession::~IpSession() -{ memory::MemoryCap::update_deallocations(sizeof(*this)); } +{ } void IpSession::clear() { diff --git a/src/stream/stream.cc b/src/stream/stream.cc index 70e715500..23f820cfb 100644 --- a/src/stream/stream.cc +++ b/src/stream/stream.cc @@ -373,10 +373,12 @@ void Stream::handle_timeouts(bool idle) TcpStreamTracker::release_held_packets(cur_time, max_remove); } -void Stream::prune_flows() +bool Stream::prune_flows() { - if ( flow_con && !FlowCache::is_pruning_in_progress()) - flow_con->prune_one(PruneReason::MEMCAP, false); + if ( !flow_con ) + return false; + + return flow_con->prune_one(PruneReason::MEMCAP, false); } //------------------------------------------------------------------------- diff --git a/src/stream/stream.h b/src/stream/stream.h index e99007a92..f16b5c05d 100644 --- a/src/stream/stream.h +++ b/src/stream/stream.h @@ -77,7 +77,7 @@ public: static void purge_flows(); static void handle_timeouts(bool idle); - static void prune_flows(); + static bool prune_flows(); static bool expected_flow(Flow*, Packet*); // Looks in the flow cache for flow session with specified key and returns diff --git a/src/stream/tcp/tcp_segment_node.cc b/src/stream/tcp/tcp_segment_node.cc index 67f4b43d6..c4d9110fb 100644 --- a/src/stream/tcp/tcp_segment_node.cc +++ b/src/stream/tcp/tcp_segment_node.cc @@ -57,7 +57,6 @@ void TcpSegmentNode::clear() { TcpSegmentNode* tsn = reserved; reserved = reserved->next; - memory::MemoryCap::update_deallocations(sizeof(*tsn) + tsn->size); tcpStats.mem_in_use -= tsn->size; snort_free(tsn); } @@ -85,7 +84,6 @@ TcpSegmentNode* TcpSegmentNode::create( #endif { size_t size = sizeof(*tsn) + len; - memory::MemoryCap::update_allocations(size); tsn = (TcpSegmentNode*)snort_alloc(size); tsn->size = len; tcpStats.mem_in_use += len; @@ -124,7 +122,6 @@ void TcpSegmentNode::term() else #endif { - memory::MemoryCap::update_deallocations(sizeof(*this) + size); tcpStats.mem_in_use -= size; snort_free(this); } diff --git a/src/stream/tcp/tcp_session.cc b/src/stream/tcp/tcp_session.cc index 38133f6db..2eadf2d14 100644 --- a/src/stream/tcp/tcp_session.cc +++ b/src/stream/tcp/tcp_session.cc @@ -86,14 +86,11 @@ TcpSession::TcpSession(Flow* f) : TcpStreamSession(f) client.session = this; server.session = this; tcpStats.instantiated++; - - memory::MemoryCap::update_allocations(sizeof(*this)); } TcpSession::~TcpSession() { clear_session(true, false, false); - memory::MemoryCap::update_deallocations(sizeof(*this)); } bool TcpSession::setup(Packet*) diff --git a/src/stream/tcp/tcp_stream_tracker.cc b/src/stream/tcp/tcp_stream_tracker.cc index 1bde9321b..d370b008a 100644 --- a/src/stream/tcp/tcp_stream_tracker.cc +++ b/src/stream/tcp/tcp_stream_tracker.cc @@ -634,10 +634,6 @@ bool TcpStreamTracker::set_held_packet(Packet* p) if ( held_packet != null_iterator ) return false; - // Temporarily increase memcap until message is finalized in case - // DAQ makes a copy of the data buffer. - memory::MemoryCap::update_allocations(daq_msg_get_data_len(p->daq_msg)); - held_packet = hpq->append(p->daq_msg, p->ptrs.tcph->seq(), *this); held_pkt_seq = p->ptrs.tcph->seq(); @@ -684,7 +680,6 @@ void TcpStreamTracker::finalize_held_packet(Packet* cp) if ( held_packet != null_iterator ) { DAQ_Msg_h msg = held_packet->get_daq_msg(); - uint32_t msglen = daq_msg_get_data_len(msg); if ( cp->active->packet_was_dropped() ) { @@ -709,8 +704,6 @@ void TcpStreamTracker::finalize_held_packet(Packet* cp) tcp_session->held_packet_dir = SSN_DIR_NONE; } - memory::MemoryCap::update_deallocations(msglen); - hpq->erase(held_packet); held_packet = null_iterator; tcpStats.current_packets_held--; @@ -725,7 +718,6 @@ void TcpStreamTracker::finalize_held_packet(Flow* flow) if ( held_packet != null_iterator ) { DAQ_Msg_h msg = held_packet->get_daq_msg(); - uint32_t msglen = daq_msg_get_data_len(msg); if ( (flow->session_state & STREAM_STATE_BLOCK_PENDING) || (flow->ssn_state.session_flags & SSNFLAG_BLOCK) ) @@ -742,8 +734,6 @@ void TcpStreamTracker::finalize_held_packet(Flow* flow) tcpStats.held_packets_passed++; } - memory::MemoryCap::update_deallocations(msglen); - hpq->erase(held_packet); held_packet = null_iterator; tcpStats.current_packets_held--; diff --git a/src/stream/udp/udp_session.cc b/src/stream/udp/udp_session.cc index 1b60005a8..968fbbf6b 100644 --- a/src/stream/udp/udp_session.cc +++ b/src/stream/udp/udp_session.cc @@ -104,10 +104,10 @@ static int ProcessUdp(Flow* lwssn, Packet* p, StreamUdpConfig*) //------------------------------------------------------------------------- UdpSession::UdpSession(Flow* f) : Session(f) -{ memory::MemoryCap::update_allocations(sizeof(*this)); } +{ } UdpSession::~UdpSession() -{ memory::MemoryCap::update_deallocations(sizeof(*this)); } +{ } bool UdpSession::setup(Packet* p) { diff --git a/src/stream/user/dev_notes.txt b/src/stream/user/dev_notes.txt index e49a66f42..731fa16b3 100644 --- a/src/stream/user/dev_notes.txt +++ b/src/stream/user/dev_notes.txt @@ -1,6 +1,6 @@ This directory contains the implementation of user session tracking and processing functions. When the source for a flow provides a TCP payload, -e.g. a socket connection, then the base Stream preprocessor delegates +e.g. a socket connection, then the base Stream inspector delegates handling of the packets on that flow to this module. The StreamUser class is implemented as a subclass of Inspector and provides diff --git a/src/stream/user/user_session.cc b/src/stream/user/user_session.cc index cf53e6314..a5af59112 100644 --- a/src/stream/user/user_session.cc +++ b/src/stream/user/user_session.cc @@ -57,7 +57,6 @@ UserSegment* UserSegment::init(const uint8_t* p, unsigned n) unsigned bucket = (n > BUCKET) ? n : BUCKET; unsigned size = sizeof(UserSegment) + bucket -1; - memory::MemoryCap::update_allocations(size); UserSegment* us = (UserSegment*)snort_alloc(size); us->size = size; @@ -71,7 +70,6 @@ UserSegment* UserSegment::init(const uint8_t* p, unsigned n) void UserSegment::term(UserSegment* us) { - memory::MemoryCap::update_deallocations(us->size); snort_free(us); } @@ -423,10 +421,10 @@ void UserSession::restart(Packet* p) //------------------------------------------------------------------------- UserSession::UserSession(Flow* f) : Session(f) -{ memory::MemoryCap::update_allocations(sizeof(*this)); } +{ } UserSession::~UserSession() -{ memory::MemoryCap::update_deallocations(sizeof(*this)); } +{ } bool UserSession::setup(Packet*) { diff --git a/src/utils/stats.cc b/src/utils/stats.cc index 047ccbd0a..fafba6708 100644 --- a/src/utils/stats.cc +++ b/src/utils/stats.cc @@ -33,6 +33,7 @@ #include "helpers/process.h" #include "log/messages.h" #include "main/snort_config.h" +#include "memory/memory_cap.h" #include "managers/module_manager.h" #include "packet_io/active.h" #include "packet_io/sfdaq.h" @@ -265,6 +266,7 @@ void DropStats(ControlConn* ctrlcon) void PrintStatistics() { DropStats(); + memory::MemoryCap::print(SnortConfig::log_verbose(), false); timing_stats(); // FIXIT-L can do flag saving with RAII (much cleaner)