PEL: Don't fix user specified action flags
Previously, the PEL object would never trust the action flags field
specified in the PEL registry and would always set some of the flags in
it itself based on a set of rules.
It turns out there are some cases where what the user needs doesn't
match the rules, so now only fix up the action flags if they weren't
specified in the registry and leave the flags alone if they were.
Most PELs in the registry should be able to leave out the action flags
field and let the PEL code set them.
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
Change-Id: I80263d779c842acac042023c468b7e979ec7158c
diff --git a/extensions/openpower-pels/README.md b/extensions/openpower-pels/README.md
index ce46a9f..c11185a 100644
--- a/extensions/openpower-pels/README.md
+++ b/extensions/openpower-pels/README.md
@@ -397,8 +397,7 @@
The `Action Flags` and `Event Type` PEL fields are optional in the message
registry, and if not present the code will set them based on certain rules
-layed out in the PEL spec. In fact, even if they were specified, the checks
-are still done to ensure consistency across all the logs.
+layed out in the PEL spec.
These rules are:
1. Always set the `Report` flag, unless the `Do Not Report` flag is already on.
diff --git a/extensions/openpower-pels/pel.cpp b/extensions/openpower-pels/pel.cpp
index 2113c0f..64b0f59 100644
--- a/extensions/openpower-pels/pel.cpp
+++ b/extensions/openpower-pels/pel.cpp
@@ -275,11 +275,17 @@
void PEL::checkRulesAndFix()
{
- auto [actionFlags, eventType] =
- pel_rules::check(_uh->actionFlags(), _uh->eventType(), _uh->severity());
+ // Only fix if the action flags are at their default value which
+ // means they weren't specified in the registry. Otherwise
+ // assume the user knows what they are doing.
+ if (_uh->actionFlags() == actionFlagsDefault)
+ {
+ auto [actionFlags, eventType] =
+ pel_rules::check(0, _uh->eventType(), _uh->severity());
- _uh->setActionFlags(actionFlags);
- _uh->setEventType(eventType);
+ _uh->setActionFlags(actionFlags);
+ _uh->setEventType(eventType);
+ }
}
void PEL::printSectionInJSON(const Section& section, std::string& buf,
diff --git a/extensions/openpower-pels/user_header.cpp b/extensions/openpower-pels/user_header.cpp
index 0e53acf..a687c55 100644
--- a/extensions/openpower-pels/user_header.cpp
+++ b/extensions/openpower-pels/user_header.cpp
@@ -116,8 +116,9 @@
_problemDomain = 0;
_problemVector = 0;
- // These will be cleaned up later in pel_rules::check()
- _actionFlags = entry.actionFlags.value_or(0);
+ // These will be set in pel_rules::check() if they're still
+ // at the default value.
+ _actionFlags = entry.actionFlags.value_or(actionFlagsDefault);
_states = 0;
diff --git a/extensions/openpower-pels/user_header.hpp b/extensions/openpower-pels/user_header.hpp
index 50c5824..07b74ae 100644
--- a/extensions/openpower-pels/user_header.hpp
+++ b/extensions/openpower-pels/user_header.hpp
@@ -13,6 +13,7 @@
{
static constexpr uint8_t userHeaderVersion = 0x01;
+static constexpr uint16_t actionFlagsDefault = 0xFFFF;
/**
* @class UserHeader
diff --git a/test/openpower-pels/pel_test.cpp b/test/openpower-pels/pel_test.cpp
index 6136733..0680185 100644
--- a/test/openpower-pels/pel_test.cpp
+++ b/test/openpower-pels/pel_test.cpp
@@ -204,6 +204,18 @@
EXPECT_EQ(mtmsCount, 1);
EXPECT_EQ(euhCount, 1);
EXPECT_EQ(udCount, 2); // AD section and sysInfo section
+
+ {
+ // The same thing, but without the action flags specified
+ // in the registry, so the constructor should set them.
+ regEntry.actionFlags = std::nullopt;
+
+ PEL pel2{
+ regEntry, 42, timestamp, phosphor::logging::Entry::Level::Error,
+ ad, ffdc, dataIface};
+
+ EXPECT_EQ(pel2.userHeader().actionFlags(), 0xA800);
+ }
}
// Test that when the AdditionalData size is over 16KB that
diff --git a/test/openpower-pels/user_header_test.cpp b/test/openpower-pels/user_header_test.cpp
index 546b480..526a59e 100644
--- a/test/openpower-pels/user_header_test.cpp
+++ b/test/openpower-pels/user_header_test.cpp
@@ -128,13 +128,26 @@
EXPECT_EQ(uh.header().componentID,
static_cast<uint16_t>(ComponentID::phosphorLogging));
- ASSERT_EQ(uh.subsystem(), 5);
- ASSERT_EQ(uh.severity(), 0x40);
- ASSERT_EQ(uh.eventType(), 1);
- ASSERT_EQ(uh.scope(), 2);
- ASSERT_EQ(uh.problemDomain(), 0);
- ASSERT_EQ(uh.problemVector(), 0);
- ASSERT_EQ(uh.actionFlags(), 0xC000);
+ EXPECT_EQ(uh.subsystem(), 5);
+ EXPECT_EQ(uh.severity(), 0x40);
+ EXPECT_EQ(uh.eventType(), 1);
+ EXPECT_EQ(uh.scope(), 2);
+ EXPECT_EQ(uh.problemDomain(), 0);
+ EXPECT_EQ(uh.problemVector(), 0);
+ EXPECT_EQ(uh.actionFlags(), 0xC000);
+
+ {
+ // The same thing, but as if the action flags weren't specified
+ // in the registry so they are a nullopt. The object should
+ // then set them to 0xFFFF.
+ EXPECT_CALL(dataIface, getSystemNames).WillOnce(ReturnRef(names));
+
+ regEntry.actionFlags = std::nullopt;
+
+ UserHeader uh(regEntry, phosphor::logging::Entry::Level::Error,
+ dataIface);
+ EXPECT_EQ(uh.actionFlags(), 0xFFFF);
+ }
}
// Test the system type based severity lookups