[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
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/inclu... ============================================================================== --- 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.... ============================================================================== --- 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... ============================================================================== --- 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/neig... ============================================================================== --- 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; }
-
cgutman＠svn.reactos.org