netipmid: make Handler asynchronous

The dbus call to the main ipmid queue was up to this point synchronous,
which means it blocks all other networking and execution until the main
queue returns (which may be on the order of seconds for some commands).
This is an unacceptable delay, especially when this queue is responsible
for timely updates of SOL traffic.

This turns the call into an asynchronous one by leveraging shared
pointers and an optional action on destruction. So as long as a
reference to the Handler object exists, it will live on, waiting to send
its response. Once the async dbus call has returned and set the reply in
the Handler, it will drop the reference to the shared pointer and the
destructor will send out the response over the channel.

Tested-by: Run multiple sessions at the same time while monitoring dbus
           traffic. See that the requests and responses may be
           interleaved instead of serial.

Change-Id: I16fca8dc3d13624eeb1592ec36d1a9af6575f115
Signed-off-by: Vernon Mauery <vernon.mauery@linux.intel.com>

5 files changed, 153 insertions, 119 deletions

diff --git a/command_table.cpp b/command_table.cpp
index e911bc6..04e1ac2 100644
--- a/command_table.cpp
+++ b/command_table.cpp
@@ -41,14 +41,12 @@ void Table::registerCommand(CommandID inCommand, std::unique_ptr<Entry>&& entry)
     command = std::move(entry);
 }
 
-std::vector<uint8_t> Table::executeCommand(uint32_t inCommand,
-                                           std::vector<uint8_t>& commandData,
-                                           const message::Handler& handler)
+void Table::executeCommand(uint32_t inCommand,
+                           std::vector<uint8_t>& commandData,
+                           std::shared_ptr<message::Handler> handler)
 {
     using namespace std::chrono_literals;
 
-    std::vector<uint8_t> response;
-
     auto iterator = commandTable.find(inCommand);
 
     if (iterator == commandTable.end())
@@ -57,54 +55,50 @@ std::vector<uint8_t> Table::executeCommand(uint32_t inCommand,
 
         auto bus = getSdBus();
         // forward the request onto the main ipmi queue
-        auto method = bus->new_method_call(
-            "xyz.openbmc_project.Ipmi.Host", "/xyz/openbmc_project/Ipmi",
-            "xyz.openbmc_project.Ipmi.Server", "execute");
+        using IpmiDbusRspType = std::tuple<uint8_t, uint8_t, uint8_t, uint8_t,
+                                           std::vector<uint8_t>>;
         uint8_t lun = command.lun();
         uint8_t netFn = command.netFn();
         uint8_t cmd = command.cmd();
         std::shared_ptr<session::Session> session =
             std::get<session::Manager&>(singletonPool)
-                .getSession(handler.sessionID);
+                .getSession(handler->sessionID);
         std::map<std::string, ipmi::Value> options = {
             {"userId", ipmi::Value(ipmi::ipmiUserGetUserId(session->userName))},
             {"privilege", ipmi::Value(static_cast<int>(session->curPrivLevel))},
         };
-        method.append(netFn, lun, cmd, commandData, options);
-        using IpmiDbusRspType = std::tuple<uint8_t, uint8_t, uint8_t, uint8_t,
-                                           std::vector<uint8_t>>;
-        IpmiDbusRspType rspTuple;
-        try
-        {
-            auto reply = bus->call(method);
-            reply.read(rspTuple);
-        }
-        catch (const sdbusplus::exception::SdBusError& e)
-        {
-            response.push_back(IPMI_CC_UNSPECIFIED_ERROR);
-            log<level::ERR>("Error sending command to ipmi queue");
-            elog<InternalFailure>();
-        }
-        auto& [rnetFn, rlun, rcmd, cc, responseData] = rspTuple;
-        if (uint8_t(netFn + 1) != rnetFn || rlun != lun || rcmd != cmd)
-        {
-            response.push_back(IPMI_CC_UNSPECIFIED_ERROR);
-            log<level::ERR>("DBus call/response mismatch from ipmi queue");
-            elog<InternalFailure>();
-        }
-        else
-        {
-            response.reserve(1 + responseData.size());
-            response.push_back(cc);
-            response.insert(response.end(), responseData.begin(),
-                            responseData.end());
-        }
+        bus->async_method_call(
+            [handler, this](const boost::system::error_code& ec,
+                            const IpmiDbusRspType& response) {
+                if (!ec)
+                {
+                    const uint8_t& cc = std::get<3>(response);
+                    const std::vector<uint8_t>& responseData =
+                        std::get<4>(response);
+                    std::vector<uint8_t> payload;
+                    payload.reserve(1 + responseData.size());
+                    payload.push_back(cc);
+                    payload.insert(payload.end(), responseData.begin(),
+                                   responseData.end());
+                    handler->outPayload = std::move(payload);
+                }
+                else
+                {
+                    std::vector<uint8_t> payload;
+                    payload.push_back(IPMI_CC_UNSPECIFIED_ERROR);
+                    handler->outPayload = std::move(payload);
+                }
+            },
+            "xyz.openbmc_project.Ipmi.Host", "/xyz/openbmc_project/Ipmi",
+            "xyz.openbmc_project.Ipmi.Server", "execute", netFn, lun, cmd,
+            commandData, options);
     }
     else
     {
         auto start = std::chrono::steady_clock::now();
 
-        response = iterator->second->executeCommand(commandData, handler);
+        handler->outPayload =
+            iterator->second->executeCommand(commandData, handler);
 
         auto end = std::chrono::steady_clock::now();
 
@@ -119,17 +113,16 @@ std::vector<uint8_t> Table::executeCommand(uint32_t inCommand,
                             entry("DELAY=%d", elapsedSeconds.count()));
         }
     }
