diff --git a/src/inet/UDPEndPointImplLwIP.cpp b/src/inet/UDPEndPointImplLwIP.cpp index ad5f25361e40cd..2a46a64217cc20 100644 --- a/src/inet/UDPEndPointImplLwIP.cpp +++ b/src/inet/UDPEndPointImplLwIP.cpp @@ -276,12 +276,10 @@ void UDPEndPointImplLwIP::Free() Release(); } -void UDPEndPointImplLwIP::HandleDataReceived(System::PacketBufferHandle && msg) +void UDPEndPointImplLwIP::HandleDataReceived(System::PacketBufferHandle && msg, IPPacketInfo * pktInfo) { if ((mState == State::kListening) && (OnMessageReceived != nullptr)) { - const IPPacketInfo * pktInfo = GetPacketInfo(msg); - if (pktInfo != nullptr) { const IPPacketInfo pktInfoCopy = *pktInfo; // copy the address info so that the app can free the @@ -296,6 +294,7 @@ void UDPEndPointImplLwIP::HandleDataReceived(System::PacketBufferHandle && msg) } } } + Platform::Delete(pktInfo); } CHIP_ERROR UDPEndPointImplLwIP::GetPCB(IPAddressType addrType) @@ -364,31 +363,36 @@ void UDPEndPointImplLwIP::LwIPReceiveUDPMessage(void * arg, struct udp_pcb * pcb { Platform::UniquePtr pbufFreeGuard(p); UDPEndPointImplLwIP * ep = static_cast(arg); - IPPacketInfo * pktInfo = nullptr; if (ep->mState == State::kClosed) { return; } + // Raw pointer is required for passing into lambda. + // The memory life cycle of `pktInfo` is manually managed. + IPPacketInfo * pktInfo = Platform::New(); + if (pktInfo == nullptr) + { + ChipLogError(Inet, "Cannot allocate packet info"); + return; + } + // TODO: Skip copying the buffer if the pbuf already meets the PacketBuffer memory model - System::PacketBufferHandle buf = System::PacketBufferHandle::New(p->tot_len); + System::PacketBufferHandle buf = System::PacketBufferHandle::New(p->tot_len, 0); if (buf.IsNull() || pbuf_copy_partial(p, buf->Start(), p->tot_len, 0) != p->tot_len) { ChipLogError(Inet, "Cannot copy received pbuf of size %u", p->tot_len); + return; } buf->SetDataLength(p->tot_len); - pktInfo = GetPacketInfo(buf); - if (pktInfo != nullptr) - { - pktInfo->SrcAddress = IPAddress(*addr); - pktInfo->DestAddress = IPAddress(*ip_current_dest_addr()); - pktInfo->Interface = InterfaceId(ip_current_netif()); - pktInfo->SrcPort = port; - pktInfo->DestPort = pcb->local_port; - } + pktInfo->SrcAddress = IPAddress(*addr); + pktInfo->DestAddress = IPAddress(*ip_current_dest_addr()); + pktInfo->Interface = InterfaceId(ip_current_netif()); + pktInfo->SrcPort = port; + pktInfo->DestPort = pcb->local_port; - CHIP_ERROR err = ep->GetSystemLayer().ScheduleLambda([ep, p = System::LwIPPacketBufferView::UnsafeGetLwIPpbuf(buf)] { - ep->HandleDataReceived(System::PacketBufferHandle::Adopt(p)); + CHIP_ERROR err = ep->GetSystemLayer().ScheduleLambda([ep, p = System::LwIPPacketBufferView::UnsafeGetLwIPpbuf(buf), pktInfo] { + ep->HandleDataReceived(System::PacketBufferHandle::Adopt(p), pktInfo); }); if (err == CHIP_NO_ERROR) @@ -396,6 +400,12 @@ void UDPEndPointImplLwIP::LwIPReceiveUDPMessage(void * arg, struct udp_pcb * pcb // If ScheduleLambda() succeeded, it has ownership of the buffer, so we need to release it (without freeing it). static_cast(std::move(buf).UnsafeRelease()); } + else + { + // If ScheduleLambda() succeeded, `pktInfo` will be deleted in `HandleDataReceived`. + // Otherwise we delete it here. + Platform::Delete(pktInfo); + } } CHIP_ERROR UDPEndPointImplLwIP::SetMulticastLoopback(IPVersion aIPVersion, bool aLoopback) @@ -499,19 +509,5 @@ struct netif * UDPEndPointImplLwIP::FindNetifFromInterfaceId(InterfaceId aInterf return (lRetval); } -IPPacketInfo * UDPEndPointImplLwIP::GetPacketInfo(const System::PacketBufferHandle & aBuffer) -{ - if (!aBuffer->EnsureReservedSize(sizeof(IPPacketInfo) + 3)) - { - return nullptr; - } - - uintptr_t lStart = (uintptr_t) aBuffer->Start(); - uintptr_t lPacketInfoStart = lStart - sizeof(IPPacketInfo); - - // Align to a 4-byte boundary - return reinterpret_cast(lPacketInfoStart & ~(sizeof(uint32_t) - 1)); -} - } // namespace Inet } // namespace chip diff --git a/src/inet/UDPEndPointImplLwIP.h b/src/inet/UDPEndPointImplLwIP.h index accb18bfcb7dbc..7bc523c913ff3a 100644 --- a/src/inet/UDPEndPointImplLwIP.h +++ b/src/inet/UDPEndPointImplLwIP.h @@ -55,28 +55,7 @@ class UDPEndPointImplLwIP : public UDPEndPoint, public EndPointStateLwIP static struct netif * FindNetifFromInterfaceId(InterfaceId aInterfaceId); static CHIP_ERROR LwIPBindInterface(struct udp_pcb * aUDP, InterfaceId intfId); - void HandleDataReceived(chip::System::PacketBufferHandle && aBuffer); - - /** - * Get LwIP IP layer source and destination addressing information. - * - * @param[in] aBuffer The packet buffer containing the IP message. - * - * @returns a pointer to the address information on success; otherwise, - * nullptr if there is insufficient space in the packet for - * the address information. - * - * When using LwIP information about the packet is 'hidden' in the reserved space before the start of the - * data in the packet buffer. This is necessary because the system layer events only have two arguments, - * which in this case are used to convey the pointer to the end point and the pointer to the buffer. - * - * In most cases this trick of storing information before the data works because the first buffer in an - * LwIP IP message contains the space that was used for the Ethernet/IP/UDP headers. However, given the - * current size of the IPPacketInfo structure (40 bytes), it is possible for there to not be enough room - * to store the structure along with the payload in a single packet buffer. In practice, this should only - * happen for extremely large IPv4 packets that arrive without an Ethernet header. - */ - static IPPacketInfo * GetPacketInfo(const chip::System::PacketBufferHandle & aBuffer); + void HandleDataReceived(System::PacketBufferHandle && msg, IPPacketInfo * pktInfo); CHIP_ERROR GetPCB(IPAddressType addrType4); static void LwIPReceiveUDPMessage(void * arg, struct udp_pcb * pcb, struct pbuf * p, const ip_addr_t * addr, u16_t port);