Changeset 83499 in vbox for trunk/src

Timestamp:

Apr 1, 2020 7:18:42 AM (5 years ago)

Author:

vboxsync

Message:

Network/DevVirtioNet_1_0.cpp: Round trip traffic happening continuously with network activity, but pings are not resolving yet. Need to dig into higher level packet traces, and validate thing like checksumming, etc...

Location:

trunk/src/VBox/Devices

Files:

• Network/DevVirtioNet_1_0.cpp (modified) (21 diffs)
• VirtIO/Virtio_1_0.cpp (modified) (5 diffs)

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

@@ -75 +75 @@
 
 #define QUEUE_NAME(a_pVirtio, a_idxQueue) ((a_pVirtio)->virtqState[(a_idxQueue)].szVirtqName)
-#define VIRTQNAME(idxQueue)           (pThis->aszVirtqNames[idxQueue])
-#define CBVIRTQNAME(idxQueue)         RTStrNLen(VIRTQNAME(idxQueue), sizeof(VIRTQNAME(idxQueue)))
+#define VIRTQNAME(idxQueue)       (pThis->aszVirtqNames[idxQueue])
+#define CBVIRTQNAME(idxQueue)     RTStrNLen(VIRTQNAME(idxQueue), sizeof(VIRTQNAME(idxQueue)))
 #define FEATURE_ENABLED(feature)  (!!(pThis->fNegotiatedFeatures & VIRTIONET_F_##feature))
 #define FEATURE_DISABLED(feature) (!FEATURE_ENABLED(feature))
@@ -82 +82 @@
 
 #define SET_LINK_UP(pState) \
+            LogFunc(("SET_LINK_UP\n")); \
             pState->virtioNetConfig.uStatus |= VIRTIONET_F_LINK_UP; \
             virtioCoreNotifyConfigChanged(&pThis->Virtio)
 
 #define SET_LINK_DOWN(pState) \
-            pState->virtioNetConfig.uStatus &= !VIRTIONET_F_LINK_UP; \
+            LogFunc(("SET_LINK_DOWN\n")); \
+            pState->virtioNetConfig.uStatus &= ~VIRTIONET_F_LINK_UP; \
             virtioCoreNotifyConfigChanged(&pThis->Virtio)
 
@@ -194 +196 @@
 #pragma pack()
 
-#define VIRTIONET_F_LINK_UP                  RT_BIT(1)          /**< config status: Link is up                      */
-#define VIRTIONET_F_ANNOUNCE                 RT_BIT(2)          /**< config status: Announce                        */
+#define VIRTIONET_F_LINK_UP                          1          /**< config status: Link is up                      */
+#define VIRTIONET_F_ANNOUNCE                         2          /**< config status: Announce                        */
 
 /** @name VirtIO 1.0 NET Host Device device specific control types
@@ -1454 +1456 @@
 
         uint32_t cbDescChainLeft = pDescChain->cbPhysReturn;
-
+        uint8_t  cbHdr = sizeof(VIRTIONET_PKT_HDR_T);
         /* Fill the Guest Rx buffer with data received from the interface */
         for (uint16_t cSegs = 0; uOffset < cb && cbDescChainLeft; )
@@ -1461 +1463 @@
             {
                 /* Lead with packet header */
-                paVirtSegsToGuest[0].cbSeg = sizeof(VIRTIONET_PKT_HDR_T);
-                paVirtSegsToGuest[0].pvSeg = RTMemAlloc(sizeof(VIRTIONET_PKT_HDR_T));
+                paVirtSegsToGuest[0].cbSeg = cbHdr;
+                paVirtSegsToGuest[0].pvSeg = RTMemAlloc(cbHdr);
                 AssertReturn(paVirtSegsToGuest[0].pvSeg, VERR_NO_MEMORY);
-                cbDescChainLeft -= sizeof(VIRTIONET_PKT_HDR_T);
-
-                memcpy(paVirtSegsToGuest[0].pvSeg, &rxPktHdr, sizeof(VIRTIONET_PKT_HDR_T));
+                cbDescChainLeft -= cbHdr;
+
+                memcpy(paVirtSegsToGuest[0].pvSeg, &rxPktHdr, cbHdr);
 
                 /* Calculate & cache the field we will need to update later in gcPhys memory */
@@ -1474 +1476 @@
                 cSegs++;
             }
