[ros-diffs] [cgutman] 54593: [TCPIP] - Avoid an extra copy operation when receiving packets - Optimize loopback code a bit - Make IP receive independent of the location of the data and continuity of buffers in...

Author: cgutman
Date: Sun Dec  4 22:31:49 2011
New Revision: 54593

URL: http://svn.reactos.org/svn/reactos?rev=54593&view=rev
Log:
[TCPIP]
- Avoid an extra copy operation when receiving packets
- Optimize loopback code a bit
- Make IP receive independent of the location of the data and continuity of buffers in the
NDIS packet for ProtocolReceivePacket support (part 1 of x)

Modified:
    trunk/reactos/drivers/network/tcpip/datalink/lan.c
    trunk/reactos/drivers/network/tcpip/include/receive.h
    trunk/reactos/lib/drivers/ip/network/ip.c
    trunk/reactos/lib/drivers/ip/network/loopback.c
    trunk/reactos/lib/drivers/ip/network/receive.c

Modified: trunk/reactos/drivers/network/tcpip/datalink/lan.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/data…
==============================================================================
--- trunk/reactos/drivers/network/tcpip/datalink/lan.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/network/tcpip/datalink/lan.c [iso-8859-1] Sun Dec  4 22:31:49
2011
@@ -278,22 +278,21 @@
 	 ("Ether Type = %x ContigSize = %d Total = %d\n",
 	  PacketType, IPPacket.ContigSize, IPPacket.TotalSize));
 
