From f77debb9afbbe007ed7e2c3f79ddeb6f1b6f7c49 Mon Sep 17 00:00:00 2001 From: Matt Spinler Date: Thu, 12 Dec 2019 10:04:33 -0600 Subject: 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 Change-Id: Ibb9692ca83b23e5e021db8b8a89b5549fb979df1 --- extensions/openpower-pels/host_notifier.cpp | 101 ++++++++++++++++++++++++++++ extensions/openpower-pels/host_notifier.hpp | 18 ++++- test/openpower-pels/host_notifier_test.cpp | 32 +++++++++ 3 files changed, 149 insertions(+), 2 deletions(-) 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 @@ namespace openpower::pels { const auto subscriptionName = "PELHostNotifier"; +const size_t maxRetryAttempts = 15; using namespace phosphor::logging; @@ -110,6 +111,41 @@ bool HostNotifier::enqueueRequired(uint32_t id) const 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::newLogCallback(const PEL& pel) 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( + "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( + std::filesystem::file_size((*attributes).get().path)); + auto rc = _hostIface->sendNewLogCmd(id, size); + + if (rc == CmdStatus::success) + { + _inProgressPEL = id; + } + else + { + // It failed. Retry + log("PLDM send failed", entry("PEL_ID=0x%X", id)); + _pelQueue.push_front(id); + _inProgressPEL = 0; + _retryTimer.restartOnce(_hostIface->getSendRetryDelay()); + } + } + else + { + log("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 @@ class HostNotifier */ 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 @@ class HostNotifier 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 @@ TEST_F(HostNotifierTest, TestPolicyAckedPEL) // 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 @@ TEST_F(HostNotifierTest, TestPolicyHiddenNoHMC) // 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 @@ TEST_F(HostNotifierTest, TestPolicyHiddenWithHMCAcked) 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 hostIface = + std::make_unique(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) { -- cgit v1.2.1