PEL: Take a PEL off the queue and send to host
Fill in the code that looks for the first PEL on the queue that still
needs to be sent up, and send it.
If the failure retry timer is active, or it has already retried the max
number of times, then don't send anything. In the former case the timer
callback will do the send, and in the latter the next time a log comes
in it will start trying again.
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
Change-Id: Ibb9692ca83b23e5e021db8b8a89b5549fb979df1
diff --git a/extensions/openpower-pels/host_notifier.cpp b/extensions/openpower-pels/host_notifier.cpp
index f9e0418..8fe6955 100644
--- a/extensions/openpower-pels/host_notifier.cpp
+++ b/extensions/openpower-pels/host_notifier.cpp
@@ -21,6 +21,7 @@
{
const auto subscriptionName = "PELHostNotifier";
+const size_t maxRetryAttempts = 15;
using namespace phosphor::logging;
@@ -110,6 +111,41 @@
return required;
}
+bool HostNotifier::notifyRequired(uint32_t id) const
+{
+ bool notify = true;
+ Repository::LogID i{Repository::LogID::Pel{id}};
+
+ if (auto attributes = _repo.getPELAttributes(i); attributes)
+ {
+ // If already acked by the host, don't send again.
+ // (A safety check as it shouldn't get to this point.)
+ auto a = attributes.value().get();
+ if (a.hostState == TransmissionState::acked)
+ {
+ notify = false;
+ }
+ else if (a.actionFlags.test(hiddenFlagBit))
+ {
+ // If hidden and acked (or will be) acked by the HMC,
+ // also don't send it. (HMC management can come and
+ // go at any time)
+ if ((a.hmcState == TransmissionState::acked) ||
+ _dataIface.isHMCManaged())
+ {
+ notify = false;
+ }
+ }
+ }
+ else
+ {
+ // Must have been deleted since put on the queue.
+ notify = false;
+ }
+
+ return notify;
+}
+
void HostNotifier::newLogCallback(const PEL& pel)
{
if (!enqueueRequired(pel.id()))
@@ -124,6 +160,71 @@
void HostNotifier::doNewLogNotify()
{
+ if (!_dataIface.isHostUp() || _retryTimer.isEnabled())
+ {
+ return;
+ }
+
+ if (_retryCount >= maxRetryAttempts)
+ {
+ // Give up until a new log comes in.
+ if (_retryCount == maxRetryAttempts)
+ {
+ // If this were to really happen, the PLDM interface
+ // would be down and isolating that shouldn't left to
+ // a logging daemon, so just trace. Also, this will start
+ // trying again when the next new log comes in.
+ log<level::ERR>(
+ "PEL Host notifier hit max retry attempts. Giving up for now.",
+ entry("PEL_ID=0x%X", _pelQueue.front()));
+ }
+ return;
+ }
+
+ bool doNotify = false;
+ uint32_t id = 0;
+
+ // Find the PEL to send
+ while (!doNotify && !_pelQueue.empty())
+ {
+ id = _pelQueue.front();
+ _pelQueue.pop_front();
+
+ if (notifyRequired(id))
+ {
+ doNotify = true;
+ }
+ }
+
+ if (doNotify)
+ {
+ // Get the size using the repo attributes
+ Repository::LogID i{Repository::LogID::Pel{id}};
+ if (auto attributes = _repo.getPELAttributes(i); attributes)
+ {
+ auto size = static_cast<size_t>(
+ std::filesystem::file_size((*attributes).get().path));
+ auto rc = _hostIface->sendNewLogCmd(id, size);
+
+ if (rc == CmdStatus::success)
+ {
+ _inProgressPEL = id;
+ }
+ else
+ {
+ // It failed. Retry
+ log<level::ERR>("PLDM send failed", entry("PEL_ID=0x%X", id));
+ _pelQueue.push_front(id);
+ _inProgressPEL = 0;
+ _retryTimer.restartOnce(_hostIface->getSendRetryDelay());
+ }
+ }
+ else
+ {
+ log<level::ERR>("PEL ID not in repository. Cannot notify host",
+ entry("PEL_ID=0x%X", id));
+ }
+ }
}
void HostNotifier::hostStateChange(bool hostUp)
diff --git a/extensions/openpower-pels/host_notifier.hpp b/extensions/openpower-pels/host_notifier.hpp
index 1bb7538..127eb03 100644
--- a/extensions/openpower-pels/host_notifier.hpp
+++ b/extensions/openpower-pels/host_notifier.hpp
@@ -67,6 +67,20 @@
*/
bool enqueueRequired(uint32_t id) const;
+ /**
+ * @brief If the host still needs to be notified of the PEL
+ * at the time of the notification.
+ *
+ * Only returns false if:
+ * - Already acked by the host
+ * - It's hidden, and the HMC already got or will get it.
+ *
+ * @param[in] id - The PEL ID
+ *
+ * @return bool - If the notify is required
+ */
+ bool notifyRequired(uint32_t id) const;
+
private:
/**
* @brief This function gets called by the Repository class
@@ -88,8 +102,8 @@
bool addPELToQueue(const PEL& pel);
/**
- * @brief Takes the PEL off the front of the queue and issues
- * the PLDM send.
+ * @brief Takes the first PEL from the queue that needs to be
+ * sent, and issues the send if conditions are right.
*/
void doNewLogNotify();
diff --git a/test/openpower-pels/host_notifier_test.cpp b/test/openpower-pels/host_notifier_test.cpp
index 2f2e1f0..3608539 100644
--- a/test/openpower-pels/host_notifier_test.cpp
+++ b/test/openpower-pels/host_notifier_test.cpp
@@ -131,15 +131,18 @@
// This is required
EXPECT_TRUE(notifier.enqueueRequired(pel->id()));
+ EXPECT_TRUE(notifier.notifyRequired(pel->id()));
// Not in the repo
EXPECT_FALSE(notifier.enqueueRequired(42));
+ EXPECT_FALSE(notifier.notifyRequired(42));
// Now set this PEL to host acked
repo.setPELHostTransState(pel->id(), TransmissionState::acked);
// Since it's acked, doesn't need to be enqueued or transmitted
EXPECT_FALSE(notifier.enqueueRequired(pel->id()));
+ EXPECT_FALSE(notifier.notifyRequired(pel->id()));
}
// Test the 'don't report' PEL flag
@@ -189,6 +192,9 @@
// Still need to enqueue this
EXPECT_TRUE(notifier.enqueueRequired(pel->id()));
+
+ // Still need to send it
+ EXPECT_TRUE(notifier.notifyRequired(pel->id()));
}
// Don't need to enqueue a hidden log already acked by the HMC
@@ -220,6 +226,32 @@
EXPECT_FALSE(notifier.enqueueRequired(pel->id()));
}
+// Test that changing the HMC manage status affects
+// the policy with hidden log notification.
+TEST_F(HostNotifierTest, TestPolicyHiddenWithHMCManaged)
+{
+ Repository repo{repoPath};
+ MockDataInterface dataIface;
+
+ std::unique_ptr<HostInterface> hostIface =
+ std::make_unique<MockHostInterface>(event, dataIface);
+
+ HostNotifier notifier{repo, dataIface, std::move(hostIface)};
+
+ // hiddenFlagBit
+ auto pel = makePEL(0x4000);
+
+ repo.add(pel);
+
+ // The first time, the HMC managed is false
+ EXPECT_TRUE(notifier.notifyRequired(pel->id()));
+
+ dataIface.setHMCManaged(true);
+
+ // This time, HMC managed is true so no need to notify
+ EXPECT_FALSE(notifier.notifyRequired(pel->id()));
+}
+
// Test that PELs are enqueued on startup
TEST_F(HostNotifierTest, TestStartup)
{