[ros-diffs] [cgutman] 43048: - Rework our aging neighbor cache system that never quite worked correctly - Fixes several memory leaks - Fix the event timer and NCE timeouts - We now keep our neighbor cache updated and remove stale entries

cgutman at svn.reactos.org cgutman at svn.reactos.org
Mon Sep 14 06:20:05 CEST 2009


Author: cgutman
Date: Mon Sep 14 06:20:05 2009
New Revision: 43048

URL: http://svn.reactos.org/svn/reactos?rev=43048&view=rev
Log:
 - Rework our aging neighbor cache system that never quite worked correctly
 - Fixes several memory leaks
 - Fix the event timer and NCE timeouts
 - We now keep our neighbor cache updated and remove stale entries

Modified:
    trunk/reactos/drivers/network/tcpip/include/neighbor.h
    trunk/reactos/lib/drivers/ip/network/arp.c
    trunk/reactos/lib/drivers/ip/network/ip.c
    trunk/reactos/lib/drivers/ip/network/neighbor.c

Modified: trunk/reactos/drivers/network/tcpip/include/neighbor.h
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/include/neighbor.h?rev=43048&r1=43047&r2=43048&view=diff
==============================================================================
--- trunk/reactos/drivers/network/tcpip/include/neighbor.h [iso-8859-1] (original)
+++ trunk/reactos/drivers/network/tcpip/include/neighbor.h [iso-8859-1] Mon Sep 14 06:20:05 2009
@@ -29,7 +29,6 @@
 typedef struct NEIGHBOR_CACHE_ENTRY {
     DEFINE_TAG
     struct NEIGHBOR_CACHE_ENTRY *Next;  /* Pointer to next entry */
-    struct NEIGHBOR_CACHE_TABLE *Table; /* Pointer to table */
     UCHAR State;                        /* State of NCE */
     UINT EventTimer;                    /* Ticks since last event */
     UINT EventCount;                    /* Number of events */
@@ -41,28 +40,18 @@
 } NEIGHBOR_CACHE_ENTRY, *PNEIGHBOR_CACHE_ENTRY;
 
 /* NCE states */
-#define NUD_NONE       0x00
 #define NUD_INCOMPLETE 0x01
 #define NUD_REACHABLE  0x02
-#define NUD_STALE      0x04
-#define NUD_DELAY      0x08
-#define NUD_PROBE      0x10
-#define NUD_FAILED     0x20
-#define NUD_NOARP      0x40
-#define NUD_PERMANENT  0x80
+#define NUD_PERMANENT  0x04
 
-#define NUD_IN_TIMER  (NUD_INCOMPLETE | NUD_DELAY | NUD_PROBE)
-#define NUD_VALID     (NUD_REACHABLE | NUD_NOARP | NUD_STALE | NUD_DELAY | \
-                       NUD_PROBE | NUD_PERMANENT)
-#define NUD_CONNECTED (NUD_PERMANENT | NUD_NOARP | NUD_REACHABLE)
+#define NUD_BROADCAST (NUD_PERMANENT | NUD_REACHABLE)
+#define NUD_LOCAL (NUD_PERMANENT | NUD_REACHABLE)
 
+/* Number of seconds before the NCE times out */
+#define ARP_TIMEOUT 30
 
-/* Maximum number of retransmissions of multicast solicits */
-#define MAX_MULTICAST_SOLICIT 3 /* 3 transmissions */
-
-/* Number of ticks between address resolution messages */
-#define RETRANS_TIMER IP_TICKS_SECOND /* One second */
-
+/* Number of seconds between ARP transmissions */
+#define ARP_RATE 10
 
 extern NEIGHBOR_CACHE_TABLE NeighborCache[NB_HASHMASK + 1];
 
@@ -84,7 +73,8 @@
     PIP_ADDRESS Address,
     PVOID LinkAddress,
     UINT LinkAddressLength,
-    UCHAR Type);
+    UCHAR Type,
+    UINT EventTimer);
 
 VOID NBUpdateNeighbor(
     PNEIGHBOR_CACHE_ENTRY NCE,

Modified: trunk/reactos/lib/drivers/ip/network/arp.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/drivers/ip/network/arp.c?rev=43048&r1=43047&r2=43048&view=diff
==============================================================================
--- trunk/reactos/lib/drivers/ip/network/arp.c [iso-8859-1] (original)
+++ trunk/reactos/lib/drivers/ip/network/arp.c [iso-8859-1] Mon Sep 14 06:20:05 2009
@@ -210,6 +210,7 @@
     /* Check if we know the sender */
 
     AddrInitIPv4(&Address, *((PULONG)SenderProtoAddress));
+
     NCE = NBLocateNeighbor(&Address);
     if (NCE) {
         /* We know the sender. Update the hardware address
@@ -220,7 +221,7 @@
            may want to communicate with us soon, so add his address
            to our address cache */
         NCE = NBAddNeighbor(Interface, &Address, SenderHWAddress,
-            Header->HWAddrLen, NUD_REACHABLE);
+            Header->HWAddrLen, NUD_REACHABLE, ARP_TIMEOUT);
     }
 
     if (Header->Opcode != ARP_OPCODE_REQUEST)