-    return response;
 }
 
 std::vector<uint8_t>
     NetIpmidEntry::executeCommand(std::vector<uint8_t>& commandData,
-                                  const message::Handler& handler)
+                                  std::shared_ptr<message::Handler> handler)
 {
     std::vector<uint8_t> errResponse;
 
     // Check if the command qualifies to be run prior to establishing a session
-    if (!sessionless && (handler.sessionID == session::SESSION_ZERO))
+    if (!sessionless && (handler->sessionID == session::SESSION_ZERO))
     {
         errResponse.resize(1);
         errResponse[0] = IPMI_CC_INSUFFICIENT_PRIVILEGE;
@@ -140,7 +133,7 @@ std::vector<uint8_t>
         return errResponse;
     }
 
-    return functor(commandData, handler);
+    return functor(commandData, *handler);
 }
 
 } // namespace command
diff --git a/command_table.hpp b/command_table.hpp
index 7e9df05..b51da9d 100644
--- a/command_table.hpp
+++ b/command_table.hpp
@@ -133,7 +133,7 @@ class Entry
      */
     virtual std::vector<uint8_t>
         executeCommand(std::vector<uint8_t>& commandData,
-                       const message::Handler& handler) = 0;
+                       std::shared_ptr<message::Handler> handler) = 0;
 
     auto getCommand() const
     {
@@ -192,7 +192,7 @@ class NetIpmidEntry final : public Entry
      */
     std::vector<uint8_t>
         executeCommand(std::vector<uint8_t>& commandData,
-                       const message::Handler& handler) override;
+                       std::shared_ptr<message::Handler> handler) override;
 
     virtual ~NetIpmidEntry() = default;
     NetIpmidEntry(const NetIpmidEntry&) = default;
@@ -251,11 +251,9 @@ class Table
      * @param[in] commandData - Request Data for the command
      * @param[in] handler - Reference to the Message Handler
      *
-     * @return Response data for the command
      */
-    std::vector<uint8_t> executeCommand(uint32_t inCommand,
-                                        std::vector<uint8_t>& commandData,
-                                        const message::Handler& handler);
+    void executeCommand(uint32_t inCommand, std::vector<uint8_t>& commandData,
+                        std::shared_ptr<message::Handler> handler);
 
   private:
     CommandTable commandTable;
