platform-mc: Handle `Connectivity` propertiesChanged signal
From Mctp codeConstruct version 2.0 [1], mctpd supports `.Connectivity`
property under `au.com.CodeConstruct.MCTP.Endpoint` interface of the
endpoint. This commit handles the propertiesChanged signal from this
interface, and updates the Availability of the MCTP Endpoint accordingly
in the source to enable or disable message sending/receiving via that
endpoint of the terminus.
[1] https://github.com/CodeConstruct/mctp/blob/v2.0/docs/endpoint-recovery.md#proposed-design
When the discovery process first starts, it will only handle the
endpoints that have `Available` `.Connectivity`. It lets the
propertiesChanged signal trigger the adding of the endpoints when they
are back to` Available`.
On interfaceAdded signal, it assumes that mctpd only publishes available
endpoints to D-Bus, so it adds the endpoints to the terminus anyway.
Tested:
1. Enable `unsafe-writable-connectivity` option in PACKAGECONFIG of mctp
recipe.
2. After PLDM discovers all the endpoints, write `Degraded` to
`.Connectivity` of one of the endpoint.
3. Write it back to `Available` to see how message is stopped from being
sent/received via the endpoint.
Signed-off-by: Chau Ly <chaul@amperecomputing.com>
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>
Change-Id: I5b7a38ae72e655b60d71396a1118f2809aaa3838
diff --git a/fw-update/manager.hpp b/fw-update/manager.hpp
index 4d2a976..14ac450 100644
--- a/fw-update/manager.hpp
+++ b/fw-update/manager.hpp
@@ -72,6 +72,17 @@
return;
}
+ /** @brief Helper function to invoke registered handlers for
+ * updating the availability status of the MCTP endpoint
+ *
+ * @param[in] mctpInfo - information of the target endpoint
+ * @param[in] availability - new availability status
+ */
+ void updateMctpEndpointAvailability(const MctpInfo&, Availability)
+ {
+ return;
+ }
+
/** @brief Handle PLDM request for the commands in the FW update
* specification
*
diff --git a/platform-mc/manager.hpp b/platform-mc/manager.hpp
index 213f1f5..5654099 100644
--- a/platform-mc/manager.hpp
+++ b/platform-mc/manager.hpp
@@ -76,6 +76,32 @@
terminusManager.removeMctpTerminus(mctpInfos);
}
+ /** @brief Helper function to invoke registered handlers for
+ * updating the availability status of the MCTP endpoint
+ *
+ * @param[in] mctpInfo - information of the target endpoint
+ * @param[in] availability - new availability status
+ */
+ void updateMctpEndpointAvailability(const MctpInfo& mctpInfo,
+ Availability availability)
+ {
+ /* Get TID of initialized terminus */
+ auto tid = terminusManager.toTid(mctpInfo);
+ if (tid)
+ {
+ if (availability)
+ {
+ sensorManager.startSensorPollTimer(tid.value());
+ }
+ else
+ {
+ sensorManager.disableTerminusSensors(tid.value());
+ }
+ updateAvailableState(tid.value(), availability);
+ }
+ terminusManager.updateMctpEndpointAvailability(mctpInfo, availability);
+ }
+
/** @brief Helper function to start sensor polling of the terminus TID
*/
void startSensorPolling(pldm_tid_t tid)
diff --git a/platform-mc/sensor_manager.cpp b/platform-mc/sensor_manager.cpp
index 5924106..d29e552 100644
--- a/platform-mc/sensor_manager.cpp
+++ b/platform-mc/sensor_manager.cpp
@@ -52,6 +52,11 @@
event.get(),
std::bind_front(&SensorManager::doSensorPolling, this, tid));
+ startSensorPollTimer(tid);
+}
+
+void SensorManager::startSensorPollTimer(pldm_tid_t tid)
+{
try
{
if (sensorPollTimers[tid] && !sensorPollTimers[tid]->isRunning())
@@ -71,6 +76,22 @@
}
}
+void SensorManager::disableTerminusSensors(pldm_tid_t tid)
+{
+ if (!termini.contains(tid))
+ {
+ return;
+ }
+
+ // numeric sensor
+ auto terminus = termini[tid];
+ for (auto& sensor : terminus->numericSensors)
+ {
+ sensor->updateReading(true, false,
+ std::numeric_limits<double>::quiet_NaN());
+ }
+}
+
void SensorManager::stopPolling(pldm_tid_t tid)
{
/* Stop polling timer */
diff --git a/platform-mc/sensor_manager.hpp b/platform-mc/sensor_manager.hpp
index 29172bb..c82b336 100644
--- a/platform-mc/sensor_manager.hpp
+++ b/platform-mc/sensor_manager.hpp
@@ -44,6 +44,15 @@
*/
void startPolling(pldm_tid_t tid);
+ /** @brief Helper function to start sensor polling timer
+ */
+ void startSensorPollTimer(pldm_tid_t tid);
+
+ /** @brief Helper function to set all terminus sensor as nan when the
+ * terminus is not available for pldm request
+ */
+ void disableTerminusSensors(pldm_tid_t tid);
+
/** @brief stopping sensor polling task
*/
void stopPolling(pldm_tid_t tid);
diff --git a/platform-mc/terminus_manager.cpp b/platform-mc/terminus_manager.cpp
index dd5ee7f..be70740 100644
--- a/platform-mc/terminus_manager.cpp
+++ b/platform-mc/terminus_manager.cpp
@@ -125,6 +125,21 @@
return true;
}
+void TerminusManager::updateMctpEndpointAvailability(const MctpInfo& mctpInfo,
+ Availability availability)
+{
+ mctpInfoAvailTable.insert_or_assign(mctpInfo, availability);
+
+ if (manager)
+ {
+ auto tid = toTid(mctpInfo);
+ if (tid)
+ {
+ manager->updateAvailableState(tid.value(), availability);
+ }
+ }
+}
+
void TerminusManager::discoverMctpTerminus(const MctpInfos& mctpInfos)
{
queuedMctpInfos.emplace(mctpInfos);
@@ -176,6 +191,7 @@
auto it = findTerminusPtr(mctpInfo);
if (it == termini.end())
{
+ mctpInfoAvailTable[mctpInfo] = true;
co_await initMctpTerminus(mctpInfo);
}
@@ -183,6 +199,7 @@
auto tid = toTid(mctpInfo);
if (!tid)
{
+ mctpInfoAvailTable.erase(mctpInfo);
co_return PLDM_ERROR;
}
addedTids.push_back(tid.value());
@@ -221,6 +238,7 @@
unmapTid(it->first);
termini.erase(it);
+ mctpInfoAvailTable.erase(mctpInfo);
}
}
@@ -627,6 +645,17 @@
co_return PLDM_ERROR_NOT_READY;
}
+ // There's a cost of maintaining another table to hold availability
+ // status as we can't ensure that it always synchronizes with the
+ // mctpInfoTable; std::map operator[] will insert a default of boolean
+ // which is false to the mctpInfoAvailTable if the mctpInfo key doesn't
+ // exist. Once we miss to initialize the availability of an available
+ // endpoint, it will drop all the messages to/from it.
+ if (!mctpInfoAvailTable[mctpInfo.value()])
+ {
+ co_return PLDM_ERROR_NOT_READY;
+ }
+
auto eid = std::get<0>(mctpInfo.value());
auto requestMsg = reinterpret_cast<pldm_msg*>(request.data());
requestMsg->hdr.instance_id = instanceIdDb.next(eid);
diff --git a/platform-mc/terminus_manager.hpp b/platform-mc/terminus_manager.hpp
index c37d74d..c4c8914 100644
--- a/platform-mc/terminus_manager.hpp
+++ b/platform-mc/terminus_manager.hpp
@@ -154,6 +154,15 @@
return localEid;
}
+ /** @brief Helper function to invoke registered handlers for
+ * updating the availability status of the MCTP endpoint
+ *
+ * @param[in] mctpInfo - information of the target endpoint
+ * @param[in] availability - new availability status
+ */
+ void updateMctpEndpointAvailability(const MctpInfo& mctpInfo,
+ Availability availability);
+
private:
/** @brief Find the terminus object pointer in termini list.
*
@@ -258,6 +267,9 @@
/** @brief local EID */
mctp_eid_t localEid;
+
+ /** @brief MCTP Endpoint available status mapping */
+ std::map<MctpInfo, Availability> mctpInfoAvailTable;
};
} // namespace platform_mc
} // namespace pldm
diff --git a/platform-mc/test/event_manager_test.cpp b/platform-mc/test/event_manager_test.cpp
index 5fb53e5..5ba9d80 100644
--- a/platform-mc/test/event_manager_test.cpp
+++ b/platform-mc/test/event_manager_test.cpp
@@ -359,6 +359,9 @@
sizeof(eventMessageSupportedResp));
EXPECT_EQ(rc, PLDM_SUCCESS);
+ terminusManager.updateMctpEndpointAvailability(
+ pldm::MctpInfo(10, "", "", 1), true);
+
// queue SetEventReceiver response
const size_t SetEventReceiverLen = 1;
PLDM_GET_PDR_REPOSITORY_INFO_RESP_BYTES;
@@ -483,6 +486,9 @@
sizeof(pollForPlatformEventMessage3Resp));
EXPECT_EQ(rc, PLDM_SUCCESS);
+ terminusManager.updateMctpEndpointAvailability(
+ pldm::MctpInfo(10, "", "", 1), true);
+
EXPECT_CALL(eventManager, processCperEvent(_, _, _, _))
.Times(1)
.WillRepeatedly(Return(1));
diff --git a/platform-mc/test/platform_manager_test.cpp b/platform-mc/test/platform_manager_test.cpp
index 09f94e5..cf6567d 100644
--- a/platform-mc/test/platform_manager_test.cpp
+++ b/platform-mc/test/platform_manager_test.cpp
@@ -183,8 +183,12 @@
sizeof(getPdrAuxNameResp));
EXPECT_EQ(rc, PLDM_SUCCESS);
+ mockTerminusManager.updateMctpEndpointAvailability(
+ pldm::MctpInfo(10, "", "", 1), true);
+
stdexec::sync_wait(platformManager.initTerminus());
EXPECT_EQ(true, terminus->initialized);
+ EXPECT_EQ(true, terminus->doesSupportCommand(PLDM_PLATFORM, PLDM_GET_PDR));
EXPECT_EQ(2, terminus->pdrs.size());
EXPECT_EQ(1, terminus->numericSensors.size());
EXPECT_EQ("S0", terminus->getTerminusName().value());
@@ -342,6 +346,9 @@
sizeof(getPdrAuxNameResp));
EXPECT_EQ(rc, PLDM_SUCCESS);
+ mockTerminusManager.updateMctpEndpointAvailability(
+ pldm::MctpInfo(10, "", "", 1), true);
+
stdexec::sync_wait(platformManager.initTerminus());
EXPECT_EQ(true, terminus->initialized);
EXPECT_EQ(2, terminus->pdrs.size());
diff --git a/requester/mctp_endpoint_discovery.cpp b/requester/mctp_endpoint_discovery.cpp
index 1b6cd30..6833684 100644
--- a/requester/mctp_endpoint_discovery.cpp
+++ b/requester/mctp_endpoint_discovery.cpp
@@ -32,13 +32,27 @@
mctpEndpointRemovedSignal(
bus, interfacesRemoved(MCTPPath),
std::bind_front(&MctpDiscovery::removeEndpoints, this)),
+ mctpEndpointPropChangedSignal(
+ bus, propertiesChangedNamespace(MCTPPath, MCTPInterfaceCC),
+ std::bind_front(&MctpDiscovery::propertiesChangedCb, this)),
handlers(list)
{
- getMctpInfos(existingMctpInfos);
+ std::map<MctpInfo, Availability> currentMctpInfoMap;
+ getMctpInfos(currentMctpInfoMap);
+ for (const auto& mapIt : currentMctpInfoMap)
+ {
+ if (mapIt.second)
+ {
+ // Only add the available endpoints to the terminus
+ // Let the propertiesChanged signal tells us when it comes back
+ // to Available again
+ addToExistingMctpInfos(MctpInfos(1, mapIt.first));
+ }
+ }
handleMctpEndpoints(existingMctpInfos);
}
-void MctpDiscovery::getMctpInfos(MctpInfos& mctpInfos)
+void MctpDiscovery::getMctpInfos(std::map<MctpInfo, Availability>& mctpInfoMap)
{
// Find all implementations of the MCTP Endpoint interface
pldm::utils::GetSubTreeResponse mapperResponse;
@@ -63,13 +77,15 @@
const MctpEndpointProps& epProps =
getMctpEndpointProps(service, path);
const UUID& uuid = getEndpointUUIDProp(service, path);
+ const Availability& availability =
+ getEndpointConnectivityProp(path);
auto types = std::get<MCTPMsgTypes>(epProps);
if (std::find(types.begin(), types.end(), mctpTypePLDM) !=
types.end())
{
- mctpInfos.emplace_back(
- MctpInfo(std::get<eid>(epProps), uuid, "",
- std::get<NetworkId>(epProps)));
+ mctpInfoMap[MctpInfo(std::get<eid>(epProps), uuid, "",
+ std::get<NetworkId>(epProps))] =
+ availability;
}
}
}
@@ -128,6 +144,29 @@
return static_cast<UUID>(emptyUUID);
}
+Availability MctpDiscovery::getEndpointConnectivityProp(const std::string& path)
+{
+ Availability available = false;
+ try
+ {
+ pldm::utils::PropertyValue propertyValue =
+ pldm::utils::DBusHandler().getDbusPropertyVariant(
+ path.c_str(), MCTPConnectivityProp, MCTPInterfaceCC);
+ if (std::get<std::string>(propertyValue) == "Available")
+ {
+ available = true;
+ }
+ }
+ catch (const sdbusplus::exception_t& e)
+ {
+ error(
+ "Error reading Endpoint Connectivity property at path '{PATH}', error - {ERROR}",
+ "PATH", path, "ERROR", e);
+ }
+
+ return available;
+}
+
void MctpDiscovery::getAddedMctpInfos(sdbusplus::message_t& msg,
MctpInfos& mctpInfos)
{
@@ -149,6 +188,7 @@
"ERROR", e);
return;
}
+ const Availability& availability = getEndpointConnectivityProp(objPath.str);
/* Get UUID */
try
@@ -176,6 +216,15 @@
auto eid = std::get<mctp_eid_t>(properties.at("EID"));
auto types = std::get<std::vector<uint8_t>>(
properties.at("SupportedMessageTypes"));
+
+ if (!availability)
+ {
+ // Log an error message here, but still add it to the
+ // terminus
+ error(
+ "mctpd added a DEGRADED endpoint {EID} networkId {NET} to D-Bus",
+ "NET", networkId, "EID", static_cast<unsigned>(eid));
+ }
if (std::find(types.begin(), types.end(), mctpTypePLDM) !=
types.end())
{
@@ -222,6 +271,75 @@
}
}
+void MctpDiscovery::propertiesChangedCb(sdbusplus::message_t& msg)
+{
+ using Interface = std::string;
+ using Property = std::string;
+ using Value = std::string;
+ using Properties = std::map<Property, std::variant<Value>>;
+
+ Interface interface;
+ Properties properties;
+ std::string objPath{};
+ std::string service{};
+
+ try
+ {
+ msg.read(interface, properties);
+ objPath = msg.get_path();
+ }
+ catch (const sdbusplus::exception_t& e)
+ {
+ error(
+ "Error handling Connectivity property changed message, error - {ERROR}",
+ "ERROR", e);
+ return;
+ }
+
+ for (const auto& [key, valueVariant] : properties)
+ {
+ Value propVal = std::get<std::string>(valueVariant);
+ auto availability = (propVal == "Available") ? true : false;
+
+ if (key == MCTPConnectivityProp)
+ {
+ service = pldm::utils::DBusHandler().getService(objPath.c_str(),
+ MCTPInterface);
+ const MctpEndpointProps& epProps =
+ getMctpEndpointProps(service, objPath);
+
+ auto types = std::get<MCTPMsgTypes>(epProps);
+ if (!std::ranges::contains(types, mctpTypePLDM))
+ {
+ return;
+ }
+ const UUID& uuid = getEndpointUUIDProp(service, objPath);
+
+ MctpInfo mctpInfo(std::get<eid>(epProps), uuid, "",
+ std::get<NetworkId>(epProps));
+ if (!std::ranges::contains(existingMctpInfos, mctpInfo))
+ {
+ if (availability)
+ {
+ // The endpoint not in existingMctpInfos and is
+ // available Add it to existingMctpInfos
+ info(
+ "Adding Endpoint networkId {NETWORK} ID {EID} by propertiesChanged signal",
+ "NETWORK", std::get<3>(mctpInfo), "EID",
+ unsigned(std::get<0>(mctpInfo)));
+ addToExistingMctpInfos(MctpInfos(1, mctpInfo));
+ handleMctpEndpoints(MctpInfos(1, mctpInfo));
+ }
+ }
+ else
+ {
+ // The endpoint already in existingMctpInfos
+ updateMctpEndpointAvailability(mctpInfo, availability);
+ }
+ }
+ }
+}
+
void MctpDiscovery::discoverEndpoints(sdbusplus::message_t& msg)
{
MctpInfos addedInfos;
@@ -234,7 +352,12 @@
{
MctpInfos mctpInfos;
MctpInfos removedInfos;
- getMctpInfos(mctpInfos);
+ std::map<MctpInfo, Availability> currentMctpInfoMap;
+ getMctpInfos(currentMctpInfoMap);
+ for (const auto& mapIt : currentMctpInfoMap)
+ {
+ mctpInfos.push_back(mapIt.first);
+ }
removeFromExistingMctpInfos(mctpInfos, removedInfos);
handleRemovedMctpEndpoints(removedInfos);
}
@@ -261,4 +384,16 @@
}
}
+void MctpDiscovery::updateMctpEndpointAvailability(const MctpInfo& mctpInfo,
+ Availability availability)
+{
+ for (const auto& handler : handlers)
+ {
+ if (handler)
+ {
+ handler->updateMctpEndpointAvailability(mctpInfo, availability);
+ }
+ }
+}
+
} // namespace pldm
diff --git a/requester/mctp_endpoint_discovery.hpp b/requester/mctp_endpoint_discovery.hpp
index 086f1cb..6bd3168 100644
--- a/requester/mctp_endpoint_discovery.hpp
+++ b/requester/mctp_endpoint_discovery.hpp
@@ -19,6 +19,8 @@
constexpr const char* MCTPInterface = "xyz.openbmc_project.MCTP.Endpoint";
constexpr const char* EndpointUUID = "xyz.openbmc_project.Common.UUID";
constexpr const char* MCTPPath = "/au/com/codeconstruct/mctp1";
+constexpr const char* MCTPInterfaceCC = "au.com.codeconstruct.MCTP.Endpoint1";
+constexpr const char* MCTPConnectivityProp = "Connectivity";
/** @class MctpDiscoveryHandlerIntf
*
@@ -30,6 +32,8 @@
public:
virtual void handleMctpEndpoints(const MctpInfos& mctpInfos) = 0;
virtual void handleRemovedMctpEndpoints(const MctpInfos& mctpInfos) = 0;
+ virtual void updateMctpEndpointAvailability(const MctpInfo& mctpInfo,
+ Availability availability) = 0;
virtual ~MctpDiscoveryHandlerIntf() {}
};
@@ -62,6 +66,9 @@
/** @brief Used to watch for the removed MCTP endpoints */
sdbusplus::bus::match_t mctpEndpointRemovedSignal;
+ /** @brief Used to watch for new MCTP endpoints */
+ sdbusplus::bus::match_t mctpEndpointPropChangedSignal;
+
/** @brief List of handlers need to notify when new MCTP
* Endpoint is Added/Removed */
std::vector<MctpDiscoveryHandlerIntf*> handlers;
@@ -69,6 +76,13 @@
/** @brief The existing MCTP endpoints */
MctpInfos existingMctpInfos;
+ /** @brief Callback function when the propertiesChanged D-Bus
+ * signal is triggered for MCTP endpoint's properties.
+ *
+ * @param[in] msg - Data associated with subscribed signal
+ */
+ void propertiesChangedCb(sdbusplus::message_t& msg);
+
/** @brief Callback function when MCTP endpoints addedInterface
* D-Bus signal raised.
*
@@ -97,11 +111,21 @@
*/
void handleRemovedMctpEndpoints(const MctpInfos& mctpInfos);
+ /** @brief Helper function to invoke registered handlers for
+ * updating the availability status of the MCTP endpoint
+ *
+ * @param[in] mctpInfo - information of the target endpoint
+ * @param[in] availability - new availability status
+ */
+ void updateMctpEndpointAvailability(const MctpInfo& mctpInfo,
+ Availability availability);
+
/** @brief Get list of MctpInfos in MCTP control interface.
*
- * @param[in] mctpInfos - information of discovered MCTP endpoints
+ * @param[in] mctpInfoMap - information of discovered MCTP endpoints
+ * and the availability status of each endpoint
*/
- void getMctpInfos(MctpInfos& mctpInfos);
+ void getMctpInfos(std::map<MctpInfo, Availability>& mctpInfoMap);
/** @brief Get list of new MctpInfos in addedInterace D-Bus signal message.
*
@@ -147,6 +171,16 @@
UUID getEndpointUUIDProp(const std::string& service,
const std::string& path);
+ /** @brief Get Endpoint Availability status from `Connectivity` D-Bus
+ * property in the `au.com.codeconstruct.MCTP.Endpoint1` D-Bus
+ * interface.
+ *
+ * @param[in] path - the MCTP endpoints object path
+ *
+ * @return Availability status: true if active false if inactive
+ */
+ Availability getEndpointConnectivityProp(const std::string& path);
+
static constexpr uint8_t mctpTypePLDM = 1;
};
diff --git a/requester/test/mctp_endpoint_discovery_test.cpp b/requester/test/mctp_endpoint_discovery_test.cpp
index f955bc0..01e5eb0 100644
--- a/requester/test/mctp_endpoint_discovery_test.cpp
+++ b/requester/test/mctp_endpoint_discovery_test.cpp
@@ -1,5 +1,6 @@
#include "config.h"
+#include "common/types.hpp"
#include "common/utils.hpp"
#include "requester/test/mock_mctp_discovery_handler_intf.hpp"
@@ -39,12 +40,12 @@
{
auto& bus = pldm::utils::DBusHandler::getBus();
pldm::MockManager manager;
- pldm::MctpInfos mctpInfos;
+ std::map<pldm::MctpInfo, pldm::Availability> currentMctpInfoMap;
auto mctpDiscoveryHandler = std::make_unique<pldm::MctpDiscovery>(
bus, std::initializer_list<pldm::MctpDiscoveryHandlerIntf*>{&manager});
- mctpDiscoveryHandler->getMctpInfos(mctpInfos);
- EXPECT_EQ(mctpInfos.size(), 0);
+ mctpDiscoveryHandler->getMctpInfos(currentMctpInfoMap);
+ EXPECT_EQ(currentMctpInfoMap.size(), 0);
}
TEST(MctpEndpointDiscoveryTest, goodAddToExistingMctpInfos)
diff --git a/requester/test/mock_mctp_discovery_handler_intf.hpp b/requester/test/mock_mctp_discovery_handler_intf.hpp
index 8aada96..02de8c5 100644
--- a/requester/test/mock_mctp_discovery_handler_intf.hpp
+++ b/requester/test/mock_mctp_discovery_handler_intf.hpp
@@ -15,6 +15,9 @@
(override));
MOCK_METHOD(void, handleRemovedMctpEndpoints, (const MctpInfos& mctpInfos),
(override));
+ MOCK_METHOD(void, updateMctpEndpointAvailability,
+ (const MctpInfo& mctpInfo, Availability availability),
+ (override));
};
} // namespace pldm