In-Memory FlightRecorder support for pldmd
pldm daemon in BMC can act both as a requester and
responder, and it is capable of talking to any device
that talks pldm spec.
With the rapid increase in the number of commands supported
by pldmd, and also with the async request/response support
enabled, its becomes extremely tough to debug the failures
in the communication.
And most of times, the essential information that is needed
to debug are the last few commands that BMC pldm responded to.
So this commit is an attempt to bring in an in-memory flight
recorder that could save the last 10(can be configurable)
pldm transactions in a circular buffer, and dumps the contents
of it into a file when it receives a SIGUR1 signal.
Resolves openbmc/pldm#24
Tested By :
1. Power on host
2. In the middle of poweron, send the SIGUSR1 signal to pldmd
root@rain118bmc:/tmp# kill -10 836
Received SIGUR1(10) Signal interrupt
root@rain118bmc:/tmp# Dumping the flight recorder into /tmp/pldm_flight_recorder
3. Make sure pldmd is not killed and does the rest of the power on
operation.
4. check the contents of /tmp/pldm_flight_recorder
root@p10bmc:~# cat /tmp/pldm_flight_recorder
UTC Nov 05 / 11:27:25.334606 : Tx :
0a 3f 0d 00
UTC Nov 05 / 11:27:26.292988 : Rx :
09 01 8b 3f 0d 00 00 7b 1e 00 50 00
UTC Nov 05 / 11:27:26.296915 : Tx :
0b 3f 0d 00
UTC Nov 05 / 11:27:27.250999 : Rx :
09 01 8c 3f 0d 00 00 7c 1e 00 50 00
UTC Nov 05 / 11:27:27.254762 : Tx :
0c 3f 0d 00
UTC Nov 05 / 11:27:28.212168 : Rx :
09 01 8d 3f 0d 00 00 7d 1e 00 50 00
UTC Nov 05 / 11:27:28.216086 : Tx :
0d 3f 0d 00
UTC Nov 05 / 11:27:29.171228 : Rx :
09 01 8e 3f 0d 00 00 7e 1e 00 50 00
UTC Nov 05 / 11:27:29.175143 : Tx :
0e 3f 0d 00
UTC Nov 05 / 11:27:25.330716 : Rx :
09 01 8a 3f 0d 00 00 7a 1e 00 50 00
5. Configure with -Dflightrecorder-max-entries=0
root@p10bmc:~# kill -10 21847
Received SIGUR1(10) Signal interrupt
Fight recorder policy is disabled
Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com>
Change-Id: I4e9c828f4ada9f1db6bf3a9b68c16e71b6e5d8f0
diff --git a/common/flight_recorder.hpp b/common/flight_recorder.hpp
new file mode 100644
index 0000000..db6314d
--- /dev/null
+++ b/common/flight_recorder.hpp
@@ -0,0 +1,127 @@
+#pragma once
+
+#include <config.h>
+
+#include <common/utils.hpp>
+
+#include <fstream>
+#include <iomanip>
+#include <iostream>
+#include <vector>
+namespace pldm
+{
+namespace flightrecorder
+{
+
+using ReqOrResponse = bool;
+using FlightRecorderData = std::vector<uint8_t>;
+using FlightRecorderTimeStamp = std::string;
+using FlightRecorderRecord =
+ std::tuple<FlightRecorderTimeStamp, ReqOrResponse, FlightRecorderData>;
+using FlightRecorderCassette = std::vector<FlightRecorderRecord>;
+static constexpr auto flightRecorderDumpPath = "/tmp/pldm_flight_recorder";
+
+/** @class FlightRecorder
+ *
+ * The class for implementing the PLDM flight recorder logic. This class
+ * handles the insertion of the data into the recorder and also provides
+ * API's to dump the flight recorder into a file.
+ */
+
+class FlightRecorder
+{
+ private:
+ FlightRecorder() : index(0)
+ {
+
+ flightRecorderPolicy = FLIGHT_RECORDER_MAX_ENTRIES ? true : false;
+ if (flightRecorderPolicy)
+ {
+ tapeRecorder = FlightRecorderCassette(FLIGHT_RECORDER_MAX_ENTRIES);
+ }
+ }
+
+ protected:
+ int index;
+ FlightRecorderCassette tapeRecorder;
+ bool flightRecorderPolicy;
+
+ public:
+ FlightRecorder(const FlightRecorder&) = delete;
+ FlightRecorder(FlightRecorder&&) = delete;
+ FlightRecorder& operator=(const FlightRecorder&) = delete;
+ FlightRecorder& operator=(FlightRecorder&&) = delete;
+ ~FlightRecorder() = default;
+
+ static FlightRecorder& GetInstance()
+ {
+ static FlightRecorder flightRecorder;
+ return flightRecorder;
+ }
+
+ /** @brief Add records to the flightRecorder
+ *
+ * @param[in] buffer - The request/respose byte buffer
+ * @param[in] isRequest - bool that captures if it is a request message or
+ * a response message
+ *
+ * @return void
+ */
+ void saveRecord(const FlightRecorderData& buffer, ReqOrResponse isRequest)
+ {
+ // if the flight recorder policy is enabled, then only insert the
+ // messages into the flight recorder, if not this function will be just
+ // a no-op
+ if (flightRecorderPolicy)
+ {
+ int currentIndex = index++;
+ tapeRecorder[currentIndex] = std::make_tuple(
+ pldm::utils::getCurrentSystemTime(), isRequest, buffer);
+ index =
+ (currentIndex == FLIGHT_RECORDER_MAX_ENTRIES - 1) ? 0 : index;
+ }
+ }
+
+ /** @brief play flight recorder
+ *
+ * @return void
+ */
+
+ void playRecorder()
+ {
+ if (flightRecorderPolicy)
+ {
+ std::ofstream recorderOutputFile(flightRecorderDumpPath);
+ std::cout << "Dumping the flight recorder into : "
+ << flightRecorderDumpPath << "\n";
+
+ for (const auto& message : tapeRecorder)
+ {
+ recorderOutputFile << std::get<FlightRecorderTimeStamp>(message)
+ << " : ";
+ if (std::get<ReqOrResponse>(message))
+ {
+ recorderOutputFile << "Tx : \n";
+ }
+ else
+ {
+ recorderOutputFile << "Rx : \n";
+ }
+ for (const auto& word : std::get<FlightRecorderData>(message))
+ {
+ recorderOutputFile << std::setfill('0') << std::setw(2)
+ << std::hex << (unsigned)word << " ";
+ }
+ recorderOutputFile << std::endl;
+ }
+ recorderOutputFile.close();
+ }
+ else
+ {
+ std::cerr << "Fight recorder policy is disabled\n";
+ }
+ }
+};
+
+} // namespace flightrecorder
+} // namespace pldm
diff --git a/common/utils.cpp b/common/utils.cpp
index ac19896..0b74833 100644
--- a/common/utils.cpp
+++ b/common/utils.cpp
@@ -568,5 +568,19 @@
return out;
}
+std::string getCurrentSystemTime()
+{
+ using namespace std::chrono;
+ const time_point<system_clock> tp = system_clock::now();
+ std::time_t tt = system_clock::to_time_t(tp);
+ auto ms = duration_cast<microseconds>(tp.time_since_epoch()) -
+ duration_cast<seconds>(tp.time_since_epoch());
+
+ std::stringstream ss;
+ ss << std::put_time(std::localtime(&tt), "%F %Z %T.")
+ << std::to_string(ms.count());
+ return ss.str();
+}
+
} // namespace utils
} // namespace pldm
diff --git a/common/utils.hpp b/common/utils.hpp
index 464816a..43a8056 100644
--- a/common/utils.hpp
+++ b/common/utils.hpp
@@ -402,6 +402,11 @@
*/
std::vector<std::string> split(std::string_view srcStr, std::string_view delim,
std::string_view trimStr = "");
+/** @brief Get the current system time in readable format
+ *
+ * @return - std::string equivalent of the system time
+ */
+std::string getCurrentSystemTime();
} // namespace utils
} // namespace pldm
diff --git a/meson.build b/meson.build
index 3c58cfd..abd5d4f 100644
--- a/meson.build
+++ b/meson.build
@@ -56,6 +56,7 @@
conf_data.set('NUMBER_OF_REQUEST_RETRIES', get_option('number-of-request-retries'))
conf_data.set('INSTANCE_ID_EXPIRATION_INTERVAL',get_option('instance-id-expiration-interval'))
conf_data.set('RESPONSE_TIME_OUT',get_option('response-time-out'))
+conf_data.set('FLIGHT_RECORDER_MAX_ENTRIES',get_option('flightrecorder-max-entries'))
if get_option('libpldm-only').disabled()
conf_data.set_quoted('HOST_EID_PATH', join_paths(package_datadir, 'host_eid'))
endif
diff --git a/meson_options.txt b/meson_options.txt
index 0cb99df..7bb863f 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -27,3 +27,5 @@
# Firmware update configuration parameters
option('maximum-transfer-size', type: 'integer', min: 16, max: 4294967295, description: 'Maximum size in bytes of the variable payload allowed to be requested by the FD, via RequestFirmwareData command', value: 4096)
+# Flight Recorder for PLDM Daemon
+option('flightrecorder-max-entries', type:'integer',min:0, max:30, description: 'The max number of pldm messages that can be stored in the recorder, this feature will be disabled if it is set to 0', value: 10)
diff --git a/pldmd/pldmd.cpp b/pldmd/pldmd.cpp
index d48844f..d64751d 100644
--- a/pldmd/pldmd.cpp
+++ b/pldmd/pldmd.cpp
@@ -3,6 +3,7 @@
#include "libpldm/pdr.h"
#include "libpldm/platform.h"
+#include "common/flight_recorder.hpp"
#include "common/utils.hpp"
#include "dbus_impl_requester.hpp"
#include "fw-update/manager.hpp"
@@ -22,6 +23,8 @@
#include <sdeventplus/event.hpp>
#include <sdeventplus/source/io.hpp>
+#include <sdeventplus/source/signal.hpp>
+#include <stdplus/signal.hpp>
#include <cstdio>
#include <cstring>
@@ -62,6 +65,17 @@
using namespace sdeventplus::source;
using namespace pldm::responder;
using namespace pldm::utils;
+using sdeventplus::source::Signal;
+using namespace pldm::flightrecorder;
+
+void interruptFlightRecorderCallBack(Signal& /*signal*/,
+ const struct signalfd_siginfo*)
+{
+ std::cerr << "\nReceived SIGUR1(10) Signal interrupt\n";
+
+ // obtain the flight recorder instance and dump the recorder
+ FlightRecorder::GetInstance().playRecorder();
+}
static std::optional<Response>
processRxMsg(const std::vector<uint8_t>& requestMsg, Invoker& invoker,
@@ -350,6 +364,7 @@
fd, static_cast<void*>(requestMsg.data()), peekedLength, 0);
if (recvDataLength == peekedLength)
{
+ FlightRecorder::GetInstance().saveRecord(requestMsg, false);
if (verbose)
{
printBuffer(Rx, requestMsg);
@@ -366,6 +381,8 @@
reqHandler, fwManager.get());
if (response.has_value())
{
+ FlightRecorder::GetInstance().saveRecord(*response,
+ true);
if (verbose)
{
printBuffer(Tx, *response);
@@ -429,6 +446,9 @@
hostPDRHandler->setHostFirmwareCondition();
}
#endif
+ stdplus::signal::block(SIGUSR1);
+ sdeventplus::source::Signal sigUsr1(
+ event, SIGUSR1, std::bind_front(&interruptFlightRecorderCallBack));
returnCode = event.loop();
if (shutdown(sockfd, SHUT_RDWR))
diff --git a/requester/request.hpp b/requester/request.hpp
index 2f2470f..8df3d03 100644
--- a/requester/request.hpp
+++ b/requester/request.hpp
@@ -3,6 +3,7 @@
#include "libpldm/base.h"
#include "libpldm/requester/pldm.h"
+#include "common/flight_recorder.hpp"
#include "common/types.hpp"
#include "common/utils.hpp"
@@ -194,6 +195,8 @@
return PLDM_ERROR;
}
}
+ pldm::flightrecorder::FlightRecorder::GetInstance().saveRecord(
+ requestMsg, true);
auto rc = pldm_send(eid, fd, requestMsg.data(), requestMsg.size());
if (rc < 0)
{