requester: support multi-host MCTP devices hot plug
Currently, pldmd listens for new MCTP endpoint exposed by mctpd, but
they only shows their EID, Network Id, and SupportedMessageTypes, which
cannot fulfill some requirements, e.g., get the device's name or check
the host slot number which contains the MCTP endpoint in multi-host
system.
In openbmc, the additional information are exposed by Entity Manager,
so that we add ConfigurationDiscoveryHandler, search for Entity
Manager's configuration when a new MCTP endpoint has been register by
mctpd.
Objects who want to obtain the configuration can include the
ConfigurationDiscoveryHandler as their attribute, get the configuration
when needed.
TESTED:
A series of unit test case to simulate the response of dbus call,
confirm that the configurations are successfully stored.
Change-Id: I59b8bf434576cbdea651848a8da11e5b870e2dfa
Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>
Signed-off-by: MandyMCHung <mandy.hung.wiwynn@gmail.com>
Signed-off-by: LioraGuo-wiwynn <liora.guo.wiwynn@gmail.com>
Signed-off-by: Unive Tien <unive.tien.wiwynn@gmail.com>
diff --git a/common/test/mocked_utils.hpp b/common/test/mocked_utils.hpp
index 8e887a5..4d5de74 100644
--- a/common/test/mocked_utils.hpp
+++ b/common/test/mocked_utils.hpp
@@ -72,4 +72,8 @@
MOCK_METHOD(pldm::utils::GetSubTreePathsResponse, getSubTreePaths,
(const std::string&, int, const std::vector<std::string>&),
(const override));
+
+ MOCK_METHOD(pldm::utils::PropertyMap, getAll,
+ (const std::string&, const std::string&, const std::string&),
+ (const override));
};
diff --git a/common/types.hpp b/common/types.hpp
index 42d626b..301d08b 100644
--- a/common/types.hpp
+++ b/common/types.hpp
@@ -68,9 +68,9 @@
using Interfaces = std::vector<std::string>;
using Property = std::string;
using PropertyType = std::string;
-using Value =
- std::variant<bool, uint8_t, int16_t, uint16_t, int32_t, uint32_t, int64_t,
- uint64_t, double, std::string, std::vector<uint8_t>>;
+using Value = std::variant<bool, uint8_t, int16_t, uint16_t, int32_t, uint32_t,
+ int64_t, uint64_t, double, std::string,
+ std::vector<uint8_t>, std::vector<uint64_t>>;
using PropertyMap = std::map<Property, Value>;
using InterfaceMap = std::map<Interface, PropertyMap>;
diff --git a/common/utils.cpp b/common/utils.cpp
index a5792f5..9e621f1 100644
--- a/common/utils.cpp
+++ b/common/utils.cpp
@@ -427,6 +427,23 @@
return bus.call(method, dbusTimeout).unpack<PropertyMap>();
}
+PropertyMap DBusHandler::getAll(const std::string& service,
+ const std::string& objPath,
+ const std::string& dbusInterface) const
+{
+ auto& bus = DBusHandler::getBus();
+ auto method =
+ bus.new_method_call(service.c_str(), objPath.c_str(),
+ "org.freedesktop.DBus.Properties", "GetAll");
+ method.append(dbusInterface);
+
+ auto response = bus.call(method);
+ PropertyMap result{};
+ response.read(result);
+
+ return result;
+}
+
PropertyValue jsonEntryToDbusVal(std::string_view type,
const nlohmann::json& value)
{
diff --git a/common/utils.hpp b/common/utils.hpp
index bfae329..1b029cb 100644
--- a/common/utils.hpp
+++ b/common/utils.hpp
@@ -174,7 +174,7 @@
using PropertyValue =
std::variant<bool, uint8_t, int16_t, uint16_t, int32_t, uint32_t, int64_t,
uint64_t, double, std::string, std::vector<uint8_t>,
- std::vector<std::string>>;
+ std::vector<uint64_t>, std::vector<std::string>>;
using DbusProp = std::string;
using DbusChangedProps = std::map<DbusProp, PropertyValue>;
using DBusInterfaceAdded = std::vector<
@@ -232,6 +232,10 @@
virtual PropertyMap
getDbusPropertiesVariant(const char* serviceName, const char* objPath,
const char* dbusInterface) const = 0;
+
+ virtual PropertyMap getAll(const std::string& service,
+ const std::string& objPath,
+ const std::string& dbusInterface) const = 0;
};
/**
@@ -338,6 +342,9 @@
getDbusPropertiesVariant(const char* serviceName, const char* objPath,
const char* dbusInterface) const override;
+ PropertyMap getAll(const std::string& service, const std::string& objPath,
+ const std::string& dbusInterface) const override;
+
/** @brief The template function to get property from the requested dbus
* path
*
diff --git a/libpldmresponder/fru.hpp b/libpldmresponder/fru.hpp
index af55f92..8fda98b 100644
--- a/libpldmresponder/fru.hpp
+++ b/libpldmresponder/fru.hpp
@@ -24,9 +24,10 @@
namespace dbus
{
-using Value = std::variant<bool, uint8_t, int16_t, uint16_t, int32_t, uint32_t,
- int64_t, uint64_t, double, std::string,
- std::vector<uint8_t>, std::vector<std::string>>;
+using Value =
+ std::variant<bool, uint8_t, int16_t, uint16_t, int32_t, uint32_t, int64_t,
+ uint64_t, double, std::string, std::vector<uint8_t>,
+ std::vector<uint64_t>, std::vector<std::string>>;
using PropertyMap = std::map<Property, Value>;
using InterfaceMap = std::map<Interface, PropertyMap>;
using ObjectValueTree = std::map<sdbusplus::message::object_path, InterfaceMap>;
diff --git a/meson.build b/meson.build
index 32684b3..b03f7a1 100644
--- a/meson.build
+++ b/meson.build
@@ -248,6 +248,7 @@
'platform-mc/numeric_sensor.cpp',
'platform-mc/event_manager.cpp',
'requester/mctp_endpoint_discovery.cpp',
+ 'requester/configuration_discovery_handler.cpp',
implicit_include_directories: false,
dependencies: deps,
install: true,
diff --git a/pldmd/pldmd.cpp b/pldmd/pldmd.cpp
index fc2b08d..3f200fd 100644
--- a/pldmd/pldmd.cpp
+++ b/pldmd/pldmd.cpp
@@ -7,6 +7,7 @@
#include "fw-update/manager.hpp"
#include "invoker.hpp"
#include "platform-mc/manager.hpp"
+#include "requester/configuration_discovery_handler.hpp"
#include "requester/handler.hpp"
#include "requester/mctp_endpoint_discovery.hpp"
#include "requester/request.hpp"
@@ -244,6 +245,10 @@
}
std::shared_ptr<HostPDRHandler> hostPDRHandler;
std::unique_ptr<DbusToPLDMEvent> dbusToPLDMEventHandler;
+
+ auto configurationDiscovery =
+ std::make_unique<pldm::ConfigurationDiscoveryHandler>(&dbusHandler);
+
std::unique_ptr<platform_config::Handler> platformConfigHandler{};
platformConfigHandler =
std::make_unique<platform_config::Handler>(PDR_JSONS_DIR);
@@ -326,7 +331,9 @@
std::unique_ptr<MctpDiscovery> mctpDiscoveryHandler =
std::make_unique<MctpDiscovery>(
bus, std::initializer_list<MctpDiscoveryHandlerIntf*>{
- fwManager.get(), platformManager.get()});
+ fwManager.get(), platformManager.get(),
+ configurationDiscovery.get()});
+
auto callback = [verbose, &invoker, &reqHandler, &fwManager, &pldmTransport,
TID](IO& io, int fd, uint32_t revents) mutable {
if (!(revents & EPOLLIN))
diff --git a/requester/configuration_discovery_handler.cpp b/requester/configuration_discovery_handler.cpp
new file mode 100644
index 0000000..8b4d956
--- /dev/null
+++ b/requester/configuration_discovery_handler.cpp
@@ -0,0 +1,178 @@
+#include "configuration_discovery_handler.hpp"
+
+#include <phosphor-logging/lg2.hpp>
+
+#include <algorithm>
+
+PHOSPHOR_LOG2_USING;
+
+namespace pldm
+{
+
+void ConfigurationDiscoveryHandler::handleMctpEndpoints(
+ const MctpInfos& newMctpInfos)
+{
+ for (const auto& newMctpInfo : newMctpInfos)
+ {
+ searchConfigurationFor(newMctpInfo);
+ }
+}
+
+void ConfigurationDiscoveryHandler::handleRemovedMctpEndpoints(
+ const MctpInfos& removedMctpInfos)
+{
+ for (const auto& removedMctpInfo : removedMctpInfos)
+ {
+ removeConfigByEid(std::get<0>(removedMctpInfo));
+ }
+}
+
+std::map<std::string, MctpEndpoint>&
+ ConfigurationDiscoveryHandler::getConfigurations()
+{
+ return configurations;
+}
+
+void ConfigurationDiscoveryHandler::searchConfigurationFor(MctpInfo mctpInfo)
+{
+ constexpr auto inventoryPath = "/xyz/openbmc_project/inventory/";
+ constexpr auto depthWithDownstreamDevices = std::ranges::count(
+ "/inventory/system/{BOARD_OR_CHASSIS}/{DEVICE}/{DOWNSTREAM_DEVICE}",
+ '/');
+
+ const std::vector<std::string> mctpEndpoint = {
+ "xyz.openbmc_project.Configuration.MCTPEndpoint"};
+
+ try
+ {
+ auto response = dBusIntf->getSubtree(
+ inventoryPath, depthWithDownstreamDevices, mctpEndpoint);
+
+ for (const auto& [objectPath, serviceMap] : response)
+ {
+ appendConfigIfEidMatch(std::get<eid>(mctpInfo), objectPath,
+ serviceMap);
+ }
+ }
+ catch (const sdbusplus::exception_t& e)
+ {
+ error("{FUNC}: Failed to getSubtree with MCTPEndpoint, error={ERROR}",
+ "FUNC", std::string(__func__), "ERROR", e.what());
+ return;
+ }
+ catch (const std::exception& e)
+ {
+ error("{FUNC}: Unpredicted error occured, error={ERROR}", "FUNC",
+ std::string(__func__), "ERROR", e.what());
+ return;
+ }
+}
+
+void ConfigurationDiscoveryHandler::appendConfigIfEidMatch(
+ uint8_t targetEid, const std::string& configPath,
+ const pldm::utils::MapperServiceMap& serviceMap)
+{
+ if (!configurations.contains(configPath))
+ {
+ const auto& serviceName = serviceMap.at(0).first;
+
+ /** mctpEndpointInterface should be
+ * "xyz.openbmc_project.Configuration.MCTPEndpoint".
+ */
+ const auto& mctpEndpointInterface = serviceMap.at(0).second.at(0);
+ try
+ {
+ auto response = dBusIntf->getAll(serviceName, configPath,
+ mctpEndpointInterface);
+ appendIfEidMatch(targetEid, configPath,
+ parseMctpEndpointFromResponse(response));
+ }
+ catch (const sdbusplus::exception_t& e)
+ {
+ error(
+ "{FUNC}: Failed to getAll of interface={INTERFACE} in path={PATH}, error={ERROR}",
+ "FUNC", std::string(__func__), "INTERFACE",
+ mctpEndpointInterface, "PATH", configPath, "ERROR", e.what());
+ return;
+ }
+ catch (const NoSuchPropertiesException& e)
+ {
+ error("{FUNC}: Insufficient properties in {PATH}, error={ERROR}",
+ "FUNC", std::string(__func__), "PATH", configPath, "ERROR",
+ e.what());
+ return;
+ }
+ catch (const std::exception& e)
+ {
+ error("{FUNC}: Unpredicted error occured, error={ERROR}", "FUNC",
+ std::string(__func__), "ERROR", e.what());
+ return;
+ }
+ }
+}
+
+MctpEndpoint ConfigurationDiscoveryHandler::parseMctpEndpointFromResponse(
+ const pldm::utils::PropertyMap& response)
+{
+ if (response.contains("Address") && response.contains("Bus") &&
+ response.contains("EndpointId") && response.contains("Name"))
+ {
+ auto addressArray =
+ std::get<std::vector<uint64_t>>(response.at("Address"));
+ uint64_t address = 0;
+
+ for (const auto& element : addressArray)
+ {
+ address = (address << 8) | element;
+ }
+
+ auto eid = std::get<uint64_t>(response.at("EndpointId"));
+ auto bus = std::get<uint64_t>(response.at("Bus"));
+ auto componentName = std::get<std::string>(response.at("Name"));
+ if (response.contains("IANA"))
+ {
+ auto iana = std::get<std::string>(response.at("IANA"));
+ return MctpEndpoint{address, eid, bus, componentName, iana};
+ }
+
+ return MctpEndpoint{address, eid, bus, componentName, std::nullopt};
+ }
+ else
+ {
+ std::vector<std::string> missingProperties{};
+ if (response.contains("Address"))
+ missingProperties.emplace_back("Address");
+ if (response.contains("Bus"))
+ missingProperties.emplace_back("Bus");
+ if (response.contains("EndpointId"))
+ missingProperties.emplace_back("EndpointId");
+ if (response.contains("Name"))
+ missingProperties.emplace_back("Name");
+
+ throw NoSuchPropertiesException(missingProperties);
+ }
+}
+
+void ConfigurationDiscoveryHandler::appendIfEidMatch(
+ uint8_t targetEid, const std::string& configPath,
+ const MctpEndpoint& endpoint)
+{
+ if (endpoint.EndpointId == targetEid)
+ {
+ configurations.emplace(configPath, endpoint);
+ }
+}
+
+void ConfigurationDiscoveryHandler::removeConfigByEid(uint8_t eid)
+{
+ for (const auto& [configDbusPath, mctpEndpoint] : configurations)
+ {
+ if (mctpEndpoint.EndpointId == eid)
+ {
+ configurations.erase(configDbusPath);
+ return;
+ }
+ }
+}
+
+} // namespace pldm
diff --git a/requester/configuration_discovery_handler.hpp b/requester/configuration_discovery_handler.hpp
new file mode 100644
index 0000000..403b15f
--- /dev/null
+++ b/requester/configuration_discovery_handler.hpp
@@ -0,0 +1,133 @@
+#pragma once
+
+#include "common/utils.hpp"
+#include "mctp_endpoint_discovery.hpp"
+
+#include <stdexcept>
+
+namespace pldm
+{
+
+struct MctpEndpoint
+{
+ uint64_t address;
+ uint64_t EndpointId;
+ uint64_t bus;
+ std::string name;
+ std::optional<std::string> iana;
+};
+
+class ConfigurationDiscoveryHandler : public MctpDiscoveryHandlerIntf
+{
+ public:
+ ConfigurationDiscoveryHandler() = delete;
+ ConfigurationDiscoveryHandler(const ConfigurationDiscoveryHandler&) =
+ delete;
+ ConfigurationDiscoveryHandler(ConfigurationDiscoveryHandler&&) = delete;
+ ConfigurationDiscoveryHandler&
+ operator=(const ConfigurationDiscoveryHandler&) = delete;
+ ConfigurationDiscoveryHandler&
+ operator=(ConfigurationDiscoveryHandler&&) = delete;
+ ~ConfigurationDiscoveryHandler() = default;
+
+ explicit ConfigurationDiscoveryHandler(
+ const pldm::utils::DBusHandler* dBusIntf) : dBusIntf(dBusIntf)
+ {}
+
+ /** @brief Discover configuration associated with the new MCTP endpoints.
+ *
+ * @param[in] newMctpInfos - information of discovered MCTP endpoint
+ */
+ void handleMctpEndpoints(const MctpInfos& newMctpInfos);
+
+ /** @brief Remove configuration associated with the removed MCTP endpoints.
+ *
+ * @param[in] removedMctpInfos - the removed MCTP endpoints
+ */
+ void handleRemovedMctpEndpoints(const MctpInfos& removedMctpInfos);
+
+ /** @brief Get existing configurations.
+ *
+ * @return The configurations.
+ */
+ std::map<std::string, MctpEndpoint>& getConfigurations();
+
+ private:
+ /** @brief Search for associated configuration for the MctpInfo.
+ *
+ * @param[in] mctpInfo - information of discovered MCTP endpoint
+ */
+ void searchConfigurationFor(MctpInfo mctpInfo);
+
+ /** @brief Append to configurations if Endpoint Id is matched.
+ *
+ * @param[in] targetEid - discovered MCTP endpoint Id
+ * @param[in] configPath - the Dbus path of a configuration
+ * @param[in] serviceMap - the map contains the service's information who
+ * expose the configuration
+ */
+ void
+ appendConfigIfEidMatch(uint8_t targetEid, const std::string& configPath,
+ const pldm::utils::MapperServiceMap& serviceMap);
+
+ /** @brief Parse MctpEndpoint from the response of GetAll method.
+ *
+ * @param[in] response - Response data of GetAll method
+ *
+ * @return Parsed MctpEndpoint object.
+ */
+ MctpEndpoint
+ parseMctpEndpointFromResponse(const pldm::utils::PropertyMap& response);
+
+ /** @brief Append to configuration if the MctpEndpoint's EID matches
+ * targetEid.
+ *
+ * @param[in] targetEid - The target EID
+ * @param[in] configPath - Discovered configuration's Dbus path
+ * @param[in] endpoint - The configuration's MctpEndpoint information.
+ */
+ void appendIfEidMatch(uint8_t targetEid, const std::string& configPath,
+ const MctpEndpoint& endpoint);
+
+ /** @brief Remove configuration associated with the removed MCTP endpoint.
+ *
+ * @param[in] eid - endpoint Id of the removed MCTP Endpoint
+ */
+ void removeConfigByEid(uint8_t eid);
+
+ /** @brief The configuration contains Dbus path and the MCTP endpoint
+ * information.
+ */
+ std::map<std::string /*configDbusPath*/, MctpEndpoint> configurations;
+
+ /** @brief D-Bus Interface object*/
+ const pldm::utils::DBusHandler* dBusIntf;
+};
+
+class NoSuchPropertiesException : public std::exception
+{
+ public:
+ NoSuchPropertiesException() = delete;
+ ~NoSuchPropertiesException() = default;
+
+ explicit NoSuchPropertiesException(
+ const std::vector<std::string> properties)
+ {
+ std::string missingProperties{};
+ for (const auto& property : properties)
+ {
+ missingProperties += std::string(property) + " ";
+ }
+ message = "Interface has no properties: " + missingProperties;
+ }
+
+ const char* what() const noexcept override
+ {
+ return message.c_str();
+ }
+
+ private:
+ std::string message;
+};
+
+} // namespace pldm
diff --git a/requester/test/configuration_discovery_handler_test.cpp b/requester/test/configuration_discovery_handler_test.cpp
new file mode 100644
index 0000000..cd28c42
--- /dev/null
+++ b/requester/test/configuration_discovery_handler_test.cpp
@@ -0,0 +1,115 @@
+#include "config.h"
+
+#include "common/test/mocked_utils.hpp"
+#include "common/types.hpp"
+#include "common/utils.hpp"
+#include "requester/configuration_discovery_handler.hpp"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+using ::testing::_;
+
+TEST(ConfigurationDiscoveryHandlerTest, succesfullyDiscoverConfig)
+{
+ constexpr uint8_t EID = 10;
+ constexpr auto mockedDbusPath =
+ "/xyz/openbmc_project/inventory/system/board/Mocked_Board_Slot_1/MockedDevice";
+ constexpr auto mockedService = "xyz.openbmc_project.EntityManager";
+ constexpr auto mockedInterface =
+ "xyz.openbmc_project.Configuration.MCTPEndpoint";
+ MockdBusHandler mockedUtils;
+ auto handler =
+ std::make_unique<pldm::ConfigurationDiscoveryHandler>(&mockedUtils);
+
+ pldm::utils::GetSubTreeResponse mockedGetSubtreeResponse{std::make_pair(
+ mockedDbusPath,
+ pldm::utils::MapperServiceMap{std::make_pair(
+ mockedService, pldm::utils::Interfaces{mockedInterface})})};
+
+ EXPECT_CALL(mockedUtils, getSubtree(_, _, _))
+ .Times(1)
+ .WillOnce(testing::Return(mockedGetSubtreeResponse));
+
+ pldm::utils::PropertyMap mockGetAllResponse{
+ {"Address", std::vector<uint64_t>{0x1}},
+ {"Bus", uint64_t(0)},
+ {"EndpointId", uint64_t(EID)},
+ {"Name", std::string("MockedDevice")}};
+
+ EXPECT_CALL(mockedUtils,
+ getAll(mockedService, mockedDbusPath, mockedInterface))
+ .Times(1)
+ .WillOnce(testing::Return(mockGetAllResponse));
+
+ pldm::MctpInfos mctpInfos;
+ mctpInfos.emplace_back(pldm::MctpInfo(EID, "emptyUUID", "", 1));
+
+ // Act
+ handler->handleMctpEndpoints(mctpInfos);
+
+ ASSERT_EQ(1, handler->getConfigurations().size());
+}
+
+TEST(ConfigurationDiscoveryHandlerTest, BlockedByNoSuchElement)
+{
+ constexpr uint8_t EID = 10;
+ constexpr auto mockedDbusPath =
+ "/xyz/openbmc_project/inventory/system/board/Mocked_Board_Slot_1/MockedDevice";
+ constexpr auto mockedService = "xyz.openbmc_project.EntityManager";
+ constexpr auto mockedInterface =
+ "xyz.openbmc_project.Configuration.MCTPEndpoint";
+ MockdBusHandler mockedUtils;
+ auto handler =
+ std::make_unique<pldm::ConfigurationDiscoveryHandler>(&mockedUtils);
+
+ pldm::utils::GetSubTreeResponse mockedGetSubtreeResponse{std::make_pair(
+ mockedDbusPath,
+ pldm::utils::MapperServiceMap{std::make_pair(
+ mockedService, pldm::utils::Interfaces{mockedInterface})})};
+
+ EXPECT_CALL(mockedUtils, getSubtree(_, _, _))
+ .Times(1)
+ .WillOnce(testing::Return(mockedGetSubtreeResponse));
+
+ pldm::utils::PropertyMap mockGetAllResponse{
+ {"Address", uint64_t(0x1)},
+ {"Bus", uint64_t(0)},
+ /* No EndpointId */
+ {"Name", std::string("MockedDevice")}};
+
+ EXPECT_CALL(mockedUtils,
+ getAll(mockedService, mockedDbusPath, mockedInterface))
+ .Times(1)
+ .WillOnce(testing::Return(mockGetAllResponse));
+
+ pldm::MctpInfos mctpInfos;
+ mctpInfos.emplace_back(pldm::MctpInfo(EID, "emptyUUID", "", 1));
+
+ // Act
+ EXPECT_NO_THROW(handler->handleMctpEndpoints(mctpInfos));
+
+ ASSERT_EQ(0, handler->getConfigurations().size());
+}
+
+TEST(ConfigurationDiscoveryHandlerTest, succesfullyRemoveConfig)
+{
+ constexpr uint8_t EID = 10;
+
+ MockdBusHandler mockedUtils;
+ auto handler =
+ std::make_unique<pldm::ConfigurationDiscoveryHandler>(&mockedUtils);
+
+ pldm::MctpEndpoint mockedMctpEndpoint = {
+ uint64_t(0x1), uint64_t(EID), uint64_t(0), "MockedDevice",
+ std::nullopt};
+ handler->getConfigurations().emplace(
+ "/xyz/openbmc_project/inventory/system/board/Mocked_Board_Slot_1/MockedDevice",
+ mockedMctpEndpoint);
+
+ pldm::MctpInfos mctpInfos;
+ mctpInfos.emplace_back(pldm::MctpInfo(EID, "emptyUUID", "", 1));
+ handler->handleRemovedMctpEndpoints(mctpInfos);
+
+ ASSERT_EQ(0, handler->getConfigurations().size());
+}
diff --git a/requester/test/meson.build b/requester/test/meson.build
index f269561..d30d884 100644
--- a/requester/test/meson.build
+++ b/requester/test/meson.build
@@ -1,8 +1,17 @@
test_src = declare_dependency(
- sources: ['../mctp_endpoint_discovery.cpp', '../../common/utils.cpp'],
+ sources: [
+ '../mctp_endpoint_discovery.cpp',
+ '../../common/utils.cpp',
+ '../configuration_discovery_handler.cpp',
+ ],
)
-tests = ['handler_test', 'request_test', 'mctp_endpoint_discovery_test']
+tests = [
+ 'handler_test',
+ 'request_test',
+ 'mctp_endpoint_discovery_test',
+ 'configuration_discovery_handler_test',
+]
foreach t : tests
test(