+
             if (cSegs >= cSegsAllocated)
             {
@@ -1507 +1510 @@
 
  /* Dump Rx Pkt after it's been written to guest physical memory via the virtio core API */
-    if (pThis->fLog)
-    {
+//    if (pThis->fLog)
+//    {
         virtioNetDumpGcPhysRxBuf(pDevIns, &rxPktHdr, cDescs, (uint8_t *)pvBuf, cb,
                                  gcPhysPktHdrNumBuffers - RT_UOFFSETOF(VIRTIONET_PKT_HDR_T, uNumBuffers),
                                  cb + sizeof(VIRTIONET_PKT_HDR_T));
-    }
+//    }
 
     virtioCoreQueueSync(pDevIns, &pThis->Virtio, RXQIDX_QPAIR(idxQueue));
@@ -1518 +1521 @@
 
 if (pThis->fLog) {
-    RTThreadSleep(500);
-    RT_BREAKPOINT();
+//    RTThreadSleep(500);
+//   RT_BREAKPOINT();
     pThis->fBp = false;
 }
@@ -1911 +1914 @@
 }
 
-static bool virtioNetR3ReadHeader(PPDMDEVINS pDevIns, RTGCPHYS GCPhys, PVIRTIONET_PKT_HDR_T pPktHdr, uint32_t cbMax)
+static int virtioNetR3ReadHeader(PPDMDEVINS pDevIns, RTGCPHYS GCPhys, PVIRTIONET_PKT_HDR_T pPktHdr, uint32_t cbMax)
 {
     int rc = PDMDevHlpPCIPhysRead(pDevIns, GCPhys, pPktHdr, sizeof(*pPktHdr));
     if (RT_FAILURE(rc))
-        return false;
-
-    Log4(("virtio-net: header flags=%x gso-type=%x len=%x gso-size=%x Chksum-start=%x Chksum-offset=%x cb=%x\n",
+        return rc;
+
+    Log(("virtio-net: header flags=%x gso-type=%x len=%x gso-size=%x Chksum-start=%x Chksum-offset=%x cb=%x\n",
           pPktHdr->uFlags, pPktHdr->uGsoType, pPktHdr->uHdrLen,
           pPktHdr->uGsoSize, pPktHdr->uChksumStart, pPktHdr->uChksumOffset, cbMax));
@@ -1926 +1929 @@
 
         /* Segmentation offloading cannot be done without checksumming, and we do not support ECN */
-        if (  RT_UNLIKELY(!(pPktHdr->uFlags & VIRTIONET_HDR_F_NEEDS_CSUM))
-            | RT_UNLIKELY(pPktHdr->uGsoType & VIRTIONET_HDR_GSO_ECN))
-                return false;
+        AssertMsgReturn(   RT_LIKELY(pPktHdr->uFlags & VIRTIONET_HDR_F_NEEDS_CSUM)
+                          && RT_UNLIKELY(pPktHdr->uGsoType & VIRTIONET_HDR_GSO_ECN),
+                          ("Unsupported ECN request in pkt header\n"), VERR_NOT_SUPPORTED);
 
         switch (pPktHdr->uGsoType)
@@ -1940 +1943 @@
                 break;
             default:
-                return false;
+                LogFunc(("Bad GSO type in packet header\n"));
+                return VERR_INVALID_PARAMETER;
         }
         /* Header + MSS must not exceed the packet size. */
-        if (RT_UNLIKELY(uMinHdrSize + pPktHdr->uChksumStart + pPktHdr->uGsoSize > cbMax))
-            return false;
-    }
-    /* Checksum must fit into the frame (validating both checksum fields). */
-    if ((   pPktHdr->uFlags & VIRTIONET_HDR_F_NEEDS_CSUM)
-         && sizeof(uint16_t) + pPktHdr->uChksumStart + pPktHdr->uChksumOffset > cbMax)
-               return false;
-    Log4Func(("returning true\n"));
-    return true;
+        AssertMsgReturn(RT_LIKELY(uMinHdrSize + pPktHdr->uChksumStart + pPktHdr->uGsoSize <= cbMax),
+                    ("Header plus message exceeds packet size"), VERR_BUFFER_OVERFLOW);
+    }
+
+    AssertMsgReturn(   !pPktHdr->uFlags & VIRTIONET_HDR_F_NEEDS_CSUM
+                      || sizeof(uint16_t) + pPktHdr->uChksumStart + pPktHdr->uChksumOffset <= cbMax,
+                ("Checksum doesn't fit into pkt header\n"), VERR_BUFFER_OVERFLOW);
+
+    return VINF_SUCCESS;
 }
 
