ipmid: Improve fetch fru sdr delays
The GetManagedObject call is very heavy and it is called on each fru sdr
entry. Update the algorithm to use GetSubtreePaths isntead.
Tested:
Getting all sdr took 25 seconds to 12 seconds on system with a lot of
sensor + fru.
Fusion-Link:
- platform11
- http://fusion2/28f7dd96-2096-3a73-90d1-3748b7975e4a
- http://fusion2/48c96ca5-e333-399a-86ad-63a8dbf5a11f
- platform5: http://fusion2/c802786c-fa76-3b05-9e3c-7a3f07817f40
- platform15: http://fusion2/446b18d8-ca74-3352-b113-a3456ae3ff9b
- platform17: http://fusion2/13f92a2f-f08e-3436-8673-5ed66d0be0c9
Platforms-Affected: IPMI
Google-Bug-Id: 408304241
Google-Bug-Id: 411479972
Change-Id: Iff2b1f8ccd6cb3c481a6c9ac3709cda781c22dda
Signed-off-by: Willy Tu <wltu@google.com>
(cherry picked from commit 263704cbb46db2861c7fea5952083ea6590f20f7)
diff --git a/recipes-phosphor/ipmi/phosphor-ipmi-host/0009-Remove-GetManagedObject-call-for-fru-sdr.patch b/recipes-phosphor/ipmi/phosphor-ipmi-host/0009-Remove-GetManagedObject-call-for-fru-sdr.patch
new file mode 100644
index 0000000..29f70dc
--- /dev/null
+++ b/recipes-phosphor/ipmi/phosphor-ipmi-host/0009-Remove-GetManagedObject-call-for-fru-sdr.patch
@@ -0,0 +1,235 @@
+From 580ca1cb798dc5a479078f0fb51ea78429220d2e Mon Sep 17 00:00:00 2001
+From: Willy Tu <wltu@google.com>
+Date: Thu, 17 Apr 2025 21:19:14 +0000
+Subject: [PATCH] Remove GetManagedObject call for fru sdr
+
+Current algorithm will make a GetManagedObject against EntityManager for
+each Fru to get entityId/Instance information. Instead of doing that
+heavy call, we can just call GetSubtreePaths against ObjectManager to
+find objects with the dus interface that we want... and then call
+EntityManager to fetch the property if it they are the one generating
+it.
+
+Tested:
+Getting all sdr took 25 seconds to 12 seconds on system with a lot of
+sensor + fru.
+
+
+Patch Tracking Bug: b/411479972
+Upstream info / review: http://go/openbmccl/79480
+Upstream-Status: Submitted
+Justification: Will need more testing to sync to top of ipmid to pull in the patch normally.
+
+Change-Id: I7e7ec6f68fe6ee3bef6bd55948d853fba06ed695
+Signed-off-by: Willy Tu <wltu@google.com>
+---
+ dbus-sdr/storagecommands.cpp | 135 ++++++++++++++++++-----------------
+ 1 file changed, 68 insertions(+), 67 deletions(-)
+
+diff --git a/dbus-sdr/storagecommands.cpp b/dbus-sdr/storagecommands.cpp
+index a680743..cd5b098 100644
+--- a/dbus-sdr/storagecommands.cpp
++++ b/dbus-sdr/storagecommands.cpp
+@@ -25,6 +25,7 @@
+ #include <ipmid/api.hpp>
+ #include <ipmid/message.hpp>
+ #include <ipmid/types.hpp>
++#include <ipmid/utils.hpp>
+ #include <phosphor-logging/log.hpp>
+ #include <sdbusplus/message/types.hpp>
+ #include <sdbusplus/timer.hpp>
+@@ -33,6 +34,7 @@
+ #include <fstream>
+ #include <functional>
+ #include <iostream>
++#include <optional>
+ #include <stdexcept>
+ #include <string_view>
+
+@@ -79,6 +81,7 @@ using ObjectType =
+ using ManagedObjectType =
+ boost::container::flat_map<sdbusplus::message::object_path, ObjectType>;
+ using ManagedEntry = std::pair<sdbusplus::message::object_path, ObjectType>;
++using Paths = std::vector<std::string>;
+
+ constexpr static const char* selLoggerServiceName =
+ "xyz.openbmc_project.Logging.IPMI";
+@@ -593,73 +596,94 @@ ipmi_ret_t getFruSdrs([[maybe_unused]] ipmi::Context::ptr ctx, size_t index,
+ return IPMI_CC_RESPONSE_ERROR;
+ }
+ std::string name;
++ uint8_t entityID = 0;
++ uint8_t entityInstance = 0x1;
+
+ #ifdef USING_ENTITY_MANAGER_DECORATORS
+-
+- boost::container::flat_map<std::string, Value>* entityData = nullptr;
+-
+- // todo: this should really use caching, this is a very inefficient lookup
+ boost::system::error_code ec;
+
+- ManagedObjectType entities = ctx->bus->yield_method_call<ManagedObjectType>(
+- ctx->yield, ec, entityManagerServiceName,
+- "/xyz/openbmc_project/inventory", "org.freedesktop.DBus.ObjectManager",
+- "GetManagedObjects");
+-
++ Paths subtreePaths = ctx->bus->yield_method_call<Paths>(
++ ctx->yield, ec, "xyz.openbmc_project.ObjectMapper",
++ "/xyz/openbmc_project/object_mapper",
++ "xyz.openbmc_project.ObjectMapper", "GetSubTreePaths",
++ "/xyz/openbmc_project/inventory", 0,
++ std::array<const char*, 2>{
++ "xyz.openbmc_project.Inventory.Decorator.I2CDevice",
++ "xyz.openbmc_project.Inventory.Decorator.Ipmi",
++ });
+ if (ec)
+ {
+- phosphor::logging::log<phosphor::logging::level::ERR>(
+- "GetMangagedObjects for ipmiStorageGetFruInvAreaInfo failed",
+- phosphor::logging::entry("ERROR=%s", ec.message().c_str()));
+-
++ std::fprintf(
++ stderr,
++ "GetSubTreePaths for ipmiStorageGetFruInvAreaInfo failed: %s\n",
++ ec.message().c_str());
+ return ipmi::ccResponseError;
+ }
+
+- auto entity =
+- std::find_if(entities.begin(), entities.end(),
+- [bus, address, &entityData, &name](ManagedEntry& entry) {
+- auto findFruDevice = entry.second.find(
+- "xyz.openbmc_project.Inventory.Decorator.I2CDevice");
+- if (findFruDevice == entry.second.end())
++ bool foundEntity = false;
++ for (const auto& path : subtreePaths)
++ {
++ ipmi::PropertyMap i2cProperties;
++ boost::system::error_code ec = ipmi::getAllDbusProperties(
++ ctx, "xyz.openbmc_project.EntityManager", path,
++ "xyz.openbmc_project.Inventory.Decorator.I2CDevice", i2cProperties);
++ if (ec)
+ {
+- return false;
++ continue;
+ }
+
+- // Integer fields added via Entity-Manager json are uint64_ts by
+- // default.
+- auto findBus = findFruDevice->second.find("Bus");
+- auto findAddress = findFruDevice->second.find("Address");
+-
+- if (findBus == findFruDevice->second.end() ||
+- findAddress == findFruDevice->second.end())
++ std::optional<uint64_t> maybeBus;
++ std::optional<uint64_t> maybeAddress;
++ std::optional<std::string> maybeName;
++ for (const auto& [key, val] : i2cProperties)
+ {
+- return false;
++ if (key == "Bus")
++ {
++ maybeBus = std::get<uint64_t>(val);
++ }
++ else if (key == "Address")
++ {
++ maybeAddress = std::get<uint64_t>(val);
++ }
++ else if (key == "Name")
++ {
++ maybeName = std::get<std::string>(val);
++ }
+ }
+- if ((std::get<uint64_t>(findBus->second) != bus) ||
+- (std::get<uint64_t>(findAddress->second) != address))
++ if (!maybeBus || *maybeBus != bus || !maybeAddress ||
++ *maybeAddress != address)
+ {
+- return false;
++ continue;
+ }
+
+- auto fruName = findFruDevice->second.find("Name");
+- if (fruName != findFruDevice->second.end())
++ if (maybeName.has_value())
+ {
+- name = std::get<std::string>(fruName->second);
++ name = *maybeName;
+ }
+
+- // At this point we found the device entry and should return
+- // true.
+- auto findIpmiDevice =
+- entry.second.find("xyz.openbmc_project.Inventory.Decorator.Ipmi");
+- if (findIpmiDevice != entry.second.end())
++ ipmi::PropertyMap entityData;
++ ec = ipmi::getAllDbusProperties(
++ ctx, "xyz.openbmc_project.EntityManager", path,
++ "xyz.openbmc_project.Inventory.Decorator.Ipmi", entityData);
++ if (!ec)
+ {
+- entityData = &(findIpmiDevice->second);
++ for (const auto& [key, val] : entityData)
++ {
++ if (key == "EntityId")
++ {
++ entityID = std::get<uint64_t>(val);
++ }
++ else if (key == "EntityInstance")
++ {
++ entityInstance = std::get<uint64_t>(val);
++ }
++ }
+ }
++ foundEntity = true;
++ break;
++ }
+
+- return true;
+- });
+-
+- if (entity == entities.end())
++ if (!foundEntity)
+ {
+ if constexpr (DEBUG)
+ {
+@@ -667,7 +691,6 @@ ipmi_ret_t getFruSdrs([[maybe_unused]] ipmi::Context::ptr ctx, size_t index,
+ "not found for Fru\n");
+ }
+ }
+-
+ #endif
+
+ #ifdef USING_FRU_DEVICE_PROPERTY_OVERRIDE_FRU_NAME
+@@ -710,28 +733,6 @@ ipmi_ret_t getFruSdrs([[maybe_unused]] ipmi::Context::ptr ctx, size_t index,
+ resp.body.deviceType = 0x10;
+ resp.body.deviceTypeModifier = 0x0;
+
+- uint8_t entityID = 0;
+- uint8_t entityInstance = 0x1;
+-
+-#ifdef USING_ENTITY_MANAGER_DECORATORS
+- if (entityData)
+- {
+- auto entityIdProperty = entityData->find("EntityId");
+- auto entityInstanceProperty = entityData->find("EntityInstance");
+-
+- if (entityIdProperty != entityData->end())
+- {
+- entityID = static_cast<uint8_t>(
+- std::get<uint64_t>(entityIdProperty->second));
+- }
+- if (entityInstanceProperty != entityData->end())
+- {
+- entityInstance = static_cast<uint8_t>(
+- std::get<uint64_t>(entityInstanceProperty->second));
+- }
+- }
+-#endif
+-
+ resp.body.entityID = entityID;
+ resp.body.entityInstance = entityInstance;
+
+--
+2.49.0.805.g082f7c87e0-goog
+
diff --git a/recipes-phosphor/ipmi/phosphor-ipmi-host_%.bbappend b/recipes-phosphor/ipmi/phosphor-ipmi-host_%.bbappend
index d157e08..499de8a 100644
--- a/recipes-phosphor/ipmi/phosphor-ipmi-host_%.bbappend
+++ b/recipes-phosphor/ipmi/phosphor-ipmi-host_%.bbappend
@@ -11,6 +11,7 @@
file://0006-dbus-sdr-fix-write-permission-for-external-sen.patch \
file://0007-dbus-sdr-Add-limited-log-on-sensorTree-Resets.patch \
file://0008-user_mgmt-Recover-corrupted-nv-files.patch \
+ file://0009-Remove-GetManagedObject-call-for-fru-sdr.patch \
"
EXTRA_OEMESON:append:gbmc = " -Dfru-device-property-override-fru-name=disabled"
