From 0ac0dd232627d9c44e53d41e2e6509a0471f013d Mon Sep 17 00:00:00 2001 From: Tom Joseph Date: Fri, 16 Feb 2018 09:14:45 +0530 Subject: sensor: Refactor get sensor thresholds command Using GetManagedObjects to get the thresholds could take upto 15~20 sec for the core temperatures. The time taken to fetch threshold properties via GetManagedObjects is proportional to the number of objects under the path. Since the dbus object path is available in the yaml it is performant to read the threshold properties directly. Change-Id: Ic22d8f4d73d8ce4eabdffd6fc7af23f73b981f66 Signed-off-by: Tom Joseph --- sensorhandler.cpp | 174 ++++++++++++++++++++++++------------------------------ sensorhandler.h | 9 ++- types.hpp | 20 ------- 3 files changed, 84 insertions(+), 119 deletions(-) diff --git a/sensorhandler.cpp b/sensorhandler.cpp index 1bd10b7..e52ab69 100644 --- a/sensorhandler.cpp +++ b/sensorhandler.cpp @@ -675,64 +675,87 @@ ipmi_ret_t ipmi_sen_get_sensor_reading(ipmi_netfn_t netfn, ipmi_cmd_t cmd, return rc; } -ipmi_ret_t ipmi_sen_get_sensor_thresholds(ipmi_netfn_t netfn, ipmi_cmd_t cmd, - ipmi_request_t request, ipmi_response_t response, - ipmi_data_len_t data_len, ipmi_context_t context) +void getSensorThresholds(uint8_t sensorNum, + get_sdr::GetSensorThresholdsResponse* response) { - constexpr auto warningThresholdInterface = + constexpr auto warningThreshIntf = "xyz.openbmc_project.Sensor.Threshold.Warning"; - constexpr auto criticalThresholdInterface = + constexpr auto criticalThreshIntf = "xyz.openbmc_project.Sensor.Threshold.Critical"; - constexpr auto valueInterface = - "xyz.openbmc_project.Sensor.Value"; - constexpr auto sensorRoot = "/xyz/openbmc_project/sensors"; - ipmi::sensor::Thresholds thresholds = + sdbusplus::bus::bus bus{ipmid_get_sd_bus_connection()}; + + const auto iter = sensors.find(sensorNum); + const auto info = iter->second; + + auto service = ipmi::getService(bus, info.sensorInterface, info.sensorPath); + + auto warnThresholds = ipmi::getAllDbusProperties(bus, + service, + info.sensorPath, + warningThreshIntf); + + double warnLow = warnThresholds["WarningLow"].get(); + double warnHigh = warnThresholds["WarningHigh"].get(); + + if (warnLow != 0) { - { - warningThresholdInterface, - { - { - "WarningLow", - ipmi::sensor::ThresholdMask::NON_CRITICAL_LOW_MASK, - ipmi::sensor::ThresholdIndex::NON_CRITICAL_LOW_IDX - }, - { - "WarningHigh", - ipmi::sensor::ThresholdMask::NON_CRITICAL_HIGH_MASK, - ipmi::sensor::ThresholdIndex::NON_CRITICAL_HIGH_IDX - } - } - }, - { - criticalThresholdInterface, - { - { - "CriticalLow", - ipmi::sensor::ThresholdMask::CRITICAL_LOW_MASK, - ipmi::sensor::ThresholdIndex::CRITICAL_LOW_IDX - }, - { - "CriticalHigh", - ipmi::sensor::ThresholdMask::CRITICAL_HIGH_MASK, - ipmi::sensor::ThresholdIndex::CRITICAL_HIGH_IDX - } - } - } - }; + warnLow *= pow(10, info.scale - info.exponentR); + response->lowerNonCritical = static_cast(( + warnLow - info.scaledOffset) / info.coefficientM); + response->validMask |= static_cast( + ipmi::sensor::ThresholdMask::NON_CRITICAL_LOW_MASK); + } - sdbusplus::bus::bus bus{ipmid_get_sd_bus_connection()}; + if (warnHigh != 0) + { + warnHigh *= pow(10, info.scale - info.exponentR); + response->upperNonCritical = static_cast(( + warnHigh - info.scaledOffset) / info.coefficientM); + response->validMask |= static_cast( + ipmi::sensor::ThresholdMask::NON_CRITICAL_HIGH_MASK); + } + + auto critThresholds = ipmi::getAllDbusProperties(bus, + service, + info.sensorPath, + criticalThreshIntf); + double critLow = critThresholds["CriticalLow"].get(); + double critHigh = critThresholds["CriticalHigh"].get(); + + if (critLow != 0) + { + critLow *= pow(10, info.scale - info.exponentR); + response->lowerCritical = static_cast(( + critLow - info.scaledOffset) / info.coefficientM); + response->validMask |= static_cast( + ipmi::sensor::ThresholdMask::CRITICAL_LOW_MASK); + } + + if (critHigh != 0) + { + critHigh *= pow(10, info.scale - info.exponentR); + response->upperCritical = static_cast(( + critHigh - info.scaledOffset)/ info.coefficientM); + response->validMask |= static_cast( + ipmi::sensor::ThresholdMask::CRITICAL_HIGH_MASK); + } +} + +ipmi_ret_t ipmi_sen_get_sensor_thresholds(ipmi_netfn_t netfn, ipmi_cmd_t cmd, + ipmi_request_t request, ipmi_response_t response, + ipmi_data_len_t data_len, ipmi_context_t context) +{ + constexpr auto valueInterface = "xyz.openbmc_project.Sensor.Value"; if (*data_len != sizeof(uint8_t)) { + *data_len = 0; return IPMI_CC_REQ_DATA_LEN_INVALID; } - auto sensorNum = *(reinterpret_cast(request)); - - auto responseData = - reinterpret_cast(response); - responseData->validMask = 0; + auto sensorNum = *(reinterpret_cast(request)); + *data_len = 0; const auto iter = sensors.find(sensorNum); if (iter == sensors.end()) @@ -740,69 +763,26 @@ ipmi_ret_t ipmi_sen_get_sensor_thresholds(ipmi_netfn_t netfn, ipmi_cmd_t cmd, return IPMI_CC_SENSOR_INVALID; } - const auto sensorInfo = iter->second; + const auto info = iter->second; //Proceed only if the sensor value interface is implemented. - if (sensorInfo.propertyInterfaces.find(valueInterface) == - sensorInfo.propertyInterfaces.end()) + if (info.propertyInterfaces.find(valueInterface) == + info.propertyInterfaces.end()) { //return with valid mask as 0 return IPMI_CC_OK; } - std::string service; - try - { - service = ipmi::getService(bus, - sensorInfo.sensorInterface, - sensorInfo.sensorPath); - } - catch (const std::runtime_error& e) - { - log(e.what()); - return IPMI_CC_UNSPECIFIED_ERROR; - } - - //prevent divide by 0 - auto coefficientM = - sensorInfo.coefficientM ? sensorInfo.coefficientM : 1; + auto responseData = + reinterpret_cast(response); try { - auto mngObjects = ipmi::getManagedObjects(bus, - service, - sensorRoot); - - auto senIter = mngObjects.find(sensorInfo.sensorPath); - if (senIter == mngObjects.end()) - { - return IPMI_CC_SENSOR_INVALID; - } - - for (const auto& threshold : thresholds) - { - auto thresholdType = senIter->second.find(threshold.first); - if (thresholdType != senIter->second.end()) - { - for (const auto& threshLevel : threshold.second) - { - auto val = thresholdType-> - second[threshLevel.property].get(); - if (val != 0) - { - auto idx = static_cast(threshLevel.idx); - responseData->data[idx] = static_cast( - (val - sensorInfo.scaledOffset) / coefficientM); - responseData->validMask |= - static_cast(threshLevel.maskValue); - } - } - } - } + getSensorThresholds(sensorNum, responseData); } - catch (InternalFailure& e) + catch (std::exception& e) { - //Not able to get the values, reset the mask. + //Mask if the property is not present responseData->validMask = 0; } diff --git a/sensorhandler.h b/sensorhandler.h index e32405d..f5b5c94 100644 --- a/sensorhandler.h +++ b/sensorhandler.h @@ -241,8 +241,13 @@ inline void set_owner_lun_channel(uint8_t channel, SensorDataRecordKey* key) */ struct GetSensorThresholdsResponse { - uint8_t validMask; //Indicates which values are valid - uint8_t data[6]; //Container for threshold values + uint8_t validMask; //!< valid mask + uint8_t lowerNonCritical; //!< lower non-critical threshold + uint8_t lowerCritical; //!< lower critical threshold + uint8_t lowerNonRecoverable;//!< lower non-recoverable threshold + uint8_t upperNonCritical; //!< upper non-critical threshold + uint8_t upperCritical; //!< upper critical threshold + uint8_t upperNonRecoverable;//!< upper non-recoverable threshold } __attribute__((packed)); // Body - full record diff --git a/types.hpp b/types.hpp index e77dc8e..eb4cf8c 100644 --- a/types.hpp +++ b/types.hpp @@ -197,26 +197,6 @@ enum class ThresholdMask CRITICAL_HIGH_MASK = 0x10, }; -enum class ThresholdIndex -{ - NON_CRITICAL_LOW_IDX = 0, - CRITICAL_LOW_IDX = 1, - NON_RECOVERABLE_LOW_IDX = 2, - NON_CRITICAL_HIGH_IDX = 3, - CRITICAL_HIGH_IDX = 4, - NON_RECOVERABLE_HIGH_IDX = 5, -}; - -struct ThresholdLevel -{ - std::string property; - ThresholdMask maskValue; - ThresholdIndex idx; -}; - -using SensorThresholds = std::vector; -using Thresholds = std::map; - }// namespace sensor namespace network -- cgit v1.2.1