diff --git a/message_handler.cpp b/message_handler.cpp
index 58630d9..a45c13c 100644
--- a/message_handler.cpp
+++ b/message_handler.cpp
@@ -17,8 +17,9 @@ using namespace phosphor::logging;
 
 namespace message
 {
+using namespace phosphor::logging;
 
-std::shared_ptr<Message> Handler::receive()
+bool Handler::receive()
 {
     std::vector<uint8_t> packet;
     auto readStatus = 0;
@@ -30,51 +31,82 @@ std::shared_ptr<Message> Handler::receive()
     if (readStatus < 0)
     {
         log<level::ERR>("Error in Read", entry("STATUS=%x", readStatus));
-        return nullptr;
+        return false;
     }
 
     // Unflatten the packet
-    std::shared_ptr<Message> message;
-    std::tie(message, sessionHeader) = parser::unflatten(packet);
+    std::tie(inMessage, sessionHeader) = parser::unflatten(packet);
 
     auto session = std::get<session::Manager&>(singletonPool)
-                       .getSession(message->bmcSessionID);
+                       .getSession(inMessage->bmcSessionID);
 
-    sessionID = message->bmcSessionID;
-    message->rcSessionID = session->getRCSessionID();
+    sessionID = inMessage->bmcSessionID;
+    inMessage->rcSessionID = session->getRCSessionID();
     session->updateLastTransactionTime();
 
-    return message;
+    return true;
 }
 
-std::shared_ptr<Message>
-    Handler::executeCommand(std::shared_ptr<Message> inMessage)
+Handler::~Handler()
+{
+    if (outPayload)
+    {
+        std::shared_ptr<Message> outMessage =
+            inMessage->createResponse(*outPayload);
+        if (!outMessage)
+        {
+            return;
+        }
+        try
+        {
+            send(outMessage);
+        }
+        catch (const std::exception& e)
+        {
+            // send failed, most likely due to a session closure
+            log<level::INFO>("Async RMCP+ reply failed",
+                             entry("EXCEPTION=%s", e.what()));
+        }
+    }
+}
+
+void Handler::processIncoming()
+{
+    // Read the incoming IPMI packet
+    if (!receive())
+    {
+        return;
+    }
+
+    // Execute the Command, possibly asynchronously
+    executeCommand();
+
+    // send happens during the destructor if a payload was set
+}
+
+void Handler::executeCommand()
 {
     // Get the CommandID to map into the command table
     auto command = inMessage->getCommand();
-    std::vector<uint8_t> output{};
-
     if (inMessage->payloadType == PayloadType::IPMI)
     {
         if (inMessage->payload.size() <
             (sizeof(LAN::header::Request) + sizeof(LAN::trailer::Request)))
         {
-            return nullptr;
+            return;
         }
 
         auto start = inMessage->payload.begin() + sizeof(LAN::header::Request);
         auto end = inMessage->payload.end() - sizeof(LAN::trailer::Request);
         std::vector<uint8_t> inPayload(start, end);
-
-        output = std::get<command::Table&>(singletonPool)
-                     .executeCommand(command, inPayload, *this);
+        std::get<command::Table&>(singletonPool)
+            .executeCommand(command, inPayload, shared_from_this());
     }
     else
     {
-        output = std::get<command::Table&>(singletonPool)
-                     .executeCommand(command, inMessage->payload, *this);
+        std::get<command::Table&>(singletonPool)
+            .executeCommand(command, inMessage->payload, shared_from_this());
     }
-    return inMessage->createResponse(output);
 }
 
 void Handler::send(std::shared_ptr<Message> outMessage)
diff --git a/message_handler.hpp b/message_handler.hpp
index 599b3d6..533ed6a 100644
--- a/message_handler.hpp
+++ b/message_handler.hpp
@@ -10,56 +10,48 @@
 namespace message
 {
 
-class Handler
+class Handler : public std::enable_shared_from_this<Handler>
 {
   public:
-    explicit Handler(
-        std::shared_ptr<udpsocket::Channel> channel,
-        uint32_t sessionID = message::Message::MESSAGE_INVALID_SESSION_ID) :
+    /**
+     * @brief Create a Handler intended for a full transaction
+     *        that may or may not use asynchronous responses
+     */
+    Handler(std::shared_ptr<udpsocket::Channel> channel,
+            std::shared_ptr<boost::asio::io_context> io,
+            uint32_t sessionID = message::Message::MESSAGE_INVALID_SESSION_ID) :
+        sessionID(sessionID),
+        channel(channel), io(io)
+    {
+    }
+
+    /**
+     * @brief Create a Handler intended for a send only (SOL)
+     */
+    Handler(std::shared_ptr<udpsocket::Channel> channel,
+            uint32_t sessionID = message::Message::MESSAGE_INVALID_SESSION_ID) :
         sessionID(sessionID),
-        channel(channel)
+        channel(channel), io(nullptr)
     {
     }
 
+    ~Handler();
     Handler() = delete;
-    ~Handler() = default;
     Handler(const Handler&) = delete;
     Handler& operator=(const Handler&) = delete;
     Handler(Handler&&) = delete;
     Handler& operator=(Handler&&) = delete;
 
     /**
-     * @brief Receive the IPMI packet
-     *
-     * Read the data on the socket, get the parser based on the Session
-     * header type and flatten the payload and generate the IPMI message
-     *
-     * @return IPMI Message on success and nullptr on failure
-     *
-     */
-    std::shared_ptr<Message> receive();
-
-    /**
      * @brief Process the incoming IPMI message
      *
-     * The incoming message payload is handled and the command handler for
-     * the Network function and Command is executed and the response message
-     * is returned
-     *
-     * @param[in] inMessage - Incoming Message
-     *
-     * @return Outgoing message on success and nullptr on failure
-     */
-    std::shared_ptr<Message> executeCommand(std::shared_ptr<Message> inMessage);
-
-    /** @brief Send the outgoing message
-     *
-     *  The payload in the outgoing message is flattened and sent out on the
-     *  socket
-     *
-     *  @param[in] outMessage - Outgoing Message
+     * The incoming payload is read from the channel. If a message is read, it
+     * is passed onto executeCommand, which may or may not execute the command
+     * asynchrounously. If the command is executed asynchrounously, a shared_ptr
+     * of self via shared_from_this will keep this object alive until the
+     * response is ready. Then on the destructor, the response will be sent.
      */
-    void send(std::shared_ptr<Message> outMessage);
+    void processIncoming();
 
     /** @brief Set socket channel in session object */
     void setChannelInSession() const;
@@ -88,11 +80,45 @@ class Handler
     // BMC Session ID for the Channel
     session::SessionID sessionID;
 
+    /** @brief response to send back */
+    std::optional<std::vector<uint8_t>> outPayload;
+
   private:
+    /**
+     * @brief Receive the IPMI packet
+     *
+     * Read the data on the socket, get the parser based on the Session
+     * header type and flatten the payload and generate the IPMI message
+     */
+    bool receive();
+
+    /**
+     * @brief Process the incoming IPMI message
+     *
+     * The incoming message payload is handled and the command handler for
+     * the Network function and Command is executed and the response message
+     * is returned
+     */
+    void executeCommand();
+
+    /** @brief Send the outgoing message
+     *
+     *  The payload in the outgoing message is flattened and sent out on the
+     *  socket
+     *
+     *  @param[in] outMessage - Outgoing Message
+     */
+    void send(std::shared_ptr<Message> outMessage);
+
     /** @brief Socket channel for communicating with the remote client.*/
     std::shared_ptr<udpsocket::Channel> channel;
 
+    /** @brief asio io context to run asynchrounously */
+    std::shared_ptr<boost::asio::io_context> io;
+
     parser::SessionHeader sessionHeader = parser::SessionHeader::IPMI20;
+
+    std::shared_ptr<message::Message> inMessage;
 };
 
 } // namespace message
diff --git a/sd_event_loop.cpp b/sd_event_loop.cpp
index 5be974a..8c9abf4 100644
--- a/sd_event_loop.cpp
+++ b/sd_event_loop.cpp
@@ -23,24 +23,9 @@ void EventLoop::handleRmcpPacket()
         auto channelPtr = std::make_shared<udpsocket::Channel>(udpSocket);
 
         // Initialize the Message Handler with the socket channel
-        auto msgHandler = std::make_shared<message::Handler>(channelPtr);
+        auto msgHandler = std::make_shared<message::Handler>(channelPtr, io);
 
-        // Read the incoming IPMI packet
-        std::shared_ptr<message::Message> inMessage(msgHandler->receive());
-        if (inMessage == nullptr)
-        {
-            return;
-        }
-
-        // Execute the Command
-        std::shared_ptr<message::Message> outMessage =
-            msgHandler->executeCommand(inMessage);
-        if (outMessage == nullptr)
-        {
-            return;
-        }
-        // Send the response IPMI Message
-        msgHandler->send(outMessage);
+        msgHandler->processIncoming();
     }
     catch (const std::exception& e)
     {