diff options
| -rw-r--r-- | extensions/openpower-pels/host_notifier.cpp | 101 | ||||
| -rw-r--r-- | extensions/openpower-pels/host_notifier.hpp | 18 | ||||
| -rw-r--r-- | 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<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 @@ 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<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) { |
