From f73596e01cf2e717ef592438c70e9ee05e44a145 Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Thu, 2 Jul 2026 16:13:13 +0300 Subject: [PATCH 01/16] sm: networkmanager: reap instance veth via namespace teardown, not sync detach Stop no longer deletes the host veth synchronously. DeleteNetworkNamespace drops the instance netns (lazy MNT_DETACH umount); the kernel then reaps the peer veth - and the host end with it, since they die as a pair - asynchronously via cleanup_net, off the critical stop path. The synchronous per-instance delete blocked on a kernel RCU grace period for every instance, serialized under rtnl_lock (O(N) on mass teardown): measured ~2.5s to delete 200 host veths one-by-one vs ~0.25s for the lazy namespace teardown (which also lets cleanup_net batch the unregister). No leak: the interfaces are gone once the namespace is reaped. Detach is kept for the AddInstanceToNetwork rollback path, where the namespace may not yet own the peer. Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- src/core/sm/networkmanager/networkmanager.cpp | 20 +++++++------------ .../networkmanager/tests/networkmanager.cpp | 9 ++++----- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/core/sm/networkmanager/networkmanager.cpp b/src/core/sm/networkmanager/networkmanager.cpp index f9863b38a..552f22612 100644 --- a/src/core/sm/networkmanager/networkmanager.cpp +++ b/src/core/sm/networkmanager/networkmanager.cpp @@ -846,19 +846,12 @@ Error NetworkManager::EnsureNodeNetworkPhysical(const String& networkID) Error NetworkManager::DeleteInstanceNetworkConfig(const String& instanceID, const String& networkID) { - StaticString bridgeIfName; StaticString hostIfName; DNSServerItf* dnsServer = nullptr; { LockGuard lock {mMutex}; - if (auto it = mNetworkProviders.Find(networkID); it != mNetworkProviders.end()) { - bridgeIfName = it->mSecond.mBridgeIfName; - } else { - LOG_WRN() << "Network provider not found for cleanup" << Log::Field("networkID", networkID); - } - if (auto it = mDNSServers.Find(networkID); it != mDNSServers.end()) { dnsServer = it->mSecond; } @@ -890,12 +883,13 @@ Error NetworkManager::DeleteInstanceNetworkConfig(const String& instanceID, cons err = AOS_ERROR_WRAP(errRemove); } - if (!bridgeIfName.IsEmpty()) { - if (auto errDetach = mBridgeNetwork->Detach(instanceID, bridgeIfName); - !errDetach.IsNone() && err.IsNone()) { - err = AOS_ERROR_WRAP(errDetach); - } - } + // The host veth is intentionally NOT detached here. DeleteNetworkNamespace + // below drops the instance netns (lazy umount); the kernel then reaps the + // peer veth - and with it the host end, since they die as a pair - + // asynchronously via cleanup_net, off the critical stop path. A synchronous + // delete here would block on a per-device RCU grace period for every + // instance (O(N) rtnl_lock serialization on mass teardown) for no benefit, + // as the namespace teardown already removes the interface. } else { LOG_DBG() << "Instance was never started, skipping itf cleanup" << Log::Field("instanceID", instanceID); } diff --git a/src/core/sm/networkmanager/tests/networkmanager.cpp b/src/core/sm/networkmanager/tests/networkmanager.cpp index 382f351f5..61296ef37 100644 --- a/src/core/sm/networkmanager/tests/networkmanager.cpp +++ b/src/core/sm/networkmanager/tests/networkmanager.cpp @@ -294,7 +294,8 @@ class NetworkManagerTest : public Test { EXPECT_CALL(mDNSServer, RemoveHost(aos::String("leftover-instance"))).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mBandwidth, Clear(_)).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mFirewall, RemoveInstance(_)).WillOnce(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mBridgeNetwork, Detach(_, _)).WillOnce(Return(aos::ErrorEnum::eNone)); + // The host veth is no longer detached synchronously on cleanup; the instance + // netns teardown reaps it asynchronously. EXPECT_CALL(mNetns, DeleteNetworkNamespace(_)).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mStorage, UpdateInstanceNetworkInfo(_)).WillOnce(Return(aos::ErrorEnum::eNone)); } @@ -325,7 +326,6 @@ class NetworkManagerTest : public Test { EXPECT_CALL(mDNSServer, RemoveHost(_)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mBandwidth, Clear(_)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mFirewall, RemoveInstance(_)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mBridgeNetwork, Detach(_, _)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone)); } StrictMock mStorage; @@ -896,7 +896,7 @@ TEST_F(NetworkManagerTest, StopReleaseAndRecreateInstance) ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID, networkID), aos::ErrorEnum::eNone); } -TEST_F(NetworkManagerTest, StopInstanceNetwork_FailOnDetachError) +TEST_F(NetworkManagerTest, StopInstanceNetwork_FailOnFirewallRemoveError) { const aos::String instanceID = "test-instance"; const aos::String networkID = "test-network"; @@ -929,8 +929,7 @@ TEST_F(NetworkManagerTest, StopInstanceNetwork_FailOnDetachError) EXPECT_CALL(mTrafficMonitor, StopInstanceMonitoring(instanceID)).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mDNSServer, RemoveHost(_)).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mBandwidth, Clear(_)).WillOnce(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mFirewall, RemoveInstance(_)).WillOnce(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mBridgeNetwork, Detach(_, _)).WillOnce(Return(aos::ErrorEnum::eRuntime)); + EXPECT_CALL(mFirewall, RemoveInstance(_)).WillOnce(Return(aos::ErrorEnum::eRuntime)); EXPECT_CALL(mNetns, DeleteNetworkNamespace(_)).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mNetIf, DeleteLink(_)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); From d7f7657957635b85e1cc9ea5635380813b6b508d Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Thu, 2 Jul 2026 20:24:39 +0300 Subject: [PATCH 02/16] sm: networkmanager: skip bandwidth clear when no shaping was applied DeleteInstanceNetworkConfig only calls Bandwidth::Clear when the instance config actually had a non-zero ingress/egress limit. For unlimited instances (the common case) nothing was installed, so the previous unconditional Clear just wasted three tc round-trips (serialized on rtnl_lock) per instance on a mass teardown. The decision uses the config already held in the network manager, so no extra state is kept in the bandwidth backend. Tests to be updated separately. Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- src/core/sm/networkmanager/networkmanager.cpp | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/core/sm/networkmanager/networkmanager.cpp b/src/core/sm/networkmanager/networkmanager.cpp index 552f22612..941b57bb3 100644 --- a/src/core/sm/networkmanager/networkmanager.cpp +++ b/src/core/sm/networkmanager/networkmanager.cpp @@ -720,13 +720,16 @@ Error NetworkManager::AddInstanceToNetwork(const String& instanceID, const Strin return AOS_ERROR_WRAP(err); } - auto cleanupBandwidth = DeferRelease(&attachResult.mHostIfName, [this, &err](const String* ifName) { - if (!err.IsNone()) { - if (auto errClear = mBandwidth->Clear(*ifName); !errClear.IsNone()) { - LOG_ERR() << "Failed to clear bandwidth" << Log::Field("ifName", *ifName) << Log::Field(errClear); - } - } - }); + const bool bandwidthApplied = bandwidthParams->mIngressRate > 0 || bandwidthParams->mEgressRate > 0; + + auto cleanupBandwidth + = DeferRelease(&attachResult.mHostIfName, [this, &err, bandwidthApplied](const String* ifName) { + if (!err.IsNone() && bandwidthApplied) { + if (auto errClear = mBandwidth->Clear(*ifName); !errClear.IsNone()) { + LOG_ERR() << "Failed to clear bandwidth" << Log::Field("ifName", *ifName) << Log::Field(errClear); + } + } + }); DNSServerItf* dnsServer = nullptr; @@ -847,7 +850,8 @@ Error NetworkManager::EnsureNodeNetworkPhysical(const String& networkID) Error NetworkManager::DeleteInstanceNetworkConfig(const String& instanceID, const String& networkID) { StaticString hostIfName; - DNSServerItf* dnsServer = nullptr; + DNSServerItf* dnsServer = nullptr; + bool hasBandwidth = false; { LockGuard lock {mMutex}; @@ -857,7 +861,8 @@ Error NetworkManager::DeleteInstanceNetworkConfig(const String& instanceID, cons } if (auto it = mInstanceNetworkInfos.Find(instanceID); it != mInstanceNetworkInfos.end()) { - hostIfName = it->mSecond.mHostIfName; + hostIfName = it->mSecond.mHostIfName; + hasBandwidth = it->mSecond.mNetworkConfig.mIngressKbit > 0 || it->mSecond.mNetworkConfig.mEgressKbit > 0; } else { LOG_WRN() << "Instance network info not found for cleanup" << Log::Field("instanceID", instanceID); } @@ -875,8 +880,10 @@ Error NetworkManager::DeleteInstanceNetworkConfig(const String& instanceID, cons << Log::Field("networkID", networkID); } - if (auto errClear = mBandwidth->Clear(hostIfName); !errClear.IsNone() && err.IsNone()) { - err = AOS_ERROR_WRAP(errClear); + if (hasBandwidth) { + if (auto errClear = mBandwidth->Clear(hostIfName); !errClear.IsNone() && err.IsNone()) { + err = AOS_ERROR_WRAP(errClear); + } } if (auto errRemove = mFirewall->RemoveInstance(instanceID); !errRemove.IsNone() && err.IsNone()) { From bbe67b948de2910e509e89a7ae2edf3d7897c996 Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Wed, 22 Jul 2026 10:05:52 +0300 Subject: [PATCH 03/16] sm: networkmanager: update tests for conditional bandwidth clear Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- src/core/sm/networkmanager/tests/networkmanager.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/sm/networkmanager/tests/networkmanager.cpp b/src/core/sm/networkmanager/tests/networkmanager.cpp index 61296ef37..df32bb9a4 100644 --- a/src/core/sm/networkmanager/tests/networkmanager.cpp +++ b/src/core/sm/networkmanager/tests/networkmanager.cpp @@ -292,8 +292,8 @@ class NetworkManagerTest : public Test { EXPECT_CALL(mDNSName, CreateServer(_, _)) .WillOnce(Return(aos::RetWithError {&mDNSServer, aos::ErrorEnum::eNone})); EXPECT_CALL(mDNSServer, RemoveHost(aos::String("leftover-instance"))).WillOnce(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mBandwidth, Clear(_)).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mFirewall, RemoveInstance(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + // The leftover instance has no bandwidth shaping, so no bandwidth clear is expected. // The host veth is no longer detached synchronously on cleanup; the instance // netns teardown reaps it asynchronously. EXPECT_CALL(mNetns, DeleteNetworkNamespace(_)).WillOnce(Return(aos::ErrorEnum::eNone)); @@ -324,7 +324,6 @@ class NetworkManagerTest : public Test { void ExpectDeleteInstanceCalls(int times = 1) { EXPECT_CALL(mDNSServer, RemoveHost(_)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mBandwidth, Clear(_)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mFirewall, RemoveInstance(_)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone)); } @@ -682,6 +681,7 @@ TEST_F(NetworkManagerTest, StartInstanceNetwork_FailOnTrafficMonitorError) EXPECT_CALL(mNetns, GetNetworkNamespacePath(_)) .WillOnce(Return(aos::RetWithError> {{}, aos::ErrorEnum::eNone})); EXPECT_CALL(mNetns, DeleteNetworkNamespace(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mBridgeNetwork, Detach(_, _)).WillOnce(Return(aos::ErrorEnum::eNone)); ExpectDeleteInstanceCalls(); EXPECT_EQ(mNetManager->StartInstanceNetwork(instanceID, networkID), aos::ErrorEnum::eRuntime); @@ -928,7 +928,6 @@ TEST_F(NetworkManagerTest, StopInstanceNetwork_FailOnFirewallRemoveError) EXPECT_CALL(mTrafficMonitor, StopInstanceMonitoring(instanceID)).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mDNSServer, RemoveHost(_)).WillOnce(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mBandwidth, Clear(_)).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mFirewall, RemoveInstance(_)).WillOnce(Return(aos::ErrorEnum::eRuntime)); EXPECT_CALL(mNetns, DeleteNetworkNamespace(_)).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mNetIf, DeleteLink(_)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); From 548135a5453349219963d50fd86fd88239f0d206 Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Wed, 22 Jul 2026 10:29:43 +0300 Subject: [PATCH 04/16] sm: networkmanager: add storage transaction interface Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- src/core/sm/networkmanager/itf/storage.hpp | 21 +++++++++++++++++++++ src/core/sm/tests/mocks/storagemock.hpp | 3 +++ 2 files changed, 24 insertions(+) diff --git a/src/core/sm/networkmanager/itf/storage.hpp b/src/core/sm/networkmanager/itf/storage.hpp index 08882dfb0..c49654258 100644 --- a/src/core/sm/networkmanager/itf/storage.hpp +++ b/src/core/sm/networkmanager/itf/storage.hpp @@ -202,6 +202,27 @@ class StorageItf { */ virtual Error RemoveTrafficMonitorData(const String& chain) = 0; + /** + * Begins a storage transaction; subsequent writes are staged until commit. + * + * @return Error. + */ + virtual Error BeginTransaction() = 0; + + /** + * Commits the current storage transaction. + * + * @return Error. + */ + virtual Error CommitTransaction() = 0; + + /** + * Rolls back the current storage transaction, discarding staged writes. + * + * @return Error. + */ + virtual Error RollbackTransaction() = 0; + /** * Destroys storage interface. */ diff --git a/src/core/sm/tests/mocks/storagemock.hpp b/src/core/sm/tests/mocks/storagemock.hpp index 9ee8195ab..eadbcc5a3 100644 --- a/src/core/sm/tests/mocks/storagemock.hpp +++ b/src/core/sm/tests/mocks/storagemock.hpp @@ -25,6 +25,9 @@ class StorageMock : public StorageItf { MOCK_METHOD(Error, SetTrafficMonitorData, (const String&, const Time&, uint64_t), (override)); MOCK_METHOD(Error, GetTrafficMonitorData, (const String&, Time&, uint64_t&), (const, override)); MOCK_METHOD(Error, RemoveTrafficMonitorData, (const String&), (override)); + MOCK_METHOD(Error, BeginTransaction, (), (override)); + MOCK_METHOD(Error, CommitTransaction, (), (override)); + MOCK_METHOD(Error, RollbackTransaction, (), (override)); }; } // namespace aos::sm::networkmanager From 615e084ef814917ef999aa2e2f55a2d8f9587082 Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Wed, 22 Jul 2026 10:30:39 +0300 Subject: [PATCH 05/16] sm: networkmanager: add firewall and traffic batch interface Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- src/core/sm/networkmanager/itf/firewall.hpp | 21 +++++++++++++++++++ .../sm/networkmanager/itf/trafficmonitor.hpp | 21 +++++++++++++++++++ .../tests/mocks/firewallmock.hpp | 3 +++ .../tests/mocks/trafficmonitormock.hpp | 3 +++ 4 files changed, 48 insertions(+) diff --git a/src/core/sm/networkmanager/itf/firewall.hpp b/src/core/sm/networkmanager/itf/firewall.hpp index 3d6af4d28..a3dddca1e 100644 --- a/src/core/sm/networkmanager/itf/firewall.hpp +++ b/src/core/sm/networkmanager/itf/firewall.hpp @@ -158,6 +158,27 @@ class FirewallItf { * @return Error. */ virtual Error RemoveMasquerade(const String& subnet, const String& outIf) = 0; + + /** + * Opens a batch; AddInstance/RemoveInstance calls are staged until flush. + * + * @return Error. + */ + virtual Error BeginBatch() = 0; + + /** + * Flushes the staged batch atomically in a single nft transaction. + * + * @return Error. + */ + virtual Error FlushBatch() = 0; + + /** + * Reverts the flushed batch, deleting everything it applied by handle. + * + * @return Error. + */ + virtual Error Revert() = 0; }; /** @}*/ diff --git a/src/core/sm/networkmanager/itf/trafficmonitor.hpp b/src/core/sm/networkmanager/itf/trafficmonitor.hpp index 488e9c1c5..d71ac7951 100644 --- a/src/core/sm/networkmanager/itf/trafficmonitor.hpp +++ b/src/core/sm/networkmanager/itf/trafficmonitor.hpp @@ -104,6 +104,27 @@ class TrafficMonitorItf { */ virtual Error GetInstanceTraffic(const String& instanceID, uint64_t& inputTraffic, uint64_t& outputTraffic) const = 0; + + /** + * Opens a batch; StartInstanceMonitoring/StopInstanceMonitoring calls are staged until flush. + * + * @return Error. + */ + virtual Error BeginBatch() = 0; + + /** + * Flushes the staged batch atomically in a single nft transaction. + * + * @return Error. + */ + virtual Error FlushBatch() = 0; + + /** + * Reverts the flushed batch, deleting everything it applied by handle. + * + * @return Error. + */ + virtual Error Revert() = 0; }; /** @}*/ diff --git a/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp b/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp index db9f668d4..37d70861a 100644 --- a/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp +++ b/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp @@ -23,6 +23,9 @@ class FirewallMock : public FirewallItf { MOCK_METHOD(Error, UpdateInstance, (const String&, const InstanceFirewallParams&), (override)); MOCK_METHOD(Error, AddMasquerade, (const String&, const String&), (override)); MOCK_METHOD(Error, RemoveMasquerade, (const String&, const String&), (override)); + MOCK_METHOD(Error, BeginBatch, (), (override)); + MOCK_METHOD(Error, FlushBatch, (), (override)); + MOCK_METHOD(Error, Revert, (), (override)); }; } // namespace aos::sm::networkmanager diff --git a/src/core/sm/networkmanager/tests/mocks/trafficmonitormock.hpp b/src/core/sm/networkmanager/tests/mocks/trafficmonitormock.hpp index af96da377..7dcc873a4 100644 --- a/src/core/sm/networkmanager/tests/mocks/trafficmonitormock.hpp +++ b/src/core/sm/networkmanager/tests/mocks/trafficmonitormock.hpp @@ -22,6 +22,9 @@ class TrafficMonitorMock : public TrafficMonitorItf { MOCK_METHOD(Error, StopInstanceMonitoring, (const String&), (override)); MOCK_METHOD(Error, GetSystemTraffic, (uint64_t&, uint64_t&), (const, override)); MOCK_METHOD(Error, GetInstanceTraffic, (const String&, uint64_t&, uint64_t&), (const, override)); + MOCK_METHOD(Error, BeginBatch, (), (override)); + MOCK_METHOD(Error, FlushBatch, (), (override)); + MOCK_METHOD(Error, Revert, (), (override)); }; } // namespace aos::sm::networkmanager From 341d48808e115723b575f0a43b8cc0e90039b108 Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Wed, 22 Jul 2026 10:37:03 +0300 Subject: [PATCH 06/16] sm: networkmanager: add batch interface to network manager Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- src/core/sm/networkmanager/itf/networkmanager.hpp | 15 +++++++++++++++ src/core/sm/networkmanager/networkmanager.cpp | 12 ++++++++++++ src/core/sm/networkmanager/networkmanager.hpp | 15 +++++++++++++++ src/core/sm/tests/mocks/networkmanagermock.hpp | 2 ++ 4 files changed, 44 insertions(+) diff --git a/src/core/sm/networkmanager/itf/networkmanager.hpp b/src/core/sm/networkmanager/itf/networkmanager.hpp index 6c6333194..e753cb14a 100644 --- a/src/core/sm/networkmanager/itf/networkmanager.hpp +++ b/src/core/sm/networkmanager/itf/networkmanager.hpp @@ -128,6 +128,21 @@ class NetworkManagerItf : public SystemTrafficProviderItf, * @return Error. */ virtual Error ReleaseInstanceNetwork(const String& instanceID, const String& networkID) = 0; + + /** + * Opens a batch; start/stop operations are staged and applied on flush. + * + * @return Error. + */ + virtual Error BeginBatch() = 0; + + /** + * Flushes the staged batch atomically across firewall, traffic and storage. + * + * @param[out] failedInstanceIDs instances that were not applied. + * @return Error. + */ + virtual Error FlushBatch(Array>& failedInstanceIDs) = 0; }; /** @}*/ diff --git a/src/core/sm/networkmanager/networkmanager.cpp b/src/core/sm/networkmanager/networkmanager.cpp index 941b57bb3..e6123de02 100644 --- a/src/core/sm/networkmanager/networkmanager.cpp +++ b/src/core/sm/networkmanager/networkmanager.cpp @@ -516,6 +516,18 @@ Error NetworkManager::ReleaseInstanceNetwork(const String& instanceID, const Str return ErrorEnum::eNone; } +Error NetworkManager::BeginBatch() +{ + return ErrorEnum::eNone; +} + +Error NetworkManager::FlushBatch(Array>& failedInstanceIDs) +{ + failedInstanceIDs.Clear(); + + return ErrorEnum::eNone; +} + Error NetworkManager::PrepareUpdateItemNetworkParams( const InstanceNetworkConfig& params, const String& networkID, UpdateItemNetworkParams& serviceData) const { diff --git a/src/core/sm/networkmanager/networkmanager.hpp b/src/core/sm/networkmanager/networkmanager.hpp index 0c12f6500..e8ab55c2d 100644 --- a/src/core/sm/networkmanager/networkmanager.hpp +++ b/src/core/sm/networkmanager/networkmanager.hpp @@ -165,6 +165,21 @@ class NetworkManager : public NetworkManagerItf { */ Error ReleaseInstanceNetwork(const String& instanceID, const String& networkID) override; + /** + * Opens a batch for start/stop operations. + * + * @return Error. + */ + Error BeginBatch() override; + + /** + * Flushes the staged batch. + * + * @param[out] failedInstanceIDs instances that were not applied. + * @return Error. + */ + Error FlushBatch(Array>& failedInstanceIDs) override; + /** * Called when pending firewall rules are resolved for an instance. * diff --git a/src/core/sm/tests/mocks/networkmanagermock.hpp b/src/core/sm/tests/mocks/networkmanagermock.hpp index 352deab33..ee9726132 100644 --- a/src/core/sm/tests/mocks/networkmanagermock.hpp +++ b/src/core/sm/tests/mocks/networkmanagermock.hpp @@ -28,6 +28,8 @@ class NetworkManagerMock : public NetworkManagerItf { MOCK_METHOD(Error, GetHosts, (const String& instanceID, Array& hosts), (const, override)); MOCK_METHOD(Error, StopInstanceNetwork, (const String& instanceID, const String& networkID), (override)); MOCK_METHOD(Error, ReleaseInstanceNetwork, (const String& instanceID, const String& networkID), (override)); + MOCK_METHOD(Error, BeginBatch, (), (override)); + MOCK_METHOD(Error, FlushBatch, (Array> & failedInstanceIDs), (override)); MOCK_METHOD(void, OnPendingFirewallUpdate, (const String& nodeID, const aos::networkmanager::PendingFirewallUpdate& update), (override)); MOCK_METHOD(void, OnConnect, (), (override)); From a522c2a424bdc54a366bc15fc5acd92eb7df12cb Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Wed, 22 Jul 2026 12:31:07 +0300 Subject: [PATCH 07/16] sm: networkmanager: orchestrate network batch flush Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- src/core/sm/networkmanager/networkmanager.cpp | 107 ++++++++++++++++++ src/core/sm/networkmanager/networkmanager.hpp | 24 ++-- 2 files changed, 123 insertions(+), 8 deletions(-) diff --git a/src/core/sm/networkmanager/networkmanager.cpp b/src/core/sm/networkmanager/networkmanager.cpp index e6123de02..ee31c8f63 100644 --- a/src/core/sm/networkmanager/networkmanager.cpp +++ b/src/core/sm/networkmanager/networkmanager.cpp @@ -285,6 +285,17 @@ Error NetworkManager::StartInstanceNetwork(const String& instanceID, const Strin err = AddInstanceToNetwork(instanceID, networkID, cachedInfo->mNetworkConfig, cachedInfo->mAllocatedParams); + if (err.IsNone()) { + LockGuard lock {mMutex}; + + if (mBatchMode) { + if (auto errBatch = mBatchEntries.PushBack({instanceID, networkID, BatchOp::eAdd}); !errBatch.IsNone()) { + LOG_ERR() << "Failed to register batch entry" << Log::Field("instanceID", instanceID) + << Log::Field(errBatch); + } + } + } + return err; } @@ -415,6 +426,17 @@ Error NetworkManager::StopInstanceNetwork(const String& instanceID, const String } } + { + LockGuard lock {mMutex}; + + if (mBatchMode) { + if (auto errBatch = mBatchEntries.PushBack({instanceID, networkID, BatchOp::eRemove}); !errBatch.IsNone()) { + LOG_ERR() << "Failed to register batch entry" << Log::Field("instanceID", instanceID) + << Log::Field(errBatch); + } + } + } + if (auto errRemove = RemoveInstanceFromCache(instanceID, networkID); !errRemove.IsNone() && err.IsNone()) { err = errRemove; } @@ -518,6 +540,47 @@ Error NetworkManager::ReleaseInstanceNetwork(const String& instanceID, const Str Error NetworkManager::BeginBatch() { + Error err; + + { + LockGuard lock {mMutex}; + + mBatchEntries.Clear(); + mBatchMode = true; + } + + auto cleanupBatchMode = DeferRelease(this, [&err](NetworkManager* self) { + if (!err.IsNone()) { + LockGuard lock {self->mMutex}; + + self->mBatchMode = false; + } + }); + + if (err = mStorage->BeginTransaction(); !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + auto cleanupStorage = DeferRelease(this, [&err](NetworkManager* self) { + if (!err.IsNone()) { + self->mStorage->RollbackTransaction(); + } + }); + + if (err = mFirewall->BeginBatch(); !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + auto cleanupFirewall = DeferRelease(this, [&err](NetworkManager* self) { + if (!err.IsNone()) { + self->mFirewall->Revert(); + } + }); + + if (err = mNetMonitor->BeginBatch(); !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + return ErrorEnum::eNone; } @@ -525,6 +588,50 @@ Error NetworkManager::FlushBatch(Array>& failedInstanceIDs) { failedInstanceIDs.Clear(); + Error err; + bool firewallApplied = false; + bool trafficApplied = false; + + auto cleanupFirewall = DeferRelease(this, [&err, &firewallApplied](NetworkManager* self) { + if (!err.IsNone() && firewallApplied) { + self->mFirewall->Revert(); + } + }); + + auto cleanupTraffic = DeferRelease(this, [&err, &trafficApplied](NetworkManager* self) { + if (!err.IsNone() && trafficApplied) { + self->mNetMonitor->Revert(); + } + }); + + if (err = mFirewall->FlushBatch(); err.IsNone()) { + firewallApplied = true; + + if (err = mNetMonitor->FlushBatch(); err.IsNone()) { + trafficApplied = true; + + err = mStorage->CommitTransaction(); + } + } + + if (!err.IsNone()) { + // nft flush failed: staged DB writes must not be committed. + if (!firewallApplied || !trafficApplied) { + mStorage->RollbackTransaction(); + } + + for (const auto& entry : mBatchEntries) { + failedInstanceIDs.PushBack(entry.mInstanceID); + } + } + + { + LockGuard lock {mMutex}; + + mBatchMode = false; + mBatchEntries.Clear(); + } + return ErrorEnum::eNone; } diff --git a/src/core/sm/networkmanager/networkmanager.hpp b/src/core/sm/networkmanager/networkmanager.hpp index e8ab55c2d..8e33e4735 100644 --- a/src/core/sm/networkmanager/networkmanager.hpp +++ b/src/core/sm/networkmanager/networkmanager.hpp @@ -195,18 +195,18 @@ class NetworkManager : public NetworkManagerItf { void OnConnect() override; private: - Error EnsureNodeNetwork(const String& networkID); - Error EnsureNodeNetworkPhysical(const String& networkID); - Error UpdateInstanceFirewall(const String& instanceID, const String& networkID, - const InstanceNetworkConfig& networkConfig, const aos::InstanceNetworkAllocation& networkParams); - - Error AddInstanceToNetwork(const String& instanceID, const String& networkID, - const InstanceNetworkConfig& networkConfig, const aos::InstanceNetworkAllocation& networkParams); - using InstanceHosts = StaticArray, cMaxNumHosts>; using InstanceCache = StaticMap, InstanceHosts, cMaxNumInstances>; using NetworkCache = StaticMap, InstanceCache, cMaxNumOwners>; + enum class BatchOp { eAdd, eRemove }; + + struct BatchEntry { + StaticString mInstanceID; + StaticString mNetworkID; + BatchOp mOp; + }; + // StartInstanceNetwork keeps its cached InstanceNetworkInfo alive across the nested call to // AddInstanceToNetwork, which in turn allocates hosts, bridge/firewall/bandwidth/DNS params and // its own InstanceNetworkInfo before returning. That is the largest concurrent footprint of any @@ -252,6 +252,12 @@ class NetworkManager : public NetworkManagerItf { Error InitInstance(const String& instanceID, const String& networkID); Error ReconcileInstances(); Error RemoveFirewallOrphans(); + Error EnsureNodeNetwork(const String& networkID); + Error EnsureNodeNetworkPhysical(const String& networkID); + Error UpdateInstanceFirewall(const String& instanceID, const String& networkID, + const InstanceNetworkConfig& networkConfig, const aos::InstanceNetworkAllocation& networkParams); + Error AddInstanceToNetwork(const String& instanceID, const String& networkID, + const InstanceNetworkConfig& networkConfig, const aos::InstanceNetworkAllocation& networkParams); Error RemoveDNSOrphans(); Error AdoptDNSServer(const String& networkID); Error PrepareBridgeParams( @@ -311,6 +317,8 @@ class NetworkManager : public NetworkManagerItf { StaticMap, DNSServerItf*, cMaxNumOwners> mDNSServers; StaticMap, InstanceNetworkInfo, cMaxNumInstances * cMaxNumOwners> mInstanceNetworkInfos; StaticArray, cMaxNumOwners> mPhysicalNetworks; + bool mBatchMode {false}; + StaticArray mBatchEntries; StaticAllocator)> mNetworkInfosAllocator; StaticAllocator)> mInstanceNetworkInfosAllocator; From ddd6a38b55dcaa98256f2c4001fc1a48efd4859d Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Wed, 22 Jul 2026 12:42:59 +0300 Subject: [PATCH 08/16] sm: networkmanager: fall back to per-instance apply on batch flush failure Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- src/core/sm/networkmanager/networkmanager.cpp | 124 +++++++++++++++--- src/core/sm/networkmanager/networkmanager.hpp | 1 + 2 files changed, 104 insertions(+), 21 deletions(-) diff --git a/src/core/sm/networkmanager/networkmanager.cpp b/src/core/sm/networkmanager/networkmanager.cpp index ee31c8f63..02632026a 100644 --- a/src/core/sm/networkmanager/networkmanager.cpp +++ b/src/core/sm/networkmanager/networkmanager.cpp @@ -592,36 +592,45 @@ Error NetworkManager::FlushBatch(Array>& failedInstanceIDs) bool firewallApplied = false; bool trafficApplied = false; - auto cleanupFirewall = DeferRelease(this, [&err, &firewallApplied](NetworkManager* self) { - if (!err.IsNone() && firewallApplied) { - self->mFirewall->Revert(); - } - }); + { + auto cleanupFirewall = DeferRelease(this, [&err, &firewallApplied](NetworkManager* self) { + if (!err.IsNone() && firewallApplied) { + self->mFirewall->Revert(); + } + }); - auto cleanupTraffic = DeferRelease(this, [&err, &trafficApplied](NetworkManager* self) { - if (!err.IsNone() && trafficApplied) { - self->mNetMonitor->Revert(); - } - }); + auto cleanupTraffic = DeferRelease(this, [&err, &trafficApplied](NetworkManager* self) { + if (!err.IsNone() && trafficApplied) { + self->mNetMonitor->Revert(); + } + }); - if (err = mFirewall->FlushBatch(); err.IsNone()) { - firewallApplied = true; + if (err = mFirewall->FlushBatch(); err.IsNone()) { + firewallApplied = true; - if (err = mNetMonitor->FlushBatch(); err.IsNone()) { - trafficApplied = true; + if (err = mNetMonitor->FlushBatch(); err.IsNone()) { + trafficApplied = true; - err = mStorage->CommitTransaction(); + err = mStorage->CommitTransaction(); + } } - } - if (!err.IsNone()) { - // nft flush failed: staged DB writes must not be committed. - if (!firewallApplied || !trafficApplied) { + if (!err.IsNone() && (!firewallApplied || !trafficApplied)) { mStorage->RollbackTransaction(); } + } - for (const auto& entry : mBatchEntries) { - failedInstanceIDs.PushBack(entry.mInstanceID); + if (!err.IsNone()) { + if (firewallApplied && trafficApplied) { + for (const auto& entry : mBatchEntries) { + failedInstanceIDs.PushBack(entry.mInstanceID); + } + } else { + for (const auto& entry : mBatchEntries) { + if (auto errReapply = ReapplyInstancePolicy(entry); !errReapply.IsNone()) { + failedInstanceIDs.PushBack(entry.mInstanceID); + } + } } } @@ -635,6 +644,79 @@ Error NetworkManager::FlushBatch(Array>& failedInstanceIDs) return ErrorEnum::eNone; } +Error NetworkManager::ReapplyInstancePolicy(const BatchEntry& entry) +{ + if (entry.mOp == BatchOp::eRemove) { + Error err; + + if (auto errFW = mFirewall->RemoveInstance(entry.mInstanceID); !errFW.IsNone()) { + err = errFW; + } + + if (auto errTR = mNetMonitor->StopInstanceMonitoring(entry.mInstanceID); !errTR.IsNone() && err.IsNone()) { + err = errTR; + } + + return err; + } + + auto info = MakeUnique(&mAllocator); + + { + LockGuard lock {mMutex}; + + auto it = mInstanceNetworkInfos.Find(entry.mInstanceID); + if (it == mInstanceNetworkInfos.end()) { + return AOS_ERROR_WRAP(Error(ErrorEnum::eNotFound, "instance network info not found")); + } + + *info = it->mSecond; + } + + auto firewallParams = MakeUnique(&mAllocator); + + if (auto err = PrepareInstanceFirewallParams(info->mNetworkConfig, info->mAllocatedParams, *firewallParams); + !err.IsNone()) { + return err; + } + + Error err; + + if (err = mFirewall->AddInstance(entry.mInstanceID, *firewallParams); !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + auto cleanupFirewall = DeferRelease(this, [&err, &entry](NetworkManager* self) { + if (!err.IsNone()) { + if (auto errRemove = self->mFirewall->RemoveInstance(entry.mInstanceID); !errRemove.IsNone()) { + LOG_ERR() << "Failed to remove firewall instance on rollback" + << Log::Field("instanceID", entry.mInstanceID) << Log::Field(errRemove); + } + } + }); + + if (err = mNetMonitor->StartInstanceMonitoring(entry.mInstanceID, info->mAllocatedParams.mIP, + info->mNetworkConfig.mDownloadLimit, info->mNetworkConfig.mUploadLimit); + !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + auto cleanupMonitoring = DeferRelease(this, [&err, &entry](NetworkManager* self) { + if (!err.IsNone()) { + if (auto errStop = self->mNetMonitor->StopInstanceMonitoring(entry.mInstanceID); !errStop.IsNone()) { + LOG_ERR() << "Failed to stop instance monitoring on rollback" + << Log::Field("instanceID", entry.mInstanceID) << Log::Field(errStop); + } + } + }); + + if (err = mStorage->UpdateInstanceNetworkInfo(*info); !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + return ErrorEnum::eNone; +} + Error NetworkManager::PrepareUpdateItemNetworkParams( const InstanceNetworkConfig& params, const String& networkID, UpdateItemNetworkParams& serviceData) const { diff --git a/src/core/sm/networkmanager/networkmanager.hpp b/src/core/sm/networkmanager/networkmanager.hpp index 8e33e4735..0e14ea9f7 100644 --- a/src/core/sm/networkmanager/networkmanager.hpp +++ b/src/core/sm/networkmanager/networkmanager.hpp @@ -258,6 +258,7 @@ class NetworkManager : public NetworkManagerItf { const InstanceNetworkConfig& networkConfig, const aos::InstanceNetworkAllocation& networkParams); Error AddInstanceToNetwork(const String& instanceID, const String& networkID, const InstanceNetworkConfig& networkConfig, const aos::InstanceNetworkAllocation& networkParams); + Error ReapplyInstancePolicy(const BatchEntry& entry); Error RemoveDNSOrphans(); Error AdoptDNSServer(const String& networkID); Error PrepareBridgeParams( From 81b62605a3312cfb3baa2f82dc2b50043652d735 Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Wed, 22 Jul 2026 14:04:37 +0300 Subject: [PATCH 09/16] sm: launcher: wrap network start phase in batch and fail unapplied instances Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- src/core/sm/launcher/launcher.cpp | 38 +++++++++++++++++++++++++++++++ src/core/sm/launcher/launcher.hpp | 1 + 2 files changed, 39 insertions(+) diff --git a/src/core/sm/launcher/launcher.cpp b/src/core/sm/launcher/launcher.cpp index a9ed66b9c..6bfea00bc 100644 --- a/src/core/sm/launcher/launcher.cpp +++ b/src/core/sm/launcher/launcher.cpp @@ -900,6 +900,11 @@ void Launcher::StartNetworks(const Array& startInstances) { LOG_INF() << "[profiling] Start networks begin" << Log::Field("count", startInstances.Size()); + auto errBegin = mNetworkManager->BeginBatch(); + if (!errBegin.IsNone()) { + LOG_ERR() << "Failed to begin network batch" << Log::Field(AOS_ERROR_WRAP(errBegin)); + } + for (const auto& instance : startInstances) { auto instanceData = FindInstanceData(instance); if (!instanceData) { @@ -932,6 +937,29 @@ void Launcher::StartNetworks(const Array& startInstances) LOG_ERR() << "Thread pool wait failed" << Log::Field(AOS_ERROR_WRAP(err)); } + if (errBegin.IsNone()) { + auto failedIDs = MakeUnique, cMaxNumInstances>>(&mAllocator); + + mNetworkManager->FlushBatch(*failedIDs); + + for (const auto& failedID : *failedIDs) { + auto instanceData = FindInstanceDataByID(failedID); + if (!instanceData) { + continue; + } + + SetInstanceState(*instanceData, InstanceStateEnum::eFailed, + AOS_ERROR_WRAP(Error(ErrorEnum::eFailed, "network batch apply failed"))); + + if (auto err + = mNetworkManager->StopInstanceNetwork(instanceData->mInstanceID, instanceData->mInfo.mOwnerID); + !err.IsNone() && !err.Is(ErrorEnum::eNotFound)) { + LOG_ERR() << "Failed to stop network" << Log::Field("instance", instanceData->mInfo) + << Log::Field(AOS_ERROR_WRAP(err)); + } + } + } + LOG_INF() << "[profiling] Start networks end"; } @@ -1127,6 +1155,16 @@ Launcher::InstanceData* Launcher::FindInstanceData(const InstanceIdent& instance return const_cast(this)->FindInstanceData(instanceIdent); } +Launcher::InstanceData* Launcher::FindInstanceDataByID(const String& instanceID) +{ + auto it = mInstances.FindIf([&instanceID](const auto& instance) { return instance.mInstanceID == instanceID; }); + if (it != mInstances.end()) { + return it; + } + + return nullptr; +} + RuntimeItf* Launcher::FindInstanceRuntime(const String& runtimeID) { auto it = mRuntimes.FindIf([&runtimeID](const auto& it) { return it.mSecond == runtimeID; }); diff --git a/src/core/sm/launcher/launcher.hpp b/src/core/sm/launcher/launcher.hpp index 801caafd3..f6c17a9a1 100644 --- a/src/core/sm/launcher/launcher.hpp +++ b/src/core/sm/launcher/launcher.hpp @@ -225,6 +225,7 @@ class Launcher : public LauncherItf, InstanceData* FindInstanceData(const InstanceIdent& instanceIdent); InstanceData* FindInstanceData(const InstanceIdent& instanceIdent) const; + InstanceData* FindInstanceDataByID(const String& instanceID); RuntimeItf* FindInstanceRuntime(const String& runtimeID); RuntimeItf* FindInstanceRuntime(const String& runtimeID) const; RuntimeItf* FindInstanceRuntime(const InstanceIdent& instanceIdent); From d3f0209329b64794513f92f471e7a2ba609be7a1 Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Wed, 22 Jul 2026 14:13:50 +0300 Subject: [PATCH 10/16] sm: launcher: wrap network stop phases in batch Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- src/core/sm/launcher/launcher.cpp | 34 +++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/core/sm/launcher/launcher.cpp b/src/core/sm/launcher/launcher.cpp index 6bfea00bc..d2c4fef42 100644 --- a/src/core/sm/launcher/launcher.cpp +++ b/src/core/sm/launcher/launcher.cpp @@ -792,6 +792,11 @@ void Launcher::StopAllNetworks() { LOG_INF() << "[profiling] Stop all networks begin" << Log::Field("count", mInstances.Size()); + auto errBegin = mNetworkManager->BeginBatch(); + if (!errBegin.IsNone()) { + LOG_ERR() << "Failed to begin network batch" << Log::Field(AOS_ERROR_WRAP(errBegin)); + } + for (auto& instance : mInstances) { if (instance.mInfo.mType != UpdateItemTypeEnum::eService) { continue; @@ -804,6 +809,20 @@ void Launcher::StopAllNetworks() } } + if (auto err = mLaunchPool.Wait(); !err.IsNone()) { + LOG_ERR() << "Thread pool wait failed" << Log::Field(AOS_ERROR_WRAP(err)); + } + + if (errBegin.IsNone()) { + auto failedIDs = MakeUnique, cMaxNumInstances>>(&mAllocator); + + mNetworkManager->FlushBatch(*failedIDs); + + if (!failedIDs->IsEmpty()) { + LOG_WRN() << "Network stop batch partially failed" << Log::Field("count", failedIDs->Size()); + } + } + LOG_INF() << "[profiling] Stop all networks end"; } @@ -985,6 +1004,11 @@ void Launcher::StopNetworks(const Array& stopInstances) { LOG_INF() << "[profiling] Stop networks begin" << Log::Field("count", stopInstances.Size()); + auto errBegin = mNetworkManager->BeginBatch(); + if (!errBegin.IsNone()) { + LOG_ERR() << "Failed to begin network batch" << Log::Field(AOS_ERROR_WRAP(errBegin)); + } + for (const auto& instance : stopInstances) { auto instanceData = FindInstanceData(instance); if (!instanceData) { @@ -1009,6 +1033,16 @@ void Launcher::StopNetworks(const Array& stopInstances) LOG_ERR() << "Thread pool wait failed" << Log::Field(AOS_ERROR_WRAP(err)); } + if (errBegin.IsNone()) { + auto failedIDs = MakeUnique, cMaxNumInstances>>(&mAllocator); + + mNetworkManager->FlushBatch(*failedIDs); + + if (!failedIDs->IsEmpty()) { + LOG_WRN() << "Network stop batch partially failed" << Log::Field("count", failedIDs->Size()); + } + } + LOG_INF() << "[profiling] Stop networks end"; } From b01616407b9a5a2668596595f81a719ca9afdb4a Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Wed, 22 Jul 2026 15:27:48 +0300 Subject: [PATCH 11/16] sm: networkmanager: test batch flush success, revert and per-instance fallback Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- .../networkmanager/tests/networkmanager.cpp | 162 ++++++++++++++++++ 1 file changed, 162 insertions(+) diff --git a/src/core/sm/networkmanager/tests/networkmanager.cpp b/src/core/sm/networkmanager/tests/networkmanager.cpp index df32bb9a4..a045d00bc 100644 --- a/src/core/sm/networkmanager/tests/networkmanager.cpp +++ b/src/core/sm/networkmanager/tests/networkmanager.cpp @@ -625,6 +625,168 @@ TEST_F(NetworkManagerTest, CreateAndStartInstanceNetwork_VerifyResolvConfFile) } } +TEST_F(NetworkManagerTest, BeginFlushBatch_ForwardToBackends) +{ + EXPECT_CALL(mStorage, BeginTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->BeginBatch(), aos::ErrorEnum::eNone); + + EXPECT_CALL(mFirewall, FlushBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, FlushBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mStorage, CommitTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + + aos::StaticArray, aos::cMaxNumInstances> failed; + + EXPECT_EQ(mNetManager->FlushBatch(failed), aos::ErrorEnum::eNone); + EXPECT_TRUE(failed.IsEmpty()); +} + +TEST_F(NetworkManagerTest, BeginBatch_PropagatesBackendError) +{ + EXPECT_CALL(mStorage, BeginTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eRuntime)); + EXPECT_CALL(mStorage, RollbackTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + + EXPECT_NE(mNetManager->BeginBatch(), aos::ErrorEnum::eNone); +} + +TEST_F(NetworkManagerTest, FlushBatch_NftFailure_RevertsAndFallsBack) +{ + const aos::String instanceID1 = "test-instance-1"; + const aos::String instanceID2 = "test-instance-2"; + const aos::String networkID = "test-network"; + auto params = CreateTestInstanceNetworkConfig(); + auto allocatedParams = CreateTestAllocatedParams(); + + SetupEnsureNodeNetworkCreateMocks(networkID, allocatedParams.mSubnet, "192.168.1.1", 100ULL); + + EXPECT_CALL(mNetworkProvider, AllocateInstanceNetwork(_, networkID, aos::String("test-node"), _, _)) + .Times(2) + .WillRepeatedly(DoAll(SetArgReferee<4>(allocatedParams), Return(aos::ErrorEnum::eNone))); + EXPECT_CALL(mStorage, AddInstanceNetworkInfo(_)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->CreateInstanceNetwork(instanceID1, networkID, params), aos::ErrorEnum::eNone); + + params.mHosts.Clear(); + params.mAliases.Clear(); + params.mHosts.PushBack(aos::Host {"10.0.0.3", "host3.example.com"}); + params.mAliases.PushBack("alias3"); + params.mHostname = "test-host-3"; + params.mInstanceIdent.mInstance = 1; + + ASSERT_EQ(mNetManager->CreateInstanceNetwork(instanceID2, networkID, params), aos::ErrorEnum::eNone); + + EXPECT_CALL(mStorage, BeginTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->BeginBatch(), aos::ErrorEnum::eNone); + + SetupEnsureNodeNetworkPhysicalMocks("192.168.1.1", allocatedParams.mSubnet, 100ULL); + + BridgeAttachResult attachResult; + attachResult.mHostIfName = "veth-test"; + attachResult.mContainerIfName = "eth0"; + + EXPECT_CALL(mBridgeNetwork, Attach(_, _, _)) + .Times(2) + .WillRepeatedly(DoAll(SetArgReferee<2>(attachResult), Return(aos::ErrorEnum::eNone))); + EXPECT_CALL(mBandwidth, Apply(_, _)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mDNSServer, AddHost(_, _)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, CreateNetworkNamespace(_)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, GetNetworkNamespacePath(_)) + .Times(2) + .WillRepeatedly(Return(aos::RetWithError> {{}, aos::ErrorEnum::eNone})); + + EXPECT_CALL(mFirewall, AddInstance(instanceID1, _)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, AddInstance(instanceID2, _)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, RemoveInstance(instanceID2)).WillOnce(Return(aos::ErrorEnum::eNone)); + + EXPECT_CALL(mTrafficMonitor, StartInstanceMonitoring(instanceID1, _, _, _)) + .Times(2) + .WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, StartInstanceMonitoring(instanceID2, _, _, _)) + .WillOnce(Return(aos::ErrorEnum::eNone)) + .WillOnce(Return(aos::ErrorEnum::eRuntime)); + + EXPECT_CALL(mStorage, UpdateInstanceNetworkInfo(_)).Times(3).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID1, networkID), aos::ErrorEnum::eNone); + ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID2, networkID), aos::ErrorEnum::eNone); + + EXPECT_CALL(mFirewall, FlushBatch()).WillOnce(Return(aos::ErrorEnum::eRuntime)); + EXPECT_CALL(mStorage, RollbackTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + + aos::StaticArray, aos::cMaxNumInstances> failed; + + EXPECT_EQ(mNetManager->FlushBatch(failed), aos::ErrorEnum::eNone); + ASSERT_EQ(failed.Size(), 1U); + EXPECT_EQ(failed[0], instanceID2); +} + +TEST_F(NetworkManagerTest, FlushBatch_CommitFailure_MarksAllFailed) +{ + const aos::String instanceID1 = "test-instance-1"; + const aos::String instanceID2 = "test-instance-2"; + const aos::String networkID = "test-network"; + auto params = CreateTestInstanceNetworkConfig(); + auto allocatedParams = CreateTestAllocatedParams(); + + SetupEnsureNodeNetworkCreateMocks(networkID, allocatedParams.mSubnet, "192.168.1.1", 100ULL); + + EXPECT_CALL(mNetworkProvider, AllocateInstanceNetwork(_, networkID, aos::String("test-node"), _, _)) + .Times(2) + .WillRepeatedly(DoAll(SetArgReferee<4>(allocatedParams), Return(aos::ErrorEnum::eNone))); + EXPECT_CALL(mStorage, AddInstanceNetworkInfo(_)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->CreateInstanceNetwork(instanceID1, networkID, params), aos::ErrorEnum::eNone); + + params.mHosts.Clear(); + params.mAliases.Clear(); + params.mHosts.PushBack(aos::Host {"10.0.0.3", "host3.example.com"}); + params.mAliases.PushBack("alias3"); + params.mHostname = "test-host-3"; + params.mInstanceIdent.mInstance = 1; + + ASSERT_EQ(mNetManager->CreateInstanceNetwork(instanceID2, networkID, params), aos::ErrorEnum::eNone); + + EXPECT_CALL(mStorage, BeginTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->BeginBatch(), aos::ErrorEnum::eNone); + + SetupEnsureNodeNetworkPhysicalMocks("192.168.1.1", allocatedParams.mSubnet, 100ULL); + + ExpectAddInstanceCalls(2); + ExpectPersistInstanceCalls(2); + EXPECT_CALL(mTrafficMonitor, StartInstanceMonitoring(_, _, _, _)) + .Times(2) + .WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, CreateNetworkNamespace(_)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, GetNetworkNamespacePath(_)) + .Times(2) + .WillRepeatedly(Return(aos::RetWithError> {{}, aos::ErrorEnum::eNone})); + + ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID1, networkID), aos::ErrorEnum::eNone); + ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID2, networkID), aos::ErrorEnum::eNone); + + EXPECT_CALL(mFirewall, FlushBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, FlushBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mStorage, CommitTransaction()).WillOnce(Return(aos::ErrorEnum::eRuntime)); + EXPECT_CALL(mFirewall, Revert()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, Revert()).WillOnce(Return(aos::ErrorEnum::eNone)); + + aos::StaticArray, aos::cMaxNumInstances> failed; + + EXPECT_EQ(mNetManager->FlushBatch(failed), aos::ErrorEnum::eNone); + ASSERT_EQ(failed.Size(), 2U); + EXPECT_EQ(failed[0], instanceID1); + EXPECT_EQ(failed[1], instanceID2); +} + TEST_F(NetworkManagerTest, StartInstanceNetwork_FailOnAttachError) { const aos::String instanceID = "test-instance"; From 9f94413081bd5bd76c04d452a7ca8fecb12a92a0 Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Wed, 22 Jul 2026 15:42:38 +0300 Subject: [PATCH 12/16] sm: launcher: expect network batch calls and cover start flush failure Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- src/core/sm/launcher/tests/launcher.cpp | 61 +++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/core/sm/launcher/tests/launcher.cpp b/src/core/sm/launcher/tests/launcher.cpp index b9da0b2ae..2eaf05cd7 100644 --- a/src/core/sm/launcher/tests/launcher.cpp +++ b/src/core/sm/launcher/tests/launcher.cpp @@ -194,6 +194,8 @@ class LauncherTest : public Test { EXPECT_CALL(mNetworkManager, StartInstanceNetwork).WillRepeatedly(Return(ErrorEnum::eNone)); EXPECT_CALL(mNetworkManager, StopInstanceNetwork).WillRepeatedly(Return(ErrorEnum::eNone)); EXPECT_CALL(mNetworkManager, ReleaseInstanceNetwork).WillRepeatedly(Return(ErrorEnum::eNone)); + EXPECT_CALL(mNetworkManager, BeginBatch()).WillRepeatedly(Return(ErrorEnum::eNone)); + EXPECT_CALL(mNetworkManager, FlushBatch(_)).WillRepeatedly(Return(ErrorEnum::eNone)); } StaticArray GetRuntimesArray() @@ -462,6 +464,65 @@ TEST_F(LauncherTest, LauncherStartsStoredInstancesOnModuleStart) ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); } +TEST_F(LauncherTest, StartNetworks_FlushFailure_FailsInstance) +{ + const std::vector cStoredInfos = { + CreateInstanceInfo("item0", 0, "1.0.0", "runtime0"), + CreateInstanceInfo("item1", 1, "1.0.0", "runtime1"), + }; + + mStorage.Init(cStoredInfos); + + auto err = mLauncher.Init(GetRuntimesArray(), mImageManager, mSender, mStorage, mOCISpec, mItemInfoProvider, + mCloudConnection, mNetworkManager, mInstanceIDProvider, mResourceInfoProvider); + ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); + + EXPECT_CALL(mInstanceIDProvider, GetInstanceID) + .WillRepeatedly(Invoke([](const InstanceIdent& instance, String& instanceID) { + instanceID = instance.mItemID; + + return ErrorEnum::eNone; + })); + + EXPECT_CALL(mNetworkManager, FlushBatch(_)) + .WillRepeatedly(DoAll(WithArg<0>([](auto& failedInstanceIDs) { failedInstanceIDs.PushBack("item0"); }), + Return(ErrorEnum::eNone))); + + EXPECT_CALL(mNetworkManager, StopInstanceNetwork(String("item0"), _)) + .Times(AtLeast(1)) + .WillRepeatedly(Return(ErrorEnum::eNone)); + + EXPECT_CALL(mRuntime1, StartInstance).WillOnce(Invoke([](const InstanceInfo& instance, InstanceStatus& status) { + SetInstanceStatus(instance, InstanceStateEnum::eActive, status); + + return ErrorEnum::eNone; + })); + + err = mLauncher.Start(); + ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); + + err = mLauncher.GetInstancesStatuses(mReceivedStatuses); + ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); + + ASSERT_EQ(mReceivedStatuses.Size(), cStoredInfos.size()); + + for (const auto& status : mReceivedStatuses) { + if (status.mItemID == "item0") { + EXPECT_EQ(status.mState, InstanceStateEnum::eFailed); + } else { + EXPECT_EQ(status.mState, InstanceStateEnum::eActive); + } + } + + EXPECT_CALL(mRuntime0, StopInstance(static_cast(cStoredInfos[0]), _)) + .WillOnce(Return(ErrorEnum::eNone)); + EXPECT_CALL(mRuntime1, StopInstance(static_cast(cStoredInfos[1]), _)) + .WillOnce(Return(ErrorEnum::eNone)); + + err = mLauncher.Stop(); + ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); +} + TEST_F(LauncherTest, StopInstancesWithExpiredOfflineTTL) { const std::vector cStoredInfos = { From c63a9e0b3d6835bb9a73f8f15234ae820423df93 Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Thu, 23 Jul 2026 14:05:11 +0300 Subject: [PATCH 13/16] sm: networkmanager: add AbortBatch to firewall and traffic monitor Discarding a staged batch without applying it is not expressible with FlushBatch/Revert: FlushBatch commits and Revert undoes an already applied batch. Add AbortBatch to both backend interfaces and mocks. Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- src/core/sm/networkmanager/itf/firewall.hpp | 8 ++++++++ src/core/sm/networkmanager/itf/trafficmonitor.hpp | 8 ++++++++ src/core/sm/networkmanager/tests/mocks/firewallmock.hpp | 1 + .../sm/networkmanager/tests/mocks/trafficmonitormock.hpp | 1 + 4 files changed, 18 insertions(+) diff --git a/src/core/sm/networkmanager/itf/firewall.hpp b/src/core/sm/networkmanager/itf/firewall.hpp index a3dddca1e..37aa47f58 100644 --- a/src/core/sm/networkmanager/itf/firewall.hpp +++ b/src/core/sm/networkmanager/itf/firewall.hpp @@ -173,6 +173,14 @@ class FirewallItf { */ virtual Error FlushBatch() = 0; + /** + * Discards the staged batch and leaves batch mode without applying anything to the kernel + * (unlike FlushBatch which commits, or Revert which undoes an already-applied batch). + * + * @return Error. + */ + virtual Error AbortBatch() = 0; + /** * Reverts the flushed batch, deleting everything it applied by handle. * diff --git a/src/core/sm/networkmanager/itf/trafficmonitor.hpp b/src/core/sm/networkmanager/itf/trafficmonitor.hpp index d71ac7951..f5566c532 100644 --- a/src/core/sm/networkmanager/itf/trafficmonitor.hpp +++ b/src/core/sm/networkmanager/itf/trafficmonitor.hpp @@ -119,6 +119,14 @@ class TrafficMonitorItf { */ virtual Error FlushBatch() = 0; + /** + * Discards the staged batch and leaves batch mode without applying anything to the kernel + * (unlike FlushBatch which commits, or Revert which undoes an already-applied batch). + * + * @return Error. + */ + virtual Error AbortBatch() = 0; + /** * Reverts the flushed batch, deleting everything it applied by handle. * diff --git a/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp b/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp index 37d70861a..325d83283 100644 --- a/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp +++ b/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp @@ -25,6 +25,7 @@ class FirewallMock : public FirewallItf { MOCK_METHOD(Error, RemoveMasquerade, (const String&, const String&), (override)); MOCK_METHOD(Error, BeginBatch, (), (override)); MOCK_METHOD(Error, FlushBatch, (), (override)); + MOCK_METHOD(Error, AbortBatch, (), (override)); MOCK_METHOD(Error, Revert, (), (override)); }; diff --git a/src/core/sm/networkmanager/tests/mocks/trafficmonitormock.hpp b/src/core/sm/networkmanager/tests/mocks/trafficmonitormock.hpp index 7dcc873a4..0e6256cf8 100644 --- a/src/core/sm/networkmanager/tests/mocks/trafficmonitormock.hpp +++ b/src/core/sm/networkmanager/tests/mocks/trafficmonitormock.hpp @@ -24,6 +24,7 @@ class TrafficMonitorMock : public TrafficMonitorItf { MOCK_METHOD(Error, GetInstanceTraffic, (const String&, uint64_t&, uint64_t&), (const, override)); MOCK_METHOD(Error, BeginBatch, (), (override)); MOCK_METHOD(Error, FlushBatch, (), (override)); + MOCK_METHOD(Error, AbortBatch, (), (override)); MOCK_METHOD(Error, Revert, (), (override)); }; From 11a78c62fde0150802c5d39b2ddc2b8b939169df Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Fri, 24 Jul 2026 12:54:42 +0300 Subject: [PATCH 14/16] sm: networkmanager: recover each backend on batch flush failure FlushBatch commits the firewall, traffic-monitor and storage backends in sequence and gives each failure its own recovery so a partial batch never leaves one backend applied while another is not: a firewall-flush failure aborts the still-staged traffic batch, a traffic-flush failure reverts the already-flushed firewall, and a storage-commit failure reverts both. The storage transaction is rolled back on every failure path, and the batch entries are re-applied per instance so one bad instance is isolated instead of failing the whole flush. Cover the firewall-, traffic- and commit-failure paths with tests. Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- src/core/sm/networkmanager/networkmanager.cpp | 87 ++++++++++--------- src/core/sm/networkmanager/networkmanager.hpp | 2 + .../networkmanager/tests/networkmanager.cpp | 66 ++++++++++++++ 3 files changed, 114 insertions(+), 41 deletions(-) diff --git a/src/core/sm/networkmanager/networkmanager.cpp b/src/core/sm/networkmanager/networkmanager.cpp index 02632026a..b2f619ac0 100644 --- a/src/core/sm/networkmanager/networkmanager.cpp +++ b/src/core/sm/networkmanager/networkmanager.cpp @@ -573,7 +573,7 @@ Error NetworkManager::BeginBatch() auto cleanupFirewall = DeferRelease(this, [&err](NetworkManager* self) { if (!err.IsNone()) { - self->mFirewall->Revert(); + self->mFirewall->AbortBatch(); } }); @@ -588,60 +588,65 @@ Error NetworkManager::FlushBatch(Array>& failedInstanceIDs) { failedInstanceIDs.Clear(); - Error err; - bool firewallApplied = false; - bool trafficApplied = false; + if (auto err = mFirewall->FlushBatch(); !err.IsNone()) { + LOG_ERR() << "Failed to flush firewall batch" << Log::Field(err); - { - auto cleanupFirewall = DeferRelease(this, [&err, &firewallApplied](NetworkManager* self) { - if (!err.IsNone() && firewallApplied) { - self->mFirewall->Revert(); - } - }); + mNetMonitor->AbortBatch(); + mStorage->RollbackTransaction(); - auto cleanupTraffic = DeferRelease(this, [&err, &trafficApplied](NetworkManager* self) { - if (!err.IsNone() && trafficApplied) { - self->mNetMonitor->Revert(); - } - }); + ReapplyBatchEntries(failedInstanceIDs); + ClearBatchState(); - if (err = mFirewall->FlushBatch(); err.IsNone()) { - firewallApplied = true; + return ErrorEnum::eNone; + } - if (err = mNetMonitor->FlushBatch(); err.IsNone()) { - trafficApplied = true; + if (auto err = mNetMonitor->FlushBatch(); !err.IsNone()) { + LOG_ERR() << "Failed to flush traffic monitor batch" << Log::Field(err); - err = mStorage->CommitTransaction(); - } - } + mFirewall->Revert(); + mStorage->RollbackTransaction(); - if (!err.IsNone() && (!firewallApplied || !trafficApplied)) { - mStorage->RollbackTransaction(); - } + ReapplyBatchEntries(failedInstanceIDs); + ClearBatchState(); + + return ErrorEnum::eNone; } - if (!err.IsNone()) { - if (firewallApplied && trafficApplied) { - for (const auto& entry : mBatchEntries) { - failedInstanceIDs.PushBack(entry.mInstanceID); - } - } else { - for (const auto& entry : mBatchEntries) { - if (auto errReapply = ReapplyInstancePolicy(entry); !errReapply.IsNone()) { - failedInstanceIDs.PushBack(entry.mInstanceID); - } - } + if (auto err = mStorage->CommitTransaction(); !err.IsNone()) { + LOG_ERR() << "Failed to commit batch transaction" << Log::Field(err); + + mFirewall->Revert(); + mNetMonitor->Revert(); + mStorage->RollbackTransaction(); + + for (const auto& entry : mBatchEntries) { + failedInstanceIDs.PushBack(entry.mInstanceID); } } - { - LockGuard lock {mMutex}; + ClearBatchState(); - mBatchMode = false; - mBatchEntries.Clear(); + return ErrorEnum::eNone; +} + +void NetworkManager::ReapplyBatchEntries(Array>& failedInstanceIDs) +{ + for (const auto& entry : mBatchEntries) { + if (auto err = ReapplyInstancePolicy(entry); !err.IsNone()) { + LOG_ERR() << "Failed to reapply instance policy" << Log::Field("instanceID", entry.mInstanceID) + << Log::Field(err); + + failedInstanceIDs.PushBack(entry.mInstanceID); + } } +} - return ErrorEnum::eNone; +void NetworkManager::ClearBatchState() +{ + LockGuard lock {mMutex}; + + mBatchMode = false; + mBatchEntries.Clear(); } Error NetworkManager::ReapplyInstancePolicy(const BatchEntry& entry) diff --git a/src/core/sm/networkmanager/networkmanager.hpp b/src/core/sm/networkmanager/networkmanager.hpp index 0e14ea9f7..eb58deb5e 100644 --- a/src/core/sm/networkmanager/networkmanager.hpp +++ b/src/core/sm/networkmanager/networkmanager.hpp @@ -259,6 +259,8 @@ class NetworkManager : public NetworkManagerItf { Error AddInstanceToNetwork(const String& instanceID, const String& networkID, const InstanceNetworkConfig& networkConfig, const aos::InstanceNetworkAllocation& networkParams); Error ReapplyInstancePolicy(const BatchEntry& entry); + void ReapplyBatchEntries(Array>& failedInstanceIDs); + void ClearBatchState(); Error RemoveDNSOrphans(); Error AdoptDNSServer(const String& networkID); Error PrepareBridgeParams( diff --git a/src/core/sm/networkmanager/tests/networkmanager.cpp b/src/core/sm/networkmanager/tests/networkmanager.cpp index a045d00bc..87d52f3bd 100644 --- a/src/core/sm/networkmanager/tests/networkmanager.cpp +++ b/src/core/sm/networkmanager/tests/networkmanager.cpp @@ -636,6 +636,7 @@ TEST_F(NetworkManagerTest, BeginFlushBatch_ForwardToBackends) EXPECT_CALL(mFirewall, FlushBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mTrafficMonitor, FlushBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mStorage, CommitTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, AbortBatch()).Times(0); aos::StaticArray, aos::cMaxNumInstances> failed; @@ -717,6 +718,8 @@ TEST_F(NetworkManagerTest, FlushBatch_NftFailure_RevertsAndFallsBack) ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID2, networkID), aos::ErrorEnum::eNone); EXPECT_CALL(mFirewall, FlushBatch()).WillOnce(Return(aos::ErrorEnum::eRuntime)); + EXPECT_CALL(mTrafficMonitor, AbortBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, FlushBatch()).Times(0); EXPECT_CALL(mStorage, RollbackTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); aos::StaticArray, aos::cMaxNumInstances> failed; @@ -726,6 +729,67 @@ TEST_F(NetworkManagerTest, FlushBatch_NftFailure_RevertsAndFallsBack) EXPECT_EQ(failed[0], instanceID2); } +TEST_F(NetworkManagerTest, FlushBatch_NftFailure_AbortsTrafficBatchBeforeReapply) +{ + const aos::String instanceID = "test-instance"; + const aos::String networkID = "test-network"; + auto params = CreateTestInstanceNetworkConfig(); + auto allocatedParams = CreateTestAllocatedParams(); + + SetupEnsureNodeNetworkCreateMocks(networkID, allocatedParams.mSubnet, "192.168.1.1", 100ULL); + + EXPECT_CALL(mNetworkProvider, AllocateInstanceNetwork(_, networkID, aos::String("test-node"), _, _)) + .WillOnce(DoAll(SetArgReferee<4>(allocatedParams), Return(aos::ErrorEnum::eNone))); + EXPECT_CALL(mStorage, AddInstanceNetworkInfo(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->CreateInstanceNetwork(instanceID, networkID, params), aos::ErrorEnum::eNone); + + EXPECT_CALL(mStorage, BeginTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->BeginBatch(), aos::ErrorEnum::eNone); + + SetupEnsureNodeNetworkPhysicalMocks("192.168.1.1", allocatedParams.mSubnet, 100ULL); + + BridgeAttachResult attachResult; + attachResult.mHostIfName = "veth-test"; + attachResult.mContainerIfName = "eth0"; + + EXPECT_CALL(mBridgeNetwork, Attach(_, _, _)) + .WillOnce(DoAll(SetArgReferee<2>(attachResult), Return(aos::ErrorEnum::eNone))); + EXPECT_CALL(mBandwidth, Apply(_, _)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mDNSServer, AddHost(_, _)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, CreateNetworkNamespace(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, GetNetworkNamespacePath(_)) + .WillOnce(Return(aos::RetWithError> {{}, aos::ErrorEnum::eNone})); + EXPECT_CALL(mFirewall, AddInstance(instanceID, _)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mStorage, UpdateInstanceNetworkInfo(_)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + + // The batch is dead once the firewall flush fails, so the traffic batch must be dropped before + // the per-instance fallback re-applies monitoring, otherwise the re-apply is staged and lost. + Sequence trafficSeq; + + EXPECT_CALL(mTrafficMonitor, StartInstanceMonitoring(instanceID, _, _, _)) + .InSequence(trafficSeq) + .WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, AbortBatch()).InSequence(trafficSeq).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, StartInstanceMonitoring(instanceID, _, _, _)) + .InSequence(trafficSeq) + .WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID, networkID), aos::ErrorEnum::eNone); + + EXPECT_CALL(mFirewall, FlushBatch()).WillOnce(Return(aos::ErrorEnum::eRuntime)); + EXPECT_CALL(mTrafficMonitor, FlushBatch()).Times(0); + EXPECT_CALL(mStorage, RollbackTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + + aos::StaticArray, aos::cMaxNumInstances> failed; + + EXPECT_EQ(mNetManager->FlushBatch(failed), aos::ErrorEnum::eNone); + EXPECT_TRUE(failed.IsEmpty()); +} + TEST_F(NetworkManagerTest, FlushBatch_CommitFailure_MarksAllFailed) { const aos::String instanceID1 = "test-instance-1"; @@ -778,6 +842,8 @@ TEST_F(NetworkManagerTest, FlushBatch_CommitFailure_MarksAllFailed) EXPECT_CALL(mStorage, CommitTransaction()).WillOnce(Return(aos::ErrorEnum::eRuntime)); EXPECT_CALL(mFirewall, Revert()).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mTrafficMonitor, Revert()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mStorage, RollbackTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, AbortBatch()).Times(0); aos::StaticArray, aos::cMaxNumInstances> failed; From 355e22ff2b3a89fe213893412f2da157d6554961 Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Mon, 3 Aug 2026 17:17:52 +0300 Subject: [PATCH 15/16] sm: networkmanager: adopt DNS server for running instances on reconcile ReconcileInstances registered the network DNS server (AdoptDNSServer) only on the leftover-cleanup path, not for instances adopted as running via InitInstance. DeleteInstanceNetworkConfig then could not find the DNS server ("DNS server not found for cleanup") and left stale addnhosts entries when such an instance was later removed. Adopt the DNS server before continuing the running-instance branch too. AdoptDNSServer is idempotent, so multiple instances on one network are safe; a failure is logged and does not abort adoption. Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- src/core/sm/networkmanager/networkmanager.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/core/sm/networkmanager/networkmanager.cpp b/src/core/sm/networkmanager/networkmanager.cpp index b2f619ac0..b4242480a 100644 --- a/src/core/sm/networkmanager/networkmanager.cpp +++ b/src/core/sm/networkmanager/networkmanager.cpp @@ -1330,6 +1330,11 @@ Error NetworkManager::ReconcileInstances() if (alive) { if (err = InitInstance(entry.mInstanceID, entry.mNetworkID); err.IsNone()) { + if (auto dnsErr = AdoptDNSServer(entry.mNetworkID); !dnsErr.IsNone()) { + LOG_WRN() << "Failed to adopt DNS server for running instance" + << Log::Field("networkID", entry.mNetworkID) << Log::Field(dnsErr); + } + continue; } else { LOG_WRN() << "Failed to adopt leftover instance, falling back to cleanup" From 06f9dc0558942d37d885f8e4d0d173d896c48ddd Mon Sep 17 00:00:00 2001 From: Mykola Solianko Date: Mon, 3 Aug 2026 17:36:45 +0300 Subject: [PATCH 16/16] sm: networkmanager: test DNS server adoption for running instances Adopt a running leftover instance on restart, assert the network DNS server is registered (CreateServer), and that a subsequent StopInstanceNetwork drops the instance host entry (RemoveHost). Without the reconcile fix neither call happens, so the test fails. Update Start_KeepsLeftoverInstanceWithLiveInterface to expect the DNS server adoption too. Signed-off-by: Mykola Solianko Reviewed-by: Mykola Kobets Reviewed-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko --- .../networkmanager/tests/networkmanager.cpp | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/src/core/sm/networkmanager/tests/networkmanager.cpp b/src/core/sm/networkmanager/tests/networkmanager.cpp index 87d52f3bd..8432aaa45 100644 --- a/src/core/sm/networkmanager/tests/networkmanager.cpp +++ b/src/core/sm/networkmanager/tests/networkmanager.cpp @@ -302,7 +302,9 @@ class NetworkManagerTest : public Test { void ExpectLeftoverInstanceUntouched() { - EXPECT_CALL(mDNSName, CreateServer(_, _)).Times(0); + // Adopting a running instance registers the network DNS server so later cleanup can reach it. + EXPECT_CALL(mDNSName, CreateServer(_, _)) + .WillOnce(Return(aos::RetWithError {&mDNSServer, aos::ErrorEnum::eNone})); EXPECT_CALL(mDNSServer, RemoveHost(_)).Times(0); EXPECT_CALL(mBandwidth, Clear(_)).Times(0); EXPECT_CALL(mFirewall, RemoveInstance(_)).Times(0); @@ -1674,6 +1676,44 @@ TEST_F(NetworkManagerTest, Start_KeepsLeftoverInstanceWithLiveInterface) mNetManager->StartInstanceNetwork(leftover.mInstanceID, network.mNetworkID).Is(aos::ErrorEnum::eAlreadyExist)); } +TEST_F(NetworkManagerTest, Start_AdoptsDNSServerForRunningInstanceCleanedOnStop) +{ + const auto network = CreateTestNetworkInfo(); + const auto leftover = CreateLeftoverInstance(network); + + aos::StaticArray networks; + aos::StaticArray instances; + networks.PushBack(network); + instances.PushBack(leftover); + + RestartWithStoredState(networks, instances); + + ExpectLinkExists(leftover.mHostIfName, LinkKindEnum::eVeth, network.mBridgeIfName); + EXPECT_CALL(mNetns, IsNetworkNamespaceExist(leftover.mInstanceID)) + .WillRepeatedly(Return(aos::RetWithError {true, aos::ErrorEnum::eNone})); + + EXPECT_CALL(mDNSName, RemoveOrphans(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + + // Adopting the running instance must register the network DNS server. + EXPECT_CALL(mDNSName, CreateServer(network.mNetworkID, _)) + .WillOnce(Return(aos::RetWithError {&mDNSServer, aos::ErrorEnum::eNone})); + EXPECT_CALL(mTrafficMonitor, + StartInstanceMonitoring(leftover.mInstanceID, leftover.mAllocatedParams.mIP, + leftover.mNetworkConfig.mDownloadLimit, leftover.mNetworkConfig.mUploadLimit)) + .WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->Start(), aos::ErrorEnum::eNone); + + // Stopping the adopted instance must reach the DNS server and drop its host entry. + EXPECT_CALL(mTrafficMonitor, StopInstanceMonitoring(leftover.mInstanceID)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mDNSServer, RemoveHost(leftover.mInstanceID)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, RemoveInstance(leftover.mInstanceID)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, DeleteNetworkNamespace(leftover.mInstanceID)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mStorage, UpdateInstanceNetworkInfo(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + + EXPECT_EQ(mNetManager->StopInstanceNetwork(leftover.mInstanceID, network.mNetworkID), aos::ErrorEnum::eNone); +} + TEST_F(NetworkManagerTest, Start_CleansLeftoverInstanceWhenNamespaceMissing) { const auto network = CreateTestNetworkInfo();