dbus-sdr: Make GetSDR more robust against dbus failure.
SDR list should be persistent while occasional SDR reading error due
to underlayer dbus failure. The improvement consists of the following
aspects:
1. SDR type is merely determined by sensorTree(from ObjectMapper),
whose mechanism is the same as SDR index.
2. Alway return next sdr id even when failure.
3. Avoid unnecessary dbus call when IPMI host doesn't requires the SDR
body.
Signed-off-by: Hao Jiang <jianghao@google.com>
Change-Id: If7d7bbb6db587c727f6ea5bcb7a7984f1ed47a80
Signed-off-by: Willy Tu <wltu@google.com>
diff --git a/dbus-sdr/sensorcommands.cpp b/dbus-sdr/sensorcommands.cpp
index 0e513c5..1f07299 100644
--- a/dbus-sdr/sensorcommands.cpp
+++ b/dbus-sdr/sensorcommands.cpp
@@ -496,22 +496,30 @@
{
std::string connection;
std::string path;
- ipmi::Cc status = getSensorConnection(ctx, sensorNumber, connection, path);
+ std::vector<std::string> interfaces;
+
+ ipmi::Cc status =
+ getSensorConnection(ctx, sensorNumber, connection, path, &interfaces);
if (status)
{
return ipmi::response(status);
}
- DbusInterfaceMap sensorMap;
- if (!getSensorMap(ctx, connection, path, sensorMap))
- {
- return ipmi::responseResponseError();
- }
-
// we can tell the sensor type by its interface type
- auto sensorObject = sensorMap.find(sensor::sensorInterface);
- if (sensorObject != sensorMap.end())
+ if (std::find(interfaces.begin(), interfaces.end(),
+ sensor::sensorInterface) != interfaces.end())
{
+ DbusInterfaceMap sensorMap;
+ if (!getSensorMap(ctx, connection, path, sensorMap))
+ {
+ return ipmi::responseResponseError();
+ }
+ auto sensorObject = sensorMap.find(sensor::sensorInterface);
+ if (sensorObject != sensorMap.end())
+ {
+ return ipmi::responseResponseError();
+ }
+
auto value =
sensor::calculateValue(reading, sensorMap, sensorObject->second);
if (!value)
@@ -548,9 +556,20 @@
return ipmi::responseSuccess();
}
- sensorObject = sensorMap.find(sensor::vrInterface);
- if (sensorObject != sensorMap.end())
+ if (std::find(interfaces.begin(), interfaces.end(), sensor::vrInterface) !=
+ interfaces.end())
{
+ DbusInterfaceMap sensorMap;
+ if (!getSensorMap(ctx, connection, path, sensorMap))
+ {
+ return ipmi::responseResponseError();
+ }
+ auto sensorObject = sensorMap.find(sensor::vrInterface);
+ if (sensorObject != sensorMap.end())
+ {
+ return ipmi::responseResponseError();
+ }
+
// VR sensors are treated as a special case and we will not check the
// write permission for VR sensors, since they always deemed writable
// and permission table are not applied to VR sensors.
@@ -1313,10 +1332,8 @@
}
// Construct a type 1 SDR for threshold sensor.
-bool constructSensorSdr(uint16_t sensorNum, uint16_t recordID,
- const std::string& path,
- const DbusInterfaceMap& sensorMap,
- get_sdr::SensorDataFullRecord& record)
+void constructSensorSdrHeaderKey(uint16_t sensorNum, uint16_t recordID,
+ get_sdr::SensorDataFullRecord& record)
{
get_sdr::header::set_record_id(
recordID, reinterpret_cast<get_sdr::SensorDataRecordHeader*>(&record));
@@ -1331,6 +1348,24 @@
record.key.owner_id = bmcI2CAddr;
record.key.owner_lun = lun;
record.key.sensor_number = sensornumber;
+}
+bool constructSensorSdr(ipmi::Context::ptr ctx, uint16_t sensorNum,
+ uint16_t recordID, const std::string& service,
+ const std::string& path,
+ get_sdr::SensorDataFullRecord& record)
+{
+ uint8_t sensornumber = static_cast<uint8_t>(sensorNum);
+ constructSensorSdrHeaderKey(sensorNum, recordID, record);
+
+ DbusInterfaceMap sensorMap;
+ if (!getSensorMap(ctx, service, path, sensorMap, sensorMapSdrUpdatePeriod))
+ {
+ phosphor::logging::log<phosphor::logging::level::ERR>(
+ "Failed to update sensor map for threshold sensor",
+ phosphor::logging::entry("SERVICE=%s", service.c_str()),
+ phosphor::logging::entry("PATH=%s", path.c_str()));
+ return false;
+ }
record.body.sensor_capabilities = 0x68; // auto rearm - todo hysteresis
record.body.sensor_type = getSensorTypeFromPath(path);
@@ -1519,10 +1554,9 @@
return true;
}
-// Construct a type 3 SDR for VR typed sensor(daemon).
-void constructVrSdr(uint16_t sensorNum, uint16_t recordID,
- const std::string& path, const DbusInterfaceMap& sensorMap,
- get_sdr::SensorDataEventRecord& record)
+// Construct type 3 SDR header and key (for VR and other discrete sensors)
+void constructEventSdrHeaderKey(uint16_t sensorNum, uint16_t recordID,
+ get_sdr::SensorDataEventRecord& record)
{
uint8_t sensornumber = static_cast<uint8_t>(sensorNum);
uint8_t lun = static_cast<uint8_t>(sensorNum >> 8);
@@ -1540,7 +1574,26 @@
record.body.entity_id = 0x00;
record.body.entity_instance = 0x01;
+}
+// Construct a type 3 SDR for VR typed sensor(daemon).
+bool constructVrSdr(ipmi::Context::ptr ctx, uint16_t sensorNum,
+ uint16_t recordID, const std::string& service,
+ const std::string& path,
+ get_sdr::SensorDataEventRecord& record)
+{
+ uint8_t sensornumber = static_cast<uint8_t>(sensorNum);
+ constructEventSdrHeaderKey(sensorNum, recordID, record);
+
+ DbusInterfaceMap sensorMap;
+ if (!getSensorMap(ctx, service, path, sensorMap, sensorMapSdrUpdatePeriod))
+ {
+ phosphor::logging::log<phosphor::logging::level::ERR>(
+ "Failed to update sensor map for VR sensor",
+ phosphor::logging::entry("SERVICE=%s", service.c_str()),
+ phosphor::logging::entry("PATH=%s", path.c_str()));
+ return false;
+ }
// follow the association chain to get the parent board's entityid and
// entityInstance
updateIpmiFromAssociation(path, sensorMap, record.body.entity_id,
@@ -1566,11 +1619,14 @@
// Remember the sensor name, as determined for this sensor number
details::sdrStatsTable.updateName(sensornumber, name);
+
+ return true;
}
-static int getSensorDataRecord(ipmi::Context::ptr ctx,
- std::vector<uint8_t>& recordData,
- uint16_t recordID)
+static int
+ getSensorDataRecord(ipmi::Context::ptr ctx,
+ std::vector<uint8_t>& recordData, uint16_t recordID,
+ uint8_t readBytes = std::numeric_limits<uint8_t>::max())
{
size_t fruCount = 0;
ipmi::Cc ret = ipmi::storage::getFruSdrCount(ctx, fruCount);
@@ -1631,20 +1687,16 @@
std::string connection;
std::string path;
- auto status = getSensorConnection(ctx, recordID, connection, path);
+ std::vector<std::string> interfaces;
+
+ auto status =
+ getSensorConnection(ctx, recordID, connection, path, &interfaces);
if (status)
{
phosphor::logging::log<phosphor::logging::level::ERR>(
"getSensorDataRecord: getSensorConnection error");
return GENERAL_ERROR;
}
- DbusInterfaceMap sensorMap;
- if (!getSensorMap(ctx, connection, path, sensorMap, sensorMapUpdatePeriod))
- {
- phosphor::logging::log<phosphor::logging::level::ERR>(
- "getSensorDataRecord: getSensorMap error");
- return GENERAL_ERROR;
- }
uint16_t sensorNum = getSensorNumberFromPath(path);
if (sensorNum == invalidSensorNumber)
{
@@ -1653,16 +1705,24 @@
return GENERAL_ERROR;
}
- auto sensorObject = sensorMap.find(sensor::sensorInterface);
// Construct full record (SDR type 1) for the threshold sensors
- if (sensorObject != sensorMap.end())
+ if (std::find(interfaces.begin(), interfaces.end(),
+ sensor::sensorInterface) != interfaces.end())
{
get_sdr::SensorDataFullRecord record = {0};
- if (!constructSensorSdr(sensorNum, recordID, path, sensorMap, record))
+ // If the request doesn't read SDR body, construct only header and key
+ // part to avoid additional DBus transaction.
+ if (readBytes <= sizeof(record.header) + sizeof(record.key))
+ {
+ constructSensorSdrHeaderKey(sensorNum, recordID, record);
+ }
+ else if (!constructSensorSdr(ctx, sensorNum, recordID, connection, path,
+ record))
{
return GENERAL_ERROR;
}
+
recordData.insert(recordData.end(), (uint8_t*)&record,
((uint8_t*)&record) + sizeof(record));
@@ -1670,12 +1730,22 @@
}
// Contruct SDR type 3 record for VR sensor (daemon)
- sensorObject = sensorMap.find(sensor::vrInterface);
- if (sensorObject != sensorMap.end())
+ if (std::find(interfaces.begin(), interfaces.end(), sensor::vrInterface) !=
+ interfaces.end())
{
get_sdr::SensorDataEventRecord record = {0};
- constructVrSdr(sensorNum, recordID, path, sensorMap, record);
+ // If the request doesn't read SDR body, construct only header and key
+ // part to avoid additional DBus transaction.
+ if (readBytes <= sizeof(record.header) + sizeof(record.key))
+ {
+ constructEventSdrHeaderKey(sensorNum, recordID, record);
+ }
+ else if (!constructVrSdr(ctx, sensorNum, recordID, connection, path,
+ record))
+ {
+ return GENERAL_ERROR;
+ }
recordData.insert(recordData.end(), (uint8_t*)&record,
((uint8_t*)&record) + sizeof(record));
}
@@ -1910,7 +1980,7 @@
}
std::vector<uint8_t> record;
- if (getSensorDataRecord(ctx, record, recordID))
+ if (getSensorDataRecord(ctx, record, recordID, offset + bytesToRead))
{
phosphor::logging::log<phosphor::logging::level::ERR>(
"ipmiStorageGetSDR: fail to get SDR");
diff --git a/include/dbus-sdr/sensorcommands.hpp b/include/dbus-sdr/sensorcommands.hpp
index 2c6af8d..860bb0e 100644
--- a/include/dbus-sdr/sensorcommands.hpp
+++ b/include/dbus-sdr/sensorcommands.hpp
@@ -124,9 +124,10 @@
return sensorTree;
}
-static ipmi_ret_t getSensorConnection(ipmi::Context::ptr ctx, uint16_t sensnum,
- std::string& connection,
- std::string& path)
+static ipmi_ret_t
+ getSensorConnection(ipmi::Context::ptr ctx, uint16_t sensnum,
+ std::string& connection, std::string& path,
+ std::vector<std::string>* interfaces = nullptr)
{
auto& sensorTree = getSensorTree();
if (!getSensorSubtree(sensorTree) && sensorTree.empty())
@@ -150,6 +151,8 @@
if (path == sensor.first)
{
connection = sensor.second.begin()->first;
+ if (interfaces)
+ *interfaces = sensor.second.begin()->second;
break;
}
}