commit 4845da0: [Rework] Further work on deps processing

Vsevolod Stakhov vsevolod at rspamd.com
Sat Apr 30 19:21:17 UTC 2022


Author: Vsevolod Stakhov
Date: 2022-04-09 21:59:34 +0100
URL: https://github.com/rspamd/rspamd/commit/4845da03b4a111ca6066974bea92952ec0b4737c

[Rework] Further work on deps processing

---
 src/libserver/symcache/symcache_impl.cxx     | 126 +++++++++++++++++++++++++++
 src/libserver/symcache/symcache_internal.hxx |  76 +++++++++++-----
 2 files changed, 182 insertions(+), 20 deletions(-)

diff --git a/src/libserver/symcache/symcache_impl.cxx b/src/libserver/symcache/symcache_impl.cxx
index e1f3cafbb..a7afae6d9 100644
--- a/src/libserver/symcache/symcache_impl.cxx
+++ b/src/libserver/symcache/symcache_impl.cxx
@@ -408,6 +408,132 @@ auto cache_item::get_parent(const symcache &cache) const -> const cache_item *
 	return nullptr;
 }
 
+auto cache_item::process_deps(const symcache &cache) -> void
+{
+	/* Allow logging macros to work */
+	auto log_tag = [&](){ return cache.log_tag(); };
+
+	for (auto &dep : deps) {
+		msg_debug_cache ("process real dependency %s on %s", symbol.c_str(), dep.sym.c_str());
+		auto *dit = cache.get_item_by_name_mut(dep.sym, true);
+
+		if (dep.vid >= 0) {
+			/* Case of the virtual symbol that depends on another (maybe virtual) symbol */
+			const auto *vdit = cache.get_item_by_name(dep.sym, false);
+
+			if (!vdit) {
+				if (dit) {
+					msg_err_cache ("cannot add dependency from %s on %s: no dependency symbol registered",
+							dep.sym.c_str(), dit->symbol.c_str());
+				}
+			}
+			else {
+				msg_debug_cache ("process virtual dependency %s(%d) on %s(%d)", symbol.c_str(),
+						dep.vid, vdit->symbol.c_str(), vdit->id);
+
+				std::size_t nids = 0;
+
+				msg_debug_cache ("check id propagation for dependency %s from %s",
+						symbol.c_str(), dit->symbol.c_str());
+
+				const auto *ids = dit->allowed_ids.get_ids(nids);
+
+				/* TODO: merge? */
+				if (nids > 0) {
+					msg_info_cache ("propagate allowed ids from %s to %s",
+							dit->symbol.c_str(), symbol.c_str());
+
+					rspamd_symcache_set_allowed_settings_ids (cache, it->symbol, ids,
+							nids);
+				}
+
+				ids = rspamd_symcache_get_forbidden_settings_ids (cache, dit->symbol, &nids);
+
+				if (nids > 0) {
+					msg_info_cache ("propagate forbidden ids from %s to %s",
+							dit->symbol, it->symbol);
+
+					rspamd_symcache_set_forbidden_settings_ids (cache, it->symbol, ids,
+							nids);
+				}
+			}
+		}
+
+		if (dit != nullptr) {
+			if (!dit->is_filter()) {
+				/*
+				 * Check sanity:
+				 * - filters -> prefilter dependency is OK and always satisfied
+				 * - postfilter -> (filter, prefilter) dep is ok
+				 * - idempotent -> (any) dep is OK
+				 *
+				 * Otherwise, emit error
+				 * However, even if everything is fine this dep is useless ¯\_(ツ)_/¯
+				 */
+				auto ok_dep = false;
+
+				if (is_filter()) {
+					if (dit->is_filter()) {
+						ok_dep = true;
+					}
+					else if (dit->type & SYMBOL_TYPE_PREFILTER) {
+						ok_dep = true;
+					}
+				}
+				else if (type & SYMBOL_TYPE_POSTFILTER) {
+					if (dit->type & SYMBOL_TYPE_PREFILTER) {
+						ok_dep = true;
+					}
+				}
+				else if (type & SYMBOL_TYPE_IDEMPOTENT) {
+					if (dit->type & (SYMBOL_TYPE_PREFILTER|SYMBOL_TYPE_POSTFILTER)) {
+						ok_dep = true;
+					}
+				}
+				else if (type & SYMBOL_TYPE_PREFILTER) {
+					if (priority < dit->priority) {
+						/* Also OK */
+						ok_dep = true;
+					}
+				}
+
+				if (!ok_dep) {
+					msg_err_cache ("cannot add dependency from %s on %s: invalid symbol types",
+							dep.sym.c_str(), symbol.c_str());
+
+					continue;
+				}
+			}
+			else {
+				if (dit->id == id) {
+					msg_err_cache ("cannot add dependency on self: %s -> %s "
+								   "(resolved to %s)",
+							symbol.c_str(), dep.sym.c_str(), dit->symbol.c_str());
+				} else {
+					/* Create a reverse dep */
+					dit->rdeps.emplace_back(getptr(), dep.sym, id, -1);
+					dep.item = dit->getptr();
+					dep.id = dit->id;
+
+					msg_debug_cache ("add dependency from %d on %d", id,
+							dit->id);
+				}
+			}
+		}
+		else if (dep.id >= 0) {
+			msg_err_cache ("cannot find dependency on symbol %s for symbol %s",
+					dep.sym.c_str(), symbol.c_str());
+
+			return;
+		}
+
+		if (vdit) {
+			/* Use virtual symbol to propagate deps */
+			rspamd_symcache_propagate_dep (cache, it, vdit);
+		}
+	}
+}
+
 auto virtual_item::get_parent(const symcache &cache) const -> const cache_item *
 {
 	if (parent) {
diff --git a/src/libserver/symcache/symcache_internal.hxx b/src/libserver/symcache/symcache_internal.hxx
index b7e18ec1e..cb7d206b0 100644
--- a/src/libserver/symcache/symcache_internal.hxx
+++ b/src/libserver/symcache/symcache_internal.hxx
@@ -25,6 +25,7 @@
 #include <cmath>
 #include <cstdlib>
 #include <cstdint>
+#include <utility>
 #include <vector>
 #include <string>
 #include <string_view>
@@ -36,19 +37,19 @@
 #include "lua/lua_common.h"
 
 #define msg_err_cache(...) rspamd_default_log_function (G_LOG_LEVEL_CRITICAL, \
-        static_pool->tag.tagname, cfg->checksum, \
+        "symcache", log_tag(), \
         RSPAMD_LOG_FUNC, \
         __VA_ARGS__)
 #define msg_warn_cache(...)   rspamd_default_log_function (G_LOG_LEVEL_WARNING, \
-        static_pool->tag.tagname, cfg->checksum, \
+        "symcache", log_tag(), \
         RSPAMD_LOG_FUNC, \
         __VA_ARGS__)
 #define msg_info_cache(...)   rspamd_default_log_function (G_LOG_LEVEL_INFO, \