@@ -1957 +1961 @@
                                PPDMNETWORKGSO pGso, PVIRTIONET_PKT_HDR_T pPktHdr)
 {
-LogFunc(("\n"));
     virtioNetR3PacketDump(pThis, (uint8_t *)pSgBuf->aSegs[0].pvSeg, pSgBuf->cbUsed, "--> Outgoing");
     if (pGso)
@@ -2009 +2012 @@
 {
 
-LogFunc(("\n"));
     PVIRTIOCORE pVirtio = &pThis->Virtio;
 
@@ -2045 +2047 @@
     }
 
-    unsigned int cbPktHdr = sizeof(VIRTIONET_PKT_HDR_T);
-
     int cPkts = virtioCoreR3QueuePendingCount(pVirtio->pDevIns, pVirtio, idxQueue);
     if (!cPkts)
@@ -2064 +2064 @@
     int rc;
     PVIRTIO_DESC_CHAIN_T pDescChain;
-    while ((rc = virtioCoreR3QueuePeek(pVirtio->pDevIns, pVirtio, idxQueue, &pDescChain)))
+    while ((rc = virtioCoreR3QueuePeek(pVirtio->pDevIns, pVirtio, idxQueue, &pDescChain)) == VINF_SUCCESS)
     {
         if (RT_SUCCESS(rc))
@@ -2074 +2074 @@
         }
 
-        uint32_t cSegsFromGuest = pDescChain->pSgPhysSend->cSegs;
-        PVIRTIOSGSEG paSegsFromGuest = pDescChain->pSgPhysSend->paSegs;
-
-        if (cSegsFromGuest < 2 || paSegsFromGuest[0].cbSeg != cbPktHdr)
-        {
-            /* This check could be made more complex, because in theory (but not likely nor
-             * seen in practice) the first segment could contain header and data. */
-            LogFunc(("%s The first segment is not the header! (%u < 2 || %u != %u).\n",
-                 INSTANCE(pThis), cSegsFromGuest, paSegsFromGuest[0].cbSeg, cbPktHdr));
-            break;
-        }
+        PVIRTIOSGBUF pSgPhysSend = pDescChain->pSgPhysSend;
+        PVIRTIOSGSEG paSegsFromGuest = pSgPhysSend->paSegs;
+        uint32_t cSegsFromGuest = pSgPhysSend->cSegs;
 
         VIRTIONET_PKT_HDR_T PktHdr;
         uint32_t uSize = 0;
 
+        Assert(paSegsFromGuest[0].cbSeg >= sizeof(PktHdr));
+
         /* Compute total frame size. */
-        for (unsigned i = 1; i < cSegsFromGuest && uSize < VIRTIONET_MAX_FRAME_SIZE; i++)
+        for (unsigned i = 0; i < cSegsFromGuest && uSize < VIRTIONET_MAX_FRAME_SIZE; i++)
             uSize +=  paSegsFromGuest[i].cbSeg;
 
@@ -2100 +2094 @@
             uSize = VIRTIONET_MAX_FRAME_SIZE;
 
-        if (pThisCC->pDrv && virtioNetR3ReadHeader(pDevIns, paSegsFromGuest[0].gcPhys, &PktHdr, uSize))
+        if (pThisCC->pDrv)
         {
             PDMNETWORKGSO Gso, *pGso = virtioNetR3SetupGsoCtx(&Gso, &PktHdr);
@@ -2109 +2103 @@
             if (RT_SUCCESS(rc))
             {
-                pSgBufToPdmLeafDevice->cbUsed = uSize;
-
-                /* Assemble a complete frame. */
-                for (unsigned i = 1; i < cSegsFromGuest && uSize > 0; i++)
+                uSize -= sizeof(PktHdr);
+                rc = virtioNetR3ReadHeader(pDevIns, paSegsFromGuest[0].gcPhys, &PktHdr, sizeof(PktHdr));
+                virtioCoreSgBufAdvance(pSgPhysSend, sizeof(PktHdr));
+
+                uint64_t uOffset = 0;
+                size_t cbCopied = 0;
+                size_t cbTotal = 0;
+                size_t cbRemain = pSgBufToPdmLeafDevice->cbUsed = uSize;
+                while (cbRemain)
                 {
-                    unsigned uOffset;
-                    unsigned cbSeg = RT_MIN(uSize, paSegsFromGuest[i].cbSeg);
-
-                    PDMDevHlpPCIPhysRead(pDevIns, paSegsFromGuest[i].gcPhys,
-                                         ((uint8_t *)pSgBufToPdmLeafDevice->aSegs[0].pvSeg) + uOffset,
-                                         cbSeg);
-                    uOffset += cbSeg;
-                    uSize -= cbSeg;
+                    PVIRTIOSGSEG paSeg  = &pSgPhysSend->paSegs[pSgPhysSend->idxSeg];
+                    uint64_t srcSgStart = (uint64_t)paSeg->gcPhys;
+                    uint64_t srcSgLen   = (uint64_t)paSeg->cbSeg;
+                    uint64_t srcSgCur   = (uint64_t)pSgPhysSend->gcPhysCur;
+                    cbCopied = RT_MIN((uint64_t)cbRemain, srcSgLen - (srcSgCur - srcSgStart));
+                    PDMDevHlpPCIPhysRead(pDevIns,
+                                         (RTGCPHYS)pSgPhysSend->gcPhysCur,
+                                         ((uint8_t *)pSgBufToPdmLeafDevice->aSegs[0].pvSeg) + uOffset, cbCopied);
+                    virtioCoreSgBufAdvance(pSgPhysSend, cbCopied);
+                    cbRemain -= cbCopied;
+                    uOffset += cbCopied;
+                    cbTotal += cbCopied;
                 }
+
+                LogFunc((".... Copied %lu bytes to %lu byte guest buffer, residual=%lu\n",
+                     cbTotal, pDescChain->cbPhysSend, pDescChain->cbPhysSend - cbTotal));
+
                 rc = virtioNetR3TransmitFrame(pThis, pThisCC, pSgBufToPdmLeafDevice, pGso, &PktHdr);
                 if (RT_FAILURE(rc))
@@ -2136 +2143 @@
                 break;
             }
+
+            /* Remove this descriptor chain from the available ring */
+            virtioCoreR3QueueSkip(pVirtio, idxQueue);
+
+            /* No data to return to guest, but call is needed put elem (e.g. desc chain) on used ring */
+            virtioCoreR3QueuePut(pVirtio->pDevIns, pVirtio, idxQueue, NULL, pDescChain, false);
+
+            virtioCoreQueueSync(pVirtio->pDevIns, pVirtio, idxQueue);
         }
-        /* Remove this descriptor chain from the available ring */
-        virtioCoreR3QueueSkip(pVirtio, idxQueue);
-
-        /* No data to return to guest, but call is needed put elem (e.g. desc chain) on used ring */
-        virtioCoreR3QueuePut(pVirtio->pDevIns, pVirtio, idxQueue, NULL, pDescChain, false);
-
-        virtioCoreQueueSync(pVirtio->pDevIns, pVirtio, idxQueue);
-
     }
     virtioNetR3SetWriteLed(pThisCC, false);
