Changeset 29635 in vbox for trunk/src/VBox

Timestamp:

May 18, 2010 1:42:54 PM (15 years ago)

Author:

vboxsync

Message:

SrvIntNetR0.cpp,VBoxNetFlt.c: Adjusted r61623 so it won't crash if the network is already destroyed when the interface is being destroyed. Added paranoia and comments.

Location:

trunk/src/VBox

Files:

• Devices/Network/SrvIntNetR0.cpp (modified) (12 diffs)
• HostDrivers/VBoxNetFlt/VBoxNetFlt.c (modified) (4 diffs)

trunk/src/VBox/Devices/Network/SrvIntNetR0.cpp

@@ -3189 +3189 @@
      * Learn the MAC address of the sender.  No re-learning as the interface
      * user will normally tell us the right MAC address.
+     *
+     * Note! We don't notify the trunk about these mainly because of the
+     *       problematic contexts we might be called in.
      */
     if (RT_UNLIKELY(    pIfSender
@@ -3607 +3610 @@
     if (pNetwork)
     {
-        RTSPINLOCKTMP Tmp = RTSPINLOCKTMP_INITIALIZER;
+        RTSPINLOCKTMP   Tmp    = RTSPINLOCKTMP_INITIALIZER;
+        PINTNETTRUNKIF  pTrunk = NULL;
+
         RTSpinlockAcquireNoInts(pNetwork->hAddrSpinlock, &Tmp);
 
@@ -3615 +3620 @@
                  hIf, &pIf->MacAddr, pMac));
 
+            /* Update the two copies. */
             PINTNETMACTABENTRY pEntry = intnetR0NetworkFindMacAddrEntry(pNetwork, pIf); Assert(pEntry);
             if (RT_LIKELY(pEntry))
@@ -3620 +3626 @@
             pIf->MacAddr        = *pMac;
             pIf->fMacSet        = true;
+
+            /* Grab a busy reference to the trunk so we release the lock before notifying it. */
+            pTrunk = pNetwork->MacTab.pTrunk;
+            if (pTrunk)
+                intnetR0BusyIncTrunk(pTrunk);
         }
 
-        PINTNETTRUNKIF pTrunk = pNetwork->MacTab.pTrunk;
         RTSpinlockReleaseNoInts(pNetwork->hAddrSpinlock, &Tmp);
-        if (pTrunk && pTrunk->pIfPort)
+
+        if (pTrunk)
         {
             Log(("IntNetR0IfSetMacAddress: pfnNotifyMacAddress hIf=%RX32\n", hIf));
-            pTrunk->pIfPort->pfnNotifyMacAddress(pTrunk->pIfPort, hIf, pMac);
+            PINTNETTRUNKIFPORT pIfPort = pTrunk->pIfPort;
+            if (pIfPort)
+                pIfPort->pfnNotifyMacAddress(pIfPort, hIf, pMac);
+            intnetR0BusyDecTrunk(pTrunk);
         }
     }
@@ -3898 +3912 @@
     /*
      * Validate and free the handle.
+     *
+     * We grab the big mutex before we free the handle to avoid any handle
+     * confusion on the trunk.
      */
     PINTNET pIntNet = g_pIntNet;
@@ -3903 +3920 @@
     AssertReturn(pIntNet->u32Magic, VERR_INVALID_MAGIC);
 
+    RTSemMutexRequest(pIntNet->hMtxCreateOpenDestroy, RT_INDEFINITE_WAIT);
+
     PINTNETIF pIf = (PINTNETIF)RTHandleTableFreeWithCtx(pIntNet->hHtIfs, hIf, pSession);
     if (!pIf)
+    {
+        RTSemMutexRelease(pIntNet->hMtxCreateOpenDestroy);
         return VERR_INVALID_HANDLE;
-
-    /* mark the handle as freed so intnetR0IfDestruct won't free it again. */
+    }
+
+    /* Mark the handle as freed so intnetR0IfDestruct won't free it again. */
     ASMAtomicWriteU32(&pIf->hIf, INTNET_HANDLE_INVALID);
 
