mctp: support bridged MCTP endpoints resolution Added support for resolving bridged MCTP endpoints by tracing the MCTPBridgeDownstreamDevice configuration chain up to the root bridge. This allows nvmesensor to correctly identify the bus information for endpoints behind bridges. Tested: Ran unit tests in isolated D-Bus session in Docker. All 18 tests passed. Google-Bug-Id: 509971428 Change-Id: I0407987c0cd545f7552c076b9abbd6b344cc316e Signed-off-by: Hao Jiang <jianghao@google.com>
diff --git a/src/MctpReactorDevice.cpp b/src/MctpReactorDevice.cpp index e6afac5..f3386b3 100644 --- a/src/MctpReactorDevice.cpp +++ b/src/MctpReactorDevice.cpp
@@ -144,7 +144,7 @@ auto weak = weak_from_this(); callbackToken = registerMctpEndpointCallback( [weak](const std::string& epPath, - const SensorData& epConfig [[maybe_unused]], + const std::vector<SensorData>& epConfig [[maybe_unused]], bool isRemoved) { auto self = weak.lock(); if (!self) @@ -171,9 +171,9 @@ }); } - for (const auto& [epPath, epConfig] : mctpEndpointConfigMap) + for (const auto& [epPath, epConfigChain] : mctpEndpointConfigMap) { - MctpReactorBusInfo epBusInfo(epConfig); + MctpReactorBusInfo epBusInfo(epConfigChain); if (epBusInfo.isValid() && epBusInfo.getProperties() == targetBusInfo) {
diff --git a/src/MctpUtil.cpp b/src/MctpUtil.cpp index 1d2aa96..8b44f20 100644 --- a/src/MctpUtil.cpp +++ b/src/MctpUtil.cpp
@@ -12,7 +12,7 @@ #include <cstdint> #include <filesystem> -std::map<std::string, SensorData> mctpEndpointConfigMap; +std::map<std::string, std::vector<SensorData>> mctpEndpointConfigMap; enum class State : std::uint8_t { @@ -179,11 +179,100 @@ lg2::info( "Recorded MCTP endpoint {ENDPOINT} with config from {PATH}", "ENDPOINT", endpointPath, "PATH", emConfigPath); - mctpEndpointConfigMap[endpointPath] = objIt->second; + + std::vector<SensorData> configChain; + configChain.push_back(objIt->second); + + // Resolve bridge chain if needed + std::string currentPath = emConfigPath; + bool resolutionFailed = false; + std::vector<std::string> visitedPaths = {currentPath}; + + while (true) + { + const auto& currentConfig = configChain.back(); + auto bridgeIt = currentConfig.find( + "xyz.openbmc_project.Configuration.MCTPBridgeDownstreamDevice"); + if (bridgeIt == currentConfig.end()) + { + break; // Not a bridged endpoint, or reached the end + // of chain + } + + auto propIt = bridgeIt->second.find("BridgeInventory"); + if (propIt == bridgeIt->second.end()) + { + lg2::error( + "BridgeInventory property missing for {PATH}", + "PATH", currentPath); + resolutionFailed = true; + break; + } + + const auto* bridgePathPtr = + std::get_if<std::string>(&propIt->second); + if (bridgePathPtr == nullptr) + { + lg2::error( + "Invalid BridgeInventory property type for {PATH}", + "PATH", currentPath); + resolutionFailed = true; + break; + } + + std::string nextPath = *bridgePathPtr; + + if (std::find(visitedPaths.begin(), visitedPaths.end(), + nextPath) != visitedPaths.end()) + { + lg2::error( + "Circular dependency detected in bridge chain at {PATH}", + "PATH", nextPath); + resolutionFailed = true; + break; + } + visitedPaths.push_back(nextPath); + + auto nextObjIt = objects.find( + sdbusplus::message::object_path(nextPath)); + + if (nextObjIt == objects.end()) + { + lg2::error("Bridge config {PATH} not found", "PATH", + nextPath); + resolutionFailed = true; + break; + } + + // Verify it contains a supported bridge interface + if (nextObjIt->second.find( + "xyz.openbmc_project.Configuration.MCTPBridgeDownstreamDevice") == + nextObjIt->second.end() && + nextObjIt->second.find( + "xyz.openbmc_project.Configuration.MCTPUSBDevice") == + nextObjIt->second.end()) + { + lg2::error("Object {PATH} is not a valid bridge", + "PATH", nextPath); + resolutionFailed = true; + break; + } + + configChain.push_back(nextObjIt->second); + currentPath = nextPath; + } + + if (resolutionFailed) + { + break; // Abort resolution for this endpoint (exits the + // do-while(false) loop) + } + + mctpEndpointConfigMap[endpointPath] = configChain; for (const auto& [token, cb] : mctpEndpointCallbacks) { - cb(endpointPath, objIt->second, false); + cb(endpointPath, configChain, false); } } } @@ -471,7 +560,8 @@ }); } -MctpReactorBusInfo::MctpReactorBusInfo(const SensorData& configData) +MctpReactorBusInfo::MctpReactorBusInfo( + const std::vector<SensorData>& configDataChain) { auto extractProp = [](const auto& dbusProps, std::map<std::string, std::string>& info, @@ -483,42 +573,51 @@ } }; - for (const auto& [intf, props] : configData) + // For a MCTP device behind (multiple layers of) MCTP bridge, + // configDataChain include all config/busInfo including its bridge routing + // info (a.k.a BridgeDownsteamMCTPDDevice) and the top most port info (a.k.a + // MCTPUSBDevice/etc). + // And MctpReactorBusInfo extracts the top most port info for difference bus + // types. + for (const auto& configData : configDataChain) { - if (absl::StrContains( - intf, "xyz.openbmc_project.Configuration.MCTPUSBDevice")) + for (const auto& [intf, props] : configData) { - type = "USB"; - valid = true; - properties["BusType"] = "USB"; + if (absl::StrContains( + intf, "xyz.openbmc_project.Configuration.MCTPUSBDevice")) + { + type = "USB"; + valid = true; + properties["BusType"] = "USB"; - extractProp(props, properties, "RootHubPath"); - extractProp(props, properties, "Port"); - extractProp(props, properties, "InterfaceNum"); - extractProp(props, properties, "Configuration"); - break; - } - if (absl::StrContains( - intf, "xyz.openbmc_project.Configuration.MCTPI2CTarget")) - { - type = "I2C"; - valid = true; - properties["BusType"] = "I2C"; + extractProp(props, properties, "RootHubPath"); + extractProp(props, properties, "Port"); + extractProp(props, properties, "InterfaceNum"); + extractProp(props, properties, "Configuration"); + break; + } + if (absl::StrContains( + intf, "xyz.openbmc_project.Configuration.MCTPI2CTarget")) + { + type = "I2C"; + valid = true; + properties["BusType"] = "I2C"; - extractProp(props, properties, "Bus"); - extractProp(props, properties, "Address"); - break; - } - if (absl::StrContains( - intf, "xyz.openbmc_project.Configuration.MCTPI3CTarget")) - { - type = "I3C"; - valid = true; - properties["BusType"] = "I3C"; + extractProp(props, properties, "Bus"); + extractProp(props, properties, "Address"); + break; + } + if (absl::StrContains( + intf, "xyz.openbmc_project.Configuration.MCTPI3CTarget")) + { + type = "I3C"; + valid = true; + properties["BusType"] = "I3C"; - extractProp(props, properties, "Bus"); - extractProp(props, properties, "Address"); - break; + extractProp(props, properties, "Bus"); + extractProp(props, properties, "Address"); + break; + } } } }
diff --git a/src/MctpUtil.hpp b/src/MctpUtil.hpp index 4d63c3f..7a049fb 100644 --- a/src/MctpUtil.hpp +++ b/src/MctpUtil.hpp
@@ -26,7 +26,7 @@ class MctpReactorBusInfo { public: - explicit MctpReactorBusInfo(const SensorData& configData); + explicit MctpReactorBusInfo(const std::vector<SensorData>& configData); std::string getType() const { return type; @@ -46,11 +46,11 @@ std::map<std::string, std::string> properties; }; -extern std::map<std::string, SensorData> mctpEndpointConfigMap; +extern std::map<std::string, std::vector<SensorData>> mctpEndpointConfigMap; -using MctpEndpointCallback = - std::function<void(const std::string& endpointPath, - const SensorData& endpointConfig, bool isRemoved)>; +using MctpEndpointCallback = std::function<void( + const std::string& endpointPath, + const std::vector<SensorData>& endpointConfig, bool isRemoved)>; using MctpCallbackToken = size_t; MctpCallbackToken registerMctpEndpointCallback(MctpEndpointCallback&& cb);
diff --git a/tests/mock_mctpd.py b/tests/mock_mctpd.py index 965c4c6..8ec619c 100644 --- a/tests/mock_mctpd.py +++ b/tests/mock_mctpd.py
@@ -19,6 +19,7 @@ 2. Use `busctl call` to trigger events via TriggerAdd/TriggerRemove. 3. Listen for QueryStarted/QueryCompleted to synchronize test steps. """ +import os import dbus import dbus.service import dbus.mainloop.glib @@ -56,19 +57,14 @@ return {} -class MockTarget(dbus.service.Object): - def __init__(self, bus): - dbus.service.Object.__init__(self, bus, '/xyz/openbmc_project/inventory/system/board/MockBoard/mctp_device') +class MockConfigObject(dbus.service.Object): + def __init__(self, bus, path, interfaces): + dbus.service.Object.__init__(self, bus, path) + self.interfaces = interfaces @dbus.service.method('org.freedesktop.DBus.Properties', in_signature='s', out_signature='a{sv}') def GetAll(self, interface_name): - if interface_name == 'xyz.openbmc_project.Configuration.BusInfo': - return { - 'BusType': 'USB', - 'Port': '1.2.5', - 'Configuration': dbus.UInt32(1) - } - return {} + return self.interfaces.get(interface_name, {}) class MockMctpd(dbus.service.Object): @@ -86,6 +82,20 @@ def InterfacesRemoved(self, object_path, interfaces): pass + @dbus.service.method('org.freedesktop.DBus.ObjectManager', out_signature='a{oa{sa{sv}}}') + def GetManagedObjects(self): + result = {} + for path, ep in self.endpoints.items(): + result[dbus.ObjectPath(path)] = { + 'xyz.openbmc_project.Association.Definitions': { + 'Associations': ep._associations + }, + 'xyz.openbmc_project.MCTP.Endpoint': { + 'SupportedMessageTypes': ep._message_types + } + } + return result + @dbus.service.method('com.example.Control', in_signature='ss') def TriggerAdd(self, endpoint_path, em_config_path): if endpoint_path not in self.endpoints: @@ -159,6 +169,46 @@ 'Type': 'MCTPUSBDevice' } }, + dbus.ObjectPath('/xyz/openbmc_project/inventory/system/board/MockBoard/bridged_endpoint_config'): { + 'xyz.openbmc_project.Configuration.MCTPBridgeDownstreamDevice': { + 'BridgeInventory': '/xyz/openbmc_project/inventory/system/board/MockBoard/mctp_device', + 'EIDOffset': dbus.UInt64(1), + 'Name': 'Bridged_EP', + 'Type': 'MCTPBridgeDownstreamDevice' + } + }, + dbus.ObjectPath('/xyz/openbmc_project/inventory/system/board/MockBoard/broken_endpoint_nonexistent'): { + 'xyz.openbmc_project.Configuration.MCTPBridgeDownstreamDevice': { + 'BridgeInventory': '/xyz/openbmc_project/inventory/system/board/MockBoard/non_existent_bridge_path', + 'EIDOffset': dbus.UInt64(1), + 'Name': 'Broken_EP_Nonexistent', + 'Type': 'MCTPBridgeDownstreamDevice' + } + }, + dbus.ObjectPath('/xyz/openbmc_project/inventory/system/board/MockBoard/broken_endpoint_invalid'): { + 'xyz.openbmc_project.Configuration.MCTPBridgeDownstreamDevice': { + 'BridgeInventory': '/xyz/openbmc_project/inventory/system/board/Ironheart/Ironheart_i2c_static_NVMe_2', + 'EIDOffset': dbus.UInt64(1), + 'Name': 'Broken_EP_Invalid', + 'Type': 'MCTPBridgeDownstreamDevice' + } + }, + dbus.ObjectPath('/xyz/openbmc_project/inventory/system/board/MockBoard/intermediate_bridge'): { + 'xyz.openbmc_project.Configuration.MCTPBridgeDownstreamDevice': { + 'BridgeInventory': '/xyz/openbmc_project/inventory/system/board/MockBoard/mctp_device', + 'EIDOffset': dbus.UInt64(1), + 'Name': 'Intermediate_Bridge', + 'Type': 'MCTPBridgeDownstreamDevice' + } + }, + dbus.ObjectPath('/xyz/openbmc_project/inventory/system/board/MockBoard/multilevel_endpoint'): { + 'xyz.openbmc_project.Configuration.MCTPBridgeDownstreamDevice': { + 'BridgeInventory': '/xyz/openbmc_project/inventory/system/board/MockBoard/intermediate_bridge', + 'EIDOffset': dbus.UInt64(2), + 'Name': 'Multilevel_EP', + 'Type': 'MCTPBridgeDownstreamDevice' + } + }, dbus.ObjectPath('/xyz/openbmc_project/inventory/system/board/Ironheart/Ironheart_i2c_static_NVMe_2'): { 'xyz.openbmc_project.Configuration.NVME1000': { 'Name': 'Ironheart_i2c_static_NVMe_2', @@ -177,7 +227,6 @@ def main(): dbus.mainloop.glib.DBusGMainLoop(set_as_default=True) - import os if 'DBUS_SESSION_BUS_ADDRESS' in os.environ: bus = dbus.SessionBus() else: @@ -190,12 +239,16 @@ signal.signal(signal.SIGTERM, signal_handler) - _target = MockTarget(bus) # noqa: F841 - _mctpd = MockMctpd(bus) # noqa: F841 - _em = MockEM(bus) # noqa: F841 + em = MockEM(bus) + managed_objects = em.GetManagedObjects() + for path, interfaces in managed_objects.items(): + MockConfigObject(bus, str(path), interfaces) - _name_em = dbus.service.BusName("xyz.openbmc_project.EntityManager", bus) # noqa: F841 - _name_mctp = dbus.service.BusName("au.com.codeconstruct.MCTP1", bus) # noqa: F841 + mctpd = MockMctpd(bus) # noqa: F841 + + name_em = dbus.service.BusName("xyz.openbmc_project.EntityManager", bus) # noqa: F841 + name_mctp = dbus.service.BusName("au.com.codeconstruct.MCTP1", bus) # noqa: F841 + print("Mock mctpd and EM running...") sys.stdout.flush() loop.run()
diff --git a/tests/test_MctpUtil.cpp b/tests/test_MctpUtil.cpp index 5a0cc3c..6695118 100644 --- a/tests/test_MctpUtil.cpp +++ b/tests/test_MctpUtil.cpp
@@ -1344,6 +1344,227 @@ EXPECT_TRUE(sensorNotified); } +// 16. Bridged Endpoint Resolution +TEST_F(MctpUtilTest, BridgedEndpoint_ResolvesParentBridge) +{ + std::string bridgePath = + "/au/com/codeconstruct/mctp1/networks/1/endpoints/mctp_device"; + std::string bridgeEmPath = + "/xyz/openbmc_project/inventory/system/board/MockBoard/mctp_device"; + std::string endpointPath = + "/au/com/codeconstruct/mctp1/networks/1/endpoints/test_bridged"; + std::string emConfigPath = + "/xyz/openbmc_project/inventory/system/board/MockBoard/bridged_endpoint_config"; + + setupMctpEndpointListener(conn, MctpMessageType::NVME_MI); + + int queryCompletedCount = 0; + const std::string queryCompletedSpec = + "type='signal',interface='com.example.Control',member='QueryCompleted'"; + auto queryCompletedMatch = std::make_unique<sdbusplus::bus::match_t>( + static_cast<sdbusplus::bus_t&>(*conn), queryCompletedSpec, + [&](sdbusplus::message_t&) { queryCompletedCount++; }); + + // Wait for priming loop to complete (finding nothing) + io.poll(); + usleep(100000); // 100ms + io.poll(); + + std::string addBridgeCmd = + "busctl call au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1 com.example.Control TriggerAdd ss \"" + + bridgePath + "\" \"" + bridgeEmPath + "\""; + runCmd(addBridgeCmd); + + // Wait for bridge query to complete + for (int i = 0; i < 50; i++) + { + io.poll(); + usleep(100000); // 100ms + if (queryCompletedCount >= 1) + { + break; + } + } + ASSERT_EQ(queryCompletedCount, 1); + + // Wait for mapperx to create association for bridge + bool found = false; + for (int i = 0; i < 50; i++) + { + if (checkPathInMapper(bridgePath + "/configured_by")) + { + found = true; + break; + } + io.poll(); + usleep(100000); // 100ms + } + ASSERT_TRUE(found) << "Timed out waiting for bridge association in mapper"; + + // 2. Trigger Add for Endpoint! + std::string addEpCmd = + "busctl call au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1 com.example.Control TriggerAdd ss \"" + + endpointPath + "\" \"" + emConfigPath + "\""; + runCmd(addEpCmd); + + // Wait for endpoint query to complete + for (int i = 0; i < 50; i++) + { + io.poll(); + usleep(100000); // 100ms + if (queryCompletedCount >= 2) + { + break; + } + } + EXPECT_EQ(queryCompletedCount, 2); + + auto it = mctpEndpointConfigMap.find(endpointPath); + ASSERT_NE(it, mctpEndpointConfigMap.end()); + EXPECT_EQ(it->second.size(), 2); + + MctpReactorBusInfo busInfo(it->second); + EXPECT_TRUE(busInfo.isValid()); + EXPECT_EQ(busInfo.getType(), "USB"); + EXPECT_EQ(busInfo.getProperties().at("Port"), "1.5.1"); +} + +// 17. Broken Bridge Chain (Non-existent bridge) +TEST_F(MctpUtilTest, BridgedEndpoint_BrokenChain_Nonexistent_Fails) +{ + std::string endpointPath = + "/au/com/codeconstruct/mctp1/networks/1/endpoints/test_broken_nonexistent"; + std::string emConfigPath = + "/xyz/openbmc_project/inventory/system/board/MockBoard/broken_endpoint_nonexistent"; + + setupMctpEndpointListener(conn, MctpMessageType::NVME_MI); + + int queryCompletedCount = 0; + const std::string queryCompletedSpec = + "type='signal',interface='com.example.Control',member='QueryCompleted'"; + auto queryCompletedMatch = std::make_unique<sdbusplus::bus::match_t>( + static_cast<sdbusplus::bus_t&>(*conn), queryCompletedSpec, + [&](sdbusplus::message_t&) { queryCompletedCount++; }); + + // Wait for priming loop to complete (finding nothing) + io.poll(); + usleep(100000); // 100ms + io.poll(); + + std::string addCmd = + "busctl call au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1 com.example.Control TriggerAdd ss \"" + + endpointPath + "\" \"" + emConfigPath + "\""; + runCmd(addCmd); + + for (int i = 0; i < 50; i++) + { + io.poll(); + usleep(100000); // 100ms + if (queryCompletedCount >= 1) + { + break; + } + } + EXPECT_EQ(queryCompletedCount, 1); + + auto it = mctpEndpointConfigMap.find(endpointPath); + EXPECT_EQ(it, mctpEndpointConfigMap.end()); +} + +// 18. Multi-Level Bridging +TEST_F(MctpUtilTest, BridgedEndpoint_MultiLevel_ResolvesRootBridge) +{ + std::string rootBridgePath = + "/au/com/codeconstruct/mctp1/networks/1/endpoints/mctp_device"; + std::string rootBridgeEmPath = + "/xyz/openbmc_project/inventory/system/board/MockBoard/mctp_device"; + std::string interBridgePath = + "/au/com/codeconstruct/mctp1/networks/1/endpoints/intermediate_bridge"; + std::string interBridgeEmPath = + "/xyz/openbmc_project/inventory/system/board/MockBoard/intermediate_bridge"; + std::string endpointPath = + "/au/com/codeconstruct/mctp1/networks/1/endpoints/test_multilevel"; + std::string emConfigPath = + "/xyz/openbmc_project/inventory/system/board/MockBoard/multilevel_endpoint"; + + setupMctpEndpointListener(conn, MctpMessageType::NVME_MI); + + int queryCompletedCount = 0; + const std::string queryCompletedSpec = + "type='signal',interface='com.example.Control',member='QueryCompleted'"; + auto queryCompletedMatch = std::make_unique<sdbusplus::bus::match_t>( + static_cast<sdbusplus::bus_t&>(*conn), queryCompletedSpec, + [&](sdbusplus::message_t&) { queryCompletedCount++; }); + + // Wait for priming loop to complete (finding nothing) + io.poll(); + usleep(100000); // 100ms + io.poll(); + + // 1. Trigger Add for Root Bridge! + std::string addRootCmd = + "busctl call au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1 com.example.Control TriggerAdd ss \"" + + rootBridgePath + "\" \"" + rootBridgeEmPath + "\""; + runCmd(addRootCmd); + + // Wait for root bridge query to complete + for (int i = 0; i < 50; i++) + { + io.poll(); + usleep(100000); // 100ms + if (queryCompletedCount >= 1) + { + break; + } + } + ASSERT_EQ(queryCompletedCount, 1); + + // 2. Trigger Add for Intermediate Bridge! + std::string addInterCmd = + "busctl call au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1 com.example.Control TriggerAdd ss \"" + + interBridgePath + "\" \"" + interBridgeEmPath + "\""; + runCmd(addInterCmd); + + // Wait for intermediate bridge query to complete + for (int i = 0; i < 50; i++) + { + io.poll(); + usleep(100000); // 100ms + if (queryCompletedCount >= 2) + { + break; + } + } + ASSERT_EQ(queryCompletedCount, 2); + + // 3. Trigger Add for Endpoint! + std::string addEpCmd = + "busctl call au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1 com.example.Control TriggerAdd ss \"" + + endpointPath + "\" \"" + emConfigPath + "\""; + runCmd(addEpCmd); + + // Wait for endpoint query to complete + for (int i = 0; i < 50; i++) + { + io.poll(); + usleep(100000); // 100ms + if (queryCompletedCount >= 3) + { + break; + } + } + EXPECT_EQ(queryCompletedCount, 3); + + auto it = mctpEndpointConfigMap.find(endpointPath); + ASSERT_NE(it, mctpEndpointConfigMap.end()); + EXPECT_EQ(it->second.size(), 3); + + MctpReactorBusInfo busInfo(it->second); + EXPECT_TRUE(busInfo.isValid()); + EXPECT_EQ(busInfo.getType(), "USB"); + EXPECT_EQ(busInfo.getProperties().at("Port"), "1.5.1"); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv);
diff --git a/tests/test_nvme_mi.cpp b/tests/test_nvme_mi.cpp index 2fd8572..3874fbc 100644 --- a/tests/test_nvme_mi.cpp +++ b/tests/test_nvme_mi.cpp
@@ -549,7 +549,7 @@ return mock.fake->miSubsystemHealthStatusPoll( std::move(cb)); }); - timer.expires_after(subsysPollTime * 2); + timer.expires_after(subsysPollTime * 5); timer.async_wait([&](boost::system::error_code) { systemBus->async_method_call( [&](boost::system::error_code,