nvidia-gpu: Fix up buffering in MctpRequester
This change does a lot, for better or worse
1. Change MctpRequester to hold both buffers for send and receive
2. This requires changing the callback structure, so the reach is far
3. Changes error reporting to be through std::error_code
4. Collapses the QueuingRequeuster and Requeuster to be MctpRequeuster
5. Doing 4 gets rid of a level indirection and an extra unordered_map
6. Adds proper iid support, which is made significantly easier by 4/5
7. Fixes issues around expiry timer's where we would cancel the timer
for a given request whenever a new packet would come in to be sent.
This could cause lockup if a packet truly did time out and an
interleaved packet finished sending. This moves each queue
to have its own timer.
This fixes an issue where we were receiving buffers in from clients
and then binding them to receive_calls without ensuring that they
are the correct message, thus when receive was called, it was called
with the last bound buffer to async_receive_from. This would cause a
number of issues, ranging from incorrect device discovery results
to core dumps as well as incorrect sensor readings.
This change moves the receive and send buffers to be owned by
the MctpRequester, and a non-owning view is provided via
callback to the client. All existing clients just decode in place
given that buffer.
Tested: loaded onto nvl32-obmc. Correct number of sensors showed up
and the readings were nominal
Change-Id: I67c843691ca79e9fcccfa16df6d611918f25f6ca
Signed-off-by: Marc Olberding <molberding@nvidia.com>
diff --git a/src/nvidia-gpu/MctpRequester.hpp b/src/nvidia-gpu/MctpRequester.hpp
index 9d05ebd..0fd4683 100644
--- a/src/nvidia-gpu/MctpRequester.hpp
+++ b/src/nvidia-gpu/MctpRequester.hpp
@@ -6,88 +6,61 @@
#pragma once
+#include <MctpAsioEndpoint.hpp>
#include <OcpMctpVdm.hpp>
#include <boost/asio/generic/datagram_protocol.hpp>
#include <boost/asio/io_context.hpp>
#include <boost/asio/steady_timer.hpp>
+#include <boost/circular_buffer.hpp>
#include <boost/container/devector.hpp>
+#include <boost/container/flat_map.hpp>
+#include <boost/container/small_vector.hpp>
#include <cstddef>
#include <cstdint>
+#include <expected>
#include <functional>
+#include <iostream>
#include <memory>
+#include <queue>
#include <span>
+#include <system_error>
#include <unordered_map>
#include <utility>
namespace mctp
{
-class Requester
+class MctpRequester
{
public:
- Requester() = delete;
+ MctpRequester() = delete;
- Requester(const Requester&) = delete;
+ MctpRequester(const MctpRequester&) = delete;
- Requester(Requester&&) = delete;
+ MctpRequester(MctpRequester&&) = delete;
- Requester& operator=(const Requester&) = delete;
+ MctpRequester& operator=(const MctpRequester&) = delete;
- Requester& operator=(Requester&&) = delete;
+ MctpRequester& operator=(MctpRequester&&) = delete;
- explicit Requester(boost::asio::io_context& ctx);
+ explicit MctpRequester(boost::asio::io_context& ctx);
void sendRecvMsg(uint8_t eid, std::span<const uint8_t> reqMsg,
- std::span<uint8_t> respMsg,
- std::move_only_function<void(int)> callback);
+ std::move_only_function<void(const std::error_code&,
+ std::span<const uint8_t>)>
+ callback);
private:
- void processRecvMsg(std::span<const uint8_t> reqMsg,
- std::span<uint8_t> respMsg,
- const boost::system::error_code& ec, size_t length);
-
- void handleSendMsgCompletion(uint8_t eid, std::span<const uint8_t> reqMsg,
- std::span<uint8_t> respMsg,
- const boost::system::error_code& ec,
- size_t length);
-
- boost::asio::generic::datagram_protocol::socket mctpSocket;
+ using cb_t = std::move_only_function<void(const std::error_code&,
+ std::span<const uint8_t>)>;
static constexpr size_t maxMessageSize = 65536 + 256;
-
- boost::asio::generic::datagram_protocol::endpoint sendEndPoint;
-
- boost::asio::generic::datagram_protocol::endpoint recvEndPoint;
-
- boost::asio::steady_timer expiryTimer;
-
- std::unordered_map<uint8_t, std::move_only_function<void(int)>>
- completionCallbacks;
-
static constexpr uint8_t msgType = ocp::accelerator_management::messageType;
-};
-class QueuingRequester
-{
- public:
- QueuingRequester() = delete;
- QueuingRequester(const QueuingRequester&) = delete;
- QueuingRequester(QueuingRequester&&) = delete;
- QueuingRequester& operator=(const QueuingRequester&) = delete;
- QueuingRequester& operator=(QueuingRequester&&) = delete;
-
- explicit QueuingRequester(boost::asio::io_context& ctx) : requester(ctx) {}
-
- void sendRecvMsg(uint8_t eid, std::span<const uint8_t> reqMsg,
- std::span<uint8_t> respMsg,
- std::move_only_function<void(int)> callback);
-
- private:
struct RequestContext
{
- std::span<const uint8_t> reqMsg;
- std::span<uint8_t> respMsg;
- std::move_only_function<void(int)> callback;
+ std::vector<uint8_t> reqMsg;
+ cb_t callback;
RequestContext(const RequestContext&) = delete;
RequestContext& operator=(const RequestContext&) = delete;
@@ -96,21 +69,42 @@
RequestContext& operator=(RequestContext&&) = default;
~RequestContext() = default;
- explicit RequestContext(std::span<const uint8_t> req,
- std::span<uint8_t> resp,
- std::move_only_function<void(int)>&& cb) :
- reqMsg(req), respMsg(resp), callback(std::move(cb))
+ explicit RequestContext(std::span<const uint8_t> req, cb_t&& cb) :
+ reqMsg(req.begin(), req.end()), callback(std::move(cb))
{}
};
- void handleResult(uint8_t eid, int result);
+ struct EidContext
+ {
+ boost::asio::steady_timer timer;
+ uint8_t iid{};
+ boost::container::devector<RequestContext> queue;
+ EidContext(boost::asio::io_context& io) : timer{io}, iid{0xFF} {}
+ EidContext(EidContext&&) noexcept = default;
+ EidContext& operator=(EidContext&&) noexcept = default;
+ EidContext& operator=(const EidContext&) = delete;
+ EidContext(const EidContext&) = delete;
+ ~EidContext() = default;
+ };
+
+ std::optional<uint8_t> getNextIid(uint8_t eid);
+ void startReceive();
+ void processRecvMsg(const boost::system::error_code& ec, size_t length);
+ void handleSendMsgCompletion(uint8_t eid,
+ const boost::system::error_code& ec,
+ size_t length);
+
+ void handleResult(uint8_t eid, const std::error_code& ec,
+ std::span<const uint8_t> buffer);
void processQueue(uint8_t eid);
- Requester requester;
- std::unordered_map<
- uint8_t, boost::container::devector<std::unique_ptr<RequestContext>>>
- requestContextQueues;
+ boost::asio::io_context& io;
+ boost::asio::generic::datagram_protocol::endpoint sendEndPoint;
+
+ boost::asio::generic::datagram_protocol::socket mctpSocket;
+ std::array<uint8_t, maxMessageSize> buffer{};
+ MctpAsioEndpoint recvEndPoint;
+ std::unordered_map<uint8_t, EidContext> requestContextQueues;
};
-using MctpRequester = QueuingRequester;
} // namespace mctp