@@ -2270 +2277 @@
              else if (IS_TX_QUEUE(idxQueue))
              {
-LogFunc(("Worker thread notified of pending Xmit packets!!!!!!\n"));
                  Log10Func(("%s Notified of data to transmit\n", INSTANCE(pThis)));
                  virtioNetR3TransmitPendingPackets(pDevIns, pThis, pThisCC,
@@ -2481 +2487 @@
         LogFunc(("%s VirtIO ready\n-----------------------------------------------------------------------------------------\n",
                  INSTANCE(pThis)));
+        pThis->virtioNetConfig.uStatus = pThis->fCableConnected ? VIRTIONET_F_LINK_UP : 0;
 
         pThis->fResetting    = false;

trunk/src/VBox/Devices/VirtIO/Virtio_1_0.cpp

@@ -766 +766 @@
     uint16_t uAvailRingIdx = virtioReadAvailRingIdx(pDevIns, pVirtio, idxQueue);
     uint16_t uNextAvailIdx = pVirtio->virtqState[idxQueue].uAvailIdx;
-    int16_t  iDelta = uAvailRingIdx - uNextAvailIdx;
     uint16_t uDelta = uAvailRingIdx - uNextAvailIdx;
-    return iDelta >= 0 ? uDelta : VIRTQ_MAX_CNT + uDelta;
+    if (uAvailRingIdx > uNextAvailIdx)
+        return uDelta;
+    return VIRTQ_MAX_CNT + uDelta;
 }
 /**
@@ -906 +907 @@
               VIRTQNAME(pVirtio, idxQueue), virtioReadUsedRingIdx(pDevIns, pVirtio, idxQueue)));
 
-    /*
-     * Copy s/g buf (virtual memory) to guest phys mem (IN direction). This virtual memory
-     * block will be small (fixed portion of response header + sense buffer area or
-     * control commands or error return values)... The bulk of req data xfers to phys mem
-     * is handled by client */
+    /* Copy s/g buf (virtual memory) to guest phys mem (IN direction). */
 
     size_t cbCopy = 0, cbTotal = 0, cbRemain = 0;
@@ -937 +934 @@
     }
 
