Changeset 31608 in vbox for trunk/src/VBox/Devices/Storage/ISCSIHDDCore.cpp

Timestamp:

Aug 12, 2010 3:48:14 PM (15 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

64735

Message:

iSCSI: Proper handling of lost connections. Reset the state and resend all active tasks after the connection was successfully established again. Handle commands which succeed but have sense data properly

File:

• trunk/src/VBox/Devices/Storage/ISCSIHDDCore.cpp (modified) (33 diffs)

trunk/src/VBox/Devices/Storage/ISCSIHDDCore.cpp

@@ -342 +342 @@
     /** Length of Target2Initiator data buffer. */
     size_t          cbT2IData;
-    /** Length of sense buffer. */
+    /** Length of sense buffer
+     * This contains the number of sense bytes received upon completion. */
     size_t          cbSense;
     /** Completion status of the command. */
@@ -376 +377 @@
     /** The sense buffer. */
     uint8_t         abSense[96];
+    /** Status code to return if we got sense data. */
+    int             rcSense;
+    /** Number of retries if the command completes with sense
+     * data before we return with an error.
+     */
+    unsigned        cSenseRetries;
     /** The number of entries in the I2T S/G list. */
     unsigned        cI2TSegs;
@@ -661 +668 @@
 static uint32_t iscsiNewITT(PISCSIIMAGE pImage);
 static int iscsiSendPDU(PISCSIIMAGE pImage, PISCSIREQ paReq, uint32_t cnReq, uint32_t uFlags);
-static int iscsiRecvPDU(PISCSIIMAGE pImage, uint32_t itt, PISCSIRES paRes, uint32_t cnRes, bool fSelect);
+static int iscsiRecvPDU(PISCSIIMAGE pImage, uint32_t itt, PISCSIRES paRes, uint32_t cnRes);
 static int iscsiRecvPDUAsync(PISCSIIMAGE pImage);
 static int iscsiSendPDUAsync(PISCSIIMAGE pImage);
@@ -777 +784 @@
 
     return pIScsiCmd;
+}
+
+/**
+ * Removes all commands from the table and returns the
+ * list head
+ *
+ * @returns Pointer to the head of teh command list.
+ * @param   pImage    iSCSI connection to use.
+ */
+static PISCSICMD iscsiCmdRemoveAll(PISCSIIMAGE pImage)
+{
+    PISCSICMD pIScsiCmdHead = NULL;
+
+    for (unsigned idx = 0; idx < RT_ELEMENTS(pImage->aCmdsWaiting); idx++)
+    {
+        PISCSICMD pHead;
+        PISCSICMD pTail;
+
+        pHead = pImage->aCmdsWaiting[idx];
+        pImage->aCmdsWaiting[idx] = NULL;
+
+        /* Get the tail. */
+        pTail = pHead;
+        while (pTail->pNext)
+            pTail = pTail->pNext;
+
+        /* Concatenate. */
+        pTail->pNext = pIScsiCmdHead;
+        pIScsiCmdHead = pHead;
+    }
+
+    return pIScsiCmdHead;
 }
 
@@ -829 +868 @@
 
 
-static int iscsiTransportRead(PISCSIIMAGE pImage, PISCSIRES paResponse, unsigned int cnResponse, bool fSelect)
+static int iscsiTransportRead(PISCSIIMAGE pImage, PISCSIRES paResponse, unsigned int cnResponse)
 {
     int rc = VINF_SUCCESS;
@@ -860 +899 @@
             }
             Assert(cMilliesRemaining < 1000000);
