hardware: Disallow writing out-of-range values into sensor-value leaf
One of our boxes started reporting nonsense value of one of the sensors.
This value is way beyond range for sensor-value and it led to crashes
because libyang refused to write such value into the sensor-value leaf
and throwed with LY_EVALID.
Let's hotfix that for now to set value as {over,under}flowed and
sensor's oper-status to nonoperational.
However, the problem of setting the correct value is more complicated
because the valid range for the int32 value in YANG is one thing but the
descriptions gives even more constraints depending on sensor-value-type.
Change-Id: I9a74991da58239e0f376f2ec3e1e86f88eab9c57
diff --git a/src/ietf-hardware/IETFHardware.cpp b/src/ietf-hardware/IETFHardware.cpp
index f167050..000fc2a 100644
--- a/src/ietf-hardware/IETFHardware.cpp
+++ b/src/ietf-hardware/IETFHardware.cpp
@@ -46,13 +46,26 @@
}
/** @brief Write a sensor-data @p value for a component @p componentName and push it into the @p res DataTree */
-void addSensorValue(velia::ietf_hardware::DataTree& res, const std::string& componentName, const int64_t& value)
+void addSensorValue(velia::Log log, velia::ietf_hardware::DataTree& res, const std::string& componentName, int64_t value)
{
- writeSensorValue(res, componentName, std::to_string(value), "ok");
+ static constexpr const int64_t YANG_SENSOR_VALUE_MIN = -1'000'000'000;
+ static constexpr const int64_t YANG_SENSOR_VALUE_MAX = 1'000'000'000;
+
+ // FIXME: Valid value range depends on sensor-type as well, see ietf-hardware's sensor-value type description
+
+ if (value <= YANG_SENSOR_VALUE_MIN) {
+ log->error("Sensor's '{}' value '{}' underflow. Setting sensor as nonoperational.", componentName, value);
+ writeSensorValue(res, componentName, std::to_string(YANG_SENSOR_VALUE_MIN), "nonoperational");
+ } else if (value >= YANG_SENSOR_VALUE_MAX) {
+ log->error("Sensor's '{}' value '{}' overflow. Setting sensor as nonoperational.", componentName, value);
+ writeSensorValue(res, componentName, std::to_string(YANG_SENSOR_VALUE_MAX), "nonoperational");
+ } else {
+ writeSensorValue(res, componentName, std::to_string(value), "ok");
+ }
}
/** @brief Write a sensor-data @p value for a component @p componentName and push it into the @p res DataTree */
-void addSensorValue(velia::ietf_hardware::DataTree& res, const std::string& componentName, const std::string& value)
+void addSensorValue(velia::Log, velia::ietf_hardware::DataTree& res, const std::string& componentName, const std::string& value)
{
// TODO: Perhaps we should check if the string value is conforming to sensor-value type
writeSensorValue(res, componentName, value, "ok");
@@ -122,6 +135,7 @@
DataReader::DataReader(std::string componentName, std::optional<std::string> parent)
: m_componentName(std::move(componentName))
, m_parent(std::move(parent))
+ , m_log(spdlog::get("hardware"))
{
}
@@ -186,7 +200,7 @@
const auto sensorComponentName = m_componentName + ":fan" + std::to_string(i) + ":rpm";
const auto attribute = "fan"s + std::to_string(i) + "_input";
- addSensorValue(res, sensorComponentName, m_hwmon->attribute(attribute));
+ addSensorValue(m_log, res, sensorComponentName, m_hwmon->attribute(attribute));
}
return res;
@@ -277,7 +291,7 @@
DataTree res(m_staticData);
int64_t sensorValue = m_hwmon->attribute(m_sysfsFile);
- addSensorValue(res, m_componentName, sensorValue);
+ addSensorValue(m_log, res, m_componentName, sensorValue);
return res;
}
@@ -336,7 +350,7 @@
DataTree res(m_staticData);
auto emmcAttrs = m_emmc->attributes();
- addSensorValue(res, m_componentName + ":lifetime", emmcAttrs.at("life_time"));
+ addSensorValue(m_log, res, m_componentName + ":lifetime", emmcAttrs.at("life_time"));
return res;
}
diff --git a/src/ietf-hardware/IETFHardware.h b/src/ietf-hardware/IETFHardware.h
index c829721..c226e96 100644
--- a/src/ietf-hardware/IETFHardware.h
+++ b/src/ietf-hardware/IETFHardware.h
@@ -104,6 +104,8 @@
/** @brief static hw-state related data */
DataTree m_staticData;
+ velia::Log m_log;
+
DataReader(std::string propertyPrefix, std::optional<std::string> parent);
};
diff --git a/tests/hardware_ietf-hardware.cpp b/tests/hardware_ietf-hardware.cpp
index 1a0cca0..b4612be 100644
--- a/tests/hardware_ietf-hardware.cpp
+++ b/tests/hardware_ietf-hardware.cpp
@@ -42,20 +42,20 @@
FAKE_EMMC(emmc, attributesEMMC);
std::array<int64_t, 4> fanValues = {777, 0, 1280, 666};
- REQUIRE_CALL(*fans, attribute("fan1_input"s)).LR_RETURN(fanValues[0]).TIMES(4);
- REQUIRE_CALL(*fans, attribute("fan2_input"s)).LR_RETURN(fanValues[1]).TIMES(4);
- REQUIRE_CALL(*fans, attribute("fan3_input"s)).LR_RETURN(fanValues[2]).TIMES(4);
- REQUIRE_CALL(*fans, attribute("fan4_input"s)).LR_RETURN(fanValues[3]).TIMES(4);
+ REQUIRE_CALL(*fans, attribute("fan1_input"s)).LR_RETURN(fanValues[0]).TIMES(5);
+ REQUIRE_CALL(*fans, attribute("fan2_input"s)).LR_RETURN(fanValues[1]).TIMES(5);
+ REQUIRE_CALL(*fans, attribute("fan3_input"s)).LR_RETURN(fanValues[2]).TIMES(5);
+ REQUIRE_CALL(*fans, attribute("fan4_input"s)).LR_RETURN(fanValues[3]).TIMES(5);
- REQUIRE_CALL(*sysfsTempCpu, attribute("temp1_input")).RETURN(41800).TIMES(4);
+ REQUIRE_CALL(*sysfsTempCpu, attribute("temp1_input")).RETURN(41800).TIMES(5);
- REQUIRE_CALL(*sysfsVoltageAc, attribute("in1_input")).RETURN(220000).TIMES(4);
- REQUIRE_CALL(*sysfsVoltageDc, attribute("in1_input")).RETURN(12000).TIMES(4);
- REQUIRE_CALL(*sysfsPower, attribute("power1_input")).RETURN(14000000).TIMES(4);
- REQUIRE_CALL(*sysfsCurrent, attribute("curr1_input")).RETURN(200).TIMES(4);
+ REQUIRE_CALL(*sysfsVoltageAc, attribute("in1_input")).RETURN(220000).TIMES(5);
+ REQUIRE_CALL(*sysfsVoltageDc, attribute("in1_input")).RETURN(12000).TIMES(5);
+ REQUIRE_CALL(*sysfsPower, attribute("power1_input")).RETURN(14000000).TIMES(5);
+ REQUIRE_CALL(*sysfsCurrent, attribute("curr1_input")).RETURN(200).TIMES(5);
attributesEMMC = {{"life_time"s, "40"s}};
- FAKE_EMMC(emmc, attributesEMMC).TIMES(4);
+ FAKE_EMMC(emmc, attributesEMMC).TIMES(5);
using velia::ietf_hardware::OneThreshold;
using velia::ietf_hardware::Thresholds;
@@ -352,4 +352,23 @@
THRESHOLD_STATE("ne:psu:child", State::WarningHigh),
});
}
+
+
+ fanValues[0] = -1'000'000'001;
+ fanValues[1] = 1'000'000'001;
+ expected["/ietf-hardware:hardware/component[name='ne:fans:fan1:rpm']/sensor-data/value"] = "-1000000000";
+ expected["/ietf-hardware:hardware/component[name='ne:fans:fan1:rpm']/sensor-data/oper-status"] = "nonoperational";
+ expected["/ietf-hardware:hardware/component[name='ne:fans:fan2:rpm']/sensor-data/value"] = "1000000000";
+ expected["/ietf-hardware:hardware/component[name='ne:fans:fan2:rpm']/sensor-data/oper-status"] = "nonoperational";
+
+ {
+ auto [data, alarms] = ietfHardware->process();
+ NUKE_LAST_CHANGE(data);
+
+ REQUIRE(data == expected);
+ REQUIRE(alarms == std::map<std::string, velia::ietf_hardware::State>{
+ THRESHOLD_STATE("ne:fans:fan1:rpm", State::CriticalLow),
+ THRESHOLD_STATE("ne:fans:fan2:rpm", State::Normal),
+ });
+ }
}