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