manager: rate limit dbus errors
If there is a misconfiguration between sysfs and led-manager,
the manager will be very spammy about dbus errors at it tries
to update the sysfs value. Rate limit this to once per hour
per LED.
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I2896377ff64318159ca3963d09c2ab5bf7452862
diff --git a/manager/lamptest/lamptest.cpp b/manager/lamptest/lamptest.cpp
index f7f9e90..8178f75 100644
--- a/manager/lamptest/lamptest.cpp
+++ b/manager/lamptest/lamptest.cpp
@@ -31,8 +31,8 @@
if (iter != forceUpdateLEDs.end())
{
- phosphor::led::Manager::drivePhysicalLED(
- path, Layout::Action::Off, it.dutyOn, it.period);
+ manager.drivePhysicalLED(path, Layout::Action::Off, it.dutyOn,
+ it.period);
}
}
@@ -45,8 +45,7 @@
if (iter != forceUpdateLEDs.end())
{
- phosphor::led::Manager::drivePhysicalLED(path, it.action,
- it.dutyOn, it.period);
+ manager.drivePhysicalLED(path, it.action, it.dutyOn, it.period);
}
}
@@ -81,8 +80,7 @@
continue;
}
- phosphor::led::Manager::drivePhysicalLED(path, Layout::Action::Off, 0,
- 0);
+ manager.drivePhysicalLED(path, Layout::Action::Off, 0, 0);
}
if (std::filesystem::exists(lampTestIndicator))
@@ -238,8 +236,7 @@
continue;
}
- phosphor::led::Manager::drivePhysicalLED(path, Layout::Action::On, 0,
- 0);
+ manager.drivePhysicalLED(path, Layout::Action::On, 0, 0);
}
}
@@ -369,8 +366,8 @@
for (const auto& path : physicalLedPaths)
{
- phosphor::led::Manager::drivePhysicalLED(
- path, phosphor::led::Layout::Action::Off, 0, 0);
+ manager.drivePhysicalLED(path, phosphor::led::Layout::Action::Off,
+ 0, 0);
}
// Also remove the lamp test on indicator file.
diff --git a/manager/lamptest/lamptest.hpp b/manager/lamptest/lamptest.hpp
index ce7bae1..577e6a1 100644
--- a/manager/lamptest/lamptest.hpp
+++ b/manager/lamptest/lamptest.hpp
@@ -79,7 +79,7 @@
* with the persisted lamp test indicator file so that there is no sign of
* lamptest.
*/
- static void clearLamps();
+ void clearLamps();
private:
/** @brief Timer used for LEDs lamp test period */
diff --git a/manager/led-main.cpp b/manager/led-main.cpp
index f671d5d..7791747 100644
--- a/manager/led-main.cpp
+++ b/manager/led-main.cpp
@@ -66,7 +66,7 @@
phosphor::led::LampTest lampTest(event, manager);
// Clear leds triggered by lamp test in previous boot
- phosphor::led::LampTest::clearLamps();
+ lampTest.clearLamps();
groups.emplace_back(std::make_unique<phosphor::led::Group>(
bus, LAMP_TEST_OBJECT, manager, serializePtr,
diff --git a/manager/manager.cpp b/manager/manager.cpp
index b571546..336f819 100644
--- a/manager/manager.cpp
+++ b/manager/manager.cpp
@@ -7,8 +7,10 @@
#include <xyz/openbmc_project/Led/Physical/server.hpp>
#include <algorithm>
+#include <chrono>
#include <iostream>
#include <string>
+
namespace phosphor
{
namespace led
@@ -232,9 +234,24 @@
}
catch (const sdbusplus::exception_t& e)
{
+ // This can be a really spammy event log, so we rate limit it to once an
+ // hour per LED.
+ auto now = std::chrono::steady_clock::now();
+
+ if (auto it = physicalLEDErrors.find(objPath);
+ it != physicalLEDErrors.end())
+ {
+ using namespace std::literals::chrono_literals;
+ if ((now - it->second) < 1h)
+ {
+ return -1;
+ }
+ }
+
lg2::error(
"Error setting property for physical LED, ERROR = {ERROR}, OBJECT_PATH = {PATH}",
"ERROR", e, "PATH", objPath);
+ physicalLEDErrors[objPath] = now;
return -1;
}
diff --git a/manager/manager.hpp b/manager/manager.hpp
index f211461..0bdcc33 100644
--- a/manager/manager.hpp
+++ b/manager/manager.hpp
@@ -7,6 +7,7 @@
#include <sdeventplus/event.hpp>
#include <sdeventplus/utility/timer.hpp>
+#include <chrono>
#include <set>
#include <string>
#include <unordered_map>
@@ -122,9 +123,8 @@
*
* @return: - 0: success, -1: LED set failed
*/
- static int drivePhysicalLED(const std::string& objPath,
- Layout::Action action, uint8_t dutyOn,
- uint16_t period);
+ int drivePhysicalLED(const std::string& objPath, Layout::Action action,
+ uint8_t dutyOn, uint16_t period);
/** @brief Set lamp test callback when enabled lamp test.
*
@@ -157,6 +157,11 @@
/** @brief Contains the required set of deassert LEDs action */
ActionSet reqLedsDeAssert;
+ /** @brief Map to store the last error time for physical LED paths */
+ std::unordered_map<std::string,
+ std::chrono::time_point<std::chrono::steady_clock>>
+ physicalLEDErrors;
+
/** @brief LEDs handler callback */
void driveLedsHandler();