pldm: Convert to using libpldm transport APIs
A significant amount of logic can be removed by exploiting the new
transport APIs provided by libpldm. Switch the pldm repository over to
use these by introducing an RAII wrapper for the APIs. The current
stance is to continue using the legacy mctp-demux transport
implementation, but we also provide a build option to switch to the
AF_MCTP transport.
We don't currently have the infrastructure in place to get the correct
TIDs, so to keep everything working as before use the EID as the TID in
the EID-to-TID mapping.
Change-Id: I366f079082b102cfc0e90db0f62208581eb8693e
Signed-off-by: Rashmica Gupta <rashmica@linux.ibm.com>
Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
diff --git a/requester/handler.hpp b/requester/handler.hpp
index 9e5d8d1..6987b16 100644
--- a/requester/handler.hpp
+++ b/requester/handler.hpp
@@ -1,11 +1,11 @@
#pragma once
#include "common/instance_id.hpp"
+#include "common/transport.hpp"
#include "common/types.hpp"
#include "request.hpp"
#include <libpldm/base.h>
-#include <libpldm/pldm.h>
#include <sys/socket.h>
#include <phosphor-logging/lg2.hpp>
@@ -87,26 +87,24 @@
/** @brief Constructor
*
- * @param[in] fd - fd of MCTP communications socket
+ * @param[in] pldm_transport - PLDM requester
* @param[in] event - reference to PLDM daemon's main event loop
* @param[in] instanceIdDb - reference to an InstanceIdDb
- * @param[in] currentSendbuffSize - current send buffer size
* @param[in] verbose - verbose tracing flag
* @param[in] instanceIdExpiryInterval - instance ID expiration interval
* @param[in] numRetries - number of request retries
* @param[in] responseTimeOut - time to wait between each retry
*/
explicit Handler(
- int fd, sdeventplus::Event& event, pldm::InstanceIdDb& instanceIdDb,
- int currentSendbuffSize, bool verbose,
+ PldmTransport* pldmTransport, sdeventplus::Event& event,
+ pldm::InstanceIdDb& instanceIdDb, bool verbose,
std::chrono::seconds instanceIdExpiryInterval =
std::chrono::seconds(INSTANCE_ID_EXPIRATION_INTERVAL),
uint8_t numRetries = static_cast<uint8_t>(NUMBER_OF_REQUEST_RETRIES),
std::chrono::milliseconds responseTimeOut =
std::chrono::milliseconds(RESPONSE_TIME_OUT)) :
- fd(fd),
- event(event), instanceIdDb(instanceIdDb),
- currentSendbuffSize(currentSendbuffSize), verbose(verbose),
+ pldmTransport(pldmTransport),
+ event(event), instanceIdDb(instanceIdDb), verbose(verbose),
instanceIdExpiryInterval(instanceIdExpiryInterval),
numRetries(numRetries), responseTimeOut(responseTimeOut)
{}
@@ -171,8 +169,8 @@
}
auto request = std::make_unique<RequestInterface>(
- fd, eid, event, std::move(requestMsg), numRetries, responseTimeOut,
- currentSendbuffSize, verbose);
+ pldmTransport, eid, event, std::move(requestMsg), numRetries,
+ responseTimeOut, verbose);
auto timer = std::make_unique<phosphor::Timer>(
event.get(), instanceIdExpiryCallBack);
@@ -243,10 +241,9 @@
}
private:
- int fd; //!< file descriptor of MCTP communications socket
+ PldmTransport* pldmTransport; //!< PLDM transport object
sdeventplus::Event& event; //!< reference to PLDM daemon's main event loop
pldm::InstanceIdDb& instanceIdDb; //!< reference to an InstanceIdDb
- int currentSendbuffSize; //!< current Send Buffer size
bool verbose; //!< verbose tracing flag
std::chrono::seconds
instanceIdExpiryInterval; //!< Instance ID expiration interval
diff --git a/requester/mctp_endpoint_discovery.cpp b/requester/mctp_endpoint_discovery.cpp
index bc50ee5..5f3451a 100644
--- a/requester/mctp_endpoint_discovery.cpp
+++ b/requester/mctp_endpoint_discovery.cpp
@@ -3,8 +3,6 @@
#include "common/types.hpp"
#include "common/utils.hpp"
-#include <libpldm/pldm.h>
-
#include <algorithm>
#include <map>
#include <string>
diff --git a/requester/request.hpp b/requester/request.hpp
index 62615f8..27d94fc 100644
--- a/requester/request.hpp
+++ b/requester/request.hpp
@@ -1,11 +1,11 @@
#pragma once
#include "common/flight_recorder.hpp"
+#include "common/transport.hpp"
#include "common/types.hpp"
#include "common/utils.hpp"
#include <libpldm/base.h>
-#include <libpldm/pldm.h>
#include <sys/socket.h>
#include <phosphor-logging/lg2.hpp>
@@ -142,7 +142,7 @@
/** @brief Constructor
*
- * @param[in] fd - fd of the MCTP communication socket
+ * @param[in] pldm_transport - PLDM transport object
* @param[in] eid - endpoint ID of the remote MCTP endpoint
* @param[in] currrentSendbuffSize - the current send buffer size
* @param[in] event - reference to PLDM daemon's main event loop
@@ -151,21 +151,20 @@
* @param[in] timeout - time to wait between each retry in milliseconds
* @param[in] verbose - verbose tracing flag
*/
- explicit Request(int fd, mctp_eid_t eid, sdeventplus::Event& event,
- pldm::Request&& requestMsg, uint8_t numRetries,
- std::chrono::milliseconds timeout, int currentSendbuffSize,
+ explicit Request(PldmTransport* pldmTransport, mctp_eid_t eid,
+ sdeventplus::Event& event, pldm::Request&& requestMsg,
+ uint8_t numRetries, std::chrono::milliseconds timeout,
bool verbose) :
RequestRetryTimer(event, numRetries, timeout),
- fd(fd), eid(eid), requestMsg(std::move(requestMsg)),
- currentSendbuffSize(currentSendbuffSize), verbose(verbose)
+ pldmTransport(pldmTransport), eid(eid),
+ requestMsg(std::move(requestMsg)), verbose(verbose)
{}
private:
- int fd; //!< file descriptor of MCTP communications socket
- mctp_eid_t eid; //!< endpoint ID of the remote MCTP endpoint
- pldm::Request requestMsg; //!< PLDM request message
- mutable int currentSendbuffSize; //!< current Send Buffer size
- bool verbose; //!< verbose tracing flag
+ PldmTransport* pldmTransport; //!< PLDM transport
+ mctp_eid_t eid; //!< endpoint ID of the remote MCTP endpoint
+ pldm::Request requestMsg; //!< PLDM request message
+ bool verbose; //!< verbose tracing flag
/** @brief Sends the PLDM request message on the socket
*
@@ -177,26 +176,23 @@
{
pldm::utils::printBuffer(pldm::utils::Tx, requestMsg);
}
- if (currentSendbuffSize >= 0 &&
- (size_t)currentSendbuffSize < requestMsg.size())
- {
- int oldSendbuffSize = currentSendbuffSize;
- currentSendbuffSize = requestMsg.size();
- int res = setsockopt(fd, SOL_SOCKET, SO_SNDBUF,
- ¤tSendbuffSize,
- sizeof(currentSendbuffSize));
- if (res == -1)
- {
- error(
- "Requester : Failed to set the new send buffer size [bytes] : {CURR_SND_BUF_SIZE} from current size [bytes]: {OLD_BUF_SIZE} , Error : {ERR}",
- "CURR_SND_BUF_SIZE", currentSendbuffSize, "OLD_BUF_SIZE",
- oldSendbuffSize, "ERR", strerror(errno));
- return PLDM_ERROR;
- }
- }
pldm::flightrecorder::FlightRecorder::GetInstance().saveRecord(
requestMsg, true);
- auto rc = pldm_send(eid, fd, requestMsg.data(), requestMsg.size());
+ const struct pldm_msg_hdr* hdr =
+ (struct pldm_msg_hdr*)(requestMsg.data());
+ if (!hdr->request)
+ {
+ return PLDM_REQUESTER_NOT_REQ_MSG;
+ }
+
+ if (pldmTransport == nullptr)
+ {
+ error("Invalid transport: Unable to send PLDM request");
+ return PLDM_ERROR;
+ }
+
+ auto rc = pldmTransport->sendMsg(static_cast<pldm_tid_t>(eid),
+ requestMsg.data(), requestMsg.size());
if (rc < 0)
{
error("Failed to send PLDM message. RC = {RC}, errno = {ERR}", "RC",
diff --git a/requester/test/handler_test.cpp b/requester/test/handler_test.cpp
index 17a40fe..b656c1a 100644
--- a/requester/test/handler_test.cpp
+++ b/requester/test/handler_test.cpp
@@ -6,6 +6,7 @@
#include "test/test_instance_id.hpp"
#include <libpldm/base.h>
+#include <libpldm/transport.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
@@ -26,6 +27,7 @@
int fd = 0;
mctp_eid_t eid = 0;
+ PldmTransport* pldmTransport = nullptr;
sdeventplus::Event event;
TestInstanceIdDb instanceIdDb;
@@ -71,9 +73,9 @@
TEST_F(HandlerTest, singleRequestResponseScenario)
{
- Handler<NiceMock<MockRequest>> reqHandler(fd, event, instanceIdDb, false,
- 90000, seconds(1), 2,
- milliseconds(100));
+ Handler<NiceMock<MockRequest>> reqHandler(pldmTransport, event,
+ instanceIdDb, false, seconds(1),
+ 2, milliseconds(100));
pldm::Request request{};
auto instanceId = instanceIdDb.next(eid);
EXPECT_EQ(instanceId, 0);
@@ -92,9 +94,9 @@
TEST_F(HandlerTest, singleRequestInstanceIdTimerExpired)
{
- Handler<NiceMock<MockRequest>> reqHandler(fd, event, instanceIdDb, false,
- 90000, seconds(1), 2,
- milliseconds(100));
+ Handler<NiceMock<MockRequest>> reqHandler(pldmTransport, event,
+ instanceIdDb, false, seconds(1),
+ 2, milliseconds(100));
pldm::Request request{};
auto instanceId = instanceIdDb.next(eid);
EXPECT_EQ(instanceId, 0);
@@ -111,9 +113,9 @@
TEST_F(HandlerTest, multipleRequestResponseScenario)
{
- Handler<NiceMock<MockRequest>> reqHandler(fd, event, instanceIdDb, false,
- 90000, seconds(2), 2,
- milliseconds(100));
+ Handler<NiceMock<MockRequest>> reqHandler(pldmTransport, event,
+ instanceIdDb, false, seconds(2),
+ 2, milliseconds(100));
pldm::Request request{};
auto instanceId = instanceIdDb.next(eid);
EXPECT_EQ(instanceId, 0);
diff --git a/requester/test/mock_request.hpp b/requester/test/mock_request.hpp
index 7e6fc41..e77a3cb 100644
--- a/requester/test/mock_request.hpp
+++ b/requester/test/mock_request.hpp
@@ -1,7 +1,10 @@
#pragma once
+#include "common/transport.hpp"
#include "requester/request.hpp"
+#include <libpldm/transport.h>
+
#include <gmock/gmock.h>
#include <gtest/gtest.h>
@@ -14,10 +17,10 @@
class MockRequest : public RequestRetryTimer
{
public:
- MockRequest(int /*fd*/, mctp_eid_t /*eid*/, sdeventplus::Event& event,
- pldm::Request&& /*requestMsg*/, uint8_t numRetries,
- std::chrono::milliseconds responseTimeOut,
- int /*currentSendbuffSize*/, bool /*verbose*/) :
+ MockRequest(PldmTransport* /*pldmTransport*/, mctp_eid_t /*eid*/,
+ sdeventplus::Event& event, pldm::Request&& /*requestMsg*/,
+ uint8_t numRetries, std::chrono::milliseconds responseTimeOut,
+ bool /*verbose*/) :
RequestRetryTimer(event, numRetries, responseTimeOut)
{}
diff --git a/requester/test/request_test.cpp b/requester/test/request_test.cpp
index 2e3060d..51b337d 100644
--- a/requester/test/request_test.cpp
+++ b/requester/test/request_test.cpp
@@ -1,3 +1,4 @@
+#include "common/transport.hpp"
#include "mock_request.hpp"
#include <libpldm/base.h>
@@ -41,14 +42,15 @@
int fd = 0;
mctp_eid_t eid = 0;
+ PldmTransport* pldmTransport = nullptr;
sdeventplus::Event event;
- std::vector<uint8_t> requestMsg;
};
TEST_F(RequestIntfTest, 0Retries100msTimeout)
{
- MockRequest request(fd, eid, event, std::move(requestMsg), 0,
- milliseconds(100), 90000, false);
+ std::vector<uint8_t> requestMsg;
+ MockRequest request(pldmTransport, eid, event, std::move(requestMsg), 0,
+ milliseconds(100), false);
EXPECT_CALL(request, send())
.Times(Exactly(1))
.WillOnce(Return(PLDM_SUCCESS));
@@ -58,8 +60,9 @@
TEST_F(RequestIntfTest, 2Retries100msTimeout)
{
- MockRequest request(fd, eid, event, std::move(requestMsg), 2,
- milliseconds(100), 90000, false);
+ std::vector<uint8_t> requestMsg;
+ MockRequest request(pldmTransport, eid, event, std::move(requestMsg), 2,
+ milliseconds(100), false);
// send() is called a total of 3 times, the original plus two retries
EXPECT_CALL(request, send()).Times(3).WillRepeatedly(Return(PLDM_SUCCESS));
auto rc = request.start();
@@ -69,8 +72,9 @@
TEST_F(RequestIntfTest, 9Retries100msTimeoutRequestStoppedAfter1sec)
{
- MockRequest request(fd, eid, event, std::move(requestMsg), 9,
- milliseconds(100), 90000, false);
+ std::vector<uint8_t> requestMsg;
+ MockRequest request(pldmTransport, eid, event, std::move(requestMsg), 9,
+ milliseconds(100), false);
// send() will be called a total of 10 times, the original plus 9 retries.
// In a ideal scenario send() would have been called 10 times in 1 sec (when
// the timer is stopped) with a timeout of 100ms. Because there are delays
@@ -92,8 +96,9 @@
TEST_F(RequestIntfTest, 2Retries100msTimeoutsendReturnsError)
{
- MockRequest request(fd, eid, event, std::move(requestMsg), 2,
- milliseconds(100), 90000, false);
+ std::vector<uint8_t> requestMsg;
+ MockRequest request(pldmTransport, eid, event, std::move(requestMsg), 2,
+ milliseconds(100), false);
EXPECT_CALL(request, send()).Times(Exactly(1)).WillOnce(Return(PLDM_ERROR));
auto rc = request.start();
EXPECT_EQ(rc, PLDM_ERROR);