Skip to content

Commit

Permalink
Treat an ERR_MEM on send as non-fatal on LwIP. (project-chip#22418)
Browse files Browse the repository at this point in the history
* Treat an ERR_MEM on send as non-fatal on LwIP.

That can be caused by a transient lack of TX buffers.  We should just
try again via MRP.

Fixes project-chip#21671

* Address review comment.
  • Loading branch information
bzbarsky-apple authored and isiu-apple committed Sep 16, 2022
1 parent c83a5a3 commit 8fa2dcd
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 13 deletions.
14 changes: 1 addition & 13 deletions src/messaging/ExchangeMessageDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,19 +85,7 @@ CHIP_ERROR ExchangeMessageDispatch::SendMessage(SessionManager * sessionManager,

ReturnErrorOnFailure(sessionManager->PrepareMessage(session, payloadHeader, std::move(message), entryOwner->retainedBuf));
CHIP_ERROR err = sessionManager->SendPreparedMessage(session, entryOwner->retainedBuf);
if (err == CHIP_ERROR_POSIX(ENOBUFS))
{
// sendmsg on BSD-based systems never blocks, no matter how the
// socket is configured, and will return ENOBUFS in situation in
// which Linux, for example, blocks.
//
// This is typically a transient situation, so we pretend like this
// packet drop happened somewhere on the network instead of inside
// sendmsg and will just resend it in the normal MRP way later.
ChipLogError(ExchangeManager, "Ignoring ENOBUFS: %" CHIP_ERROR_FORMAT " on exchange " ChipLogFormatExchangeId,
err.Format(), ChipLogValueExchangeId(exchangeId, isInitiator));
err = CHIP_NO_ERROR;
}
err = ReliableMessageMgr::MapSendError(err, exchangeId, isInitiator);
ReturnErrorOnFailure(err);
reliableMessageMgr->StartRetransmision(entryOwner.release());
}
Expand Down
29 changes: 29 additions & 0 deletions src/messaging/ReliableMessageMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ CHIP_ERROR ReliableMessageMgr::SendFromRetransTable(RetransTableEntry * entry)

auto * sessionManager = entry->ec->GetExchangeMgr()->GetSessionManager();
CHIP_ERROR err = sessionManager->SendPreparedMessage(entry->ec->GetSessionHandle(), entry->retainedBuf);
err = MapSendError(err, entry->ec->GetExchangeId(), entry->ec->IsInitiator());

if (err == CHIP_NO_ERROR)
{
Expand Down Expand Up @@ -416,6 +417,34 @@ void ReliableMessageMgr::RegisterSessionUpdateDelegate(SessionUpdateDelegate * s
mSessionUpdateDelegate = sessionUpdateDelegate;
}

CHIP_ERROR ReliableMessageMgr::MapSendError(CHIP_ERROR error, uint16_t exchangeId, bool isInitiator)
{
if (
#if CHIP_SYSTEM_CONFIG_USE_LWIP
error == System::MapErrorLwIP(ERR_MEM)
#else
error == CHIP_ERROR_POSIX(ENOBUFS)
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP
)
{
// sendmsg on BSD-based systems never blocks, no matter how the
// socket is configured, and will return ENOBUFS in situation in
// which Linux, for example, blocks.
//
// This is typically a transient situation, so we pretend like this
// packet drop happened somewhere on the network instead of inside
// sendmsg and will just resend it in the normal MRP way later.
//
// Similarly, on LwIP an ERR_MEM on send indicates a likely
// temporary lack of TX buffers.
ChipLogError(ExchangeManager, "Ignoring transient send error: %" CHIP_ERROR_FORMAT " on exchange " ChipLogFormatExchangeId,
error.Format(), ChipLogValueExchangeId(exchangeId, isInitiator));
error = CHIP_NO_ERROR;
}

return error;
}

#if CHIP_CONFIG_TEST
int ReliableMessageMgr::TestGetCountRetransTable()
{
Expand Down
7 changes: 7 additions & 0 deletions src/messaging/ReliableMessageMgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,13 @@ class ReliableMessageMgr
*/
void RegisterSessionUpdateDelegate(SessionUpdateDelegate * sessionUpdateDelegate);

/**
* Map a send error code to the error code we should actually use for
* success checks. This maps some error codes to CHIP_NO_ERROR as
* appropriate.
*/
static CHIP_ERROR MapSendError(CHIP_ERROR error, uint16_t exchangeId, bool isInitiator);

#if CHIP_CONFIG_TEST
// Functions for testing
int TestGetCountRetransTable();
Expand Down

0 comments on commit 8fa2dcd

Please sign in to comment.