Modified: trunk/reactos/lib/drivers/ip/network/ip.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/drivers/ip/network/ip.c?rev=43048&r1=43047&r2=43048&view=diff
==============================================================================
--- trunk/reactos/lib/drivers/ip/network/ip.c [iso-8859-1] (original)
+++ trunk/reactos/lib/drivers/ip/network/ip.c [iso-8859-1] Mon Sep 14 06:20:05 2009
@@ -212,7 +212,7 @@
     /* Add a permanent neighbor for this NTE */
     NCE = NBAddNeighbor(IF, &IF->Unicast,
 			IF->Address, IF->AddressLength,
-			NUD_PERMANENT);
+			NUD_LOCAL, 0);
     if (!NCE) {
 	TI_DbgPrint(MIN_TRACE, ("Could not create NCE.\n"));
         return;

Modified: trunk/reactos/lib/drivers/ip/network/neighbor.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/drivers/ip/network/neighbor.c?rev=43048&r1=43047&r2=43048&view=diff
==============================================================================
--- trunk/reactos/lib/drivers/ip/network/neighbor.c [iso-8859-1] (original)
+++ trunk/reactos/lib/drivers/ip/network/neighbor.c [iso-8859-1] Mon Sep 14 06:20:05 2009
@@ -30,8 +30,7 @@
     PNEIGHBOR_PACKET Packet;
     UINT HashValue;
 
-    if(!(NCE->State & NUD_CONNECTED))
-       return;
+    ASSERT(NCE->State & NUD_REACHABLE);
 
     HashValue  = *(PULONG)(&NCE->Address.Address);
     HashValue ^= HashValue >> 16;
@@ -89,58 +88,6 @@
     }
 }
 
-VOID NCETimeout(
-  PNEIGHBOR_CACHE_ENTRY NCE)
-/*
- * FUNCTION: Neighbor cache entry timeout handler
- * NOTES:
- *   The neighbor cache lock must be held
- */
-{
-    TI_DbgPrint(DEBUG_NCACHE, ("Called. NCE (0x%X).\n", NCE));
-    TI_DbgPrint(DEBUG_NCACHE, ("NCE->State is (0x%X).\n", NCE->State));
-
-    switch (NCE->State)
-    {
-    case NUD_INCOMPLETE:
-        /* Retransmission timer expired */
-        if (NCE->EventCount++ > MAX_MULTICAST_SOLICIT)
-	{
-            /* We have retransmitted too many times */
-
-            /* Calling IPSendComplete with cache lock held is not
-	       a great thing to do. We don't get here very often
-	       so maybe it's not that big a problem */
-
-            /* Flush packet queue */
-	    NBFlushPacketQueue( NCE, NDIS_STATUS_REQUEST_ABORTED );
-            NCE->EventCount = 0;
-	}
-        else
-	{
-            /* Retransmit request */
-            NBSendSolicit(NCE);
-	}
-        break;
-
-    case NUD_DELAY:
-        /* FIXME: Delayed state */
-        TI_DbgPrint(DEBUG_NCACHE, ("NCE delay state.\n"));
-        break;
-
-    case NUD_PROBE:
-        /* FIXME: Probe state */
-        TI_DbgPrint(DEBUG_NCACHE, ("NCE probe state.\n"));
-        break;
-
-    default:
-        /* Should not happen since the event timer is not used in the other states */
-        TI_DbgPrint(MIN_TRACE, ("Invalid NCE state (%d).\n", NCE->State));
-        break;
-    }
-}
-
-
 VOID NBTimeout(VOID)
 /*
  * FUNCTION: Neighbor address cache timeout handler
@@ -151,21 +98,33 @@
 {
     UINT i;
     KIRQL OldIrql;
+    PNEIGHBOR_CACHE_ENTRY *PrevNCE;
     PNEIGHBOR_CACHE_ENTRY NCE;
 
     for (i = 0; i <= NB_HASHMASK; i++) {
         TcpipAcquireSpinLock(&NeighborCache[i].Lock, &OldIrql);
 
-        for (NCE = NeighborCache[i].Cache;
-            NCE != NULL; NCE = NCE->Next) {
+        for (PrevNCE = &NeighborCache[i].Cache;
+             (NCE = *PrevNCE) != NULL;) {
             /* Check if event timer is running */
             if (NCE->EventTimer > 0)  {
-                NCE->EventTimer--;
-                if (NCE->EventTimer == 0) {
-                    /* Call timeout handler for NCE */
-                    NCETimeout(NCE);
+                NCE->EventCount++;
+                if (NCE->EventCount % ARP_RATE == 0)
+                    NBSendSolicit(NCE);
+                if (NCE->EventTimer - NCE->EventCount == 0) {
+                    ASSERT(!(NCE->State & NUD_PERMANENT));
+
+                    /* Flush packet queue */
+                    NBFlushPacketQueue( NCE, NDIS_STATUS_REQUEST_ABORTED );
+
+                    *PrevNCE = NCE->Next;
+
+                    exFreePool(NCE);
+
+                    continue;
                 }
             }
+            PrevNCE = &NCE->Next;
         }
 
         TcpipReleaseSpinLock(&NeighborCache[i].Lock, OldIrql);