-
     /* If this write-ahead crosses threshold where the driver wants to get an event flag it */
     if (pVirtio->uDriverFeatures & VIRTIO_F_EVENT_IDX)
@@ -946 +942 @@
      * Place used buffer's descriptor in used ring but don't update used ring's slot index.
      * That will be done with a subsequent client call to virtioCoreQueueSync() */
-    virtioWriteUsedElem(pDevIns, pVirtio, idxQueue, pVirtq->uUsedIdx++, pDescChain->uHeadIdx, (uint32_t)cbCopy);
-
-    Log6Func((".... Copied %zu bytes in %d segs to %u byte buffer, residual=%zu\n",
-              cbTotal - cbRemain, pSgVirtReturn->cSegs, pDescChain->cbPhysReturn, pDescChain->cbPhysReturn - cbCopy));
+    virtioWriteUsedElem(pDevIns, pVirtio, idxQueue, pVirtq->uUsedIdx++, pDescChain->uHeadIdx, (uint32_t)cbTotal);
+
+    if (pSgVirtReturn)
+        Log6Func((".... Copied %zu bytes in %d segs to %u byte buffer, residual=%zu\n",
+              cbTotal - cbRemain, pSgVirtReturn->cSegs, pDescChain->cbPhysReturn, pDescChain->cbPhysReturn - cbTotal));
 
     Log6Func(("Write ahead used_idx=%u, %s used_idx=%u\n",
@@ -1069 +1066 @@
             return;
         }
-/* REMOVE THIS!!!!!!! DON'T INTEGRATE! EXPERIMENTAL/DEBUG */
-//virtioKick(pDevIns, pVirtio, VIRTIO_ISR_VIRTQ_INTERRUPT, pVirtio->uQueueMsixVector[idxQueue], fForce);
-//        Log6Func(("...skipping interrupt. Guest flagged VIRTQ_AVAIL_F_NO_INTERRUPT for queue\n"));
+
+        Log6Func(("...skipping interrupt. Guest flagged VIRTQ_AVAIL_F_NO_INTERRUPT for queue\n"));
     }
 }