Skip to content

Commit

Permalink
Adjust storage and processing in SystemPacketBuffer (#31776)
Browse files Browse the repository at this point in the history
and associated code for large payloads.

Extend uint16_t type variables to size_t for the APIs
and all applicable places.
  • Loading branch information
pidarped committed Apr 25, 2024
1 parent f9a3601 commit 7d988c5
Show file tree
Hide file tree
Showing 35 changed files with 252 additions and 196 deletions.
2 changes: 1 addition & 1 deletion src/app/BufferedReadCallback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ CHIP_ERROR BufferedReadCallback::GenerateListTLV(TLV::ScopedBufferTLVReader & aR
//
// To avoid that, a single contiguous buffer is the best likely approach for now.
//
uint32_t totalBufSize = 0;
size_t totalBufSize = 0;
for (const auto & packetBuffer : mBufferedList)
{
totalBufSize += packetBuffer->TotalLength();
Expand Down
3 changes: 2 additions & 1 deletion src/app/server/EchoHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ chip::Protocols::Echo::EchoServer gEchoServer;
*/
void HandleEchoRequestReceived(chip::Messaging::ExchangeContext * ec, chip::System::PacketBufferHandle && payload)
{
ChipLogProgress(AppServer, "Echo Request, len=%u ... sending response.\n", payload->DataLength());
ChipLogProgress(AppServer, "Echo Request, len=%" PRIu32 "... sending response.\n",
static_cast<uint32_t>(payload->DataLength()));
}

} // namespace
Expand Down
10 changes: 5 additions & 5 deletions src/ble/BtpEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ CHIP_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle &&
// mRxFragmentSize may be smaller than the characteristic size. Make sure
// we're not truncating to a data length smaller than what we have already consumed.
VerifyOrExit(reader.OctetsRead() <= mRxFragmentSize, err = BLE_ERROR_REASSEMBLER_INCORRECT_STATE);
data->SetDataLength(chip::min(data->DataLength(), mRxFragmentSize));
data->SetDataLength(chip::min(data->DataLength(), static_cast<size_t>(mRxFragmentSize)));

// Now mark the bytes we consumed as consumed.
data->ConsumeHead(static_cast<uint16_t>(reader.OctetsRead()));
Expand Down Expand Up @@ -346,11 +346,11 @@ CHIP_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle &&
if (rx_flags.Has(HeaderFlags::kEndMessage))
{
// Trim remainder, if any, of the received packet buffer based on sender-specified length of reassembled message.
int padding = mRxBuf->DataLength() - mRxLength;
int padding = static_cast<uint16_t>(mRxBuf->DataLength()) - mRxLength;

if (padding > 0)
{
mRxBuf->SetDataLength(mRxLength);
mRxBuf->SetDataLength(static_cast<size_t>(mRxLength));
}

// Ensure all received fragments add up to sender-specified total message size.
Expand All @@ -375,7 +375,7 @@ CHIP_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle &&
}
if (!mRxBuf.IsNull())
{
ChipLogError(Ble, "With rx buf data length = %u", mRxBuf->DataLength());
ChipLogError(Ble, "With rx buf data length = %u", static_cast<unsigned>(mRxBuf->DataLength()));
}
LogState();

Expand Down Expand Up @@ -428,7 +428,7 @@ bool BtpEngine::HandleCharacteristicSend(System::PacketBufferHandle data, bool s

mTxBuf = std::move(data);
mTxState = kState_InProgress;
mTxLength = mTxBuf->DataLength();
mTxLength = static_cast<uint16_t>(mTxBuf->DataLength());

ChipLogDebugBtpEngine(Ble, ">>> CHIPoBle preparing to send whole message:");
PrintBufDebug(mTxBuf);
Expand Down
8 changes: 4 additions & 4 deletions src/inet/TCPEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ CHIP_ERROR TCPEndPoint::SetReceivedDataForTesting(System::PacketBufferHandle &&
return CHIP_NO_ERROR;
}

uint32_t TCPEndPoint::PendingSendLength()
size_t TCPEndPoint::PendingSendLength()
{
if (!mSendQueue.IsNull())
{
Expand All @@ -135,7 +135,7 @@ uint32_t TCPEndPoint::PendingSendLength()
return 0;
}

uint32_t TCPEndPoint::PendingReceiveLength()
size_t TCPEndPoint::PendingReceiveLength()
{
if (!mRcvQueue.IsNull())
{
Expand Down Expand Up @@ -333,8 +333,8 @@ void TCPEndPoint::DriveReceiving()
{
// Acknowledgement is done after handling the buffers to allow the
// application processing to throttle flow.
uint16_t ackLength = mRcvQueue->TotalLength();
CHIP_ERROR err = OnDataReceived(this, std::move(mRcvQueue));
size_t ackLength = mRcvQueue->TotalLength();
CHIP_ERROR err = OnDataReceived(this, std::move(mRcvQueue));
if (err != CHIP_NO_ERROR)
{
DoClose(err, false);
Expand Down
8 changes: 4 additions & 4 deletions src/inet/TCPEndPoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis<TCPEndPoint>
* received. The operational semantics are undefined if \c len is larger
* than the total outstanding unacknowledged received data.
*/
virtual CHIP_ERROR AckReceive(uint16_t len) = 0;
virtual CHIP_ERROR AckReceive(size_t len) = 0;

/**
* @brief Set the receive queue, for testing.
Expand All @@ -295,15 +295,15 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis<TCPEndPoint>
*
* @return Number of untransmitted bytes in the transmit queue.
*/
uint32_t PendingSendLength();
size_t PendingSendLength();

/**
* @brief Extract the length of the unacknowledged receive data.
*
* @return Number of bytes in the receive queue that have not yet been
* acknowledged with <tt>AckReceive(uint16_t len)</tt>.
*/
uint32_t PendingReceiveLength();
size_t PendingReceiveLength();

/**
* @brief Initiate TCP half close, in other words, finished with sending.
Expand Down Expand Up @@ -447,7 +447,7 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis<TCPEndPoint>
* is the length of the message text added to the TCP transmit window,
* which are eligible for sending by the underlying network stack.
*/
typedef void (*OnDataSentFunct)(TCPEndPoint * endPoint, uint16_t len);
typedef void (*OnDataSentFunct)(TCPEndPoint * endPoint, size_t len);

/**
* The endpoint's message text transmission event handling function
Expand Down
10 changes: 6 additions & 4 deletions src/inet/TCPEndPointImplLwIP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ CHIP_ERROR TCPEndPointImplLwIP::DriveSendingImpl()
{
VerifyOrDie(!startOfUnsent.buffer.IsNull());

uint16_t bufDataLen = startOfUnsent.buffer->DataLength();
uint16_t bufDataLen = static_cast<uint16_t>(startOfUnsent.buffer->DataLength());

// Get a pointer to the start of unsent data within the first buffer on the unsent queue.
const uint8_t * sendData = startOfUnsent.buffer->Start() + startOfUnsent.offset;
Expand Down Expand Up @@ -503,16 +503,18 @@ void TCPEndPointImplLwIP::DoCloseImpl(CHIP_ERROR err, State oldState)
}
}

CHIP_ERROR TCPEndPointImplLwIP::AckReceive(uint16_t len)
CHIP_ERROR TCPEndPointImplLwIP::AckReceive(size_t len)
{
VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
CHIP_ERROR res = CHIP_NO_ERROR;

VerifyOrReturnError(len < UINT16_MAX, CHIP_ERROR_INVALID_ARGUMENT);

// Lock LwIP stack
LOCK_TCPIP_CORE();

if (mTCP != nullptr)
tcp_recved(mTCP, len);
tcp_recved(mTCP, static_cast<uint16_t>(len));
else
res = CHIP_ERROR_CONNECTION_ABORTED;

Expand Down Expand Up @@ -570,7 +572,7 @@ TCPEndPointImplLwIP::BufferOffset TCPEndPointImplLwIP::FindStartOfUnsent()
while (leftToSkip > 0)
{
VerifyOrDie(!startOfUnsent.buffer.IsNull());
uint16_t bufDataLen = startOfUnsent.buffer->DataLength();
uint16_t bufDataLen = static_cast<uint16_t>(startOfUnsent.buffer->DataLength());
if (leftToSkip >= bufDataLen)
{
// We have more to skip than current packet buffer size.
Expand Down
2 changes: 1 addition & 1 deletion src/inet/TCPEndPointImplLwIP.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class TCPEndPointImplLwIP : public TCPEndPoint, public EndPointStateLwIP
CHIP_ERROR EnableNoDelay() override;
CHIP_ERROR EnableKeepAlive(uint16_t interval, uint16_t timeoutCount) override;
CHIP_ERROR DisableKeepAlive() override;
CHIP_ERROR AckReceive(uint16_t len) override;
CHIP_ERROR AckReceive(size_t len) override;
#if INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
void TCPUserTimeoutHandler() override;
#endif // INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
Expand Down
2 changes: 1 addition & 1 deletion src/inet/TCPEndPointImplOpenThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ CHIP_ERROR TCPEndPointImplOT::DisableKeepAlive()
{
return CHIP_ERROR_NOT_IMPLEMENTED;
}
CHIP_ERROR TCPEndPointImplOT::AckReceive(uint16_t len)
CHIP_ERROR TCPEndPointImplOT::AckReceive(size_t len)
{
return CHIP_ERROR_NOT_IMPLEMENTED;
}
Expand Down
2 changes: 1 addition & 1 deletion src/inet/TCPEndPointImplOpenThread.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class TCPEndPointImplOT : public TCPEndPoint, public EndPointStateOpenThread
CHIP_ERROR EnableNoDelay() override;
CHIP_ERROR EnableKeepAlive(uint16_t interval, uint16_t timeoutCount) override;
CHIP_ERROR DisableKeepAlive() override;
CHIP_ERROR AckReceive(uint16_t len) override;
CHIP_ERROR AckReceive(size_t len) override;
#if INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
void TCPUserTimeoutHandler() override;
#endif // INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
Expand Down
10 changes: 5 additions & 5 deletions src/inet/TCPEndPointImplSockets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ CHIP_ERROR TCPEndPointImplSockets::DisableKeepAlive()
return CHIP_NO_ERROR;
}

CHIP_ERROR TCPEndPointImplSockets::AckReceive(uint16_t len)
CHIP_ERROR TCPEndPointImplSockets::AckReceive(size_t len)
{
VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);

Expand Down Expand Up @@ -483,7 +483,7 @@ CHIP_ERROR TCPEndPointImplSockets::DriveSendingImpl()

while (!mSendQueue.IsNull())
{
uint16_t bufLen = mSendQueue->DataLength();
uint32_t bufLen = static_cast<uint32_t>(mSendQueue->DataLength());

ssize_t lenSentRaw = send(mSocket, mSendQueue->Start(), bufLen, sendFlags);

Expand All @@ -496,14 +496,14 @@ CHIP_ERROR TCPEndPointImplSockets::DriveSendingImpl()
break;
}

if (lenSentRaw < 0 || lenSentRaw > bufLen)
if (lenSentRaw < 0 || bufLen < static_cast<uint32_t>(lenSentRaw))
{
err = CHIP_ERROR_INCORRECT_STATE;
break;
}

// Cast is safe because bufLen is uint16_t.
uint16_t lenSent = static_cast<uint16_t>(lenSentRaw);
// Cast is safe because bufLen is uint32_t.
uint32_t lenSent = static_cast<uint32_t>(lenSentRaw);

// Mark the connection as being active.
MarkActive();
Expand Down
4 changes: 2 additions & 2 deletions src/inet/TCPEndPointImplSockets.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class TCPEndPointImplSockets : public TCPEndPoint, public EndPointStateSockets
CHIP_ERROR EnableNoDelay() override;
CHIP_ERROR EnableKeepAlive(uint16_t interval, uint16_t timeoutCount) override;
CHIP_ERROR DisableKeepAlive() override;
CHIP_ERROR AckReceive(uint16_t len) override;
CHIP_ERROR AckReceive(size_t len) override;
#if INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
void TCPUserTimeoutHandler() override;
#endif // INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
Expand All @@ -72,7 +72,7 @@ class TCPEndPointImplSockets : public TCPEndPoint, public EndPointStateSockets

#if INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
/// This counts the number of bytes written on the TCP socket since thelast probe into the TCP outqueue was made.
uint32_t mBytesWrittenSinceLastProbe;
size_t mBytesWrittenSinceLastProbe;

/// This is the measured size(in bytes) of the kernel TCP send queue at the end of the last user timeout window.
uint32_t mLastTCPKernelSendQueueLen;
Expand Down
2 changes: 1 addition & 1 deletion src/inet/UDPEndPointImplLwIP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ void UDPEndPointImplLwIP::LwIPReceiveUDPMessage(void * arg, struct udp_pcb * pcb
if (buf->HasChainedBuffer())
{
// Have to allocate a new big-enough buffer and copy.
uint16_t messageSize = buf->TotalLength();
size_t messageSize = buf->TotalLength();
System::PacketBufferHandle copy = System::PacketBufferHandle::New(messageSize, 0);
if (copy.IsNull() || buf->Read(copy->Start(), messageSize) != CHIP_NO_ERROR)
{
Expand Down
2 changes: 1 addition & 1 deletion src/inet/UDPEndPointImplOpenThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ CHIP_ERROR UDPEndPointImplOT::SendMsgImpl(const IPPacketInfo * aPktInfo, System:
message = otUdpNewMessage(mOTInstance, NULL);
VerifyOrExit(message != NULL, error = OT_ERROR_NO_BUFS);

error = otMessageAppend(message, msg->Start(), msg->DataLength());
error = otMessageAppend(message, msg->Start(), static_cast<uint16_t>(msg->DataLength()));

if (error == OT_ERROR_NONE)
{
Expand Down
7 changes: 5 additions & 2 deletions src/inet/UDPEndPointImplSockets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,10 @@ CHIP_ERROR UDPEndPointImplSockets::SendMsgImpl(const IPPacketInfo * aPktInfo, Sy
{
return CHIP_ERROR_POSIX(errno);
}
if (lenSent != msg->DataLength())

size_t len = static_cast<size_t>(lenSent);

if (len != msg->DataLength())
{
return CHIP_ERROR_OUTBOUND_MESSAGE_TOO_BIG;
}
Expand Down Expand Up @@ -611,7 +614,7 @@ void UDPEndPointImplSockets::HandlePendingIO(System::SocketEvents events)
{
lStatus = CHIP_ERROR_POSIX(errno);
}
else if (rcvLen > lBuffer->AvailableDataLength())
else if (lBuffer->AvailableDataLength() < static_cast<size_t>(rcvLen))
{
lStatus = CHIP_ERROR_INBOUND_MESSAGE_TOO_BIG;
}
Expand Down
2 changes: 1 addition & 1 deletion src/inet/tests/TestInetLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ static void HandleTCPConnectionClosed(TCPEndPoint * aEndPoint, CHIP_ERROR aError
}
}

static void HandleTCPDataSent(TCPEndPoint * aEndPoint, uint16_t len) {}
static void HandleTCPDataSent(TCPEndPoint * aEndPoint, size_t len) {}

static CHIP_ERROR HandleTCPDataReceived(TCPEndPoint * aEndPoint, PacketBufferHandle && aBuffer)
{
Expand Down
Loading

0 comments on commit 7d988c5

Please sign in to comment.