-            if (fSelect)
-            {
-                rc = pImage->pInterfaceNetCallbacks->pfnSelectOne(pImage->Socket,
-                                                                  cMilliesRemaining);
-                if (RT_FAILURE(rc))
-                    break;
-            }
+            rc = pImage->pInterfaceNetCallbacks->pfnSelectOne(pImage->Socket,
+                                                              cMilliesRemaining);
+            if (RT_FAILURE(rc))
+                break;
             rc = pImage->pInterfaceNetCallbacks->pfnRead(pImage->Socket,
                                                          pDst, residual,
@@ -1351 +1387 @@
             cnISCSIRes++;
 
-            rc = iscsiRecvPDU(pImage, itt, aISCSIRes, cnISCSIRes, true);
+            rc = iscsiRecvPDU(pImage, itt, aISCSIRes, cnISCSIRes);
             if (RT_FAILURE(rc))
                 break;
@@ -1642 +1678 @@
             aISCSIRes.pvSeg = aResBHS;
             aISCSIRes.cbSeg = sizeof(aResBHS);
-            rc = iscsiRecvPDU(pImage, itt, &aISCSIRes, 1, true);
+            rc = iscsiRecvPDU(pImage, itt, &aISCSIRes, 1);
             if (RT_SUCCESS(rc))
             {
@@ -1802 +1838 @@
         cnISCSIRes++;
 
-        rc = iscsiRecvPDU(pImage, itt, aISCSIRes, cnISCSIRes, true);
+        rc = iscsiRecvPDU(pImage, itt, aISCSIRes, cnISCSIRes);
         if (RT_FAILURE(rc))
             break;
@@ -1981 +2017 @@
  * @param   cnRes       Number of valid iSCSI response sections in the array.
  */
-static int iscsiRecvPDU(PISCSIIMAGE pImage, uint32_t itt, PISCSIRES paRes, uint32_t cnRes, bool fSelect)
+static int iscsiRecvPDU(PISCSIIMAGE pImage, uint32_t itt, PISCSIRES paRes, uint32_t cnRes)
 {
     int rc = VINF_SUCCESS;
@@ -1990 +2026 @@
         aResBuf.pvSeg = pImage->pvRecvPDUBuf;
         aResBuf.cbSeg = pImage->cbRecvPDUBuf;
-        rc = iscsiTransportRead(pImage, &aResBuf, 1, fSelect);
+        rc = iscsiTransportRead(pImage, &aResBuf, 1);
         if (RT_FAILURE(rc))
         {
@@ -2145 +2181 @@
 
 /**
+ * Reset the PDU buffer
+ *
+ * @param   pImage      The iSCSI connection state to be used.
+ */
+static void iscsiRecvPDUReset(PISCSIIMAGE pImage)
+{
+    pImage->cbRecvPDUResidual = ISCSI_BHS_SIZE;
+    pImage->fRecvPDUBHS       = true;
+    pImage->pbRecvPDUBufCur   = (uint8_t *)pImage->pvRecvPDUBuf;
+}
+
+static void iscsiPDUTxAdd(PISCSIIMAGE pImage, PISCSIPDUTX pIScsiPDUTx, bool fFront)
+{
+    if (!fFront)
+    {
+        /* Link the PDU at the tail of the list. */
+        if (!pImage->pIScsiPDUTxHead)
+            pImage->pIScsiPDUTxHead = pIScsiPDUTx;
+        else
+            pImage->pIScsiPDUTxTail->pNext = pIScsiPDUTx;
+        pImage->pIScsiPDUTxTail = pIScsiPDUTx;
+    }
+    else
+    {
+        /* Link PDU to at the front of the list. */
+        pIScsiPDUTx->pNext = pImage->pIScsiPDUTxHead;
+        pImage->pIScsiPDUTxHead = pIScsiPDUTx;
+        if (!pImage->pIScsiPDUTxTail)
+            pImage->pIScsiPDUTxTail = pIScsiPDUTx;
+    }
+}
+
+/**
  * Receives a PDU in a non blocking way.
  *
@@ -2162 +2231 @@
         /*
          * We are receiving a new PDU, don't read more than the BHS initially
-         * until we now the real size of the PDU.
+         * until we know the real size of the PDU.
          */
-        pImage->cbRecvPDUResidual = ISCSI_BHS_SIZE;
-        pImage->fRecvPDUBHS       = true;
-        pImage->pbRecvPDUBufCur   = (uint8_t *)pImage->pvRecvPDUBuf;
+        iscsiRecvPDUReset(pImage);
         LogFlow(("Receiving new PDU\n"));
     }
@@ -2378 +2445 @@
 
                     /* Link the PDU to the list. */
-                    if (!pImage->pIScsiPDUTxHead)
-                        pImage->pIScsiPDUTxHead = pIScsiPDUTx;
-                    else
-                        pImage->pIScsiPDUTxTail->pNext = pIScsiPDUTx;
-                    pImage->pIScsiPDUTxTail = pIScsiPDUTx;
+                    iscsiPDUTxAdd(pImage, pIScsiPDUTx, false /* fFront */);
 
                     /* Start transfer of a PDU if there is no one active at the moment. */
@@ -2468 +2531 @@
             break;
         case ISCSIOP_ASYN_MSG:
-            /* Asynchronous Messages must not have the final bit unser and may not contain
+            /* Asynchronous Messages must not have the final bit unset and may not contain
              * an initiator task tag. */
             if (    ((hw0 & ISCSI_FINAL_BIT) == 0)
@@ -2490 +2553 @@
 }
 
+
 /**
  * Prepares a PDU to transfer for the given command and adds it to the list.
@@ -2498 +2562 @@
     uint32_t *paReqBHS;
     size_t cbData = 0;
+    size_t cbSegs = 0;
     PSCSIREQ pScsiReq;
     PISCSIPDUTX pIScsiPDU = NULL;
@@ -2547 +2612 @@
     pIScsiPDU->aISCSIReq[cnISCSIReq].pvSeg = pIScsiPDU->aBHS;
     cnISCSIReq++;
+    cbSegs = sizeof(pIScsiPDU->aBHS);
     /* Padding is not necessary for the BHS. */
 
@@ -2556 +2622 @@
             pIScsiPDU->aISCSIReq[cnISCSIReq].cbSeg = pScsiReq->paI2TSegs[cSeg].cbSeg;
             pIScsiPDU->aISCSIReq[cnISCSIReq].pvSeg = pScsiReq->paI2TSegs[cSeg].pvSeg;
+            cbSegs += pScsiReq->paI2TSegs[cSeg].cbSeg;
             cnISCSIReq++;
 
@@ -2564 +2631 @@
                 pIScsiPDU->aISCSIReq[cnISCSIReq].pvSeg = &pImage->aPadding[0];
                 pIScsiPDU->aISCSIReq[cnISCSIReq].cbSeg = 4 - (pScsiReq->paI2TSegs[cSeg].cbSeg & 3);
+                cbSegs += pIScsiPDU->aISCSIReq[cnISCSIReq].cbSeg;
                 cnISCSIReq++;
             }
@@ -2570 +2638 @@
 
     pIScsiPDU->cISCSIReq = cnISCSIReq;
-    pIScsiPDU->cbSgLeft  = pScsiReq->cbI2TData + sizeof(pIScsiPDU->aBHS);
+    pIScsiPDU->cbSgLeft  = cbSegs;
     RTSgBufInit(&pIScsiPDU->SgBuf, pIScsiPDU->aISCSIReq, cnISCSIReq);
 
     /* Link the PDU to the list. */
-    if (!pImage->pIScsiPDUTxHead)
-        pImage->pIScsiPDUTxHead = pIScsiPDU;
-    else
-        pImage->pIScsiPDUTxTail->pNext = pIScsiPDU;
-    pImage->pIScsiPDUTxTail = pIScsiPDU;
+    iscsiPDUTxAdd(pImage, pIScsiPDU, false /* fFront */);
 
     /* Start transfer of a PDU if there is no one active at the moment. */
@@ -2673 +2737 @@
             if (final && cbData > pScsiReq->cbT2IData)
             {
-                /* The received PDU is partially stored in the buffer for status.
+                /* The received PDU is bigger than what we requested.
                  * Must not happen under normal circumstances and is a target error. */
                 rc = VERR_BUFFER_OVERFLOW;
@@ -2695 +2759 @@
     }
 
+    /* Log any errors here but ignore the PDU. */
+    if (RT_FAILURE(rc))
+    {
+        LogRel(("iSCSI: Received malformed PDU from target %s (rc=%Rrc), ignoring\n", pImage->pszTargetName, rc));
+        rc = VINF_SUCCESS;
+    }
+
     return rc;
 }
-
 
 /**
@@ -3069 +3139 @@
     PISCSIIMAGE pImage = (PISCSIIMAGE)pvUser;
 
-    /* Initialise the initial event mask. */
+    /* Initialize the initial event mask. */
     pImage->fPollEvents = VD_INTERFACETCPNET_EVT_READ | VD_INTERFACETCPNET_EVT_ERROR;
 
@@ -3093 +3163 @@
                     {
                         rc = iscsiPDUTxPrepare(pImage, pIScsiCmd);
+                        AssertRC(rc);
                         break;
                     }
@@ -3110 +3181 @@
         else if (RT_SUCCESS(rc))
         {
+            Assert(pImage->state == ISCSISTATE_NORMAL);
             LogFlow(("Got socket events %#x\n", fEvents));
 
@@ -3129 +3201 @@
             else if (fEvents & VD_INTERFACETCPNET_EVT_ERROR)
             {
-                /** @todo: Determine type of error, reset states, reconnect
-                 * and resend all active PDUs.
+                PISCSICMD pIScsiCmdHead = NULL;
+                PISCSICMD pIScsiCmd = NULL;
+                PISCSICMD pIScsiCmdCur = NULL;
+                PISCSIPDUTX pIScsiPDUTx = NULL;
+                LogFlow(("An error ocurred\n"));
+
+                /* Close connection. */
+                iscsiTransportClose(pImage);
+                pImage->state = ISCSISTATE_FREE;
+
+                /* Reset PDU we are receiving. */
+                iscsiRecvPDUReset(pImage);
+
+                /*
+                 * Abort all PDUs we are about to transmit,
+                 * the command need a new Itt if the relogin is successful.
                  */
-                LogFlow(("An error ocurred\n"));
+                while (pImage->pIScsiPDUTxHead)
+                {
+                    pIScsiPDUTx = pImage->pIScsiPDUTxHead;
+                    pImage->pIScsiPDUTxHead = pIScsiPDUTx->pNext;
+
+                    pIScsiCmd = pIScsiPDUTx->pIScsiCmd;
+
+                    if (pIScsiCmd)
+                    {
+                        /* Place on command list. */
+                        pIScsiCmd->pNext = pIScsiCmdHead;
+                        pIScsiCmdHead = pIScsiCmd;
+                    }
+                    RTMemFree(pIScsiPDUTx);
+                }
+
+                /* Clear the current PDU too. */
+                if (pImage->pIScsiPDUTxCur)
+                {
+                    pIScsiPDUTx = pImage->pIScsiPDUTxCur;
+
+                    pImage->pIScsiPDUTxCur = NULL;
+                    pIScsiCmd = pIScsiPDUTx->pIScsiCmd;
+
+                    if (pIScsiCmd)
+                    {
+                        pIScsiCmd->pNext = pIScsiCmdHead;
+                        pIScsiCmdHead = pIScsiCmd;
+                    }
+                    RTMemFree(pIScsiPDUTx);
+                }
+
+                /*
+                 * Get all commands which are waiting for a response
+                 * They need to be resend too after a successful reconnect.
+                 */
+                pIScsiCmd = iscsiCmdRemoveAll(pImage);
+
+                pIScsiCmdCur = pIScsiCmd;
+                while (pIScsiCmdCur->pNext)
+                    pIScsiCmdCur = pIScsiCmdCur->pNext;
+
+                /*
+                 * Place them in front of the list because they are the oldest requests
+                 * and need to be processed first to minimize the risk to time out.
+                 */
+                pIScsiCmdCur->pNext = pIScsiCmdHead;
+                pIScsiCmdHead = pIScsiCmd;
+
+                /* Try to attach. */
+                rc = iscsiAttach(pImage);
+                if (RT_SUCCESS(rc))
+                {
+                    /* Phew, we have a connection again.
+                     * Prepare new PDUs for the aborted commands.
+                     */
+                    while (pIScsiCmdHead)
+                    {
+                        pIScsiCmd = pIScsiCmdHead;
+                        pIScsiCmdHead = pIScsiCmdHead->pNext;
+
+                        rc = iscsiPDUTxPrepare(pImage, pIScsiCmd);
+                        AssertRC(rc);
+                    }
+                }
+                else
+                {
+                    /*
+                     * Still no luck, complete commands with error so the caller
+                     * has a chance to inform the user and maybe resend the command.
+                     */
+                    while (pIScsiCmdHead)
+                    {
+                        pIScsiCmd = pIScsiCmdHead;
+                        pIScsiCmdHead = pIScsiCmdHead->pNext;
+
+                        iscsiCmdComplete(pImage, pIScsiCmd, VERR_BROKEN_PIPE);
+                    }
+                }
             }
             else
@@ -3144 +3308 @@
 
     return VINF_SUCCESS;
-}
-
-static void iscsiCommandAsyncComplete(PISCSIIMAGE pImage, int rcReq, void *pvUser)
-{
-    size_t cbTransfered = 0;
-    PSCSIREQASYNC pReqAsync = (PSCSIREQASYNC)pvUser;
-    PSCSIREQ pScsiReq = pReqAsync->pScsiReq;
-
-    /** @todo Retry and sense buffer handling. */
-
-    if (pScsiReq->enmXfer == SCSIXFER_FROM_TARGET)
-        cbTransfered = pScsiReq->cbT2IData;
-    else if (pScsiReq->enmXfer == SCSIXFER_TO_TARGET)
-        cbTransfered = pScsiReq->cbI2TData;
-    else
-        AssertMsg(pScsiReq->enmXfer == SCSIXFER_NONE, ("To/From transfers are not supported yet\n"));
-
-    /* Continue I/O context. */
-    pImage->pInterfaceIoCallbacks->pfnIoCtxCompleted(pImage->pInterfaceIo->pvUser,
-                                                     pReqAsync->pIoCtx, rcReq,
-                                                     cbTransfered);
-
-    RTMemFree(pScsiReq);
-    RTMemFree(pReqAsync);
-}
-
-static void iscsiCommandCompleteSync(PISCSIIMAGE pImage, int rcReq, void *pvUser)
-{
-    PISCSICMDSYNC pIScsiCmdSync = (PISCSICMDSYNC)pvUser;
-
-    pIScsiCmdSync->rcCmd = rcReq;
-    int rc = RTSemEventSignal(pIScsiCmdSync->EventSem);
-    AssertRC(rc);
 }
 
@@ -3208 +3339 @@
 
     return rc;
+}
+
+static void iscsiCommandCompleteSync(PISCSIIMAGE pImage, int rcReq, void *pvUser)
+{
+    PISCSICMDSYNC pIScsiCmdSync = (PISCSICMDSYNC)pvUser;
+
+    pIScsiCmdSync->rcCmd = rcReq;
+    int rc = RTSemEventSignal(pIScsiCmdSync->EventSem);
+    AssertRC(rc);
 }
 
