|
|
9/2/1998 JosephJ As part of implementing promiscuous mode multicast, we need to make some changes to the mars server.
Some observations: * RCF2022 has special support for routers wanting to monitor blocks of NONOVERLAPPING addresses. It handles these blocks somewhat independantly from individual group joins: a block can overlap one or more individual group joins. On leaving the block, the mars server maintains the individual group joins.
Changes/fixes required: 1. MarsPunchHoles needs to be written to change time complexity from O(number of addresses in block) to O(number of mcs address-ranges + number of individual groups).
Currently, it runs through all 268 million possible addresses when processing a join of the entire class-D address space!
Fix is jump to the end of the hole (if in a hole) or to the beginning of the next hole (if not in a hole).
Also, it was enumerating the list of groups wrongly -- using pGroup->Next instead of pGroup->NextGroup.
2. MarsAddClusterMemberTogroups: When processing an join of the entire class-D space, it needs to check if the member has already joined the entire space, in which case it should treat the join request as a duplicate (send it back on the private vc). Currently it will treat it as a new entry and hence will send it on the cluster-control-vc. We would end up with duplicate entries. MarsDelClusterMemberFromGroups: Similarly, when when processing a leave of the entire class-D space when in fact the member has already left, we must make sure that we reflect it privately.
3. MARS_MULTI processing: MarsHandleRequest had to be modified to also return the hw addresses of members monitoring the entire class-D address space (but also make sure that it doesn't send duplicate hw addresses if the member is also minitoring the specific group address).
9/14/1998 JosephJ
There are reports of bugchecks during boot up. One is bug# 212412 (ATMARPS: MarsSendRedirect referenced past end of pool). The stack was: f7cfbcdc f276ba36 00000000 fa502000 00000000 ntoskrnl!KiTrap0E+0xc3 f7cfbd80 f276cc04 80a11828 f92cff18 00000000 atmarps!MarsSendRedirect+0xbe
MarsSendRedirect is a timer callback function. It it creates and sends a redirect packet containing all the configured and registered addresses. However it doesn't claim any lock. So it's possible that it's in the middle of doing this when registered addresses are added (they are added on ArpSCoRequestComplete callbacks.
So we fix this by adding a lock to the interface when reading/modifying these addresses: in MarsSendRedirect and in ArpSCoRequestComplete. Checked in today.
10/24/1998 JosephJ
Added statistics -- created ARP_SERVER_STATISTICS and MARS_SERVER_STATISTICS (iocto.h, ioctl.c, also the added functionality to atmarp.exe to dump this stuff).
10/25/1998 JosephJ Added support to actually fill out the statistics: ndis.c, mars.c, arps.c, etc.
Some clarifications...
Mcast Joins: failed doesn't include dups.
Mcast requests: MCMesh requests include responses to those nodes that are in promiscuous mode.
Discarded pkts: usually means pkts discarded due to error/unsupported format or because a resource allocation failure. Also includes pkts discarded because they are not from a cluster member. If the discarded pkt is known to be for a particular task, say a join, then the count of failed joins is also incremented. We do not explicitly keep track of failed mars or arp requests which are ignored (either acked or nacked) -- so if you see the arp request count increment but the ack+nak not incrementing, you can conclude that the request is being ignored.
10/25/1998 JosephJ While implementing the above, found the following existing bugs:
MarsReqThread -- if it gets a pkt with an unrecognized opcode, it simply leaves it in limbo -- it should put it back into the free list! Otherwise we'll soon run out of pkts if we get barraged with bad pkts.
MarsDelClusterMemberFromGroups: it should (a) delete a mars entry when it's list of members goes to NULL -- delete consists of removing it from the hash table and calling ArpSFreeBlock on it. (b) actually free the pGroup, calling FREE_MEM.
checked in 10/28/1998
12/22/1998
Made a change to the following code in ArpSCoRequestComplete: [Old code] pIntF->NumAddressesRegd ++; if (pIntF->NumAddressesRegd < pIntF->NumRegdAddresses) { ... } [New code] We inly increment NumAddressRegd if (pIntF->NumAddressesRegd < pIntF->NumRegdAddresses). We assert otherwise. See 12/22/1998 Note in ArpSCoRequestComplete for details.
2/26/1999 JosephJ Bug: #297656 ATMARPS: Unbinding arp/mars on server A does not unregister the well-known address from server A. Fix: I unregister the registered addresses in shutdowninterface, and block until all the unregistrations complete. I decided to block because the completions will likely be asynchronous and we immediately go on to close the Af and then deallocate the object, so I was not comfortable with not waiting for completions) of the unregistrations.
2/29/1999 JosephJ
Stress hit the following assert with checked atmarpc.sys, 1990: Assert NdisPartyHandle != NULL failed: file mars.c, line 3247
The assert was because the ndispartyhandle was NULL in the context of the dropparty handler.
The crutial observation is the following 2 identical lines of debug spew before the assert: 0:MARS: AddMemberToCC: pVc 868178c8, pMember 87736808, ConnState 2 0:MARS: AddMemberToCC: pVc 868178c8, pMember 87736808, ConnState 2
The 2nd call to NdisAddParty (from MarsAddMemberToClusterControlVc) clobbers the previous value of pMember->NdisPartyHandle on entry and then probably fails (because there's already a party).
Looking for ways that MarsAddMemberToClusterControlVc could be called twice (which is not supposed to happen), the only way I can see this happening consistant with the debug spew, is that an incoming registration-join from a new member (MarsHandleJoin) came at the same time that we were handling post-processing of the initial CC MakeCall complete (ArpSMakeCallComplete, 1195,ndis.c).
There are holes in the way vc and member flags are set and checked that would lead to MarsAddMemberToClusterControlVc being called from both code paths. (The enumeration in line 1185 of ArpSMakeCallComplete is also dangerous in that it could potentially deref a freed pMember, but that's not what happened here).
The following functions have similary dangerous emumeration/assumption that pMember will be valid: MarsAbortAllMembers ArpSDropPartyComplete MarsDelMemberFromClusterControlVc
FIXES to all of the above: 1. Added new function MarsIsValidClusterMember that makes sure a particular member is in the cluster list. 2. This function is called from: MarsDelMemberFromClusterControlVc (which simply returns if invalid). ArpSMakeCallComplete (which stops enumeration if a pMember is invalid). 3. MarsAbortAllMembers fixed to do a safe enumeration of all members. 4. MarsAddMemberToClusterControlVc returns without doing anything if if (MARS_GET_CM_CONN_STATE(pMember) != CM_CONN_IDLE) (check is made AFTER getting the IF lock).
3/3/1999 JosephJ Revisiting MarsAbortAllMembers and DelMembersfromVc -- safe enumeration used.
3/3/1999 JosephJ CM_INVALID use -- fix del member on add-party complete, but be careful on make-call complete (maybe ok if there are no other members at this time).
3/16/1999 JosephJ Actually checked in the following files... ioctl.c v13 fix uninit var mars.c v12 299201 -- various robustness-re mars.h v2 299201 -- various robustness-re ndis.c v16 299201 -- various robustness-re protos.h v9 299201 -- various robustness-re
All of the above are the fixes explained in 2/29 and 3/3 entries above
registry.c v5 ArpSReadAdapterConfig... p -- p (make sure proper default values are in place in case the corresponding call to read registry values fails -- The are two cases: - pConfig->RegAddresses -- this is benign in all cases (because pConfig->NumAllocedRegdAddresses is set to 0 on failure), EXCEPT in ArpSReadAdapterConfiguration, where PrevRegAddresses is freed if it is non NULL. Fix is to initialize pConfig->RegAddresses to NULL before calling NdisReadocnfiguration. - pConfig->pMcsList -- this is not benign. Fix is to init it to NULL before calling NdisReadocnfiguration.
04/20/1999 JosephJ Fix for 327626 Need Rouge ARP Server detection on ARP registration.
1st version of fix: BEFORE registering address, we try to make a call to the address. If it succeeds, we log an event in the event log and fail the initialization.
2nd version of fix: keep call open -- if we get an incoming close, we then try again.
Note: it's possible that if two notes are doing this at the same time they may still both come up, i.e., rogue detection will fail.
05/05/1999 JosephJ 331517 - bugchecks due to wrong VC PacketSizeThe bugchk was triggered by the fact that the mars server got several incoming VCswith very large max packet size (they should all be 9188, but we saw 64008 forseveral and 18200 for one).
Fix (in MarsHandleRequest (mars.c)) is to replace SHORT and USHORT local variablesby ULONG.
05/05/1999 JosephJ rogue ARP server detection contd... ArpSCoAfRegisterNotify -> ArpSOpenAfComplete -> ArpSRegisterSap -> ArpSRegisterSapComplete
ArpSBindAdapter -> ArpSReadAdapterConfiguration -> ArpSQueryAndSetAddresses
ArpSCoRequestComplete
Configured address (pIntF->ConfiguredAddress) filled by GET OID_CO_GET_ADDRESSESregistered addresses (pIntF->RegAddresses[])
Fri 05/14/1999 JosephJ Rogue ARP server detection contd. Things are kicked off on getting a OID_CO_ADDRESS_CHANGE notification. We (as before) query the adapter for the configured address. On completion of this (OID_CO_GET_ADDRESS), we start the process of validating and registering all addresses, by calling ArpSValidateAndSetRegdAddresses.
ArpSValidateAndSetRegdAddresses allocates and initializes pIntF->pRegAddrCtxt (which keeps all context associated with the validation and registration of addresses). A reference is added to pIntF for pRegAddrCtxt. The function then calls ArpSValidateOneRegdAddress.
ArpSValidateOneRegdAddress attempts to initiate the validation & registration of a single address. If there are no addresses left to be processed, it will unlink pIntF->pRegAddrCtxt (deref pIntF and deallocate pRegAddrCtxt). "Validation" consists of making a point-to-point to call to the address, using the same call params as the atmarp client uses. If the call fails, the address is considered "validated". The protocol's context for the VC is pRegAddrCtxt itself.
The next stage happens in the make-call complete handler (ArpSMakeRegAddrCallComplete).
ArpSMakeRegAddrCallComplete: - on successful make call (which is a failed validation), it it immediately closes the call. The close call handler (ArpSCloseRegAddrCallComplete) deltes the vc, increments pRegAddrCtxt->RegAddrIndex and calls ArpSValidateOneRegdAddress (to initiate the validation & registration of the NEXT address, if any).
-- On failed make call (which is a successful validation), it deletes the vc and initiates registration of the address (calls NdisRequest with OID_CO_ADD_ADDRESS). On completion of the OID_CO_ADD_ADDRESS (ArpSCoRequestComplete), we do the following: - on success, copy over the addres to pIntF->RegAddresses[pIntF->NumAddressesRegd] and increment pIntF->NumAddressesRegd ++ - on failure or success, we increment pRegAddrCtxt->RegAddrIndex and call ArpSValidateOneRegdAddress (to initiate the validation & registration of the NEXT address, if any).
A note on the use of pIntF->RegAddresses[...] This array is initialized with all the user-specified addresses read from the registry. As validation proceeds, however, the successfully validated and registered addresses are copied sequentially into this array. If *all* the addresses are successfully validated and registered, then the end values in the array is the same as the initial values. However, if some intermediate addresses are not validated or registered succcessfully, then the end result will be different. In all cases, the first pIntF->NumAddressesRegd entries will contain the registered addresses.
09/30/1999 JosephJ Bug 405851 *RC3SS: ATMARPS: Bugcheck unloading atmarps during shutdown The basic problem is that ArpSIfList contains a pointer to a just-freed pIF. The biggest problem I've found is that INTF_CLOSING state is set only in the ArpSCloseAdapterComplete handler. This means that ArpSStopInterface (which calls on ArpSReferenceIntF which *does* check the INTF_CLOSING flag) can be called multiple times for the same adapter. Also ArpSReferenceIntF is called from some other places. ArpSStopInterface is called from: ArpSShutDown (called from arp's unload handler as WELL as when handling IRP_MJ_SHUTDOWN) ArpSCoRequest(AF closing) and ArpSUnbindAdapter(unbind adapter handler). ArpSStopInterface is NOT idempotent (it expects to be called only once per pIF), but given the above, it CAN becalled multiple times per IF (i.e., because INTF_CLOSING is only set on close adapter complete). One specific problem with this: it assumes it can use pIntF->CleanupEvent. Another problem: ArpSShutDown: it goes through each IF in ArpSIfList, refs it (which would fail if the IF is INTF_CLOSING), release list lock, then calls ArpSStopInterface. It is definately flawed in the way it uses pIF->Next -- pIF->Next could well be gone by the time it gets to it. HOWEVER that's not the problem we're seeing -- we're seeing the case of the ArpSIfList ITSELF being corrupted. FIXES: 1. arps.c ArpSShutdown now refs the pNext pointer -- a little bit of intricate code that makes sure pNext is still around when we need it. 2. We use the INTF_ADAPTER_OPENED flag, set on successful completion of open adapter (in ArpSOpenAdapterComplete). We use this to make sure that NdisCloseAdapter is called only once. NdisCloseAdapter was called in a bunch of places -- now we call ArpSTryCloseAdapter, which checks the INTF_ADAPTER_OPENED flag first. 3. ArpSStopInterface now doesn't clobber pIntF->CleanupEvent -- if it's NON-NULL it simply doesn't wait.
10/07/1999 JosephJ fix for 412018 *RC3SS: ATMARPS bugchecks on unloading if no call manager bound to adapter. On unloading, we were trying to deregister addresses which had never been registered (in fact the AF was never opened). Fix is in DeregisterAllAddresses (ndis.c): it used to check that pIntF->NumAllocedRegdAddresses is non-null ; now it checks that NumAddressesRegd is non-null.
01/06/2000 JosephJ fix for 416301 corrupt ArpSIfList ArpSOpenAfComplete(ndis.c): we were setting a pInfF flag without holding the IF lock (we just had the IF list lock). This was possibly corrupting the other bits in the flag field. This is most likely the cause of the problem. Also got rid of INTF_IN_LIST field, which is not required.
ArpSDereferenceIntF(arps.c): Arvindm added code to make sure two threads don't try to deref the interface to zero at the same time. This is not likely to be the cause of the bug (bug is that the list is corrupted, not that an IF entry was de-allocated prematurely or twice) but is a hole that needs to be fixed.03/30/2000 JosephJ Hit an assert in arps.c when the close-call handler for the validation make call (ArpSCloseRegAddrCallComplete) is called during shutdown -- we've nuked pIntF->NumAllocedRegdAddresses by this time as part of shutdown, so the following assert in the function fails: ASSERT(pRegAddrCtxt->RegAddrIndex < pIntF->NumAllocedRegdAddresses); Fix is to bracket this code by if (!(pIntF->Flags & INTF_STOPPING)).
04/18/2000 JosephJ Removed assert from the code below in ArpSValidateAndSetRegdAddresses
if (pIntF->pRegAddrCtxt != NULL) { ASSERT(FALSE); break; }
This could happen if we get an OID_CO_ADDRESS_CHANGE when we are either processing an earlier one, or are in the process of initializing. We get this case (and hit the assert) during pnp stress ( 1c_reset script against an Olicom 616X) -- Whistler bug#102805
|
