pldmd: move to libpldm instance ID alloc/free
Refactor the dbus_api::Requester class to be implemented in terms of
libpldm's instance ID database. To make that easier to deal with we
introduce a light-weight RAII C++ binding along with a helper class for
unit tests.
Change-Id: Ia03de8245dfb114e6266ba36dcf26ca4398a4ce0
Signed-off-by: Rashmica Gupta <rashmica@linux.ibm.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
diff --git a/pldmd/dbus_impl_requester.cpp b/pldmd/dbus_impl_requester.cpp
deleted file mode 100644
index 38ddb44..0000000
--- a/pldmd/dbus_impl_requester.cpp
+++ /dev/null
@@ -1,36 +0,0 @@
-#include "dbus_impl_requester.hpp"
-
-#include "xyz/openbmc_project/Common/error.hpp"
-
-#include <iostream>
-
-using namespace sdbusplus::xyz::openbmc_project::Common::Error;
-
-namespace pldm
-{
-namespace dbus_api
-{
-
-uint8_t Requester::getInstanceId(uint8_t eid)
-{
- if (ids.find(eid) == ids.end())
- {
- InstanceId id;
- ids.emplace(eid, InstanceId());
- }
-
- uint8_t id{};
- try
- {
- id = ids[eid].next();
- }
- catch (const std::runtime_error& e)
- {
- throw TooManyResources();
- }
-
- return id;
-}
-
-} // namespace dbus_api
-} // namespace pldm
diff --git a/pldmd/dbus_impl_requester.hpp b/pldmd/dbus_impl_requester.hpp
index a1dfcf5..2daaf18 100644
--- a/pldmd/dbus_impl_requester.hpp
+++ b/pldmd/dbus_impl_requester.hpp
@@ -1,6 +1,7 @@
#pragma once
#include "instance_id.hpp"
+#include "xyz/openbmc_project/Common/error.hpp"
#include "xyz/openbmc_project/PLDM/Requester/server.hpp"
#include <sdbusplus/bus.hpp>
@@ -34,26 +35,53 @@
/** @brief Constructor to put object onto bus at a dbus path.
* @param[in] bus - Bus to attach to.
* @param[in] path - Path to attach at.
+ * @param[in] db - The database to use for allocating instance IDs
+ * @note will throw TooManyResources() if there were no free instance IDs
+ * Throws std::system_category().default_error_condition if there is
+ * something wrong with the instance ID database.
*/
- Requester(sdbusplus::bus_t& bus, const std::string& path) :
- RequesterIntf(bus, path.c_str()){};
+ Requester(sdbusplus::bus_t& bus, const std::string& path,
+ InstanceIdDb& db) :
+ RequesterIntf(bus, path.c_str()),
+ pldmInstanceIdDb(db){};
/** @brief Implementation for RequesterIntf.GetInstanceId */
- uint8_t getInstanceId(uint8_t eid) override;
+ uint8_t getInstanceId(uint8_t eid) override
+ {
+ int id;
+
+ // Ideally we would be able to look up the TID for a given EID. We don't
+ // have that infrastructure in place yet. So use the EID value for the
+ // TID. This is an interim step towards the PLDM requester logic moving
+ // into libpldm, and eventually this won't be needed.
+ try
+ {
+ id = pldmInstanceIdDb.next(eid);
+ }
+ catch (const std::runtime_error& e)
+ {
+ throw sdbusplus::xyz::openbmc_project::Common::Error::
+ TooManyResources();
+ }
+
+ return id;
+ }
/** @brief Mark an instance id as unused
* @param[in] eid - MCTP eid to which this instance id belongs
* @param[in] instanceId - PLDM instance id to be freed
- * @note will throw std::out_of_range if instanceId > 31
+ * @note will throw std::runtime_error if the instance ID was not
+ * previously allocated.
+ * Throws std::system_category().default_error_condition if there is
+ * something wrong with the instance ID database.
*/
void markFree(uint8_t eid, uint8_t instanceId)
{
- ids[eid].markFree(instanceId);
+ pldmInstanceIdDb.free(eid, instanceId);
}
private:
- /** @brief EID to PLDM Instance ID map */
- std::map<uint8_t, InstanceId> ids;
+ InstanceIdDb& pldmInstanceIdDb;
};
} // namespace dbus_api
diff --git a/pldmd/instance_id.cpp b/pldmd/instance_id.cpp
deleted file mode 100644
index 52cd1d6..0000000
--- a/pldmd/instance_id.cpp
+++ /dev/null
@@ -1,25 +0,0 @@
-#include "instance_id.hpp"
-
-#include <stdexcept>
-
-namespace pldm
-{
-
-uint8_t InstanceId::next()
-{
- uint8_t idx = 0;
- while (idx < id.size() && id.test(idx))
- {
- ++idx;
- }
-
- if (idx == id.size())
- {
- throw std::runtime_error("No free instance ids");
- }
-
- id.set(idx);
- return idx;
-}
-
-} // namespace pldm
diff --git a/pldmd/instance_id.hpp b/pldmd/instance_id.hpp
index 4f88c43..1f003fa 100644
--- a/pldmd/instance_id.hpp
+++ b/pldmd/instance_id.hpp
@@ -1,34 +1,99 @@
#pragma once
-#include <bitset>
+#include "libpldm/instance-id.h"
+
+#include <phosphor-logging/lg2.hpp>
+
+#include <cerrno>
#include <cstdint>
+#include <exception>
+#include <string>
+#include <system_error>
+
+PHOSPHOR_LOG2_USING;
namespace pldm
{
-constexpr size_t maxInstanceIds = 32;
-
/** @class InstanceId
* @brief Implementation of PLDM instance id as per DSP0240 v1.0.0
*/
-class InstanceId
+class InstanceIdDb
{
public:
- /** @brief Get next unused instance id
- * @return - PLDM instance id
+ InstanceIdDb()
+ {
+ int rc = pldm_instance_db_init_default(&pldmInstanceIdDb);
+ if (rc)
+ {
+ throw std::system_category().default_error_condition(rc);
+ }
+ }
+
+ /** @brief Constructor
+ *
+ * @param[in] path - instance ID database path
*/
- uint8_t next();
+ InstanceIdDb(const std::string& path)
+ {
+ int rc = pldm_instance_db_init(&pldmInstanceIdDb, path.c_str());
+ if (rc)
+ {
+ throw std::system_category().default_error_condition(rc);
+ }
+ }
+
+ ~InstanceIdDb()
+ {
+ int rc = pldm_instance_db_destroy(pldmInstanceIdDb);
+ if (rc)
+ {
+ error("pldm_instance_db_destroy failed, rc= {RC}", "RC", rc);
+ }
+ }
+
+ /** @brief Allocate an instance ID for the given terminus
+ * @param[in] tid - the terminus ID the instance ID is associated with
+ * @return - PLDM instance id or -EAGAIN if there are no available instance
+ * IDs
+ */
+ uint8_t next(uint8_t tid)
+ {
+ uint8_t id;
+ int rc = pldm_instance_id_alloc(pldmInstanceIdDb, tid, &id);
+
+ if (rc == -EAGAIN)
+ {
+ throw std::runtime_error("No free instance ids");
+ }
+
+ if (rc)
+ {
+ throw std::system_category().default_error_condition(rc);
+ }
+
+ return id;
+ }
/** @brief Mark an instance id as unused
+ * @param[in] tid - the terminus ID the instance ID is associated with
* @param[in] instanceId - PLDM instance id to be freed
*/
- void markFree(uint8_t instanceId)
+ void free(uint8_t tid, uint8_t instanceId)
{
- id.set(instanceId, false);
+ int rc = pldm_instance_id_free(pldmInstanceIdDb, tid, instanceId);
+ if (rc == -EINVAL)
+ {
+ throw std::runtime_error("Invalid instance ID");
+ }
+ if (rc)
+ {
+ throw std::system_category().default_error_condition(rc);
+ }
}
private:
- std::bitset<maxInstanceIds> id;
+ struct pldm_instance_db* pldmInstanceIdDb = nullptr;
};
} // namespace pldm
diff --git a/pldmd/pldmd.cpp b/pldmd/pldmd.cpp
index 9f5bbaa..5da52f0 100644
--- a/pldmd/pldmd.cpp
+++ b/pldmd/pldmd.cpp
@@ -2,6 +2,7 @@
#include "common/utils.hpp"
#include "dbus_impl_requester.hpp"
#include "fw-update/manager.hpp"
+#include "instance_id.hpp"
#include "invoker.hpp"
#include "requester/handler.hpp"
#include "requester/mctp_endpoint_discovery.hpp"
@@ -196,7 +197,10 @@
auto& bus = pldm::utils::DBusHandler::getBus();
sdbusplus::server::manager_t objManager(bus,
"/xyz/openbmc_project/software");
- dbus_api::Requester dbusImplReq(bus, "/xyz/openbmc_project/pldm");
+
+ InstanceIdDb instanceIdDb;
+ dbus_api::Requester dbusImplReq(bus, "/xyz/openbmc_project/pldm",
+ instanceIdDb);
Invoker invoker{};
requester::Handler<requester::Request> reqHandler(