Acl improvements

common/actions.h

@@ -22,13 +22,13 @@ class dump_t
 	        dump_tag(std::move(dump_tag))
 	{}
 
-	inline bool operator==(const dump_t& o) const
+	bool operator==(const dump_t& o) const
 	{
 		return std::tie(dump_id, dump_tag) ==
 		       std::tie(o.dump_id, o.dump_tag);
 	}
 
-	inline bool operator!=(const dump_t& o) const
+	bool operator!=(const dump_t& o) const
 	{
 		return !operator==(o);
 	}
@@ -119,8 +119,6 @@ struct state_timeout_t
 
 /**
  * @brief Represents an action that dumps packets to a specified ring.
- *
- * Only one DumpAction is allowed in an Actions object.
  */
 struct DumpAction final
 {
@@ -207,8 +205,6 @@ struct FlowAction final
  *
  * This class doesn't have any specific info to store,
  * because check-state rule doesn't need anything.
- *
- * Only one CheckStateAction is allowed in an Actions object.
  */
 struct CheckStateAction final
 {
@@ -554,48 +550,60 @@ struct IntermediateActions
 
 } // namespace acl
 
-template<bool HasCheckState>
+enum class ActionsPath
+{
+	Default,
+	WithCheckState
+};
+
+template<ActionsPath>
 class BaseActions;
 
 template<>
-class BaseActions<false>
+class BaseActions<ActionsPath::Default>
 {
-private:
+protected:
 	std::vector<Action> path_{};
 
 public:
 	BaseActions() = default;
 
 	BaseActions(acl::IntermediateActions&& actions) :
-	        path_(std::move(actions.path)) {}
+	        path_(std::move(actions.path))
+	{
+		if (path_.empty())
+		{
+			YANET_THROW("Path cannot be empty");
+		}
+		if (!std::holds_alternative<FlowAction>(default_path_last_raw_action()))
+		{
+			YANET_THROW("Last action in the default path must be a FlowAction");
+		}
+	}
 
-	[[nodiscard]] const Action& get_last() const
+	[[nodiscard]] const std::vector<Action>& default_path() const
 	{
-		assert(!path_.empty());
-		return path_.back();
+		return path_;
 	}
 
-	Action& get_last()
+	[[nodiscard]] size_t default_path_size() const
 	{
-		assert(!path_.empty());
-		return path_.back();
+		return path_.size();
 	}
 
-	[[nodiscard]] const std::vector<Action>& get_actions() const
+	[[nodiscard]] const RawAction& default_path_raw_action(size_t idx) const
 	{
-		return path_;
+		return path_[idx].raw_action;
 	}
 
-	[[nodiscard]] const common::globalBase::tFlow& get_flow() const
+	[[nodiscard]] const RawAction& default_path_last_raw_action() const
 	{
-		assert(std::holds_alternative<FlowAction>(get_last().raw_action));
-		return std::get<FlowAction>(get_last().raw_action).flow;
+		return path_.back().raw_action;
 	}
 
-	[[nodiscard]] common::globalBase::tFlow& get_flow()
+	[[nodiscard]] const common::globalBase::tFlow& get_flow() const
 	{
-		assert(std::holds_alternative<FlowAction>(get_last().raw_action));
-		return std::get<FlowAction>(get_last().raw_action).flow;
+		return std::get<FlowAction>(default_path_last_raw_action()).flow;
 	}
 
 	bool operator<(const BaseActions& second) const
@@ -615,18 +623,22 @@ class BaseActions<false>
 };
 
 template<>
-class BaseActions<true>
+class BaseActions<ActionsPath::WithCheckState> : public BaseActions<ActionsPath::Default>
 {
 private:
-	std::vector<Action> path_{};
 	// TODO: This is a prefix of a path_, in C++-20 I would use std::span to avoid extra copying
 	std::vector<Action> check_state_path_{};
 
 public:
 	BaseActions() = default;
+
 	BaseActions(acl::IntermediateActions&& actions)
 	{
-		assert(actions.indices.get<common::CheckStateAction>().has_value());
+		if (!actions.indices.get<common::CheckStateAction>().has_value())
+		{
+			YANET_THROW("Check-state index should be provided");
+		}
+
 		auto check_state_index = actions.indices.get<common::CheckStateAction>().value();
 
 		path_ = std::move(actions.path);
@@ -638,43 +650,24 @@ class BaseActions<true>
 		path_.erase(path_.begin() + check_state_index);
 	}
 
-	[[nodiscard]] const std::vector<Action>& get_actions() const
-	{
-		return path_;
-	}
-
-	[[nodiscard]] const std::vector<Action>& get_check_state_actions() const
+	[[nodiscard]] const std::vector<Action>& check_state_path() const
 	{
 		return check_state_path_;
 	}
 
-	[[nodiscard]] const Action& get_last() const
+	[[nodiscard]] size_t check_state_path_size() const
 	{
-		assert(!path_.empty());
-		return path_.back();
+		return check_state_path_.size();
 	}
 
-	Action& get_last()
+	[[nodiscard]] const RawAction& check_state_path_raw_action(size_t idx) const
 	{
-		assert(!path_.empty());
-		return path_.back();
+		return check_state_path_[idx].raw_action;
 	}
 
-	[[nodiscard]] const common::globalBase::tFlow& get_flow() const
+	[[nodiscard]] const RawAction& check_state_path_last_raw_action() const
 	{
-		assert(std::holds_alternative<FlowAction>(get_last().raw_action));
-		return std::get<FlowAction>(get_last().raw_action).flow;
-	}
-
-	[[nodiscard]] common::globalBase::tFlow& get_flow()
-	{
-		assert(std::holds_alternative<FlowAction>(get_last().raw_action));
-		return std::get<FlowAction>(get_last().raw_action).flow;
-	}
-
-	bool operator<(const BaseActions& second) const
-	{
-		return get_flow() < second.get_flow();
+		return check_state_path_.back().raw_action;
 	}
 
 	void pop(stream_in_t& stream)
@@ -692,8 +685,8 @@ class BaseActions<true>
 
 /**
  * The Actions type is defined as a std::variant to efficiently handle two possible states of action sequences:
- * - BaseActions<true>: This specialization is used when the action sequence contains a check-state action.
- * - BaseActions<false>: This specialization is used when the action sequence does not contain a check-state action.
+ * - BaseActions<WithCheckState>: This specialization is used when the action sequence contains a check-state action.
+ * - BaseActions<Default>: This specialization is used when the action sequence does not contain a check-state action.
  *
  * This approach allows us to avoid runtime branching to check for the presence of a check-state action, thereby
  * enhancing performance. Instead, the decision is made once when constructing the Actions object.
@@ -704,6 +697,6 @@ class BaseActions<true>
  * that will be only once. Once the result of a check-state is determined, we will choose correct path and execute it
  * without any additional checks.
  */
-using Actions = std::variant<BaseActions<true>, BaseActions<false>>;
+using Actions = std::variant<BaseActions<ActionsPath::Default>, BaseActions<ActionsPath::WithCheckState>>;
 
 } // namespace common

common/define.h

@@ -1,10 +1,12 @@
 #pragma once
 
 #include <cstdio>
+#include <cstdlib>
 #include <ctime>
 #include <map>
 #include <optional>
 #include <set>
+#include <string_view>
 #include <vector>
 
 #include <inttypes.h>
@@ -90,13 +92,13 @@ extern LogPriority logPriority;
 #define CALCULATE_LOGICALPORT_ID(portId, vlanId) ((portId << 13) | ((vlanId & 0xFFF) << 1) | 1)
 
 #if __cpp_exceptions
-#define YANET_THROW(string) throw string
+#define YANET_THROW(message) throw std::runtime_error(std::string(message))
 #else // __cpp_exceptions
-#define YANET_THROW(string)                             \
-	do                                              \
-	{                                               \
-		YANET_LOG_ERROR("%s\n", string.data()); \
-		std::abort();                           \
+#define YANET_THROW(message)                                               \
+	do                                                                 \
+	{                                                                  \
+		YANET_LOG_ERROR("%s\n", std::string_view(message).data()); \
+		std::abort();                                              \
 	} while (0)
 #endif // __cpp_exceptions
 

controlplane/acl.cpp

@@ -464,56 +464,33 @@ static bool unwind(int64_t start_from, firewall_rules_t& fw, const dispatcher_ru
 			ids.insert(ids.end(), rule.ids.begin(), rule.ids.end());
 
 			ACL_DBGMSG("advancing further...");
-			if (std::holds_alternative<int64_t>(rule.action))
-			{
-				// handle skipto && allow action
-				start_from = std::get<int64_t>(rule.action);
-				if (start_from != DISPATCHER)
+			std::visit([&](const auto& action) {
+				using T = std::decay_t<decltype(action)>;
+
+				if constexpr (std::is_same_v<T, int64_t>)
 				{
-					ACL_DBGMSG("skipto " << start_from);
-					term_rule = unwind(start_from, fw, dispatcher, result_filter, iface, ids, rules, log || rule.log, recursion_limit + 1);
-					// if we have reached DISPATCHER, it will be handled next
+					// handle skipto && allow action
+					start_from = action;
+					if (start_from != DISPATCHER)
+					{
+						ACL_DBGMSG("skipto " << start_from);
+						term_rule = unwind(start_from, fw, dispatcher, result_filter, iface, ids, rules, log || rule.log, recursion_limit + 1);
+						// if we have reached DISPATCHER, it will be handled next
+					}
+					if (start_from == DISPATCHER)
+					{
+						ACL_DBGMSG("go to dispatcher...");
+						term_rule = unwind_dispatcher(dispatcher, result_filter, iface, ids, rules, log || rule.log);
+					}
 				}
-
-				if (start_from == DISPATCHER)
+				else
 				{
-					ACL_DBGMSG("go to dispatcher...");
-					term_rule = unwind_dispatcher(dispatcher, result_filter, iface, ids, rules, log || rule.log);
+					// Handle other types by emplacing them directly into rules
+					rules.emplace_back(std::move(result_filter), action, ids, log || rule.log);
+					ACL_DBGMSG(typeid(T).name() << " gathered...");
 				}
-			}
-			else if (std::holds_alternative<common::globalBase::tFlow>(rule.action))
-			{
-				// handle tFlows
-				rules.emplace_back(std::move(result_filter),
-				                   std::get<common::globalBase::tFlow>(rule.action),
-				                   ids,
-				                   log || rule.log);
-				ACL_DBGMSG("tFlow gathered...");
-			}
-			else if (std::holds_alternative<common::acl::check_state_t>(rule.action))
-			{
-				rules.emplace_back(std::move(result_filter),
-				                   std::get<common::acl::check_state_t>(rule.action),
-				                   ids,
-				                   log || rule.log);
-				ACL_DBGMSG("check_state gathered...");
-			}
-			else if (std::holds_alternative<common::acl::state_timeout_t>(rule.action))
-			{
-				rules.emplace_back(std::move(result_filter),
-				                   std::get<common::acl::state_timeout_t>(rule.action),
-				                   ids,
-				                   log || rule.log);
-				ACL_DBGMSG("state_timeout gathered...");
-			}
-			else
-			{
-				rules.emplace_back(std::move(result_filter),
-				                   std::get<common::acl::dump_t>(rule.action),
-				                   ids,
-				                   log || rule.log);
-				ACL_DBGMSG("dump_t gathered...");
-			}
+			},
+			           rule.action);
 
 			ids.resize(idSize);
 			if (is_term_filter(rule.filter) && (rule.is_term() || rule.is_skipto()))