-    /*
-     * Release the references to the interface object (handle + free lookup).
-     * But signal the event semaphore first so any waiter holding a reference
-     * will wake up too (he'll see hIf == invalid and return correctly).
+    /* Notify the trunk that the interface has been disconnected. */
+    PINTNETNETWORK pNetwork = pIf->pNetwork;
+    PINTNETTRUNKIF pTrunk   = pNetwork ? pNetwork->MacTab.pTrunk : NULL;
+    if (pTrunk && pTrunk->pIfPort)
+        pTrunk->pIfPort->pfnDisconnectInterface(pTrunk->pIfPort, hIf);
+
+    RTSemMutexRelease(pIntNet->hMtxCreateOpenDestroy);
+
+    /*
+     * Signal the event semaphore to wake up any threads in IntNetR0IfWait
+     * and give them a moment to get out and release the interface.
      */
     uint32_t i = pIf->cSleepers;
@@ -3923 +3952 @@
     RTSemEventSignal(pIf->hRecvEvent);
 
+    /*
+     * Release the references to the interface object (handle + free lookup).
+     */
     void *pvObj = pIf->pvObj;
     intnetR0IfRelease(pIf, pSession); /* (RTHandleTableFreeWithCtx) */
@@ -3970 +4002 @@
     ASMAtomicWriteBool(&pIf->fDestroying, true);
 
-    PINTNETNETWORK pNetwork = pIf->pNetwork;
-    PINTNETTRUNKIF pTrunk = pNetwork->MacTab.pTrunk;
-    if (pTrunk && pTrunk->pIfPort)
-    {
-        Log(("intnetR0IfDestruct: pfnDisconnectInterface hIf=%RX32\n", pIf->hIf));
-        pTrunk->pIfPort->pfnDisconnectInterface(pTrunk->pIfPort, pIf->hIf);
-    }
-
     /*
      * Delete the interface handle so the object no longer can be used.
@@ -3993 +4017 @@
      * of cleanup order we might have been orphaned by the network destructor.
      */
+    PINTNETNETWORK pNetwork = pIf->pNetwork;
     if (pNetwork)
     {
@@ -4014 +4039 @@
             }
 
+        PINTNETTRUNKIF pTrunk = pNetwork->MacTab.pTrunk;
+
         RTSpinlockReleaseNoInts(pNetwork->hAddrSpinlock, &Tmp);
+
+        /* If we freed the handle just now, notify the trunk about the
+           interface being destroyed. */
+        if (hIf != INTNET_HANDLE_INVALID && pTrunk && pTrunk->pIfPort)
+            pTrunk->pIfPort->pfnDisconnectInterface(pTrunk->pIfPort, hIf);
 
         /* Wait for the interface to quiesce while we still can. */
@@ -4214 +4246 @@
                     pIf->pNetwork = pNetwork;
 
-                    /** @todo handle failure of pfnConnectInterface */
+                    /*
+                     * Grab a busy reference (paranoia) to the trunk before releaseing
+                     * the spinlock and then notify it about the new interface.
+                     */
                     PINTNETTRUNKIF pTrunk = pNetwork->MacTab.pTrunk;
-                    if (pTrunk && pTrunk->pIfPort)
+                    if (pTrunk)
+                        intnetR0BusyIncTrunk(pTrunk);
+
+                    RTSpinlockReleaseNoInts(pNetwork->hAddrSpinlock, &Tmp);
+
+                    if (pTrunk)
                     {
                         Log(("intnetR0NetworkCreateIf: pfnConnectInterface hIf=%RX32\n", pIf->hIf));
-                        pTrunk->pIfPort->pfnConnectInterface(pTrunk->pIfPort, pIf->hIf);
+                        if (pTrunk->pIfPort)
+                            rc = pTrunk->pIfPort->pfnConnectInterface(pTrunk->pIfPort, pIf->hIf);
+                        intnetR0BusyDecTrunk(pTrunk);
                     }
