This is an automated email from the ASF dual-hosted git repository.
dmeden pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new d2cfc82e1e UDP/QUIC: Make sure quiche gets the entire rx buffer from
the udp (#9765)
d2cfc82e1e is described below
commit d2cfc82e1e08214f3233213b956b78bc4859b915
Author: Damian Meden <[email protected]>
AuthorDate: Fri Jun 2 12:51:20 2023 +0100
UDP/QUIC: Make sure quiche gets the entire rx buffer from the udp (#9765)
packet.
It seems that it was only getting the first slice of the entire chain.
Currently each of the iov buffer size is hardcoded to 2k, if happens
that we need to fit more than 2k inside the buffer the current code will
only hand over the first part of the chain which will make QUICHE to
complain as the data is incomplete.
This change makes sure that quiche gets the entire buffer.
---
iocore/net/I_UDPPacket.h | 2 ++
iocore/net/QUICNetVConnection_quiche.cc | 6 ++----
iocore/net/QUICPacketHandler_quiche.cc | 6 ++----
iocore/net/UnixUDPNet.cc | 31 +++++++++++++++++++++++++++++++
4 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/iocore/net/I_UDPPacket.h b/iocore/net/I_UDPPacket.h
index d3705ed7fa..4d88eb1242 100644
--- a/iocore/net/I_UDPPacket.h
+++ b/iocore/net/I_UDPPacket.h
@@ -73,6 +73,7 @@ public:
UDPConnection *getConnection();
IOBufferBlock *getIOBlockChain();
int64_t getPktLength();
+ uint8_t *get_entire_chain_buffer(size_t *buf_len);
/**
Add IOBufferBlock (chain) to end of packet.
@@ -114,6 +115,7 @@ public:
private:
SLINK(UDPPacket, alink); // atomic link
UDPPacketInternal p;
+ ats_unique_buf _payload{nullptr};
};
// Inline definitions
diff --git a/iocore/net/QUICNetVConnection_quiche.cc
b/iocore/net/QUICNetVConnection_quiche.cc
index 090a9d1280..3c5795876d 100644
--- a/iocore/net/QUICNetVConnection_quiche.cc
+++ b/iocore/net/QUICNetVConnection_quiche.cc
@@ -345,10 +345,8 @@ QUICNetVConnection::reset_quic_connection()
void
QUICNetVConnection::handle_received_packet(UDPPacket *packet)
{
- IOBufferBlock *block = packet->getIOBlockChain();
- uint8_t *buf = reinterpret_cast<uint8_t *>(block->buf());
- uint64_t buf_len = block->size();
-
+ size_t buf_len{0};
+ uint8_t *buf = packet->get_entire_chain_buffer(&buf_len);
net_activity(this, this_ethread());
quiche_recv_info recv_info = {
&packet->from.sa,
diff --git a/iocore/net/QUICPacketHandler_quiche.cc
b/iocore/net/QUICPacketHandler_quiche.cc
index bad50d651d..65b8fae054 100644
--- a/iocore/net/QUICPacketHandler_quiche.cc
+++ b/iocore/net/QUICPacketHandler_quiche.cc
@@ -190,10 +190,8 @@ QUICPacketHandlerIn::_get_continuation()
void
QUICPacketHandlerIn::_recv_packet(int event, UDPPacket *udp_packet)
{
- // Assumption: udp_packet has only one IOBufferBlock
- IOBufferBlock *block = udp_packet->getIOBlockChain();
- const uint8_t *buf = reinterpret_cast<uint8_t *>(block->buf());
- uint64_t buf_len = block->size();
+ uint64_t buf_len{0};
+ uint8_t *buf = udp_packet->get_entire_chain_buffer(&buf_len);
constexpr int MAX_TOKEN_LEN = 1200;
constexpr int DEFAULT_MAX_DATAGRAM_SIZE = 1350;
diff --git a/iocore/net/UnixUDPNet.cc b/iocore/net/UnixUDPNet.cc
index 429beae24d..9f2da492de 100644
--- a/iocore/net/UnixUDPNet.cc
+++ b/iocore/net/UnixUDPNet.cc
@@ -134,9 +134,40 @@ UDPPacket::free()
if (p.conn)
p.conn->Release();
p.conn = nullptr;
+
+ if (this->_payload) {
+ _payload.release();
+ }
udpPacketAllocator.free(this);
}
+uint8_t *
+UDPPacket::get_entire_chain_buffer(size_t *buf_len)
+{
+ if (this->_payload == nullptr) {
+ IOBufferBlock *block = this->getIOBlockChain();
+
+ // No need to allocate, we will use the first slice. With the current
slice size
+ // 2048 more likely we will using this block most of the time.
+ if (block && block->next.get() == nullptr) {
+ *buf_len = this->getPktLength();
+ return reinterpret_cast<uint8_t *>(block->buf());
+ }
+
+ // Store it. Should we try to avoid allocating here?
+ this->_payload = ats_unique_malloc(this->getPktLength());
+ uint64_t len{0};
+ while (block) {
+ memcpy(this->_payload.get() + len, block->start(), block->read_avail());
+ len += block->read_avail();
+ block = block->next.get();
+ }
+ ink_assert(len == static_cast<size_t>(this->getPktLength()));
+ }
+
+ *buf_len = this->getPktLength();
+ return this->_payload.get();
+}
//
// Global Data
//
