Network batching - #279
Merged
Merged
Conversation
MykolaSuperman
force-pushed
the
network-batching
branch
2 times, most recently
from
July 24, 2026 10:54
48c3a6a to
6ff5d9b
Compare
al1img
reviewed
Jul 30, 2026
| if(NOT EXISTS ${AOS_CORE_LIB_DIR}) | ||
| FetchContent_Populate( | ||
| aos_core_lib_cpp | ||
| GIT_REPOSITORY https://github.com/aosedge/aos_core_lib_cpp.git |
Collaborator
There was a problem hiding this comment.
Do not forget to remove this commit.
| { | ||
| LOG_DBG() << "Abort firewall batch"; | ||
|
|
||
| std::lock_guard<std::mutex> lock {mBatchMutex}; |
| std::vector<std::string> failedInstances; | ||
|
|
||
| { | ||
| std::lock_guard<std::mutex> lock {mBatchMutex}; |
mykola-kobets-epam
approved these changes
Jul 30, 2026
mykola-kobets-epam
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
MykolaSuperman
force-pushed
the
network-batching
branch
2 times, most recently
from
August 3, 2026 14:54
4da5b08 to
bc76a46
Compare
mlohvynenko
approved these changes
Aug 4, 2026
mlohvynenko
left a comment
Member
There was a problem hiding this comment.
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
al1img
approved these changes
Aug 4, 2026
al1img
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
| // per-instance state we added. Nothing to do if the table is already gone. | ||
| if (auto err = mBackend->ListChainRules(mTable, cForwardChain, forwardRules); !err.IsNone()) { | ||
| { | ||
| std::lock_guard<std::mutex> lock {mBatchMutex}; |
Author
There was a problem hiding this comment.
it was done in separate commit
| } | ||
|
|
||
| { | ||
| std::lock_guard<std::mutex> lock {mBatchMutex}; |
Author
There was a problem hiding this comment.
it was done in separate commit
added 2 commits
August 4, 2026 11:27
Add a Commit overload that echoes the batch (NFT_CTX_OUTPUT_ECHO) and returns the kernel-assigned handles of the rules it added, parsed from the same "# handle N" output ListChainRules already relies on. This lets callers record handles at creation time and delete those rules later by handle, without re-listing the chain. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
The root TBF and ingress qdiscs sit on the host veth and are destroyed with it when the teardown path removes the interface (veth / namespace deletion), so Clear no longer issues the two redundant qdisc deletes and removes only the separate IFB netdev (which does not die with the veth). Cuts the per-instance teardown from three tc/netlink round-trips to one. Tests to be updated separately. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
MykolaSuperman
force-pushed
the
network-batching
branch
from
August 4, 2026 08:27
d224421 to
bc67cac
Compare
added 18 commits
August 4, 2026 12:06
Implement BeginTransaction/CommitTransaction/RollbackTransaction on the SM database using the Poco session begin/commit/rollback, so a burst of writes (e.g. a mass instance start or teardown) commits in a single fsync instead of one per statement and is discarded as a whole when the batch fails. Each call is a no-op when it does not match the session transaction state. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Implement BeginBatch/FlushBatch/Revert. In batch mode AddInstance and RemoveInstance stage their nft operations into a single shared transaction (serialized by a mutex, since the launcher stages from its worker pool) instead of committing per instance; FlushBatch commits the lot in one nft round-trip and keeps the handles it reported. Revert deletes those handles from the forward chain and drops the instance chains the batch created, so the caller can fall back to a per-instance apply. Outside batch mode AddInstance/RemoveInstance commit immediately as before. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Mirror the firewall batch primitives. StartInstanceMonitoring and StopInstanceMonitoring stage their nft operations into a single shared transaction while a batch is open (guarded by its own mutex, the launcher stages concurrently); FlushBatch commits them in one nft round-trip and keeps the reported handles. Revert deletes those handles from the forward chain, drops the counter chains the batch created and discards their monitoring state without persisting it, so a per-instance retry starts from scratch. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
The veth's root and ingress qdiscs die with the interface, so Clear only removes the IFB device: drop the DelRootTBFQDisc/DelIngressQDisc expectations from the Clear tests and make the failure case fail on the IFB delete. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Cover the batch primitives: AddInstance/RemoveInstance stage into a single transaction that is committed once by FlushBatch, the immediate per-instance commit is back after the flush, Revert deletes the flushed handles and the chains the batch created, and a failed flush leaves nothing to revert. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Mirror the firewall batch coverage: start and stop stage into a single transaction committed once by FlushBatch, Revert deletes the flushed handles and the counter chains the batch created without persisting their counters, and the reverted instance is unknown again so a per-instance retry really re-applies instead of returning early. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Cover the transaction primitives against the real SQLite database: writes staged in a transaction survive CommitTransaction, are discarded by RollbackTransaction, and commit/rollback outside a transaction are no-ops that leave later writes working. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Implement AbortBatch: leave batch mode and drop the staged transaction, which discards everything queued in it without touching the kernel. The recorded batch chains and flushed handles go with it, so a later Revert has nothing to undo. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Implement AbortBatch: leave batch mode and drop the staged transaction without touching the kernel. StartInstanceMonitoring registers the instance even while staging and returns early for an already known instance, so the abort must also drop that state or the per-instance retry would report success without applying anything. Revert shares the same state-clearing helper. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
A failed batch commit left the staged instances registered in mInstanceChains, so the per-instance retry that follows returned early and the traffic monitor was silently never applied for them. Drop the staged monitoring state on the failure path too, the same way AbortBatch and Revert already do. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Stage an instance in a batch whose commit fails, then assert the per-instance retry builds and commits its own transaction instead of returning early for an instance the failed batch left registered. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Add a Commit overload that echoes the batch and parses the added rules (like ListChainRules), so callers can attribute added jumps to their target chains when many instances are committed together. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
AddInstance/UpdateInstance and the batch flush now record the two forward-chain jump handles per instance, and RemoveInstance deletes them directly by handle instead of listing and scanning the whole forward chain. This turns a mass teardown from O(N^2) into O(N). It falls back to the chain scan when handles are not tracked (e.g. after an SM restart). Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
… handle The batch flush records the two forward-chain jump handles per instance; StopInstanceMonitoring deletes them directly by handle instead of scanning the forward chain, turning a mass teardown from O(N^2) into O(N). It falls back to the chain scan when handles are not tracked. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
…ardown Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
ParseRuleLine constructed a dozen std::regex objects on every call, so parsing the echoed rules of a mass batch commit recompiled thousands of regexes and dominated the flush. Make them static const so each is compiled once, restoring the mass-start flush time. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
The batch Commit overload only needs the added forward-chain jumps to map handles to their target chains, so parse just the jump target and handle per line instead of running the full rule parser over every added rule. This restores the mass-start flush time. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Rely on C++17 class template argument deduction for std::lock_guard on the batch mutex instead of spelling out std::lock_guard<std::mutex>. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
MykolaSuperman
force-pushed
the
network-batching
branch
from
August 4, 2026 09:26
bc67cac to
34a49f6
Compare
|
This file contains hidden or 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
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.