add handler logic to handle SysEntityName
Add handler logic to handler for SysEntityName such that it splits the
true IPMI processing from the business logic.
Tested: Only ran unit-tests (added new ones).
Change-Id: I6d672a80f85843ff98c2c7e5daf4689932ff96f9
Signed-off-by: Patrick Venture <venture@google.com>
diff --git a/entity_name.cpp b/entity_name.cpp
index c070c84..ac04275 100644
--- a/entity_name.cpp
+++ b/entity_name.cpp
@@ -16,25 +16,19 @@
#include "entity_name.hpp"
+#include "errors.hpp"
+#include "handler.hpp"
#include "main.hpp"
#include <cstdint>
#include <cstring>
-#include <filesystem>
-#include <fstream>
-#include <map>
-#include <nlohmann/json.hpp>
-#include <phosphor-logging/elog-errors.hpp>
-#include <phosphor-logging/log.hpp>
#include <string>
#include <vector>
-#include <xyz/openbmc_project/Common/error.hpp>
namespace google
{
namespace ipmi
{
-namespace fs = std::filesystem;
namespace
{
@@ -44,65 +38,6 @@
#define MAX_IPMI_BUFFER 64
#endif
-using Json = nlohmann::json;
-
-using namespace phosphor::logging;
-using InternalFailure =
- sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
-
-static constexpr auto configFile =
- "/usr/share/ipmi-entity-association/entity_association_map.json";
-
-static const std::map<uint8_t, std::string> entityIdToName{
- {0x03, "cpu"},
- {0x04, "storage_device"},
- {0x06, "system_management_module"},
- {0x08, "memory_module"},
- {0x0B, "add_in_card"},
- {0x17, "system_chassis"},
- {0x20, "memory_device"}};
-
-Json parse_config()
-{
- std::ifstream jsonFile(configFile);
- if (!jsonFile.is_open())
- {
- log<level::ERR>("Entity association JSON file not found");
- elog<InternalFailure>();
- }
-
- auto data = Json::parse(jsonFile, nullptr, false);
- if (data.is_discarded())
- {
- log<level::ERR>("Entity association JSON parser failure");
- elog<InternalFailure>();
- }
-
- return data;
-}
-
-std::string read(const std::string& type, uint8_t instance, const Json& config)
-{
- static const std::vector<Json> empty{};
- std::vector<Json> readings = config.value(type, empty);
- std::string name = "";
- for (const auto& j : readings)
- {
- uint8_t instanceNum = j.value("instance", 0);
- // Not the instance we're interested in
- if (instanceNum != instance)
- {
- continue;
- }
-
- // Found the instance we're interested in
- name = j.value("name", "");
-
- break;
- }
- return name;
-}
-
} // namespace
struct GetEntityNameRequest
@@ -120,7 +55,7 @@
} __attribute__((packed));
ipmi_ret_t GetEntityName(const uint8_t* reqBuf, uint8_t* replyBuf,
- size_t* dataLen)
+ size_t* dataLen, HandlerInterface* handler)
{
struct GetEntityNameRequest request;
@@ -132,37 +67,15 @@
}
std::memcpy(&request, &reqBuf[0], sizeof(request));
-
- // Check if we support this Entity ID.
- auto it = entityIdToName.find(request.entity_id);
- if (it == entityIdToName.end())
- {
- log<level::ERR>("Unknown Entity ID",
- entry("ENTITY_ID=%d", request.entity_id));
- return IPMI_CC_INVALID_FIELD_REQUEST;
- }
-
- static Json config{};
- static bool parsed = false;
-
std::string entityName;
try
{
- // Parse the JSON config file.
- if (!parsed)
- {
- config = parse_config();
- parsed = true;
- }
-
- // Find the "entity id:entity instance" mapping to entity name.
- entityName = read(it->second, request.entity_instance, config);
- if (entityName.empty())
- return IPMI_CC_INVALID_FIELD_REQUEST;
+ entityName =
+ handler->getEntityName(request.entity_id, request.entity_instance);
}
- catch (InternalFailure& e)
+ catch (const IpmiException& e)
{
- return IPMI_CC_UNSPECIFIED_ERROR;
+ return e.getIpmiError();
}
int length = sizeof(struct GetEntityNameReply) + entityName.length();
diff --git a/entity_name.hpp b/entity_name.hpp
index a1bc9f4..cf35e0d 100644
--- a/entity_name.hpp
+++ b/entity_name.hpp
@@ -1,5 +1,7 @@
#pragma once
+#include "handler.hpp"
+
#include <ipmid/api.h>
namespace google
@@ -10,7 +12,8 @@
// Handle the "entity id:entity instance" to entity name mapping command.
// Sys can query the entity name for a particular "entity id:entity instance".
ipmi_ret_t GetEntityName(const uint8_t* reqBuf, uint8_t* replyBuf,
- size_t* dataLen);
+ size_t* dataLen,
+ HandlerInterface* handler = &handlerImpl);
} // namespace ipmi
} // namespace google
diff --git a/handler.cpp b/handler.cpp
index 8a076f2..2018ef6 100644
--- a/handler.cpp
+++ b/handler.cpp
@@ -24,11 +24,15 @@
#include <cstdio>
#include <filesystem>
#include <fstream>
+#include <map>
+#include <nlohmann/json.hpp>
+#include <phosphor-logging/elog-errors.hpp>
#include <phosphor-logging/log.hpp>
#include <sdbusplus/bus.hpp>
#include <sstream>
#include <string>
#include <tuple>
+#include <xyz/openbmc_project/Common/error.hpp>
// The phosphor-host-ipmi daemon requires a configuration that maps
// the if_name to the IPMI LAN channel. However, that doesn't strictly
@@ -51,6 +55,10 @@
namespace ipmi
{
namespace fs = std::filesystem;
+using Json = nlohmann::json;
+using namespace phosphor::logging;
+using InternalFailure =
+ sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
std::tuple<std::uint8_t, std::string> Handler::getEthDetails() const
{
@@ -190,6 +198,93 @@
}
}
+static Json config{};
+static bool parsed = false;
+static const std::map<uint8_t, std::string> entityIdToName{
+ {0x03, "cpu"},
+ {0x04, "storage_device"},
+ {0x06, "system_management_module"},
+ {0x08, "memory_module"},
+ {0x0B, "add_in_card"},
+ {0x17, "system_chassis"},
+ {0x20, "memory_device"}};
+static constexpr auto configFile =
+ "/usr/share/ipmi-entity-association/entity_association_map.json";
+
+Json parse_config()
+{
+ std::ifstream jsonFile(configFile);
+ if (!jsonFile.is_open())
+ {
+ log<level::ERR>("Entity association JSON file not found");
+ elog<InternalFailure>();
+ }
+
+ auto data = Json::parse(jsonFile, nullptr, false);
+ if (data.is_discarded())
+ {
+ log<level::ERR>("Entity association JSON parser failure");
+ elog<InternalFailure>();
+ }
+
+ return data;
+}
+
+std::string read(const std::string& type, uint8_t instance, const Json& config)
+{
+ static const std::vector<Json> empty{};
+ std::vector<Json> readings = config.value(type, empty);
+ std::string name = "";
+ for (const auto& j : readings)
+ {
+ uint8_t instanceNum = j.value("instance", 0);
+ // Not the instance we're interested in
+ if (instanceNum != instance)
+ {
+ continue;
+ }
+
+ // Found the instance we're interested in
+ name = j.value("name", "");
+
+ break;
+ }
+ return name;
+}
+
+std::string Handler::getEntityName(std::uint8_t id, std::uint8_t instance)
+{
+ // Check if we support this Entity ID.
+ auto it = entityIdToName.find(id);
+ if (it == entityIdToName.end())
+ {
+ log<level::ERR>("Unknown Entity ID", entry("ENTITY_ID=%d", id));
+ throw IpmiException(IPMI_CC_INVALID_FIELD_REQUEST);
+ }
+
+ std::string entityName;
+ try
+ {
+ // Parse the JSON config file.
+ if (!parsed)
+ {
+ config = parse_config();
+ parsed = true;
+ }
+
+ // Find the "entity id:entity instance" mapping to entity name.
+ entityName = read(it->second, instance, config);
+ if (entityName.empty())
+ throw IpmiException(IPMI_CC_INVALID_FIELD_REQUEST);
+ }
+ catch (InternalFailure& e)
+ {
+ throw IpmiException(IPMI_CC_UNSPECIFIED_ERROR);
+ }
+
+ return entityName;
+}
+
Handler handlerImpl;
} // namespace ipmi
diff --git a/handler.hpp b/handler.hpp
index a068fce..be5f4d0 100644
--- a/handler.hpp
+++ b/handler.hpp
@@ -49,6 +49,20 @@
* @throw IpmiException on failure.
*/
virtual void psuResetDelay(std::uint32_t delay) const = 0;
+
+ /**
+ * Return the entity name.
+ * On the first call to this method it'll build the list of entities.
+ * @todo Consider moving the list building to construction time (and ignore
+ * failures).
+ *
+ * @param[in] id - the entity id value
+ * @param[in] instance - the entity instance
+ * @return the entity's name
+ * @throw IpmiException on failure.
+ */
+ virtual std::string getEntityName(std::uint8_t id,
+ std::uint8_t instance) = 0;
};
class Handler : public HandlerInterface
@@ -61,6 +75,7 @@
std::int64_t getRxPackets(const std::string& name) const override;
VersionTuple getCpldVersion(unsigned int id) const override;
void psuResetDelay(std::uint32_t delay) const override;
+ std::string getEntityName(std::uint8_t id, std::uint8_t instance) override;
};
extern Handler handlerImpl;
diff --git a/test/Makefile.am b/test/Makefile.am
index 04b6836..0243d44 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -34,3 +34,7 @@
check_PROGRAMS += psu_unittest
psu_unittest_SOURCES = psu_unittest.cpp
psu_unittest_LDADD = $(top_builddir)/libsyscmds_common.la
+
+check_PROGRAMS += entity_unittest
+entity_unittest_SOURCES = entity_unittest.cpp
+entity_unittest_LDADD = $(top_builddir)/libsyscmds_common.la
diff --git a/test/entity_unittest.cpp b/test/entity_unittest.cpp
new file mode 100644
index 0000000..74495c6
--- /dev/null
+++ b/test/entity_unittest.cpp
@@ -0,0 +1,53 @@
+#include "entity_name.hpp"
+#include "handler_mock.hpp"
+#include "main.hpp"
+
+#include <cstdint>
+#include <cstring>
+#include <string>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+#define MAX_IPMI_BUFFER 64
+
+using ::testing::Return;
+
+namespace google
+{
+namespace ipmi
+{
+
+TEST(EntityNameCommandTest, InvalidCommandLength)
+{
+ // GetEntityNameRequest is three bytes, let's send 2.
+ std::vector<std::uint8_t> request = {SysOEMCommands::SysEntityName, 0x01};
+ size_t dataLen = request.size();
+ std::uint8_t reply[MAX_IPMI_BUFFER];
+
+ HandlerMock hMock;
+ EXPECT_EQ(IPMI_CC_REQ_DATA_LEN_INVALID,
+ GetEntityName(request.data(), reply, &dataLen, &hMock));
+}
+
+TEST(EntityNameCommandTest, ValidRequest)
+{
+ std::uint8_t entityId = 3;
+ std::uint8_t entityInstance = 5;
+ std::vector<std::uint8_t> request = {SysOEMCommands::SysEntityName,
+ entityId, entityInstance};
+ size_t dataLen = request.size();
+ std::uint8_t reply[MAX_IPMI_BUFFER];
+ std::string entityName = "asdf";
+
+ HandlerMock hMock;
+ EXPECT_CALL(hMock, getEntityName(entityId, entityInstance))
+ .WillOnce(Return(entityName));
+ EXPECT_EQ(IPMI_CC_OK,
+ GetEntityName(request.data(), reply, &dataLen, &hMock));
+ EXPECT_EQ(reply[1], entityName.length());
+ EXPECT_EQ(0, std::memcmp(&reply[2], entityName.c_str(), reply[1]));
+}
+
+} // namespace ipmi
+} // namespace google
diff --git a/test/handler_mock.hpp b/test/handler_mock.hpp
index 6eb929a..6394f68 100644
--- a/test/handler_mock.hpp
+++ b/test/handler_mock.hpp
@@ -21,6 +21,7 @@
std::tuple<std::uint8_t, std::uint8_t, std::uint8_t,
std::uint8_t>(unsigned int));
MOCK_CONST_METHOD1(psuResetDelay, void(std::uint32_t));
+ MOCK_METHOD2(getEntityName, std::string(std::uint8_t, std::uint8_t));
};
} // namespace ipmi