pdr: Add APIs for creating and locating remote PDRs
In the current state, the tree_find() and tree_add() utilities work by
replacing the existing container ID that comes from the remote
endpoints. But we need the remote container ID when we do the PDR
normalization (merging) in BMC.
Two boolean properties are introduced:
* is_remote: used to indicate if the entity is BMC owned or of remote
endpoint owned.
* is_update_container_id: used to indicate if the container id of the
entity needs to be updated or not.
We handle 3 different combinations,
1. Both are false - when it is the parent node in the tree
example: - https://github.com/ibm-openbmc/pldm/blob/1050/
libpldmresponder/pdr_state_sensor.hpp#L136
2. is_remote is false and update_container_id is true - when it is
a BMC entity but we have a container id change needed.
example: - https://github.com/ibm-openbmc/pldm/blob/1050/
libpldmresponder/fru.cpp#L130
3. is_remote is true and update_container_id is false - In IBM
PLDM stack talks to two different remote firmware implementations
over PLDM, and we had a conflicting requirement to not merge the
container ID from one endpoint but merge the container ID from
another endpoint. Example: -https://github.com/ibm-openbmc/
pldm/blob/1050/host-bmc/host_pdr_handler.cpp#L509
Change-Id: I8541b76aa46ba23172ffbc4d72e686909b7147fc
Signed-off-by: Pavithra Barithaya <pavithra.b@ibm.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6f66f63..d76c6a8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -22,6 +22,7 @@
1. Add encode/decode pldmMessagePollEvent data
2. README: Add a section on working with libpldm
3. pdr: Introduce remote_container_id and associated APIs
+4. pdr: Add APIs for creating and locating remote PDRs
### Changed
diff --git a/include/libpldm/pdr.h b/include/libpldm/pdr.h
index 6a72028..7542624 100644
--- a/include/libpldm/pdr.h
+++ b/include/libpldm/pdr.h
@@ -242,7 +242,7 @@
*/
pldm_entity_association_tree *pldm_entity_association_tree_init(void);
-/** @brief Add an entity into the entity association tree
+/** @brief Add a local entity into the entity association tree
*
* @param[in/out] tree - opaque pointer acting as a handle to the tree
* @param[in/out] entity - pointer to the entity to be added. Input has the
@@ -262,6 +262,35 @@
uint16_t entity_instance_number, pldm_entity_node *parent,
uint8_t association_type);
+/** @brief Add an entity into the entity association tree based on remote field
+ * set or unset.
+ *
+ * @param[in/out] tree - opaque pointer acting as a handle to the tree
+ * @param[in/out] entity - pointer to the entity to be added. Input has the
+ * entity type. On output, instance number and the
+ * container id are populated.
+ * @param[in] entity_instance_number - entity instance number, we can use the
+ * entity instance number of the entity by
+ * default if its value is equal 0xFFFF.
+ * @param[in] parent - pointer to the node that should be the parent of input
+ * entity. If this is NULL, then the entity is the root
+ * @param[in] association_type - relation with the parent : logical or physical
+ * @param[in] is_remote - used to denote whether we are adding a BMC entity to
+ * the tree or a host entity
+ * @param[in] is_update_contanier_id - Used to determine whether need to update
+ * contanier id.
+ * true: should be changed
+ * false: should not be changed
+ * @param[in] container_id - container id of the entity added.
+ *
+ * @return pldm_entity_node* - opaque pointer to added entity
+ */
+pldm_entity_node *pldm_entity_association_tree_add_entity(
+ pldm_entity_association_tree *tree, pldm_entity *entity,
+ uint16_t entity_instance_number, pldm_entity_node *parent,
+ uint8_t association_type, bool is_remote, bool is_update_container_id,
+ uint16_t container_id);
+
/** @brief Visit and note each entity in the entity association tree
*
* @param[in] tree - opaque pointer acting as a handle to the tree
@@ -377,6 +406,21 @@
pldm_entity_association_tree_find(pldm_entity_association_tree *tree,
pldm_entity *entity);
+/** @brief Find an entity in the entity association tree if remote
+ *
+ * @param[in] tree - pointer to entity association tree
+ * @param[in/out] entity - entity type and instance id set on input, container
+ * id set on output
+ * @param[in] is_remote - variable to denote whether we are finding a host
+ * entity or a BMC entity
+ *
+ * @return pldm_entity_node* pointer to entity if found, NULL otherwise
+ */
+pldm_entity_node *
+pldm_entity_association_tree_find_if_remote(pldm_entity_association_tree *tree,
+ pldm_entity *entity,
+ bool is_remote);
+
/** @brief Create a copy of an existing entity association tree
*
* @param[in] org_tree - pointer to source tree
diff --git a/src/pdr.c b/src/pdr.c
index ff4fbd1..752fd57 100644
--- a/src/pdr.c
+++ b/src/pdr.c
@@ -447,8 +447,22 @@
uint16_t entity_instance_number, pldm_entity_node *parent,
uint8_t association_type)
{
- assert(tree != NULL);
- assert(entity != NULL);
+ return pldm_entity_association_tree_add_entity(tree, entity,
+ entity_instance_number,
+ parent, association_type,
+ false, true, 0xFFFF);
+}
+
+LIBPLDM_ABI_TESTING
+pldm_entity_node *pldm_entity_association_tree_add_entity(
+ pldm_entity_association_tree *tree, pldm_entity *entity,
+ uint16_t entity_instance_number, pldm_entity_node *parent,
+ uint8_t association_type, bool is_remote, bool is_update_container_id,
+ uint16_t container_id)
+{
+ if ((!tree) || (!entity)) {
+ return NULL;
+ }
if (entity_instance_number != 0xFFFF && parent != NULL) {
pldm_entity node;
@@ -458,11 +472,14 @@
return NULL;
}
}
-
- assert(association_type == PLDM_ENTITY_ASSOCIAION_PHYSICAL ||
- association_type == PLDM_ENTITY_ASSOCIAION_LOGICAL);
+ if (association_type != PLDM_ENTITY_ASSOCIAION_PHYSICAL &&
+ association_type != PLDM_ENTITY_ASSOCIAION_LOGICAL) {
+ return NULL;
+ }
pldm_entity_node *node = malloc(sizeof(pldm_entity_node));
- assert(node != NULL);
+ if (!node) {
+ return NULL;
+ }
node->first_child = NULL;
node->next_sibling = NULL;
node->parent.entity_type = 0;
@@ -473,9 +490,11 @@
entity_instance_number != 0xFFFF ? entity_instance_number : 1;
node->association_type = association_type;
node->remote_container_id = 0;
-
if (tree->root == NULL) {
- assert(parent == NULL);
+ if (parent != NULL) {
+ free(node);
+ return NULL;
+ }
tree->root = node;
/* container_id 0 here indicates this is the top-most entry */
node->entity.entity_container_id = 0;
@@ -483,16 +502,41 @@
} else if (parent != NULL && parent->first_child == NULL) {
parent->first_child = node;
node->parent = parent->entity;
- node->entity.entity_container_id = next_container_id(tree);
+
+ if (is_remote) {
+ node->remote_container_id = entity->entity_container_id;
+ }
+ if (is_update_container_id) {
+ if (container_id != 0xFFFF) {
+ node->entity.entity_container_id = container_id;
+ } else {
+ node->entity.entity_container_id =
+ next_container_id(tree);
+ }
+ } else {
+ node->entity.entity_container_id =
+ entity->entity_container_id;
+ }
+
+ if (!is_remote) {
+ node->remote_container_id =
+ node->entity.entity_container_id;
+ }
} else {
pldm_entity_node *start = parent == NULL ? tree->root :
parent->first_child;
pldm_entity_node *prev =
find_insertion_at(start, entity->entity_type);
- assert(prev != NULL);
+ if (!prev) {
+ free(node);
+ return NULL;
+ }
pldm_entity_node *next = prev->next_sibling;
if (prev->entity.entity_type == entity->entity_type) {
- assert(prev->entity.entity_instance_num != UINT16_MAX);
+ if (prev->entity.entity_instance_num == UINT16_MAX) {
+ free(node);
+ return NULL;
+ }
node->entity.entity_instance_num =
entity_instance_number != 0xFFFF ?
entity_instance_number :
@@ -506,8 +550,9 @@
node->remote_container_id = entity->entity_container_id;
}
entity->entity_instance_num = node->entity.entity_instance_num;
- entity->entity_container_id = node->entity.entity_container_id;
-
+ if (is_update_container_id) {
+ entity->entity_container_id = node->entity.entity_container_id;
+ }
return node;
}
@@ -921,6 +966,51 @@
}
}
+static void entity_association_tree_find_if_remote(pldm_entity_node *node,
+ pldm_entity *entity,
+ pldm_entity_node **out,
+ bool is_remote)
+{
+ assert(out != NULL && *out == NULL);
+ if (node == NULL) {
+ return;
+ }
+ bool is_entity_type;
+ bool is_entity_instance_num;
+
+ is_entity_type = node->entity.entity_type == entity->entity_type;
+ is_entity_instance_num = node->entity.entity_instance_num ==
+ entity->entity_instance_num;
+
+ if (!is_remote ||
+ node->remote_container_id == entity->entity_container_id) {
+ if (is_entity_type && is_entity_instance_num) {
+ entity->entity_container_id =
+ node->entity.entity_container_id;
+ *out = node;
+ return;
+ }
+ }
+ entity_association_tree_find_if_remote(node->next_sibling, entity, out,
+ is_remote);
+ entity_association_tree_find_if_remote(node->first_child, entity, out,
+ is_remote);
+}
+
+LIBPLDM_ABI_TESTING
+pldm_entity_node *
+pldm_entity_association_tree_find_if_remote(pldm_entity_association_tree *tree,
+ pldm_entity *entity, bool is_remote)
+{
+ if (!tree || !entity) {
+ return NULL;
+ }
+ pldm_entity_node *node = NULL;
+ entity_association_tree_find_if_remote(tree->root, entity, &node,
+ is_remote);
+ return node;
+}
+
LIBPLDM_ABI_STABLE
void entity_association_tree_find(pldm_entity_node *node, pldm_entity *entity,
pldm_entity_node **out)
diff --git a/tests/libpldm_pdr_test.cpp b/tests/libpldm_pdr_test.cpp
index 6b99d5e..72860d7 100644
--- a/tests/libpldm_pdr_test.cpp
+++ b/tests/libpldm_pdr_test.cpp
@@ -748,6 +748,91 @@
pldm_entity_association_tree_destroy(tree);
}
+#if LIBPLDM_API_TESTING
+TEST(EntityAssociationPDR, findAndAddRemotePDR)
+{
+ // Tree - 1
+ //
+ // 11521(1,0)
+ // |
+ // 45 (1,1)
+ // |
+ // 64 (1,2)
+ // |
+ // ------------------
+ // | |
+ // 67(0,3) 67(1,3)
+ // | |
+ // 135(0,4) 135(0,5)
+ // | |
+ // 32903(0,6) 32903(0,7)
+
+ pldm_entity entities[9]{};
+ entities[0].entity_type = 11521;
+ entities[1].entity_type = 45;
+ entities[2].entity_type = 64;
+ entities[3].entity_type = 67;
+ entities[4].entity_type = 67;
+ entities[5].entity_type = 135;
+ entities[5].entity_container_id = 2;
+ entities[6].entity_type = 135;
+ entities[6].entity_container_id = 3;
+ entities[7].entity_type = 32903;
+ entities[8].entity_type = 32903;
+ auto tree = pldm_entity_association_tree_init();
+ auto l1 = pldm_entity_association_tree_add_entity(
+ tree, &entities[0], 0xFFFF, nullptr, PLDM_ENTITY_ASSOCIAION_LOGICAL,
+ false, true, 0xFFFF);
+ EXPECT_NE(l1, nullptr);
+ auto l2 = pldm_entity_association_tree_add_entity(
+ tree, &entities[1], 0xFFFF, l1, PLDM_ENTITY_ASSOCIAION_PHYSICAL, false,
+ false, 0xFFFF);
+ EXPECT_NE(l2, nullptr);
+ auto l3 = pldm_entity_association_tree_add_entity(
+ tree, &entities[2], 0xFFFF, l2, PLDM_ENTITY_ASSOCIAION_PHYSICAL, false,
+ true, 0xFFFF);
+ EXPECT_NE(l3, nullptr);
+ auto l4a = pldm_entity_association_tree_add_entity(
+ tree, &entities[3], 0, l3, PLDM_ENTITY_ASSOCIAION_PHYSICAL, false,
+ false, 0xFFFF);
+ EXPECT_NE(l4a, nullptr);
+ auto l4b = pldm_entity_association_tree_add_entity(
+ tree, &entities[4], 1, l3, PLDM_ENTITY_ASSOCIAION_PHYSICAL, true, true,
+ 0xFFFF);
+ EXPECT_NE(l4b, nullptr);
+ auto l5a = pldm_entity_association_tree_add_entity(
+ tree, &entities[5], 0, l4a, PLDM_ENTITY_ASSOCIAION_PHYSICAL, false,
+ false, 0xFFFF);
+ EXPECT_NE(l5a, nullptr);
+ auto l5b = pldm_entity_association_tree_add_entity(
+ tree, &entities[6], 0, l4b, PLDM_ENTITY_ASSOCIAION_PHYSICAL, false,
+ false, 0xFFFF);
+ EXPECT_NE(l5b, nullptr);
+ pldm_entity entity{};
+ entity.entity_type = 135;
+ entity.entity_instance_num = 0;
+ entity.entity_container_id = 2;
+ auto result1 = pldm_entity_association_tree_find(tree, &entity);
+ EXPECT_EQ(result1, l5a);
+ EXPECT_EQ(entities[5].entity_container_id, 2);
+ auto l6a = pldm_entity_association_tree_add_entity(
+ tree, &entities[7], 0, result1, PLDM_ENTITY_ASSOCIAION_PHYSICAL, false,
+ false, 0xFFFF);
+ EXPECT_NE(l6a, nullptr);
+ entity.entity_type = 135;
+ entity.entity_instance_num = 0;
+ entity.entity_container_id = 3;
+ auto result2 = pldm_entity_association_tree_find(tree, &entity);
+ EXPECT_NE(result2, l5b);
+ EXPECT_EQ(entities[6].entity_container_id, 3);
+ auto l7a = pldm_entity_association_tree_add_entity(
+ tree, &entities[8], 0, result2, PLDM_ENTITY_ASSOCIAION_PHYSICAL, false,
+ false, 0xFFFF);
+ EXPECT_EQ(l7a, nullptr);
+ pldm_entity_association_tree_destroy(tree);
+}
+#endif
+
TEST(EntityAssociationPDR, testSpecialTrees)
{
pldm_entity entities[3]{};