-
-                    RTSpinlockReleaseNoInts(pNetwork->hAddrSpinlock, &Tmp);
-
-                    *phIf = pIf->hIf;
-                    Log(("intnetR0NetworkCreateIf: returns VINF_SUCCESS *phIf=%RX32 cbSend=%u cbRecv=%u cbBuf=%u\n",
-                         *phIf, pIf->pIntBufDefault->cbSend, pIf->pIntBufDefault->cbRecv, pIf->pIntBufDefault->cbBuf));
-                    return VINF_SUCCESS;
+                    if (RT_SUCCESS(rc))
+                    {
+                        /*
+                         * We're good!
+                         */
+                        *phIf = pIf->hIf;
+                        Log(("intnetR0NetworkCreateIf: returns VINF_SUCCESS *phIf=%RX32 cbSend=%u cbRecv=%u cbBuf=%u\n",
+                             *phIf, pIf->pIntBufDefault->cbSend, pIf->pIntBufDefault->cbRecv, pIf->pIntBufDefault->cbBuf));
+                        return VINF_SUCCESS;
+                    }
                 }
 
@@ -4852 +4897 @@
         intnetR0BusyWait(pNetwork, &pNetwork->MacTab.paEntries[iIf].pIf->cBusy);
 
-    /* Orphan the interfaces (not trunk). */
+    /* Orphan the interfaces (not trunk).  Don't bother with calling
+       pfnDisconnectInterface here since the networking is going away. */
     RTSpinlockAcquireNoInts(pNetwork->hAddrSpinlock, &Tmp);
     while ((iIf = pNetwork->MacTab.cEntries) > 0)

trunk/src/VBox/HostDrivers/VBoxNetFlt/VBoxNetFlt.c

@@ -558 +558 @@
 static DECLCALLBACK(int) vboxNetFltPortConnectInterface(PINTNETTRUNKIFPORT pIfPort,  INTNETIFHANDLE hIf)
 {
-    PVBOXNETFLTINS pThis = IFPORT_2_VBOXNETFLTINS(pIfPort);
-    int rc = VINF_SUCCESS;
+    PVBOXNETFLTINS  pThis = IFPORT_2_VBOXNETFLTINS(pIfPort);
+    int             rc;
 
     /*
@@ -578 +578 @@
  * @copydoc INTNETTRUNKIFPORT::pfnDisconnectInterface
  */
-static DECLCALLBACK(int) vboxNetFltPortDisconnectInterface(PINTNETTRUNKIFPORT pIfPort, INTNETIFHANDLE hIf)
-{
-    PVBOXNETFLTINS pThis = IFPORT_2_VBOXNETFLTINS(pIfPort);
-    int rc = VINF_SUCCESS;
+static DECLCALLBACK(void) vboxNetFltPortDisconnectInterface(PINTNETTRUNKIFPORT pIfPort, INTNETIFHANDLE hIf)
+{
+    PVBOXNETFLTINS  pThis = IFPORT_2_VBOXNETFLTINS(pIfPort);
+    int             rc;
 
     /*
@@ -592 +592 @@
     rc = vboxNetFltPortOsDisconnectInterface(pThis, hIf);
     vboxNetFltRelease(pThis, false /* fBusy */);
-
-    return rc;
+    AssertRC(rc); /** @todo fix vboxNetFltPortOsDisconnectInterface. */
 }
 
@@ -998 +997 @@
     pNew->MyPort.pfnNotifyMacAddress    = vboxNetFltPortNotifyMacAddress;
     pNew->MyPort.pfnConnectInterface    = vboxNetFltPortConnectInterface;
-    pNew->MyPort.pfnDisconnectInterface = vboxNetFltPortDisconnectInterface;    
+    pNew->MyPort.pfnDisconnectInterface = vboxNetFltPortDisconnectInterface;
     pNew->MyPort.u32VersionEnd          = INTNETTRUNKIFPORT_VERSION;
     pNew->pSwitchPort                   = pSwitchPort;