entity_association_tree: Keep tree always updated
This commit keeps the entity association tree updated across Host
reboots. It deletes the Host entries from the entity associaton
tree when Host is powered off.
Without this fix the entity association tree keeps on growing with
duplicate Host entries during each power on(PDR exchange) resulting
a crash of pldm daemon after few power on-off sequence
Change-Id: I8fdc258195aef998d10e33577f3ec98da48b3d40
Signed-off-by: Sampa Misra <sampmisr@in.ibm.com>
diff --git a/host-bmc/host_pdr_handler.cpp b/host-bmc/host_pdr_handler.cpp
index e6bf7d7..b4ef645 100644
--- a/host-bmc/host_pdr_handler.cpp
+++ b/host-bmc/host_pdr_handler.cpp
@@ -25,11 +25,12 @@
sdeventplus::Event& event, pldm_pdr* repo,
const std::string& eventsJsonsDir,
pldm_entity_association_tree* entityTree,
+ pldm_entity_association_tree* bmcEntityTree,
Requester& requester, bool verbose) :
mctp_fd(mctp_fd),
mctp_eid(mctp_eid), event(event), repo(repo),
stateSensorHandler(eventsJsonsDir), entityTree(entityTree),
- requester(requester), verbose(verbose)
+ bmcEntityTree(bmcEntityTree), requester(requester), verbose(verbose)
{
fs::path hostFruJson(fs::path(HOST_JSONS_DIR) / fruJson);
if (fs::exists(hostFruJson))
@@ -69,7 +70,8 @@
pldm::utils::DBusHandler::getBus(),
propertiesChanged("/xyz/openbmc_project/state/host0",
"xyz.openbmc_project.State.Host"),
- [this, repo](sdbusplus::message::message& msg) {
+ [this, repo, entityTree,
+ bmcEntityTree](sdbusplus::message::message& msg) {
DbusChangedProps props{};
std::string intf;
msg.read(intf, props);
@@ -81,6 +83,9 @@
if (propVal == "xyz.openbmc_project.State.Host.HostState.Off")
{
pldm_pdr_remove_remote_pdrs(repo);
+ pldm_entity_association_tree_destroy_root(entityTree);
+ pldm_entity_association_tree_copy_root(bmcEntityTree,
+ entityTree);
this->sensorMap.clear();
}
}
diff --git a/host-bmc/host_pdr_handler.hpp b/host-bmc/host_pdr_handler.hpp
index a8866a2..8e898af 100644
--- a/host-bmc/host_pdr_handler.hpp
+++ b/host-bmc/host_pdr_handler.hpp
@@ -89,6 +89,7 @@
sdeventplus::Event& event, pldm_pdr* repo,
const std::string& eventsJsonsDir,
pldm_entity_association_tree* entityTree,
+ pldm_entity_association_tree* bmcEntityTree,
Requester& requester, bool verbose = false);
/** @brief fetch PDRs from host firmware. See @class.
@@ -173,8 +174,12 @@
pldm_pdr* repo;
StateSensorHandler stateSensorHandler;
- /** @brief Pointer to BMC's entity association tree */
+ /** @brief Pointer to BMC's and Host's entity association tree */
pldm_entity_association_tree* entityTree;
+
+ /** @brief Pointer to BMC's entity association tree */
+ pldm_entity_association_tree* bmcEntityTree;
+
/** @brief reference to Requester object, primarily used to access API to
* obtain PLDM instance id.
*/
diff --git a/libpldm/pdr.c b/libpldm/pdr.c
index 65a2591..562c9f9 100644
--- a/libpldm/pdr.c
+++ b/libpldm/pdr.c
@@ -694,6 +694,45 @@
return node;
}
+static void entity_association_tree_copy(pldm_entity_node *org_node,
+ pldm_entity_node **new_node)
+{
+ if (org_node == NULL) {
+ return;
+ }
+ *new_node = malloc(sizeof(pldm_entity_node));
+ (*new_node)->entity = org_node->entity;
+ (*new_node)->association_type = org_node->association_type;
+ (*new_node)->first_child = NULL;
+ (*new_node)->next_sibling = NULL;
+ entity_association_tree_copy(org_node->first_child,
+ &((*new_node)->first_child));
+ entity_association_tree_copy(org_node->next_sibling,
+ &((*new_node)->next_sibling));
+}
+
+void pldm_entity_association_tree_copy_root(
+ pldm_entity_association_tree *org_tree,
+ pldm_entity_association_tree *new_tree)
+{
+ new_tree->last_used_container_id = org_tree->last_used_container_id;
+ entity_association_tree_copy(org_tree->root, &(new_tree->root));
+}
+
+void pldm_entity_association_tree_destroy_root(
+ pldm_entity_association_tree *tree)
+{
+ assert(tree != NULL);
+ entity_association_tree_destroy(tree->root);
+ tree->last_used_container_id = 0;
+ tree->root = NULL;
+}
+
+bool pldm_is_empty_entity_assoc_tree(pldm_entity_association_tree *tree)
+{
+ return ((tree->root == NULL) ? true : false);
+}
+
void pldm_entity_association_pdr_extract(const uint8_t *pdr, uint16_t pdr_len,
size_t *num_entities,
pldm_entity **entities)
diff --git a/libpldm/pdr.h b/libpldm/pdr.h
index 68702f3..bf31405 100644
--- a/libpldm/pdr.h
+++ b/libpldm/pdr.h
@@ -305,6 +305,29 @@
pldm_entity_association_tree_find(pldm_entity_association_tree *tree,
pldm_entity *entity);
+/** @brief Create a copy of an existing entity association tree
+ *
+ * @param[in] org_tree - pointer to source tree
+ * @param[in/out] new_tree - pointer to destination tree
+ */
+void pldm_entity_association_tree_copy_root(
+ pldm_entity_association_tree *org_tree,
+ pldm_entity_association_tree *new_tree);
+
+/** @brief Destroy all the nodes of the entity association tree
+ *
+ * @param[in] tree - pointer to entity association tree
+ */
+void pldm_entity_association_tree_destroy_root(
+ pldm_entity_association_tree *tree);
+
+/** @brief Check whether the entity association tree is empty
+ *
+ * @param[in] tree - pointer to entity association tree
+ * @return bool, true if tree is empty
+ */
+bool pldm_is_empty_entity_assoc_tree(pldm_entity_association_tree *tree);
+
/** @brief Extract entities from entity association PDR
*
* @param[in] pdr - entity association PDR
diff --git a/libpldm/tests/libpldm_pdr_test.cpp b/libpldm/tests/libpldm_pdr_test.cpp
index 416764b..8dcb413 100644
--- a/libpldm/tests/libpldm_pdr_test.cpp
+++ b/libpldm/tests/libpldm_pdr_test.cpp
@@ -1171,6 +1171,47 @@
pldm_entity_association_tree_destroy(tree);
}
+TEST(EntityAssociationPDR, testCopyTree)
+{
+ pldm_entity entities[4]{};
+ entities[0].entity_type = 1;
+ entities[1].entity_type = 2;
+ entities[2].entity_type = 2;
+ entities[3].entity_type = 3;
+
+ auto orgTree = pldm_entity_association_tree_init();
+ auto newTree = pldm_entity_association_tree_init();
+ auto l1 = pldm_entity_association_tree_add(orgTree, &entities[0], nullptr,
+ PLDM_ENTITY_ASSOCIAION_PHYSICAL);
+ EXPECT_NE(l1, nullptr);
+ auto l2a = pldm_entity_association_tree_add(
+ orgTree, &entities[1], l1, PLDM_ENTITY_ASSOCIAION_PHYSICAL);
+ EXPECT_NE(l2a, nullptr);
+ auto l2b = pldm_entity_association_tree_add(
+ orgTree, &entities[2], l1, PLDM_ENTITY_ASSOCIAION_PHYSICAL);
+ EXPECT_NE(l2b, nullptr);
+ auto l2c = pldm_entity_association_tree_add(
+ orgTree, &entities[3], l1, PLDM_ENTITY_ASSOCIAION_PHYSICAL);
+ EXPECT_NE(l2c, nullptr);
+ size_t orgNum{};
+ pldm_entity* orgOut = nullptr;
+ pldm_entity_association_tree_visit(orgTree, &orgOut, &orgNum);
+ EXPECT_EQ(orgNum, 4u);
+
+ pldm_entity_association_tree_copy_root(orgTree, newTree);
+ size_t newNum{};
+ pldm_entity* newOut = nullptr;
+ pldm_entity_association_tree_visit(newTree, &newOut, &newNum);
+ EXPECT_EQ(newNum, orgNum);
+ EXPECT_EQ(newOut[0].entity_type, 1u);
+ EXPECT_EQ(newOut[0].entity_instance_num, 1u);
+ EXPECT_EQ(newOut[0].entity_container_id, 0u);
+ free(orgOut);
+ free(newOut);
+ pldm_entity_association_tree_destroy(orgTree);
+ pldm_entity_association_tree_destroy(newTree);
+}
+
TEST(EntityAssociationPDR, testExtract)
{
std::vector<uint8_t> pdr{};
diff --git a/libpldmresponder/fru.cpp b/libpldmresponder/fru.cpp
index 1c04a12..ea19806 100644
--- a/libpldmresponder/fru.cpp
+++ b/libpldmresponder/fru.cpp
@@ -110,6 +110,8 @@
}
pldm_entity_association_pdr_add(entityTree, pdrRepo, false);
+ // save a copy of bmc's entity association tree
+ pldm_entity_association_tree_copy_root(entityTree, bmcEntityTree);
if (table.size())
{
diff --git a/libpldmresponder/fru.hpp b/libpldmresponder/fru.hpp
index 5ae593a..18c7882 100644
--- a/libpldmresponder/fru.hpp
+++ b/libpldmresponder/fru.hpp
@@ -57,11 +57,14 @@
* for PLDM FRU
* @param[in] pdrRepo - opaque pointer to PDR repository
* @param[in] entityTree - opaque pointer to the entity association tree
+ * @param[in] bmcEntityTree - opaque pointer to bmc's entity association
+ * tree
*/
FruImpl(const std::string& configPath, pldm_pdr* pdrRepo,
- pldm_entity_association_tree* entityTree) :
+ pldm_entity_association_tree* entityTree,
+ pldm_entity_association_tree* bmcEntityTree) :
parser(configPath),
- pdrRepo(pdrRepo), entityTree(entityTree)
+ pdrRepo(pdrRepo), entityTree(entityTree), bmcEntityTree(bmcEntityTree)
{}
/** @brief Total length of the FRU table in bytes, this excludes the pad
@@ -159,6 +162,7 @@
fru_parser::FruParser parser;
pldm_pdr* pdrRepo;
pldm_entity_association_tree* entityTree;
+ pldm_entity_association_tree* bmcEntityTree;
std::map<dbus::ObjectPath, pldm_entity_node*> objToEntityNode{};
@@ -187,8 +191,9 @@
public:
Handler(const std::string& configPath, pldm_pdr* pdrRepo,
- pldm_entity_association_tree* entityTree) :
- impl(configPath, pdrRepo, entityTree)
+ pldm_entity_association_tree* entityTree,
+ pldm_entity_association_tree* bmcEntityTree) :
+ impl(configPath, pdrRepo, entityTree, bmcEntityTree)
{
handlers.emplace(PLDM_GET_FRU_RECORD_TABLE_METADATA,
[this](const pldm_msg* request, size_t payloadLength) {
diff --git a/pldmd/pldmd.cpp b/pldmd/pldmd.cpp
index bfd3d71..5c25587 100644
--- a/pldmd/pldmd.cpp
+++ b/pldmd/pldmd.cpp
@@ -167,6 +167,10 @@
decltype(&pldm_entity_association_tree_destroy)>
entityTree(pldm_entity_association_tree_init(),
pldm_entity_association_tree_destroy);
+ std::unique_ptr<pldm_entity_association_tree,
+ decltype(&pldm_entity_association_tree_destroy)>
+ bmcEntityTree(pldm_entity_association_tree_init(),
+ pldm_entity_association_tree_destroy);
std::unique_ptr<HostPDRHandler> hostPDRHandler;
std::unique_ptr<pldm::host_effecters::HostEffecterParser>
hostEffecterParser;
@@ -177,7 +181,7 @@
{
hostPDRHandler = std::make_unique<HostPDRHandler>(
sockfd, hostEID, event, pdrRepo.get(), EVENTS_JSONS_DIR,
- entityTree.get(), dbusImplReq, verbose);
+ entityTree.get(), bmcEntityTree.get(), dbusImplReq, verbose);
hostEffecterParser =
std::make_unique<pldm::host_effecters::HostEffecterParser>(
&dbusImplReq, sockfd, pdrRepo.get(), dbusHandler.get(),
@@ -203,7 +207,7 @@
invoker.registerHandler(PLDM_BIOS, std::make_unique<bios::Handler>(
sockfd, hostEID, &dbusImplReq));
auto fruHandler = std::make_unique<fru::Handler>(
- FRU_JSONS_DIR, pdrRepo.get(), entityTree.get());
+ FRU_JSONS_DIR, pdrRepo.get(), entityTree.get(), bmcEntityTree.get());
// FRU table is built lazily when a FRU command or Get PDR command is
// handled. To enable building FRU table, the FRU handler is passed to the
// Platform handler.