@@ -3337 +3477 @@
     return rc;
 }
+
+
+static void iscsiCommandAsyncComplete(PISCSIIMAGE pImage, int rcReq, void *pvUser)
+{
+    bool fComplete = true;
+    size_t cbTransfered = 0;
+    PSCSIREQASYNC pReqAsync = (PSCSIREQASYNC)pvUser;
+    PSCSIREQ pScsiReq = pReqAsync->pScsiReq;
+
+    if (   RT_SUCCESS(rcReq)
+        && pScsiReq->cbSense > 0)
+    {
+        /* Try again if possible. */
+        if (pReqAsync->cSenseRetries > 0)
+        {
+            pReqAsync->cSenseRetries--;
+            pScsiReq->cbSense = sizeof(pReqAsync->abSense);
+            int rc = iscsiCommandAsync(pImage, pScsiReq, iscsiCommandAsyncComplete, pReqAsync);
+            if (RT_SUCCESS(rc))
+                fComplete = false;
+            else
+                rcReq = pReqAsync->rcSense;
+        }
+        else
+            rcReq = pReqAsync->rcSense;
+    }
+
+    if (fComplete)
+    {
+        if (pScsiReq->enmXfer == SCSIXFER_FROM_TARGET)
+            cbTransfered = pScsiReq->cbT2IData;
+        else if (pScsiReq->enmXfer == SCSIXFER_TO_TARGET)
+            cbTransfered = pScsiReq->cbI2TData;
+        else
+            AssertMsg(pScsiReq->enmXfer == SCSIXFER_NONE, ("To/From transfers are not supported yet\n"));
+
+        /* Continue I/O context. */
+        pImage->pInterfaceIoCallbacks->pfnIoCtxCompleted(pImage->pInterfaceIo->pvUser,
+                                                         pReqAsync->pIoCtx, rcReq,
+                                                         cbTransfered);
+
+        RTMemFree(pScsiReq);
+        RTMemFree(pReqAsync);
+    }
+}
+
 
 /**
@@ -5018 +5204 @@
             pReqAsync->pIoCtx = pIoCtx;
             pReqAsync->pScsiReq = pReq;
+            pReqAsync->cSenseRetries = 10;
+            pReqAsync->rcSense       = VERR_READ_ERROR;
 
             pbCDB[0] = SCSI_READ_10;
@@ -5116 +5304 @@
             pReqAsync->pIoCtx = pIoCtx;
             pReqAsync->pScsiReq = pReq;
+            pReqAsync->cSenseRetries = 10;
+            pReqAsync->rcSense       = VERR_WRITE_ERROR;
 
             pbCDB[0] = SCSI_WRITE_10;
@@ -5178 +5368 @@
             uint8_t *pbCDB = &pReqAsync->abCDB[0];
 
-            pReqAsync->pIoCtx   = pIoCtx;
-            pReqAsync->pScsiReq = pReq;
+            pReqAsync->pIoCtx        = pIoCtx;
+            pReqAsync->pScsiReq      = pReq;
+            pReqAsync->cSenseRetries = 0;
+            pReqAsync->rcSense       = VINF_SUCCESS;
 
             pbCDB[0] = SCSI_SYNCHRONIZE_CACHE;