monitor: Subscribe to tach target and feedback services
Subscribes to nameOwnerChanged signals for the services of the sensor
and target interfaces for each configured fan. If those services go
offline, the fan tach sensors should get marked nonfunctional due to no
longer receiving updated target or feedback values. In this design, we
use the existing method of determining when a fan tach sensor should be
marked nonfunctional to allow a recovery window, wherein a brief
offline/online transition (such as during a restart) will not trigger a
nonfunctional state change.
Change-Id: I0a935ccad5a864dc952d023185356a1ef1226830
Signed-off-by: Mike Capps <mikepcapps@gmail.com>
diff --git a/monitor/fan.cpp b/monitor/fan.cpp
index c5322b1..0e2d835 100644
--- a/monitor/fan.cpp
+++ b/monitor/fan.cpp
@@ -356,15 +356,15 @@
bool Fan::outOfRange(const TachSensor& sensor)
{
- auto actual = static_cast<uint64_t>(sensor.getInput());
- auto range = sensor.getRange(_deviation);
-
- if ((actual < range.first) || (actual > range.second))
+ if (!sensor.hasOwner())
{
return true;
}
- return false;
+ auto actual = static_cast<uint64_t>(sensor.getInput());
+ auto range = sensor.getRange(_deviation);
+
+ return ((actual < range.first) || (actual > range.second));
}
void Fan::updateState(TachSensor& sensor)
diff --git a/monitor/system.cpp b/monitor/system.cpp
index 01f1a9b..9b27b7e 100644
--- a/monitor/system.cpp
+++ b/monitor/system.cpp
@@ -1,5 +1,5 @@
/**
- * Copyright © 2020 IBM Corporation
+ * Copyright © 2021 IBM Corporation
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -20,6 +20,7 @@
#include "tach_sensor.hpp"
#include "trust_manager.hpp"
#include "types.hpp"
+#include "utility.hpp"
#ifdef MONITOR_USE_JSON
#include "json_parser.hpp"
#endif
@@ -75,6 +76,70 @@
rule->check(PowerRuleState::runtime, _fanHealth);
});
}
+
+ auto sensorMap = buildNameOwnerChangedMap();
+
+ namespace match = sdbusplus::bus::match;
+
+ // for each service, register a callback handler for nameOwnerChanged
+ for (const auto& service_itr : sensorMap)
+ {
+ _sensorMatch.push_back(std::make_unique<match::match>(
+ _bus, match::rules::nameOwnerChanged(service_itr.first),
+ std::bind(&System::tachSignalOffline, this, std::placeholders::_1,
+ sensorMap)));
+ }
+}
+
+SensorMapType System::buildNameOwnerChangedMap() const
+{
+ SensorMapType sensorMap;
+
+ // build a list of all interfaces, always including the value interface
+ // using set automatically guards against duplicates
+ std::set<std::string> unique_interfaces{util::FAN_SENSOR_VALUE_INTF};
+
+ for (const auto& fan : _fans)
+ {
+ for (const auto& sensor : fan->sensors())
+ {
+ unique_interfaces.insert(sensor->getInterface());
+ }
+ }
+ // convert them to vector to pass into getSubTreeRaw
+ std::vector<std::string> interfaces(unique_interfaces.begin(),
+ unique_interfaces.end());
+
+ // get service information for all service names that are
+ // hosting these interfaces
+ auto serviceObjects =
+ util::SDBusPlus::getSubTreeRaw(_bus, FAN_SENSOR_PATH, interfaces, 0);
+
+ for (const auto& fan : _fans)
+ {
+ // For every sensor in each fan
+ for (const auto& sensor : fan->sensors())
+ {
+ const auto itServ = serviceObjects.find(sensor->name());
+
+ if (serviceObjects.end() == itServ || itServ->second.empty())
+ {
+ getLogger().log(
+ fmt::format("Fan sensor entry {} not found in D-Bus",
+ sensor->name()),
+ Logger::error);
+ continue;
+ }
+
+ for (const auto& [serviceName, unused] : itServ->second)
+ {
+ // map its service name to the sensor
+ sensorMap[serviceName].insert(sensor);
+ }
+ }
+ }
+
+ return sensorMap;
}
void System::sighupHandler(sdeventplus::source::Signal&,
@@ -157,6 +222,40 @@
}
}
+// callback indicating a service went [on|off]line.
+// Determine on/offline status, set all sensors for that service
+// to new state
+//
+void System::tachSignalOffline(sdbusplus::message::message& msg,
+ SensorMapType const& sensorMap)
+{
+ std::string serviceName, oldOwner, newOwner;
+
+ msg.read(serviceName);
+ msg.read(oldOwner);
+ msg.read(newOwner);
+
+ // true if sensor server came back online, false -> went offline
+ bool hasOwner = !newOwner.empty() && oldOwner.empty();
+
+ std::string stateStr(hasOwner ? "online" : "offline");
+ getLogger().log(fmt::format("Changing sensors for service {} to {}",
+ serviceName, stateStr),
+ Logger::info);
+
+ auto sensorItr(sensorMap.find(serviceName));
+
+ if (sensorItr != sensorMap.end())
+ {
+ // set all sensors' owner state to not-owned
+ for (auto& sensor : sensorItr->second)
+ {
+ sensor->setOwner(hasOwner);
+ sensor->getFan().process(*sensor);
+ }
+ }
+}
+
void System::updateFanHealth(const Fan& fan)
{
std::vector<bool> sensorStatus;
diff --git a/monitor/system.hpp b/monitor/system.hpp
index 6725b00..a690567 100644
--- a/monitor/system.hpp
+++ b/monitor/system.hpp
@@ -37,6 +37,10 @@
using json = nlohmann::json;
+// Mapping from service name to sensor
+using SensorMapType =
+ std::map<std::string, std::set<std::shared_ptr<TachSensor>>>;
+
class System
{
public:
@@ -164,6 +168,11 @@
ThermalAlertObject _thermalAlert;
/**
+ * @brief The tach sensors D-Bus match objects
+ */
+ std::vector<std::unique_ptr<sdbusplus::bus::match::match>> _sensorMatch;
+
+ /**
* @brief If start() has been called
*/
bool _started = false;
@@ -177,6 +186,14 @@
json captureSensorData();
/**
+ * @brief Builds a mapping for sensors to be identified
+ * for a given service name.
+ *
+ * @return a map of service_name->[sensor1,sensor2...]
+ */
+ SensorMapType buildNameOwnerChangedMap() const;
+
+ /**
* @brief Retrieve the configured trust groups
*
* @param[in] jsonObj - JSON object to parse from
@@ -216,6 +233,16 @@
void updateFanHealth(const Fan& fan);
/**
+ * @brief callback when a tach sensor signal goes offline
+ *
+ * @param[in] msg - D-Bus message containing details (inc. service name)
+ *
+ * @param[in] sensorMap - map providing sensor access for each service
+ */
+ void tachSignalOffline(sdbusplus::message::message& msg,
+ const SensorMapType& sensorMap);
+
+ /**
* @brief The function that runs when the power state changes
*
* @param[in] powerStateOn - If power is now on or not
diff --git a/monitor/tach_sensor.cpp b/monitor/tach_sensor.cpp
index daeb1f0..91aecdd 100644
--- a/monitor/tach_sensor.cpp
+++ b/monitor/tach_sensor.cpp
@@ -35,7 +35,6 @@
namespace monitor
{
-constexpr auto FAN_SENSOR_VALUE_INTF = "xyz.openbmc_project.Sensor.Value";
constexpr auto FAN_TARGET_PROPERTY = "Target";
constexpr auto FAN_VALUE_PROPERTY = "Value";
@@ -131,7 +130,7 @@
// object can be functional with a missing D-bus sensor.
}
- auto match = getMatchString(FAN_SENSOR_VALUE_INTF);
+ auto match = getMatchString(util::FAN_SENSOR_VALUE_INTF);
tachSignal = std::make_unique<sdbusplus::server::match::match>(
_bus, match.c_str(),
@@ -169,7 +168,7 @@
void TachSensor::updateTachAndTarget()
{
_tachInput = util::SDBusPlus::getProperty<decltype(_tachInput)>(
- _bus, _name, FAN_SENSOR_VALUE_INTF, FAN_VALUE_PROPERTY);
+ _bus, _name, util::FAN_SENSOR_VALUE_INTF, FAN_VALUE_PROPERTY);
if (_hasTarget)
{
@@ -272,8 +271,8 @@
void TachSensor::handleTachChange(sdbusplus::message::message& msg)
{
- readPropertyFromMessage(msg, FAN_SENSOR_VALUE_INTF, FAN_VALUE_PROPERTY,
- _tachInput);
+ readPropertyFromMessage(msg, util::FAN_SENSOR_VALUE_INTF,
+ FAN_VALUE_PROPERTY, _tachInput);
// Check just this sensor against the target
_fan.tachChanged(*this);
diff --git a/monitor/tach_sensor.hpp b/monitor/tach_sensor.hpp
index ece1d85..27174f5 100644
--- a/monitor/tach_sensor.hpp
+++ b/monitor/tach_sensor.hpp
@@ -170,6 +170,24 @@
}
/**
+ * @brief Returns true if sensor has a D-Bus owner
+ */
+ inline bool hasOwner() const
+ {
+ return _hasOwner;
+ }
+
+ /**
+ * @brief sets D-Bus owner status
+ *
+ * @param[in] val - new owner status
+ */
+ inline void setOwner(bool val)
+ {
+ _hasOwner = val;
+ }
+
+ /**
* @brief Returns the factor of the sensor target
*/
inline double getFactor() const
@@ -178,6 +196,14 @@
}
/**
+ * @brief Returns a reference to the sensor's Fan object
+ */
+ inline Fan& getFan() const
+ {
+ return _fan;
+ }
+
+ /**
* @brief Returns the offset of the sensor target
*/
inline int64_t getOffset() const
@@ -397,6 +423,11 @@
const bool _hasTarget;
/**
+ * @brief If the sensor has a D-Bus owner
+ */
+ bool _hasOwner = true;
+
+ /**
* @brief Amount of time to delay updating to functional
*/
const size_t _funcDelay;
diff --git a/utility.hpp b/utility.hpp
index 6563243..ac1ebbc 100644
--- a/utility.hpp
+++ b/utility.hpp
@@ -33,6 +33,7 @@
constexpr auto FUNCTIONAL_PROPERTY = "Functional";
constexpr auto INV_ITEM_IFACE = "xyz.openbmc_project.Inventory.Item";
+constexpr auto FAN_SENSOR_VALUE_INTF = "xyz.openbmc_project.Sensor.Value";
class FileDescriptor
{