Timeouts for states #231
Merged
Timeouts for states #231
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ol-imorozko
force-pushed
the
timeouts-for-states
branch
from
d484f2f
to
3d0d6d3
Compare
ol-imorozko
force-pushed
the
timeouts-for-states
branch
2 times, most recently
from
a378c58
to
5abcb27
Compare
ol-imorozko
force-pushed
the
timeouts-for-states
branch
7 times, most recently
from
c95aa37
to
f907835
Compare
ol-imorozko
force-pushed
the
timeouts-for-states
branch
3 times, most recently
from
45d1b4a
to
972ba95
Compare
Merged
ol-imorozko
force-pushed
the
timeouts-for-states
branch
from
95397a0
to
a0c4833
Compare
Makes it easier to read and avoid writing decltype<Actions::raw_action> where we need such type
This commit introduces parsing for the state-timeout rule. The tests and the actual semantics will be implemented later. Note that the current implementation does not properly manage StateTimeout yet, as we are only interested in the last action for performance optimization. Storing only the last state-timeout action reduces the number of actions processed in the dataplane, enhancing performance. In general, we can optimize performance for actions that only need to be executed once. For instance, only the first execution of CheckStateAction matters, as we cannot add new states while processing the current packet actions. Similarly, only the last StateTimeoutAction is relevant, as it provides the most accurate timeout value. By storing only the relevant action and tracking its index during the "compile" step, we can use this information to optimize performance. For example, instead of storing a StateTimeoutAction, we could directly add the timeout field to the FlowAction. The next commit will propose a generic way to handle this optimization across various action types.
Adds simple structures to make the folliwing VarinatTraitMap implementation easier
Introduce the `VariantTraitMap`, a generic helper that maps types within a
`std::variant` to a value (e.g., `std::optional<size_t>`) based on a given trait.
Why this is needed:
Some actions, (those with `MAX_COUNT=1`), only need to be executed once or
only the first/last occurrence of them matters. For example:
- CheckStateAction:
Only the first occurrence of this action is meaningful.
- StateTimeoutAction:
Only the last occurrence of this action should be stored and processed,
as it provides the most accurate or latest information.
To optimize contolplane, we can store only one of those actions with their index.
This way, on "compile" step in value_t we can do something with this information
to increase contolplane performance. For example, compeletly ditch the
StateTimeoutAction and just add the timeout to the FlowAction as a field.
Therefore, to avoid writing boilerplate code like
`std::optional<size_t> check_state_index, state_timeout_index`, we can
use VariantTraitMap to directly map Actions with MAX_COUNT=1 to their index.
This commit introduces a new common/utils.h file to store small utility functions, as such a file didn't previously exist. It also adds the always_false trait, which is a trait that evaluates to false for any type. This trait is useful for triggering static assertions in order to identify the exact type that caused a compilation error.
This commit refactors the way actions with `MAX_COUNT=1` are tracked by using `VariantTraitMap`. The change enables storing the index of the first or last occurrence of specific actions, like `CheckStateAction` or `StateTimeoutAction`, in a more generic way. Previously, actions like `check_state_index` and `state_timeout_index` were handled manually with specific member variables. Now, these actions are handled through a type-safe mechanism based on traits, allowing easy extension for other actions in the future. This improves control plane performance by reducing redundant actions, as only the relevant instances are processed and stored.
This is a representation of a processed action, but not quite ready for use in dataplane yet. Here we build the actions for the packet, and on "compile" step we will shape the final form of a structure, hence Intermediate name
These are helpful methods that will reduce code duplication later
Refactored the compile method by splitting it into two functions
Remove StateTimeoutAction to improve performance of dataplane, since now it wont have to execute this action. By tracking only the last timeout and adding its value directly to FlowAction we can just fill the required timeout on FlowAction execution
acl_touch_state is used only in acl_create_state and acl_checkstate. The former creates the state, while the latter attempts to match a packet with it. During state creation, we need to fill the `value` with the appropriate timeout, which is initialized to zero by default. However, in acl_checkstate, we still need to "touch" the state, ensuring that the last_seen field is updated with the current time. The issue with the current code is that it unnecessarily fills the state_timeout value every time, even though this value doesn't change once set. It is defined during state creation and later used to check for timeouts, but we don't modify it afterward. Recalculating it each time adds extra complexity/branches to the dataplane without any benefit. This commit removes that behavior.
Modifies `acl_create_state` to accept an optional timeout value. If provided, this value is used to set the state's timeout. If not, get timeout as before.
Verify the correct behavior when moving state-timeout timeout to flow action. Checks that only the last state timeout is stored in the flow action.
Some tests were missing clearFWState, which resulted in other tests wrong behaviour due to the presence of states from previous runs
ol-imorozko
force-pushed
the
timeouts-for-states
branch
from
a0c4833
to
8483781
Compare
GeorgyKirichenko
approved these changes
Sep 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.