+    /* NDIS packet is freed in all of these cases */
     switch (PacketType) {
-    case ETYPE_IPv4:
-    case ETYPE_IPv6:
-	TI_DbgPrint(MID_TRACE,("Received IP Packet\n"));
-	IPReceive(Adapter->Context, &IPPacket);
-	break;
-    case ETYPE_ARP:
-	TI_DbgPrint(MID_TRACE,("Received ARP Packet\n"));
-	ARPReceive(Adapter->Context, &IPPacket);
-    break;
-    default:
-        IPPacket.Free(&IPPacket);
-	break;
-    }
-
-    FreeNdisPacket( Packet );
+        case ETYPE_IPv4:
+        case ETYPE_IPv6:
+            TI_DbgPrint(MID_TRACE,("Received IP Packet\n"));
+            IPReceive(Adapter->Context, &IPPacket);
+            break;
+        case ETYPE_ARP:
+            TI_DbgPrint(MID_TRACE,("Received ARP Packet\n"));
+            ARPReceive(Adapter->Context, &IPPacket);
+            break;
+        default:
+            IPPacket.Free(&IPPacket);
+            break;
+    }
 }
 
 VOID LanSubmitReceiveWork(

Modified: trunk/reactos/drivers/network/tcpip/include/receive.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/incl…
==============================================================================
--- trunk/reactos/drivers/network/tcpip/include/receive.h [iso-8859-1] (original)
+++ trunk/reactos/drivers/network/tcpip/include/receive.h [iso-8859-1] Sun Dec  4 22:31:49
2011
@@ -15,7 +15,8 @@
 /* IP datagram fragment descriptor. Used to store IP datagram fragments */
 typedef struct IP_FRAGMENT {
     LIST_ENTRY ListEntry; /* Entry on list */
-    PVOID Data;           /* Pointer to fragment data */
+    PNDIS_PACKET Packet;  /* NDIS packet containing fragment data */
+    UINT PacketOffset;    /* Offset into NDIS packet where data is */
     UINT Offset;          /* Offset into datagram where this fragment is */
     UINT Size;            /* Size of this fragment */
 } IP_FRAGMENT, *PIP_FRAGMENT;

Modified: trunk/reactos/lib/drivers/ip/network/ip.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/lib/drivers/ip/network/ip.…
==============================================================================
--- trunk/reactos/lib/drivers/ip/network/ip.c [iso-8859-1] (original)
+++ trunk/reactos/lib/drivers/ip/network/ip.c [iso-8859-1] Sun Dec  4 22:31:49 2011
@@ -30,14 +30,22 @@
 VOID
 TCPUnregisterInterface(PIP_INTERFACE IF);
 
-VOID DontFreePacket(
+VOID DeinitializePacket(
     PVOID Object)
 /*
- * FUNCTION: Do nothing for when the IPPacket struct is part of another
+ * FUNCTION: Frees buffers attached to the packet
  * ARGUMENTS:
  *     Object = Pointer to an IP packet structure
  */
 {
+    PIP_PACKET IPPacket = Object;
+
+    /* Detect double free */
+    ASSERT(IPPacket->Type != 0xFF);
+    IPPacket->Type = 0xFF;
+
+    if (IPPacket->NdisPacket != NULL)
+        FreeNdisPacket(IPPacket->NdisPacket);
 }
 
 VOID FreeIF(
@@ -62,10 +70,9 @@
  *     Pointer to the created IP packet. NULL if there was not enough free resources.
  */
 {
-    /* FIXME: Is this needed? */
     RtlZeroMemory(IPPacket, sizeof(IP_PACKET));
 
-    IPPacket->Free     = DontFreePacket;
+    IPPacket->Free     = DeinitializePacket;
     IPPacket->Type     = Type;
 
     return IPPacket;

Modified: trunk/reactos/lib/drivers/ip/network/loopback.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/lib/drivers/ip/network/loo…
==============================================================================
--- trunk/reactos/lib/drivers/ip/network/loopback.c [iso-8859-1] (original)
+++ trunk/reactos/lib/drivers/ip/network/loopback.c [iso-8859-1] Sun Dec  4 22:31:49 2011
@@ -17,10 +17,10 @@
 {
   PIP_PACKET IPPacket = Context;
 
+  /* IPReceive() takes care of the NDIS packet */
   IPReceive(Loopback, IPPacket);
-  FreeNdisPacket(IPPacket->NdisPacket);
 
-  ExFreePool(Context);
+  ExFreePool(IPPacket);
 }
 
 VOID LoopTransmit(
@@ -43,9 +43,8 @@
     UINT PacketLength;
     PNDIS_PACKET XmitPacket;
     NDIS_STATUS NdisStatus;
-    IP_PACKET IPPacket;
+    PIP_PACKET IPPacket;
     PNDIS_BUFFER NdisBuffer;
-    PVOID WorkerBuffer;
 
     ASSERT_KM_POINTER(NdisPacket);
     ASSERT_KM_POINTER(PC(NdisPacket));
@@ -59,24 +58,23 @@
         ( &XmitPacket, PacketBuffer, PacketLength );
 
     if( NT_SUCCESS(NdisStatus) ) {
-        IPInitializePacket(&IPPacket, 0);
-		
-        IPPacket.NdisPacket = XmitPacket;
-		
-        NdisGetFirstBufferFromPacket(XmitPacket,
-                                     &NdisBuffer,
-                                     &IPPacket.Header,
-                                     &IPPacket.ContigSize,
-                                     &IPPacket.TotalSize);
+        IPPacket = ExAllocatePool(NonPagedPool, sizeof(IP_PACKET));
+        if (IPPacket)
+        {
+            IPInitializePacket(IPPacket, 0);
 
-        
-        WorkerBuffer = ExAllocatePool(NonPagedPool, sizeof(IPPacket));
-        if (WorkerBuffer)
-        {
-            RtlCopyMemory(WorkerBuffer, &IPPacket, sizeof(IPPacket));
-            if (!ChewCreate(LoopPassiveWorker, WorkerBuffer))
+            IPPacket->NdisPacket = XmitPacket;
+
+            NdisGetFirstBufferFromPacket(XmitPacket,
+                                         &NdisBuffer,
+                                         &IPPacket->Header,
+                                         &IPPacket->ContigSize,
+                                         &IPPacket->TotalSize);
+
+            if (!ChewCreate(LoopPassiveWorker, IPPacket))
             {
-                ExFreePool(WorkerBuffer);
+                IPPacket->Free(IPPacket);
+                ExFreePool(IPPacket);
                 NdisStatus = NDIS_STATUS_RESOURCES;
             }
         }

Modified: trunk/reactos/lib/drivers/ip/network/receive.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/lib/drivers/ip/network/rec…
==============================================================================
--- trunk/reactos/lib/drivers/ip/network/receive.c [iso-8859-1] (original)
+++ trunk/reactos/lib/drivers/ip/network/receive.c [iso-8859-1] Sun Dec  4 22:31:49 2011
@@ -89,10 +89,10 @@
     /* Unlink it from the list */
     RemoveEntryList(CurrentEntry);
 
-    TI_DbgPrint(DEBUG_IP, ("Freeing fragment data at (0x%X).\n",
CurrentF->Data));
+    TI_DbgPrint(DEBUG_IP, ("Freeing fragment packet at (0x%X).\n",
CurrentF->Packet));
 
     /* Free the fragment data buffer */
-    ExFreePoolWithTag(CurrentF->Data, FRAGMENT_DATA_TAG);
+    FreeNdisPacket(CurrentF->Packet);
 
     TI_DbgPrint(DEBUG_IP, ("Freeing fragment at (0x%X).\n", CurrentF));
 
@@ -186,20 +186,16 @@
  */
 {
   PLIST_ENTRY CurrentEntry;
-  PIP_FRAGMENT Current;
-  PVOID Data;
+  PIP_FRAGMENT Fragment;
+  PCHAR Data;
 
   TI_DbgPrint(DEBUG_IP, ("Reassembling datagram from IPDR at (0x%X).\n",
IPDR));
   TI_DbgPrint(DEBUG_IP, ("IPDR->HeaderSize = %d\n", IPDR->HeaderSize));
   TI_DbgPrint(DEBUG_IP, ("IPDR->DataSize = %d\n", IPDR->DataSize));
 
-  TI_DbgPrint(DEBUG_IP, ("Fragment header:\n"));
-  //OskitDumpBuffer((PCHAR)IPDR->IPv4Header, IPDR->HeaderSize);
-
   IPPacket->TotalSize  = IPDR->HeaderSize + IPDR->DataSize;
   IPPacket->ContigSize = IPPacket->TotalSize;
   IPPacket->HeaderSize = IPDR->HeaderSize;
-  /*IPPacket->Position   = IPDR->HeaderSize;*/
 
   RtlCopyMemory(&IPPacket->SrcAddr, &IPDR->SrcAddr, sizeof(IP_ADDRESS));
   RtlCopyMemory(&IPPacket->DstAddr, &IPDR->DstAddr, sizeof(IP_ADDRESS));
@@ -221,15 +217,14 @@
   /* Copy data from all fragments into buffer */
   CurrentEntry = IPDR->FragmentListHead.Flink;
   while (CurrentEntry != &IPDR->FragmentListHead) {
-    Current = CONTAINING_RECORD(CurrentEntry, IP_FRAGMENT, ListEntry);
-
-    TI_DbgPrint(DEBUG_IP, ("Copying (%d) bytes of fragment data from (0x%X) to
offset (%d).\n",
-      Current->Size, Data, Current->Offset));
-    /* Copy fragment data to the destination buffer at the correct offset */
-    RtlCopyMemory((PVOID)((ULONG_PTR)Data + Current->Offset),
-		  Current->Data,
-		  Current->Size);
-    //OskitDumpBuffer( Data, Current->Offset + Current->Size );
+    Fragment = CONTAINING_RECORD(CurrentEntry, IP_FRAGMENT, ListEntry);
+
+    /* Copy fragment data into datagram buffer */
+    CopyPacketToBuffer(Data + Fragment->Offset,
+                       Fragment->Packet,
+                       Fragment->PacketOffset,
+                       Fragment->Size);
+
     CurrentEntry = CurrentEntry->Flink;
   }
 
@@ -410,24 +405,12 @@
     TI_DbgPrint(DEBUG_IP, ("Fragment descriptor allocated at (0x%X).\n",
Fragment));
 
     Fragment->Size = IPPacket->TotalSize - IPPacket->HeaderSize;
-    Fragment->Data = ExAllocatePoolWithTag(NonPagedPool, Fragment->Size,
FRAGMENT_DATA_TAG);
-    if (!Fragment->Data) {
-      /* We don't have the resources to process this packet, discard it */
-      ExFreeToNPagedLookasideList(&IPFragmentList, Fragment);
-      Cleanup(&IPDR->Lock, OldIrql, IPDR);
-      return;
-    }
-
-    /* Position here is an offset from the NdisPacket start, not the header */
-    TI_DbgPrint(DEBUG_IP, ("Fragment data buffer allocated at (0x%X)  Size (%d) Pos
(%d).\n",
-			   Fragment->Data, Fragment->Size, IPPacket->Position));
-
-    /* Copy datagram data into fragment buffer */
-    CopyPacketToBuffer(Fragment->Data,
-		       IPPacket->NdisPacket,
-		       IPPacket->HeaderSize,
-		       Fragment->Size);
+    Fragment->Packet = IPPacket->NdisPacket;
+    Fragment->PacketOffset = IPPacket->Position + IPPacket->HeaderSize;
     Fragment->Offset = FragFirst;
+
+    /* Disassociate the NDIS packet so it isn't freed upon return from IPReceive()
*/
+    IPPacket->NdisPacket = NULL;
 
     /* If this is the last fragment, compute and save the datagram data size */
     if (!MoreFragments)
@@ -581,7 +564,6 @@
     AddrInitIPv4(&IPPacket->SrcAddr,
((PIPv4_HEADER)IPPacket->Header)->SrcAddr);
     AddrInitIPv4(&IPPacket->DstAddr,
((PIPv4_HEADER)IPPacket->Header)->DstAddr);
 
-    IPPacket->Position += IPPacket->HeaderSize;
     IPPacket->Data     = (PVOID)((ULONG_PTR)IPPacket->Header +
IPPacket->HeaderSize);
 
     TI_DbgPrint(MID_TRACE,("IPPacket->Position = %d\n",