fw_update: Introduce AggregateUpdateManager
Description:
This commit introduces the `AggregateUpdateManager` class for handling
multiple firmware update sessions simultaneously. The
`AggregateUpdateManager` acts as a reverse proxy for PLDM messages,
routing them to the appropriate `UpdateManager` instance based on the
`instanceId`. This allows for concurrent firmware updates, improving
the efficiency and flexibility of the firmware update process.
Motivation:
We have introduced the `FirmwareInventory`/`FirmwareInventoryManager`
classes to manage D-Bus interfaces for firmware update, After that, we
would like to implement the `StartUpdate` method for `FirmwareInventory`
properly for the update process implementation[1], which
`AggregateUpdateManager` will help route the command to the specific
update sessions.
Details of the PLDM message reverse proxy:
By the implementation, different update session would be handled with
different `instanceId`s, which can help `UpdateManager`s to identify
whether the message is for them or not.
Whenever a PLDM response message is received, the flow that
the `AggregateUpdateManager` do:
1. Handle the message for existing update flow if the instance_id
(the pldm instance numbers) matches the update task which using the
existing update flow.
2. Traverse forward the message to each `ItemUpdateManager`
(implementation in the next patch) instances if there's no match in
step 1.
3. Return error message PLDM_FWUP_COMMAND_NOT_EXPECTED if no matches in
step 2.
[1]: https://gerrit.openbmc.org/c/openbmc/pldm/+/74774
Change-Id: Icfdb8d238121f9f44a624396e00b378e491ce652
Signed-off-by: Unive Tien <unive.tien.wiwynn@gmail.com>
diff --git a/fw-update/aggregate_update_manager.cpp b/fw-update/aggregate_update_manager.cpp
new file mode 100644
index 0000000..c27db95
--- /dev/null
+++ b/fw-update/aggregate_update_manager.cpp
@@ -0,0 +1,54 @@
+#include "aggregate_update_manager.hpp"
+
+namespace pldm::fw_update
+{
+
+Response AggregateUpdateManager::handleRequest(
+ mctp_eid_t eid, uint8_t command, const pldm_msg* request, size_t reqMsgLen)
+{
+ Response response;
+ response = UpdateManager::handleRequest(eid, command, request, reqMsgLen);
+ auto responseMsg = new (response.data()) pldm_msg;
+ if (responseMsg->payload[0] != PLDM_FWUP_COMMAND_NOT_EXPECTED)
+ {
+ return response;
+ }
+ for (auto& [_, updateManager] : updateManagers)
+ {
+ response =
+ updateManager->handleRequest(eid, command, request, reqMsgLen);
+ if (responseMsg->payload[0] != PLDM_FWUP_COMMAND_NOT_EXPECTED)
+ {
+ return response;
+ }
+ }
+ return response;
+}
+
+void AggregateUpdateManager::eraseUpdateManager(
+ const SoftwareIdentifier& softwareIdentifier)
+{
+ updateManagers.erase(softwareIdentifier);
+ descriptorMap.erase(softwareIdentifier);
+ componentInfoMap.erase(softwareIdentifier);
+}
+
+void AggregateUpdateManager::eraseUpdateManagerIf(
+ std::function<bool(const SoftwareIdentifier&)>&& predicate)
+{
+ for (auto it = updateManagers.begin(); it != updateManagers.end();)
+ {
+ if (predicate(it->first))
+ {
+ descriptorMap.erase(it->first);
+ componentInfoMap.erase(it->first);
+ it = updateManagers.erase(it);
+ }
+ else
+ {
+ ++it;
+ }
+ }
+}
+
+} // namespace pldm::fw_update
diff --git a/fw-update/aggregate_update_manager.hpp b/fw-update/aggregate_update_manager.hpp
new file mode 100644
index 0000000..eb15dc9
--- /dev/null
+++ b/fw-update/aggregate_update_manager.hpp
@@ -0,0 +1,116 @@
+#pragma once
+
+#include "update_manager.hpp"
+
+namespace pldm::fw_update
+{
+
+class AggregateUpdateManager : public UpdateManager
+{
+ public:
+ AggregateUpdateManager() = delete;
+ AggregateUpdateManager(const AggregateUpdateManager&) = delete;
+ AggregateUpdateManager(AggregateUpdateManager&&) = delete;
+ AggregateUpdateManager& operator=(const AggregateUpdateManager&) = delete;
+ AggregateUpdateManager& operator=(AggregateUpdateManager&&) = delete;
+
+ /**
+ * @brief Constructor for AggregateUpdateManager
+ *
+ * @param[in] event - Reference to the PLDM daemon's main event loop
+ * @param[in] handler - PLDM request handler
+ * @param[in] instanceIdDb - Reference to the instance ID database
+ * @param[in] descriptorMap - Descriptor map for the update manager
+ * @param[in] componentInfoMap - Component information map for the update
+ * manager
+ */
+ explicit AggregateUpdateManager(
+ Event& event,
+ pldm::requester::Handler<pldm::requester::Request>& handler,
+ InstanceIdDb& instanceIdDb, const DescriptorMap& descriptorMap,
+ const ComponentInfoMap& componentInfoMap) :
+ UpdateManager(event, handler, instanceIdDb, descriptorMap,
+ componentInfoMap)
+ {}
+
+ /**
+ * @brief Handle PLDM requests for the aggregate update manager
+ *
+ * This function processes incoming PLDM requests and dispatches them to the
+ * appropriate update manager based on the software identifier.
+ *
+ * @param[in] eid - Remote MCTP Endpoint ID
+ * @param[in] command - PLDM command code
+ * @param[in] request - PLDM request message
+ * @param[in] reqMsgLen - PLDM request message length
+ * @return PLDM response message
+ */
+ Response handleRequest(mctp_eid_t eid, uint8_t command,
+ const pldm_msg* request, size_t reqMsgLen) override;
+
+ /**
+ * @brief Create a new UpdateManager instance for a specific software
+ * identifier
+ *
+ * This function creates and stores a new UpdateManager instance associated
+ * with the given software identifier, along with its corresponding
+ * descriptor and component information maps.
+ *
+ * @param[in] softwareIdentifier - The software identifier (pair of eid and
+ * component identifier)
+ * @param[in] descriptors - The descriptors associated with the software
+ * identifier
+ * @param[in] componentInfo - The component information associated with the
+ * software identifier
+ * @param[in] updateObjPath - The D-Bus object path for the update manager
+ */
+ void createUpdateManager(const SoftwareIdentifier& softwareIdentifier,
+ const Descriptors& descriptors,
+ const ComponentInfo& componentInfo,
+ const std::string& updateObjPath);
+
+ /**
+ * @brief Erase an existing UpdateManager instance associated with a
+ * specific software identifier
+ *
+ * This function removes the UpdateManager instance and its associated
+ * descriptor and component information maps from the internal storage based
+ * on the provided software identifier.
+ *
+ * @param[in] softwareIdentifier - The software identifier (pair of eid and
+ * component identifier)
+ */
+ void eraseUpdateManager(const SoftwareIdentifier& softwareIdentifier);
+
+ /**
+ * @brief Erase UpdateManager instances that satisfy a given predicate
+ *
+ * This function iterates through the stored UpdateManager instances and
+ * removes those that satisfy the provided predicate function. It also
+ * removes the associated descriptor and component information maps.
+ *
+ * @param[in] predicate - A function that takes a SoftwareIdentifier and
+ * returns true if the corresponding UpdateManager should be erased
+ */
+ void eraseUpdateManagerIf(
+ std::function<bool(const SoftwareIdentifier&)>&& predicate);
+
+ private:
+ /**
+ * @brief Map of UpdateManager instances keyed by software identifier
+ */
+ std::map<SoftwareIdentifier, std::unique_ptr<UpdateManager>> updateManagers;
+
+ /**
+ * @brief Map of descriptor maps keyed by software identifier
+ */
+ std::map<SoftwareIdentifier, std::unique_ptr<Descriptors>> descriptorMap;
+
+ /**
+ * @brief Map of component information maps keyed by software identifier
+ */
+ std::map<SoftwareIdentifier, std::unique_ptr<ComponentInfo>>
+ componentInfoMap;
+};
+
+} // namespace pldm::fw_update
diff --git a/fw-update/firmware_inventory.cpp b/fw-update/firmware_inventory.cpp
index e59b5ba..ffbb4e8 100644
--- a/fw-update/firmware_inventory.cpp
+++ b/fw-update/firmware_inventory.cpp
@@ -6,7 +6,6 @@
FirmwareInventory::FirmwareInventory(
SoftwareIdentifier /*softwareIdentifier*/, const std::string& softwarePath,
const std::string& softwareVersion, const std::string& associatedEndpoint,
- const Descriptors& /*descriptors*/, const ComponentInfo& /*componentInfo*/,
SoftwareVersionPurpose purpose) :
softwarePath(softwarePath),
association(this->bus, this->softwarePath.c_str()),
diff --git a/fw-update/firmware_inventory.hpp b/fw-update/firmware_inventory.hpp
index 729989e..9d6dda8 100644
--- a/fw-update/firmware_inventory.hpp
+++ b/fw-update/firmware_inventory.hpp
@@ -52,8 +52,6 @@
SoftwareIdentifier /*softwareIdentifier*/,
const std::string& softwarePath, const std::string& softwareVersion,
const std::string& associatedEndpoint,
- const Descriptors& /*descriptors*/,
- const ComponentInfo& /*componentInfo*/,
SoftwareVersionPurpose purpose = SoftwareVersionPurpose::Unknown);
private:
diff --git a/fw-update/firmware_inventory_manager.cpp b/fw-update/firmware_inventory_manager.cpp
index bb9a6aa..a3e0ac9 100644
--- a/fw-update/firmware_inventory_manager.cpp
+++ b/fw-update/firmware_inventory_manager.cpp
@@ -13,7 +13,7 @@
void FirmwareInventoryManager::createFirmwareEntry(
const SoftwareIdentifier& softwareIdentifier,
const SoftwareName& softwareName, const std::string& activeVersion,
- const Descriptors& descriptors, const ComponentInfo& componentInfo)
+ const Descriptors& /*descriptors*/, const ComponentInfo& /*componentInfo*/)
{
struct timespec ts;
clock_gettime(CLOCK_REALTIME, &ts);
@@ -42,15 +42,19 @@
softwareName, utils::generateSwId());
softwareMap.insert_or_assign(
- softwareIdentifier, std::make_unique<FirmwareInventory>(
- softwareIdentifier, softwarePath, activeVersion,
- *boardPath, descriptors, componentInfo));
+ softwareIdentifier,
+ std::make_unique<FirmwareInventory>(softwareIdentifier, softwarePath,
+ activeVersion, *boardPath));
}
void FirmwareInventoryManager::deleteFirmwareEntry(const pldm::eid& eid)
{
std::erase_if(softwareMap,
[&](const auto& pair) { return pair.first.first == eid; });
+ updateManager.eraseUpdateManagerIf(
+ [&](const SoftwareIdentifier& softwareIdentifier) {
+ return softwareIdentifier.first == eid;
+ });
}
std::optional<std::filesystem::path> getBoardPath(
diff --git a/fw-update/firmware_inventory_manager.hpp b/fw-update/firmware_inventory_manager.hpp
index e6616ce..45ffd8e 100644
--- a/fw-update/firmware_inventory_manager.hpp
+++ b/fw-update/firmware_inventory_manager.hpp
@@ -1,5 +1,6 @@
#pragma once
+#include "aggregate_update_manager.hpp"
#include "common/types.hpp"
#include "firmware_inventory.hpp"
@@ -40,8 +41,9 @@
*/
explicit FirmwareInventoryManager(
const pldm::utils::DBusHandler* dbusHandler,
- const Configurations& config) :
- dbusHandler(dbusHandler), configurations(config)
+ const Configurations& config, AggregateUpdateManager& updateManager) :
+ dbusHandler(dbusHandler), configurations(config),
+ updateManager(updateManager)
{}
/**
@@ -96,6 +98,11 @@
* the initialization of the FirmwareInventoryManager.
*/
const Configurations& configurations;
+
+ /**
+ * @brief Reference to the aggregate update manager
+ */
+ AggregateUpdateManager& updateManager;
};
} // namespace pldm::fw_update
diff --git a/fw-update/inventory_manager.hpp b/fw-update/inventory_manager.hpp
index 673b513..e100a52 100644
--- a/fw-update/inventory_manager.hpp
+++ b/fw-update/inventory_manager.hpp
@@ -1,5 +1,6 @@
#pragma once
+#include "aggregate_update_manager.hpp"
#include "common/instance_id.hpp"
#include "common/types.hpp"
#include "firmware_inventory_manager.hpp"
@@ -47,12 +48,13 @@
InstanceIdDb& instanceIdDb, DescriptorMap& descriptorMap,
DownstreamDescriptorMap& downstreamDescriptorMap,
ComponentInfoMap& componentInfoMap,
- const Configurations& configurations) :
+ const Configurations& configurations,
+ AggregateUpdateManager& updateManager) :
handler(handler), instanceIdDb(instanceIdDb),
descriptorMap(descriptorMap),
downstreamDescriptorMap(downstreamDescriptorMap),
componentInfoMap(componentInfoMap), configurations(configurations),
- firmwareInventoryManager(dbusHandler, configurations)
+ firmwareInventoryManager(dbusHandler, configurations, updateManager)
{}
/** @brief Discover the firmware identifiers and component details of FDs
diff --git a/fw-update/manager.hpp b/fw-update/manager.hpp
index 2e75f6c..39e8376 100644
--- a/fw-update/manager.hpp
+++ b/fw-update/manager.hpp
@@ -1,13 +1,13 @@
#pragma once
#include "activation.hpp"
+#include "aggregate_update_manager.hpp"
#include "common/instance_id.hpp"
#include "common/types.hpp"
#include "device_updater.hpp"
#include "inventory_manager.hpp"
#include "requester/handler.hpp"
#include "requester/mctp_endpoint_discovery.hpp"
-#include "update_manager.hpp"
#include <unordered_map>
#include <vector>
@@ -48,10 +48,11 @@
explicit Manager(const pldm::utils::DBusHandler* dbusHandler, Event& event,
requester::Handler<requester::Request>& handler,
pldm::InstanceIdDb& instanceIdDb) :
- inventoryMgr(dbusHandler, handler, instanceIdDb, descriptorMap,
- downstreamDescriptorMap, componentInfoMap, configurations),
updateManager(event, handler, instanceIdDb, descriptorMap,
- componentInfoMap)
+ componentInfoMap),
+ inventoryMgr(dbusHandler, handler, instanceIdDb, descriptorMap,
+ downstreamDescriptorMap, componentInfoMap, configurations,
+ updateManager)
{}
/** @brief Helper function to invoke registered handlers for
@@ -134,11 +135,11 @@
/** Configuration bindings from the Entity Manager */
Configurations configurations;
+ /** @brief PLDM firmware update manager */
+ AggregateUpdateManager updateManager;
+
/** @brief PLDM firmware inventory manager */
InventoryManager inventoryMgr;
-
- /** @brief PLDM firmware update manager */
- UpdateManager updateManager;
};
} // namespace fw_update
diff --git a/fw-update/test/firmware_inventory_manager_test.cpp b/fw-update/test/firmware_inventory_manager_test.cpp
index 4814838..c7effaa 100644
--- a/fw-update/test/firmware_inventory_manager_test.cpp
+++ b/fw-update/test/firmware_inventory_manager_test.cpp
@@ -1,6 +1,7 @@
#include "common/test/mocked_utils.hpp"
#include "fw-update/firmware_inventory.hpp"
#include "fw-update/firmware_inventory_manager.hpp"
+#include "test/test_instance_id.hpp"
#include <gmock/gmock.h>
#include <gtest/gtest.h>
@@ -33,8 +34,9 @@
{
public:
FirmwareInventoryManagerTest(const pldm::utils::DBusHandler* handler,
- const Configurations& config) :
- FirmwareInventoryManager(handler, config)
+ const Configurations& config,
+ AggregateUpdateManager& updateManager) :
+ FirmwareInventoryManager(handler, config, updateManager)
{}
SoftwareMap& getSoftwareMap()
@@ -64,7 +66,19 @@
endpointId, endpointUuid, endpointMedium, endpointNetId, endpointName);
configurations[boardInventoryPath] = endpointInfo;
- FirmwareInventoryManagerTest inventoryManager(&mockHandler, configurations);
+ Event event(sdeventplus::Event::get_default());
+ TestInstanceIdDb instanceIdDb;
+ requester::Handler<requester::Request> handler(
+ nullptr, event, instanceIdDb, false, seconds(1), 2, milliseconds(100));
+
+ DescriptorMap descriptorMap{};
+ ComponentInfoMap componentInfoMap{};
+
+ AggregateUpdateManager updateManager(event, handler, instanceIdDb,
+ descriptorMap, componentInfoMap);
+
+ FirmwareInventoryManagerTest inventoryManager(&mockHandler, configurations,
+ updateManager);
SoftwareIdentifier softwareIdentifier{endpointId, 100};
SoftwareName softwareName{"TestDevice"};
diff --git a/fw-update/test/firmware_inventory_test.cpp b/fw-update/test/firmware_inventory_test.cpp
index 46611bd..47d8396 100644
--- a/fw-update/test/firmware_inventory_test.cpp
+++ b/fw-update/test/firmware_inventory_test.cpp
@@ -36,10 +36,9 @@
ComponentInfo firmwareComponentInfo;
SoftwareVersionPurpose expectedPurpose = SoftwareVersionPurpose::Unknown;
- FirmwareInventoryTest inventory(
- softwareIdentifier, expectedSoftwarePath, expectedSoftwareVersion,
- expectedEndpointPath, firmwareDescriptors, firmwareComponentInfo,
- expectedPurpose);
+ FirmwareInventoryTest inventory(softwareIdentifier, expectedSoftwarePath,
+ expectedSoftwareVersion,
+ expectedEndpointPath, expectedPurpose);
EXPECT_EQ(inventory.getSoftwarePath(), expectedSoftwarePath);
auto associationTuples = inventory.getAssociation().associations();
diff --git a/fw-update/test/inventory_manager_test.cpp b/fw-update/test/inventory_manager_test.cpp
index f2fd789..5e76b46 100644
--- a/fw-update/test/inventory_manager_test.cpp
+++ b/fw-update/test/inventory_manager_test.cpp
@@ -1,4 +1,5 @@
#include "common/utils.hpp"
+#include "fw-update/aggregate_update_manager.hpp"
#include "fw-update/inventory_manager.hpp"
#include "requester/test/mock_request.hpp"
#include "test/test_instance_id.hpp"
@@ -18,9 +19,11 @@
event(sdeventplus::Event::get_default()), instanceIdDb(),
reqHandler(nullptr, event, instanceIdDb, false, seconds(1), 2,
milliseconds(100)),
+ updateManager(event, reqHandler, instanceIdDb, outDescriptorMap,
+ outComponentInfoMap),
inventoryManager(&dBusHandler, reqHandler, instanceIdDb,
outDescriptorMap, outDownstreamDescriptorMap,
- outComponentInfoMap, configurations)
+ outComponentInfoMap, configurations, updateManager)
{}
int fd = -1;
@@ -28,6 +31,7 @@
sdeventplus::Event event;
TestInstanceIdDb instanceIdDb;
requester::Handler<requester::Request> reqHandler;
+ AggregateUpdateManager updateManager;
InventoryManager inventoryManager;
DescriptorMap outDescriptorMap{};
DownstreamDescriptorMap outDownstreamDescriptorMap{};
diff --git a/fw-update/test/meson.build b/fw-update/test/meson.build
index ba78fcd..7ea17cd 100644
--- a/fw-update/test/meson.build
+++ b/fw-update/test/meson.build
@@ -1,12 +1,14 @@
fw_update_test_src = declare_dependency(
sources: [
'../activation.cpp',
+ '../aggregate_update_manager.cpp',
'../inventory_manager.cpp',
'../package_parser.cpp',
'../device_updater.cpp',
'../update_manager.cpp',
'../firmware_inventory_manager.cpp',
'../firmware_inventory.cpp',
+ '../update.cpp',
'../../common/utils.cpp',
],
)
diff --git a/fw-update/update_manager.hpp b/fw-update/update_manager.hpp
index 5aa80ec..4320de1 100644
--- a/fw-update/update_manager.hpp
+++ b/fw-update/update_manager.hpp
@@ -47,7 +47,7 @@
UpdateManager(UpdateManager&&) = delete;
UpdateManager& operator=(const UpdateManager&) = delete;
UpdateManager& operator=(UpdateManager&&) = delete;
- ~UpdateManager() = default;
+ virtual ~UpdateManager() = default;
explicit UpdateManager(
Event& event,
@@ -80,8 +80,8 @@
*
* @return PLDM response message
*/
- Response handleRequest(mctp_eid_t eid, uint8_t command,
- const pldm_msg* request, size_t reqMsgLen);
+ virtual Response handleRequest(mctp_eid_t eid, uint8_t command,
+ const pldm_msg* request, size_t reqMsgLen);
int processPackage(const std::filesystem::path& packageFilePath);