fru: fix in the FRU Record Table
In the current design, when PLDM builds the FRU table, it
incorporates padbytes and checksum. However, the process of
adding or deleting a FRU triggers a rebuild of the table, including
the addition of padbytes again.This repetition is leading to a crash
in pldmtool after multiple add/remove operations.
To address the above issue, the proposed commit resolves the problem
by eliminating the inclusion of padbytes and checksum during the
initial construction of the FRU table. Instead, these elements are
added based on requests from the remote endpoint via the
getFRURecordTable command.
Tested: Powered on fine with the change. The pldmtool crash was
not seen.
Change-Id: Ia2fe72034c58e1f7a7f666b54de18989ebd7a618
Signed-off-by: Pavithra Barithaya <pavithra.b@ibm.com>
diff --git a/libpldmresponder/fru.cpp b/libpldmresponder/fru.cpp
index 67af845..2b2dc23 100644
--- a/libpldmresponder/fru.cpp
+++ b/libpldmresponder/fru.cpp
@@ -227,14 +227,6 @@
// save a copy of bmc's entity association tree
pldm_entity_association_tree_copy_root(entityTree, bmcEntityTree);
- if (table.size())
- {
- padBytes = pldm::utils::getNumPadBytes(table.size());
- table.resize(table.size() + padBytes, 0);
-
- // Calculate the checksum
- checksum = crc32(table.data(), table.size());
- }
isBuilt = true;
}
std::string FruImpl::populatefwVersion()
@@ -358,19 +350,48 @@
}
}
+std::vector<uint8_t> FruImpl::tableResize()
+{
+ std::vector<uint8_t> tempTable;
+
+ if (table.size())
+ {
+ std::copy(table.begin(), table.end(), std::back_inserter(tempTable));
+ padBytes = pldm::utils::getNumPadBytes(table.size());
+ tempTable.resize(tempTable.size() + padBytes, 0);
+ }
+ return tempTable;
+}
+
void FruImpl::getFRUTable(Response& response)
{
auto hdrSize = response.size();
+ std::vector<uint8_t> tempTable;
- response.resize(hdrSize + table.size() + sizeof(checksum), 0);
- std::copy(table.begin(), table.end(), response.begin() + hdrSize);
+ if (table.size())
+ {
+ tempTable = tableResize();
+ checksum = crc32(tempTable.data(), tempTable.size());
+ }
+ response.resize(hdrSize + tempTable.size() + sizeof(checksum), 0);
+ std::copy(tempTable.begin(), tempTable.end(), response.begin() + hdrSize);
// Copy the checksum to response data
- auto iter = response.begin() + hdrSize + table.size();
+ auto iter = response.begin() + hdrSize + tempTable.size();
std::copy_n(reinterpret_cast<const uint8_t*>(&checksum), sizeof(checksum),
iter);
}
+void FruImpl::getFRURecordTableMetadata()
+{
+ std::vector<uint8_t> tempTable;
+ if (table.size())
+ {
+ tempTable = tableResize();
+ checksum = crc32(tempTable.data(), tempTable.size());
+ }
+}
+
int FruImpl::getFRURecordByOption(std::vector<uint8_t>& fruData,
uint16_t /* fruTableHandle */,
uint16_t recordSetIdentifer,
@@ -426,6 +447,8 @@
0);
auto responsePtr = reinterpret_cast<pldm_msg*>(response.data());
+ impl.getFRURecordTableMetadata();
+
auto rc = encode_get_fru_record_table_metadata_resp(
request->hdr.instance_id, PLDM_SUCCESS, major, minor, maxSize,
impl.size(), impl.numRSI(), impl.numRecords(), impl.checkSum(),
diff --git a/libpldmresponder/fru.hpp b/libpldmresponder/fru.hpp
index 78a1a3e..da19d00 100644
--- a/libpldmresponder/fru.hpp
+++ b/libpldmresponder/fru.hpp
@@ -68,14 +68,14 @@
pdrRepo(pdrRepo), entityTree(entityTree), bmcEntityTree(bmcEntityTree)
{}
- /** @brief Total length of the FRU table in bytes, this excludes the pad
+ /** @brief Total length of the FRU table in bytes, this includes the pad
* bytes and the checksum.
*
* @return size of the FRU table
*/
uint32_t size() const
{
- return table.size() - padBytes;
+ return table.size();
}
/** @brief The checksum of the contents of the FRU table
@@ -111,6 +111,11 @@
*/
void getFRUTable(Response& response);
+ /** @brief Get the Fru Table MetaData
+ *
+ */
+ void getFRURecordTableMetadata();
+
/** @brief Get FRU Record Table By Option
* @param[out] response - Populate response with the FRU table got by
* options
@@ -177,6 +182,12 @@
*/
std::string populatefwVersion();
+ /* @brief Method to resize the table
+ *
+ * @return resized table
+ */
+ std::vector<uint8_t> tableResize();
+
private:
uint16_t nextRSI()
{