PacketBuffer: Make discontiguous LwIP pbuf adoptable (#25679)

project-chip · Aug 29, 2023 · 1140075 · 1140075
1 parent 5d4283c
commit 1140075
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 45 deletions.
diff --git a/src/inet/UDPEndPointImplLwIP.cpp b/src/inet/UDPEndPointImplLwIP.cpp
@@ -59,22 +59,6 @@ static_assert(LWIP_VERSION_MAJOR > 1, "CHIP requires LwIP 2.0 or later");
 #undef HAVE_IPV6_MULTICAST
 #endif
 
-#if (LWIP_VERSION_MAJOR == 2) && (LWIP_VERSION_MINOR == 0)
-#define PBUF_STRUCT_DATA_CONTIGUOUS(pbuf) (pbuf)->type == PBUF_RAM || (pbuf)->type == PBUF_POOL
-#else // (LWIP_VERSION_MAJOR == 2) && (LWIP_VERSION_MINOR == 0)
-#define PBUF_STRUCT_DATA_CONTIGUOUS(pbuf) (pbuf)->type_internal & PBUF_TYPE_FLAG_STRUCT_DATA_CONTIGUOUS
-#endif // (LWIP_VERSION_MAJOR == 2) && (LWIP_VERSION_MINOR == 0)
-
-namespace chip {
-namespace Platform {
-template <>
-struct Deleter<struct pbuf>
-{
-    void operator()(struct pbuf * p) { pbuf_free(p); }
-};
-} // namespace Platform
-} // namespace chip
-
 namespace chip {
 namespace Inet {
 
@@ -381,9 +365,7 @@ CHIP_ERROR UDPEndPointImplLwIP::GetPCB(IPAddressType addrType)
 void UDPEndPointImplLwIP::LwIPReceiveUDPMessage(void * arg, struct udp_pcb * pcb, struct pbuf * p, const ip_addr_t * addr,
                                                 u16_t port)
 {
-    Platform::UniquePtr<struct pbuf> pbufFreeGuard(p);
     UDPEndPointImplLwIP * ep = static_cast<UDPEndPointImplLwIP *>(arg);
-    System::PacketBufferHandle buf;
     if (ep->mState == State::kClosed)
     {
         return;
@@ -396,37 +378,22 @@ void UDPEndPointImplLwIP::LwIPReceiveUDPMessage(void * arg, struct udp_pcb * pcb
         return;
     }
 
-    if (PBUF_STRUCT_DATA_CONTIGUOUS(p))
+    System::PacketBufferHandle buf = System::PacketBufferHandle::Adopt(p);
+    if (buf->HasChainedBuffer())
     {
-        buf = System::PacketBufferHandle::Adopt(p);
-        // Release pbufFreeGuard since the buf has the ownership of the pbuf.
-        pbufFreeGuard.release();
-        if (buf->HasChainedBuffer())
-        {
-            buf->CompactHead();
-        }
-        if (buf->HasChainedBuffer())
-        {
-            // Have to allocate a new big-enough buffer and copy.
-            uint16_t messageSize            = buf->TotalLength();
-            System::PacketBufferHandle copy = System::PacketBufferHandle::New(messageSize, 0);
-            if (copy.IsNull() || buf->Read(copy->Start(), messageSize) != CHIP_NO_ERROR)
-            {
-                ChipLogError(Inet, "No memory to flatten incoming packet buffer chain of size %u", buf->TotalLength());
-                return;
-            }
-            buf = std::move(copy);
-        }
+        buf->CompactHead();
     }
-    else
+    if (buf->HasChainedBuffer())
     {
-        buf = System::PacketBufferHandle::New(p->tot_len, 0);
-        if (buf.IsNull() || pbuf_copy_partial(p, buf->Start(), p->tot_len, 0) != p->tot_len)
+        // Have to allocate a new big-enough buffer and copy.
+        uint16_t messageSize            = buf->TotalLength();
+        System::PacketBufferHandle copy = System::PacketBufferHandle::New(messageSize, 0);
+        if (copy.IsNull() || buf->Read(copy->Start(), messageSize) != CHIP_NO_ERROR)
         {
-            ChipLogError(Inet, "Cannot copy received pbuf of size %u", p->tot_len);
+            ChipLogError(Inet, "No memory to flatten incoming packet buffer chain of size %u", buf->TotalLength());
             return;
         }
-        buf->SetDataLength(p->tot_len);
+        buf = std::move(copy);
     }
 
     pktInfo->SrcAddress  = IPAddress(*addr);

diff --git a/src/system/SystemPacketBuffer.cpp b/src/system/SystemPacketBuffer.cpp
@@ -51,6 +51,11 @@
 #if CHIP_SYSTEM_CONFIG_USE_LWIP
 #include <lwip/mem.h>
 #include <lwip/pbuf.h>
+#if LWIP_VERSION_MAJOR == 2 && LWIP_VERSION_MINOR < 1
+#define PBUF_STRUCT_DATA_CONTIGUOUS(pbuf) (pbuf)->type == PBUF_RAM || (pbuf)->type == PBUF_POOL
+#else
+#define PBUF_STRUCT_DATA_CONTIGUOUS(pbuf) (pbuf)->type_internal & PBUF_TYPE_FLAG_STRUCT_DATA_CONTIGUOUS
+#endif
 #endif // CHIP_SYSTEM_CONFIG_USE_LWIP
 
 #if CHIP_SYSTEM_PACKETBUFFER_FROM_CHIP_HEAP
@@ -194,7 +199,7 @@ void PacketBufferHandle::InternalRightSize()
 void PacketBuffer::SetStart(uint8_t * aNewStart)
 {
     uint8_t * const kStart = ReserveStart();
-    uint8_t * const kEnd   = kStart + this->AllocSize();
+    uint8_t * const kEnd   = this->Start() + this->MaxDataLength();
 
     if (aNewStart < kStart)
         aNewStart = kStart;
@@ -236,6 +241,12 @@ void PacketBuffer::SetDataLength(uint16_t aNewLen, PacketBuffer * aChainHead)
 
 uint16_t PacketBuffer::MaxDataLength() const
 {
+#if CHIP_SYSTEM_CONFIG_USE_LWIP
+    if (!(PBUF_STRUCT_DATA_CONTIGUOUS(this)))
+    {
+        return DataLength();
+    }
+#endif
     return static_cast<uint16_t>(AllocSize() - ReservedSize());
 }
 
@@ -253,11 +264,23 @@ uint16_t PacketBuffer::ReservedSize() const
 
 uint8_t * PacketBuffer::ReserveStart()
 {
+#if CHIP_SYSTEM_CONFIG_USE_LWIP
+    if (!(PBUF_STRUCT_DATA_CONTIGUOUS(this)))
+    {
+        return reinterpret_cast<uint8_t *>(this->Start());
+    }
+#endif
     return reinterpret_cast<uint8_t *>(this) + kStructureSize;
 }
 
 const uint8_t * PacketBuffer::ReserveStart() const
 {
+#if CHIP_SYSTEM_CONFIG_USE_LWIP
+    if (!(PBUF_STRUCT_DATA_CONTIGUOUS(this)))
+    {
+        return reinterpret_cast<const uint8_t *>(this->Start());
+    }
+#endif
     return reinterpret_cast<const uint8_t *>(this) + kStructureSize;
 }
 
@@ -400,7 +423,12 @@ bool PacketBuffer::EnsureReservedSize(uint16_t aReservedSize)
 
     if ((aReservedSize + this->len) > this->AllocSize())
         return false;
-
+#if CHIP_SYSTEM_CONFIG_USE_LWIP
+    if (!(PBUF_STRUCT_DATA_CONTIGUOUS(this)) && aReservedSize > 0)
+    {
+        return false;
+    }
+#endif
     // Cast is safe because aReservedSize > kCurrentReservedSize.
     const uint16_t kMoveLength = static_cast<uint16_t>(aReservedSize - kCurrentReservedSize);
     memmove(static_cast<uint8_t *>(this->payload) + kMoveLength, this->payload, this->len);