-        static_pool->tag.tagname, cfg->checksum, \
+        "symcache", log_tag(), \
         RSPAMD_LOG_FUNC, \
         __VA_ARGS__)
 #define msg_debug_cache(...)  rspamd_conditional_debug_fast (NULL, NULL, \
-        rspamd_symcache_log_id, "symcache", cfg->checksum, \
+        rspamd_symcache_log_id, "symcache", log_tag(), \
         RSPAMD_LOG_FUNC, \
         __VA_ARGS__)
 #define msg_debug_cache_task(...)  rspamd_conditional_debug_fast (NULL, NULL, \
@@ -171,6 +172,8 @@ struct id_list {
 			}
 		}
 	}
+
+
 };
 
 class symcache;
@@ -222,14 +225,18 @@ struct cache_dependency {
 	std::string sym; /* Symbolic dep name */
 	int id; /* Real from */
 	int vid; /* Virtual from */
+public:
+	/* Default piecewise constructor */
+	cache_dependency(cache_item_ptr _item, std::string _sym, int _id, int _vid) :
+		item(std::move(_item)), sym(std::move(_sym)), id(_id), vid(_vid) {}
 };
 
-struct cache_item {
+struct cache_item : std::enable_shared_from_this<cache_item> {
 	/* This block is likely shared */
 	struct rspamd_symcache_item_stat *st;
 	struct rspamd_counter_data *cd;
 
-	std::uint64_t last_count;
+	std::uint64_t last_count = 0;
 	std::string symbol;
 	std::string_view type_descr;
 	int type;
@@ -238,16 +245,16 @@ struct cache_item {
 	std::variant<normal_item, virtual_item> specific;
 
 	/* Condition of execution */
-	bool enabled;
+	bool enabled = true;
 
 	/* Priority */
-	int priority;
+	int priority = 0;
 	/* Topological order */
-	unsigned int order;
+	unsigned int order = 0;
 	/* Unique id - counter */
-	int id;
+	int id = 0;
 
-	int frequency_peaks;
+	int frequency_peaks = 0;
 	/* Settings ids */
 	id_list allowed_ids;
 	/* Allows execution but not symbols insertion */
@@ -259,7 +266,33 @@ struct cache_item {
 	/* Reverse dependencies */
 	std::vector<cache_item_ptr> rdeps;
 
+public:
+	[[nodiscard]] static auto create() -> cache_item_ptr {
+		return std::shared_ptr<cache_item>(new cache_item());
+	}
+	/**
+	 * Share ownership on the item
+	 * @return
+	 */
+	auto getptr() -> cache_item_ptr {
+		return shared_from_this();
+	}
+	/**
+	 * Process and resolve dependencies for the item
+	 * @param cache
+	 */
+	auto process_deps(const symcache &cache) -> void;
 	auto is_virtual() const -> bool { return std::holds_alternative<virtual_item>(specific); }
+	auto is_filter() const -> bool {
+		static constexpr const auto forbidden_flags = SYMBOL_TYPE_PREFILTER|
+				SYMBOL_TYPE_COMPOSITE|
+				SYMBOL_TYPE_CONNFILTER|
+				SYMBOL_TYPE_POSTFILTER|
+				SYMBOL_TYPE_IDEMPOTENT;
+
+		return std::holds_alternative<normal_item>(specific) &&
+		        (type & forbidden_flags) == 0;
+	}
 	auto get_parent(const symcache &cache) const -> const cache_item *;
 	auto add_condition(lua_State *L, int cbref) -> bool {
 		if (!is_virtual()) {
@@ -271,6 +304,9 @@ struct cache_item {
 
 		return false;
 	}
+
+private:
+	cache_item() = default;
 };
 
 struct delayed_cache_dependency {
@@ -377,11 +413,19 @@ public:
 	 */
 	auto add_dependency(int id_from, std::string_view to, int virtual_id_from) -> void;
 
-	/*
+	/**
 	 * Initialises the symbols cache, must be called after all symbols are added
 	 * and the config file is loaded
 	 */
 	auto init() -> bool;
+
+	/**
+	 * Log helper that returns cfg checksum
+	 * @return
+	 */
+	auto log_tag() const -> const char* {
+		return cfg->checksum;
+	}
 };
 
 /*
@@ -396,14 +440,6 @@ struct cache_dynamic_item {
 	std::uint32_t async_events;
 };
 
-
-struct cache_dependency {
-	cache_item_ptr item; /* Owning pointer to the real dep */
-	std::string_view sym; /* Symbolic dep name */
-	int id; /* Real from */
-	int vid; /* Virtual from */
-};
-
 struct cache_savepoint {
 	unsigned order_gen;
 	unsigned items_inflight;


More information about the Commits mailing list