From cc3b64aebb1760750888324f116d11a952acd203 Mon Sep 17 00:00:00 2001 From: Matt Spinler Date: Thu, 12 Dec 2019 11:27:10 -0600 Subject: PEL: Support for the host acking a PEL After the host firmware successfully transfers a PEL to the OS, it will respond with an 'Ack' command that the PLDM daemon sends over to this daemon via a D-Bus method call. Add support to the HostNotifier class for this. It will change the state field in the PEL to 'acked' so that it doesn't get sent up again. Signed-off-by: Matt Spinler Change-Id: Id2a9985965017d9431419c1375d5374a2d0ae00b --- extensions/openpower-pels/host_notifier.cpp | 12 +++++ extensions/openpower-pels/host_notifier.hpp | 9 ++++ test/openpower-pels/host_notifier_test.cpp | 72 +++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+) diff --git a/extensions/openpower-pels/host_notifier.cpp b/extensions/openpower-pels/host_notifier.cpp index f2d951f..b4e92a8 100644 --- a/extensions/openpower-pels/host_notifier.cpp +++ b/extensions/openpower-pels/host_notifier.cpp @@ -345,4 +345,16 @@ void HostNotifier::stopCommand() } } +void HostNotifier::ackPEL(uint32_t id) +{ + _repo.setPELHostTransState(id, TransmissionState::acked); + + // No longer just 'sent', so remove it from the sent list. + auto sent = std::find(_sentPELs.begin(), _sentPELs.end(), id); + if (sent != _sentPELs.end()) + { + _sentPELs.erase(sent); + } +} + } // namespace openpower::pels diff --git a/extensions/openpower-pels/host_notifier.hpp b/extensions/openpower-pels/host_notifier.hpp index 3e50c83..d160dd4 100644 --- a/extensions/openpower-pels/host_notifier.hpp +++ b/extensions/openpower-pels/host_notifier.hpp @@ -82,6 +82,15 @@ class HostNotifier */ bool notifyRequired(uint32_t id) const; + /** + * @brief Called when the host sends the 'ack' PLDM command. + * + * This means the PEL never needs to be sent up again. + * + * @param[in] id - The PEL ID + */ + void ackPEL(uint32_t id); + private: /** * @brief This function gets called by the Repository class diff --git a/test/openpower-pels/host_notifier_test.cpp b/test/openpower-pels/host_notifier_test.cpp index afb5dc2..62c79b2 100644 --- a/test/openpower-pels/host_notifier_test.cpp +++ b/test/openpower-pels/host_notifier_test.cpp @@ -571,3 +571,75 @@ TEST_F(HostNotifierTest, TestCancelCmd) EXPECT_EQ(mockHostIface.numCmdsProcessed(), 1); EXPECT_EQ(notifier.queueSize(), 0); } + +// Test that acking a PEL persist across power cycles +TEST_F(HostNotifierTest, TestPowerCycleAndAcks) +{ + Repository repo{repoPath}; + MockDataInterface dataIface; + + sdeventplus::Event sdEvent{event}; + + std::unique_ptr hostIface = + std::make_unique(event, dataIface); + + MockHostInterface& mockHostIface = + reinterpret_cast(*hostIface); + + HostNotifier notifier{repo, dataIface, std::move(hostIface)}; + + auto send = [&mockHostIface](uint32_t id, uint32_t size) { + return mockHostIface.send(0); + }; + + EXPECT_CALL(mockHostIface, sendNewLogCmd(_, _)) + .WillRepeatedly(Invoke(send)); + + // Add 2 PELs with host off + auto pel = makePEL(); + repo.add(pel); + auto id1 = pel->id(); + + pel = makePEL(); + repo.add(pel); + auto id2 = pel->id(); + + dataIface.changeHostState(true); + + runEvents(sdEvent, 2); + + // The were both sent. + EXPECT_EQ(mockHostIface.numCmdsProcessed(), 2); + EXPECT_EQ(notifier.queueSize(), 0); + + dataIface.changeHostState(false); + + // Those PELs weren't acked, so they will get sent again + EXPECT_EQ(notifier.queueSize(), 2); + + // Power back on and send them again + dataIface.changeHostState(true); + runEvents(sdEvent, 2); + + EXPECT_EQ(mockHostIface.numCmdsProcessed(), 4); + EXPECT_EQ(notifier.queueSize(), 0); + + // Ack them and verify the state in the PEL. + notifier.ackPEL(id1); + notifier.ackPEL(id2); + + Repository::LogID id{Repository::LogID::Pel{id1}}; + auto data = repo.getPELData(id); + PEL pelFromRepo1{*data}; + EXPECT_EQ(pelFromRepo1.hostTransmissionState(), TransmissionState::acked); + + id.pelID.id = id2; + data = repo.getPELData(id); + PEL pelFromRepo2{*data}; + EXPECT_EQ(pelFromRepo2.hostTransmissionState(), TransmissionState::acked); + + // Power back off, and they should't get re-added + dataIface.changeHostState(false); + + EXPECT_EQ(notifier.queueSize(), 0); +} -- cgit v1.2.1