@@ -211,6 +170,8 @@
           /* Flush wait queue */
 	  NBFlushPacketQueue( CurNCE, NDIS_STATUS_NOT_ACCEPTED );
 
+          exFreePool(CurNCE);
+
 	  CurNCE = NextNCE;
       }
 
@@ -233,18 +194,7 @@
 {
     TI_DbgPrint(DEBUG_NCACHE, ("Called. NCE (0x%X).\n", NCE));
 
-    if (NCE->State & NUD_INCOMPLETE)
-    {
-	/* This is the first solicitation of this neighbor. Broadcast
-	   a request for the neighbor */
-
-	TI_DbgPrint(MID_TRACE,("NCE: %x\n", NCE));
-
-	ARPTransmit(&NCE->Address, NCE->Interface);
-    } else {
-	/* FIXME: Unicast solicitation since we have a cached address */
-	TI_DbgPrint(MIN_TRACE, ("Uninplemented unicast solicitation.\n"));
-    }
+    ARPTransmit(&NCE->Address, NCE->Interface);
 }
 
 PNEIGHBOR_CACHE_ENTRY NBAddNeighbor(
@@ -252,7 +202,8 @@
   PIP_ADDRESS Address,
   PVOID LinkAddress,
   UINT LinkAddressLength,
-  UCHAR State)
+  UCHAR State,
+  UINT EventTimer)
 /*
  * FUNCTION: Adds a neighbor to the neighbor cache
  * ARGUMENTS:
@@ -298,7 +249,8 @@
   else
       memset(NCE->LinkAddress, 0xff, LinkAddressLength);
   NCE->State = State;
-  NCE->EventTimer = 0; /* Not in use */
+  NCE->EventTimer = EventTimer;
+  NCE->EventCount = 0;
   InitializeListHead( &NCE->PacketQueue );
 
   TI_DbgPrint(MID_TRACE,("NCE: %x\n", NCE));
@@ -309,8 +261,6 @@
   HashValue ^= HashValue >> 4;
   HashValue &= NB_HASHMASK;
 
-  NCE->Table = &NeighborCache[HashValue];
-
   TcpipAcquireSpinLock(&NeighborCache[HashValue].Lock, &OldIrql);
 
   NCE->Next = NeighborCache[HashValue].Cache;
@@ -350,10 +300,11 @@
 
     RtlCopyMemory(NCE->LinkAddress, LinkAddress, NCE->LinkAddressLength);
     NCE->State = State;
+    NCE->EventCount = 0;
 
     TcpipReleaseSpinLock(&NeighborCache[HashValue].Lock, OldIrql);
 
-    if( NCE->State & NUD_CONNECTED )
+    if( NCE->State & NUD_REACHABLE )
 	NBSendPackets( NCE );
 }
 
@@ -425,16 +376,12 @@
             AddrIsUnspecified(Address) ) {
             TI_DbgPrint(MID_TRACE,("Packet targeted at broadcast addr\n"));
             NCE = NBAddNeighbor(Interface, Address, NULL,
-                                Interface->AddressLength, NUD_CONNECTED);
-            if (!NCE) return NULL;
-            NCE->EventTimer = 0;
-            NCE->EventCount = 0;
+                                Interface->AddressLength, NUD_BROADCAST, 0);
         } else {
             NCE = NBAddNeighbor(Interface, Address, NULL,
-                                Interface->AddressLength, NUD_INCOMPLETE);
+                                Interface->AddressLength, NUD_INCOMPLETE, ARP_TIMEOUT);
             if (!NCE) return NULL;
-            NCE->EventTimer = 1;
-            NCE->EventCount = 0;
+            NBSendSolicit(NCE);
         }
     }
 
@@ -483,12 +430,11 @@
 
   TcpipReleaseSpinLock(&NeighborCache[HashValue].Lock, OldIrql);
 
-  if( NCE->State & NUD_CONNECTED )
+  if( NCE->State & NUD_REACHABLE )
       NBSendPackets( NCE );
 
   return TRUE;
 }
-
 
 VOID NBRemoveNeighbor(
   PNEIGHBOR_CACHE_ENTRY NCE)
@@ -559,10 +505,8 @@
 		  ArpTable[Size].LogAddr = CurNCE->Address.Address.IPv4Address;
 		  if( CurNCE->State & NUD_PERMANENT )
 		      ArpTable[Size].Type = ARP_ENTRY_STATIC;
-		  else if( CurNCE->State & NUD_CONNECTED )
+		  else if( CurNCE->State & NUD_REACHABLE )
 		      ArpTable[Size].Type = ARP_ENTRY_DYNAMIC;
-		  else if( !(CurNCE->State & NUD_VALID) )
-		      ArpTable[Size].Type = ARP_ENTRY_INVALID;
 		  else
 		      ArpTable[Size].Type = ARP_ENTRY_OTHER;
 	      }




More information about the Ros-diffs mailing list