nvme: support bridged MCTP endpoints
Updated nvmed to support bridged MCTP endpoints by parsing a list of
BusInfo interfaces from Entity-Manager. Moved the matching logic into
MctpReactorDevice::setup() to keep the device self-resolving and
encapsulate discovery logic. Refactored MctpReactorBusInfo to
maintain root-to-leaf order for consistency, and added an operator==
overload for subset matching.
Example EM config for a bridged device:
```
{
"Protocol": "mi_mctp",
"Type": "NVME1000",
"xyz.openbmc_project.Configuration.NVME1000.BusInfo0": {
"BusType": "USB",
"Port": "1.5.1",
"RootHubPath": "/sys/devices/platform/ahb/f0806000.usb"
},
"xyz.openbmc_project.Configuration.NVME1000.BusInfo1": {
"BusType": "MCTP Bridge",
"EIDOffset": 2
}
}
```
Tested: Ran unit tests in Docker container (test_mctp_util, test_nvme).
Google-Bug-Id: 509971428
Change-Id: I8633ed947d1c58d6fb948f5e6bd43e8521fa042e
Signed-off-by: Hao Jiang <jianghao@google.com>
diff --git a/src/MctpReactorDevice.cpp b/src/MctpReactorDevice.cpp
index f3386b3..fb369af 100644
--- a/src/MctpReactorDevice.cpp
+++ b/src/MctpReactorDevice.cpp
@@ -122,7 +122,7 @@
MctpReactorDevice::MctpReactorDevice(
const std::shared_ptr<sdbusplus::asio::connection>& connection,
- const std::string& deviceObjectPath, const BusInfo& busInfo,
+ const std::string& deviceObjectPath, const std::vector<BusInfo>& busInfo,
std::optional<int> busNumber, std::optional<int> address) :
connection(connection), deviceObjectPath(deviceObjectPath),
prettyObjectName(extractSuffixFromObjectPath(deviceObjectPath)),
@@ -173,9 +173,13 @@
for (const auto& [epPath, epConfigChain] : mctpEndpointConfigMap)
{
- MctpReactorBusInfo epBusInfo(epConfigChain);
- if (epBusInfo.isValid() &&
- epBusInfo.getProperties() == targetBusInfo)
+ MctpReactorBusInfo epBusInfoObj(epConfigChain);
+ if (!epBusInfoObj.isValid())
+ {
+ continue;
+ }
+
+ if (epBusInfoObj == targetBusInfo)
{
finaliseEndpoint(epPath, std::move(cb));
return;
diff --git a/src/MctpReactorDevice.hpp b/src/MctpReactorDevice.hpp
index 212a355..19f24cb 100644
--- a/src/MctpReactorDevice.hpp
+++ b/src/MctpReactorDevice.hpp
@@ -28,8 +28,9 @@
std::optional<int> address);
MctpReactorDevice(
const std::shared_ptr<sdbusplus::asio::connection>& connection,
- const std::string& deviceObjectPath, const BusInfo& busInfo,
- std::optional<int> busNumber, std::optional<int> address);
+ const std::string& deviceObjectPath,
+ const std::vector<BusInfo>& busInfo, std::optional<int> busNumber,
+ std::optional<int> address);
MctpReactorDevice(const MctpDevice& other) = delete;
MctpReactorDevice(MctpDevice&& other) = delete;
~MctpReactorDevice() override;
@@ -62,7 +63,7 @@
std::shared_ptr<MctpdEndpoint> endpoint;
std::unique_ptr<sdbusplus::bus::match_t> removeMatch;
- BusInfo targetBusInfo;
+ std::vector<BusInfo> targetBusInfo;
std::optional<MctpCallbackToken> callbackToken;
std::function<void(const std::error_code&,
const std::shared_ptr<MctpEndpoint>&)>
diff --git a/src/MctpUtil.cpp b/src/MctpUtil.cpp
index 85deb4c..c1a3f48 100644
--- a/src/MctpUtil.cpp
+++ b/src/MctpUtil.cpp
@@ -11,6 +11,7 @@
#include <chrono>
#include <cstdint>
#include <filesystem>
+#include <ranges>
std::map<std::string, std::vector<SensorData>> mctpEndpointConfigMap;
@@ -617,20 +618,27 @@
};
// 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)
+ // configDataChain include all config/busInfo from its bottom-most bridge
+ // routing info (a.k.a BridgeDownsteamMCTPDDevice) to the top-most port info
+ // (a.k.a MCTPUSBDevice/etc).
+
+ // And MctpReactorBusInfo extracts the config for
+ // both routing and the top most port info and put them into a reverse order
+ // (from top most port to the bottom most routing) to accomodate the
+ // NVMe1000 bus config
+ for (const auto& configData : std::views::reverse(configDataChain))
{
+ std::string type;
+ std::map<std::string, std::string> properties;
+ bool levelValid = false;
+
for (const auto& [intf, props] : configData)
{
if (absl::StrContains(
intf, "xyz.openbmc_project.Configuration.MCTPUSBDevice"))
{
type = "USB";
- valid = true;
+ levelValid = true;
properties["BusType"] = "USB";
extractProp(props, properties, "RootHubPath");
@@ -643,7 +651,7 @@
intf, "xyz.openbmc_project.Configuration.MCTPI2CTarget"))
{
type = "I2C";
- valid = true;
+ levelValid = true;
properties["BusType"] = "I2C";
extractProp(props, properties, "Bus");
@@ -654,17 +662,60 @@
intf, "xyz.openbmc_project.Configuration.MCTPI3CTarget"))
{
type = "I3C";
- valid = true;
+ levelValid = true;
properties["BusType"] = "I3C";
extractProp(props, properties, "Bus");
extractProp(props, properties, "Address");
break;
}
+ if (absl::StrContains(
+ intf,
+ "xyz.openbmc_project.Configuration.MCTPBridgeDownstreamDevice"))
+ {
+ type = "MCTP Bridge";
+ levelValid = true;
+ properties["BusType"] = "MCTP Bridge";
+
+ extractProp(props, properties, "EIDOffset");
+ break;
+ }
+ }
+
+ if (levelValid)
+ {
+ types.push_back(type);
+ propertiesList.push_back(properties);
+ valid = true;
}
}
}
+bool MctpReactorBusInfo::operator==(
+ const std::vector<BusInfo>& targetBusInfo) const
+{
+ if (propertiesList.size() != targetBusInfo.size())
+ {
+ return false;
+ }
+
+ for (size_t i = 0; i < targetBusInfo.size(); ++i)
+ {
+ const auto& configBusInfo = targetBusInfo[i];
+ const auto& epProps = propertiesList[i];
+
+ for (const auto& [key, val] : configBusInfo)
+ {
+ auto it = epProps.find(key);
+ if (it == epProps.end() || it->second != val)
+ {
+ return false;
+ }
+ }
+ }
+ return true;
+}
+
void cleanupMctpEndpointListener()
{
associationMatch.reset();
diff --git a/src/MctpUtil.hpp b/src/MctpUtil.hpp
index 7a049fb..3372235 100644
--- a/src/MctpUtil.hpp
+++ b/src/MctpUtil.hpp
@@ -27,23 +27,25 @@
{
public:
explicit MctpReactorBusInfo(const std::vector<SensorData>& configData);
- std::string getType() const
+ bool operator==(const std::vector<BusInfo>& targetBusInfo) const;
+ std::vector<std::string> getType() const
{
- return type;
+ return types;
}
bool isValid() const
{
return valid;
}
- const std::map<std::string, std::string>& getProperties() const
+ const std::vector<std::map<std::string, std::string>> &
+ getProperties() const
{
- return properties;
+ return propertiesList;
}
private:
- std::string type;
+ std::vector<std::string> types;
bool valid = false;
- std::map<std::string, std::string> properties;
+ std::vector<std::map<std::string, std::string>> propertiesList;
};
extern std::map<std::string, std::vector<SensorData>> mctpEndpointConfigMap;
diff --git a/src/NVMeSensorMain.cpp b/src/NVMeSensorMain.cpp
index c64b106..3a71371 100644
--- a/src/NVMeSensorMain.cpp
+++ b/src/NVMeSensorMain.cpp
@@ -200,9 +200,13 @@
return false;
}
-static BusInfo extractNVMeBusInfo(const SensorData& configData)
+static std::vector<BusInfo> extractNVMeBusInfo(const SensorData& configData)
{
- BusInfo busInfo;
+ std::vector<BusInfo> busInfoList;
+ std::map<int, BusInfo> indexedBusInfo;
+ BusInfo legacyBusInfo;
+ bool hasLegacy = false;
+
for (const auto& pair : configData)
{
const std::string& intf = pair.first;
@@ -210,6 +214,7 @@
if (absl::StrContains(
intf, "xyz.openbmc_project.Configuration.NVME1000.BusInfo"))
{
+ BusInfo busInfo;
std::string busType;
auto it = props.find("BusType");
if (it != props.end())
@@ -249,14 +254,55 @@
extractProp("Bus");
extractProp("Address");
}
+ else if (busType == "MCTP Bridge")
+ {
+ extractProp("EIDOffset");
+ }
else if (!busType.empty())
{
lg2::warning("Unsupported BusType {TYPE} in NVME1000 config",
"TYPE", busType);
}
+
+ std::string suffix = intf.substr(
+ std::string(
+ "xyz.openbmc_project.Configuration.NVME1000.BusInfo")
+ .length());
+ if (suffix.empty())
+ {
+ legacyBusInfo = busInfo;
+ hasLegacy = true;
+ }
+ else
+ {
+ try
+ {
+ int index = std::stoi(suffix);
+ indexedBusInfo[index] = busInfo;
+ }
+ catch (const std::exception& e)
+ {
+ lg2::error(
+ "Failed to parse index from interface {INTF}: {ERROR}",
+ "INTF", intf, "ERROR", e.what());
+ }
+ }
}
}
- return busInfo;
+
+ if (hasLegacy)
+ {
+ busInfoList.push_back(legacyBusInfo);
+ }
+ else
+ {
+ for (const auto& [index, busInfo] : indexedBusInfo)
+ {
+ busInfoList.push_back(busInfo);
+ }
+ }
+
+ return busInfoList;
}
static void handleConfigurations(
@@ -287,7 +333,8 @@
std::optional<int> busNumber;
std::optional<int> address;
- BusInfo busInfo;
+ std::vector<BusInfo> busInfoList;
+ std::string endpointPath;
if (*nvmeProtocol == "mi_basic" || *nvmeProtocol == "mi_i2c")
{
@@ -300,33 +347,32 @@
}
else if (*nvmeProtocol == "mi_mctp")
{
- busInfo = extractNVMeBusInfo(configData);
- if (busInfo.empty())
+ busInfoList = extractNVMeBusInfo(configData);
+ if (busInfoList.empty())
{
lg2::error("Missing BusInfo for mi_mctp device {PATH}", "PATH",
nvmeObjectPath.str);
continue;
}
- auto itType = busInfo.find("BusType");
- if (itType != busInfo.end() && itType->second == "I2C")
- {
- auto itBus = busInfo.find("Bus");
- if (itBus != busInfo.end())
- {
- busNumber = std::stoi(itBus->second);
- }
- auto itAddr = busInfo.find("Address");
- if (itAddr != busInfo.end())
- {
- address = std::stoi(itAddr->second, nullptr, 0);
- }
+ // Matching is now handled in MctpReactorDevice::setup()
- if (!busNumber)
+ if (!busInfoList.empty())
+ {
+ const auto& rootBusInfo = busInfoList.front();
+ auto itType = rootBusInfo.find("BusType");
+ if (itType != rootBusInfo.end() && itType->second == "I2C")
{
- lg2::error("Missing Bus in BusInfo for {PATH}", "PATH",
- nvmeObjectPath.str);
- continue;
+ auto itBus = rootBusInfo.find("Bus");
+ if (itBus != rootBusInfo.end())
+ {
+ busNumber = std::stoi(itBus->second);
+ }
+ auto itAddr = rootBusInfo.find("Address");
+ if (itAddr != rootBusInfo.end())
+ {
+ address = std::stoi(itAddr->second, nullptr, 0);
+ }
}
}
}
@@ -432,8 +478,8 @@
if (*nvmeProtocol == "mi_mctp")
{
mctpDev = std::make_shared<MctpReactorDevice>(
- dbusConnection, nvmeObjectPath.str, busInfo, busNumber,
- address);
+ dbusConnection, nvmeObjectPath.str, busInfoList,
+ busNumber, address);
}
else if (!isMctpReactorDevice)
{
diff --git a/tests/test_MctpUtil.cpp b/tests/test_MctpUtil.cpp
index ee110d2..cf2bfe3 100644
--- a/tests/test_MctpUtil.cpp
+++ b/tests/test_MctpUtil.cpp
@@ -1050,7 +1050,8 @@
// Create MctpReactorDevice
auto mctpDev = std::make_shared<MctpReactorDevice>(
- conn, emConfigPath, targetBusInfo, std::nullopt, std::nullopt);
+ conn, emConfigPath, std::vector<BusInfo>{targetBusInfo}, std::nullopt,
+ std::nullopt);
mctpDev->setup([&](const std::error_code& ec,
const std::shared_ptr<MctpEndpoint>& ep) {
setupCompleted = true;
@@ -1126,7 +1127,8 @@
// Create MctpReactorDevice
auto mctpDev = std::make_shared<MctpReactorDevice>(
- conn, emConfigPath, targetBusInfo, std::nullopt, std::nullopt);
+ conn, emConfigPath, std::vector<BusInfo>{targetBusInfo}, std::nullopt,
+ std::nullopt);
mctpDev->setup([&](const std::error_code& ec,
const std::shared_ptr<MctpEndpoint>& ep) {
setupCompleted = true;
@@ -1185,7 +1187,8 @@
// Create MctpReactorDevice
auto mctpDev = std::make_shared<MctpReactorDevice>(
- conn, emConfigPath, targetBusInfo, std::nullopt, std::nullopt);
+ conn, emConfigPath, std::vector<BusInfo>{targetBusInfo}, std::nullopt,
+ std::nullopt);
mctpDev->setup([&](const std::error_code& ec,
const std::shared_ptr<MctpEndpoint>& ep) {
setupCompleted = true;
@@ -1243,7 +1246,8 @@
// Create MctpReactorDevice
auto mctpDev = std::make_shared<MctpReactorDevice>(
- conn, emConfigPath, targetBusInfo, std::nullopt, std::nullopt);
+ conn, emConfigPath, std::vector<BusInfo>{targetBusInfo}, std::nullopt,
+ std::nullopt);
mctpDev->setup([&](const std::error_code& ec,
const std::shared_ptr<MctpEndpoint>& ep) {
setupCompleted = true;
@@ -1312,7 +1316,8 @@
// Create MctpReactorDevice
auto mctpDev = std::make_shared<MctpReactorDevice>(
- conn, emConfigPath, targetBusInfo, std::nullopt, std::nullopt);
+ conn, emConfigPath, std::vector<BusInfo>{targetBusInfo},
+ std::nullopt, std::nullopt);
mctpDev->setup([&](const std::error_code&,
const std::shared_ptr<MctpEndpoint>& ep) {
@@ -1425,8 +1430,8 @@
MctpReactorBusInfo busInfo(it->second);
EXPECT_TRUE(busInfo.isValid());
- EXPECT_EQ(busInfo.getType(), "USB");
- EXPECT_EQ(busInfo.getProperties().at("Port"), "1.5.1");
+ EXPECT_EQ(busInfo.getType().front(), "USB");
+ EXPECT_EQ(busInfo.getProperties().front().at("Port"), "1.5.1");
}
// 17. Broken Bridge Chain (Non-existent bridge)
@@ -1561,8 +1566,8 @@
MctpReactorBusInfo busInfo(it->second);
EXPECT_TRUE(busInfo.isValid());
- EXPECT_EQ(busInfo.getType(), "USB");
- EXPECT_EQ(busInfo.getProperties().at("Port"), "1.5.1");
+ EXPECT_EQ(busInfo.getType().front(), "USB");
+ EXPECT_EQ(busInfo.getProperties().front().at("Port"), "1.5.1");
}
// 19. Out-of-Order Triggers (Endpoint before Bridge) - Expect Success after
@@ -1776,6 +1781,73 @@
EXPECT_NE(it, mctpEndpointConfigMap.end());
}
+// 22. MctpReactorBusInfo chain parsing and operator== matching
+TEST_F(MctpUtilTest, MctpReactorBusInfo_ChainParsingAndMatching)
+{
+ std::vector<SensorData> chain;
+
+ // Leaf (endpoint)
+ SensorBaseConfigMap epProps;
+ epProps["EIDOffset"] = uint64_t(2);
+ SensorData epData;
+ epData["xyz.openbmc_project.Configuration.MCTPBridgeDownstreamDevice"] =
+ epProps;
+ chain.push_back(epData);
+
+ // Bridge
+ SensorBaseConfigMap bridgeProps;
+ bridgeProps["BusType"] = std::string("USB");
+ bridgeProps["Port"] = std::string("1.5.1");
+ bridgeProps["Configuration"] =
+ std::string("1"); // Extra property not in target
+ SensorData bridgeData;
+ bridgeData["xyz.openbmc_project.Configuration.MCTPUSBDevice"] = bridgeProps;
+ chain.push_back(bridgeData);
+
+ MctpReactorBusInfo busInfo(chain);
+
+ EXPECT_TRUE(busInfo.isValid());
+ auto propsList = busInfo.getProperties();
+ ASSERT_EQ(propsList.size(), 2);
+
+ // Verify order is root-to-leaf (from Test 22)
+ EXPECT_EQ(propsList[0].at("BusType"), "USB");
+ EXPECT_EQ(propsList[0].at("Port"), "1.5.1");
+
+ EXPECT_EQ(propsList[1].at("BusType"), "MCTP Bridge");
+ EXPECT_EQ(propsList[1].at("EIDOffset"), "2");
+
+ // Verify operator== subset matching (from Test 23)
+ std::vector<BusInfo> targetConfig;
+
+ // Root level
+ BusInfo targetBridge;
+ targetBridge["BusType"] = "USB";
+ targetBridge["Port"] = "1.5.1";
+ targetConfig.push_back(targetBridge);
+
+ // Leaf level
+ BusInfo targetEp;
+ targetEp["EIDOffset"] = "2";
+ targetConfig.push_back(targetEp);
+
+ // Should match because target is a subset of extracted properties
+ EXPECT_TRUE(busInfo == targetConfig);
+
+ // Test mismatch in value
+ targetConfig[1]["EIDOffset"] = "3";
+ EXPECT_FALSE(busInfo == targetConfig);
+
+ // Test missing property in extracted props
+ targetConfig[1]["EIDOffset"] = "2";
+ targetConfig[1]["ExtraProp"] = "value";
+ EXPECT_FALSE(busInfo == targetConfig);
+
+ // Test size mismatch
+ targetConfig.pop_back();
+ EXPECT_FALSE(busInfo == targetConfig);
+}
+
int main(int argc, char** argv)
{
::testing::InitGoogleTest(&argc, argv);
