VirtualBox

Changeset 107830 in vbox for trunk


Ignore:
Timestamp:
Jan 16, 2025 10:24:40 PM (3 weeks ago)
Author:
vboxsync
Message:

DrvNAT-libslirp: Some more cleanups & fixes (drvNAT_TimerFreeCb, ++). Using the approprate prefixes for timeouts and(/or) documenting the unit really helps with readability. Converted a bunch of unnecessary 'opaque' parameters for internal functions (drvNAT_CheckTimeout, drvNAT_UpdateTimeout, ++) Left a few todos behind. bugref:10268

File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/VBox/Devices/Network/DrvNATlibslirp.cpp

    r107826 r107830  
    9292*********************************************************************************************************************************/
    9393#define DRVNAT_MAXFRAMESIZE (16 * 1024)
    94 #define DRVNAT_DEFAULT_TIMEOUT (3600*1000)
     94/** The maximum (default) poll/WSAPoll timeout. */
     95#define DRVNAT_DEFAULT_TIMEOUT (int)RT_MS_1HOUR
    9596#define MAX_IP_ADDRESS_STR_LEN_W_NULL 16
    9697
     98/** @todo r=bird: this is a load of weirdness... 'extradata' != cfgm.   */
    9799#define GET_EXTRADATA(pdrvins, node, name, rc, type, type_name, var)                                  \
    98100    do {                                                                                                \
    … …  
    160162{
    161163    struct slirpTimer *next;
    162     int64_t uTimeExpire;
     164    /** The time deadline (milliseconds, RTTimeMilliTS).   */
     165    int64_t msExpire;
    163166    SlirpTimerCb pHandler;
    164167    void *opaque;
    … …  
    178181    unsigned int uPollCap = 0;
    179182
     183    /** List of timers (in reverse creation order).
     184     * @note There is currently only one libslirp timer (v4.8 / 2025-01-16).  */
    180185    SlirpTimer *pTimerHead;
    181186    bool fPassDomain;
    … …  
    229234    RTPIPE                  hPipeRead;
    230235#endif
    231     /* count of bytes sent to notify NAT thread */
     236    /** count of unconsumed bytes sent to notify NAT thread */
    232237    volatile uint64_t       cbWakeupNotifs;
    233238
    … …  
    264269*********************************************************************************************************************************/
    265270static void drvNATNotifyNATThread(PDRVNAT pThis, const char *pszWho);
    266 static void drvNAT_UpdateTimeout(int *i32Timeout, void *opaque);
    267 static void drvNAT_CheckTimeout(void *opaque);
     271static int  drvNATTimersAdjustTimeoutDown(PDRVNAT pThis, int cMsTimeout);
     272static void drvNATTimersRunExpired(PDRVNAT pThis);
    268273static DECLCALLBACK(int) drvNAT_AddPollCb(int iFd, int iEvents, void *opaque);
    269274static DECLCALLBACK(int64_t) drvNAT_ClockGetNsCb(void *opaque);
    … …  
    394399    if (pThis->enmLinkState == PDMNETWORKLINKSTATE_UP)
    395400    {
    396         const uint8_t *m = static_cast<const uint8_t*>(pSgBuf->pvAllocator);
    397         if (m)
     401        if (pSgBuf->pvAllocator)
    398402        {
    399403            /*
    400404             * A normal frame.
    401405             */
    402             LogFlowFunc(("m=%p\n", m));
     406            LogFlowFunc(("pvAllocator=%p LB %#zx\n", pSgBuf->pvAllocator, pSgBuf->cbUsed));
    403407            slirp_input(pThis->pNATState->pSlirp, (uint8_t const *)pSgBuf->pvAllocator, (int)pSgBuf->cbUsed);
    404408        }
    … …  
    406410        {
    407411            /*
    408              * M_EXT buf, need to segment it.
     412             * Do segmentation offloading.
    409413             */
    410 
    411             uint8_t const  *pbFrame = (uint8_t const *)pSgBuf->aSegs[0].pvSeg;
    412             PCPDMNETWORKGSO pGso    = (PCPDMNETWORKGSO)pSgBuf->pvUser;
    413414            /* Do not attempt to segment frames with invalid GSO parameters. */
    414             if (PDMNetGsoIsValid((const PDMNETWORKGSO *)pGso, sizeof(*pGso), pSgBuf->cbUsed))
     415            PCPDMNETWORKGSO pGso = (PCPDMNETWORKGSO)pSgBuf->pvUser;
     416            if (PDMNetGsoIsValid(pGso, sizeof(*pGso), pSgBuf->cbUsed))
    415417            {
    416418                uint32_t const cSegs = PDMNetGsoCalcSegmentCount(pGso, pSgBuf->cbUsed);
    417419                Assert(cSegs > 1);
    418                 for (uint32_t iSeg = 0; iSeg < cSegs; iSeg++)
     420                uint8_t * const pbSeg = (uint8_t *)RTMemAlloc(DRVNAT_MAXFRAMESIZE); /** @todo r=bird: we could use a stack buffer here... */
     421                if (pbSeg)
    419422                {
    420                     void  *pvSeg;
    421                     pvSeg = RTMemAlloc(DRVNAT_MAXFRAMESIZE);
    422 
    423                     uint32_t cbPayload, cbHdrs;
    424                     uint32_t offPayload = PDMNetGsoCarveSegment(pGso, pbFrame, pSgBuf->cbUsed,
    425                                                                 iSeg, cSegs, (uint8_t *)pvSeg, &cbHdrs, &cbPayload);
    426                     memcpy((uint8_t *)pvSeg + cbHdrs, pbFrame + offPayload, cbPayload);
    427 
    428                     Assert((size_t)cbPayload > 0 && (size_t)cbHdrs > 0);
    429                     slirp_input(pThis->pNATState->pSlirp, (uint8_t const *)pvSeg, cbPayload + cbHdrs);
    430                     RTMemFree(pvSeg);
     423                    uint8_t const * const pbFrame = (uint8_t const *)pSgBuf->aSegs[0].pvSeg;
     424                    LogFlowFunc(("GSO %p LB %#zx - creating %u segments out of it\n", pbFrame, pSgBuf->cbUsed, cSegs));
     425                    for (uint32_t iSeg = 0; iSeg < cSegs; iSeg++)
     426                    {
     427                        uint32_t cbPayload, cbHdrs;
     428                        uint32_t offPayload = PDMNetGsoCarveSegment(pGso, pbFrame, pSgBuf->cbUsed,
     429                                                                    iSeg, cSegs, pbSeg, &cbHdrs, &cbPayload);
     430                        Assert(cbHdrs > 0);
     431                        Assert(cbHdrs < DRVNAT_MAXFRAMESIZE);
     432                        Assert(cbPayload > 0);
     433                        Assert(cbPayload < DRVNAT_MAXFRAMESIZE);
     434                        AssertBreak((uint64_t)cbHdrs + cbPayload <= DRVNAT_MAXFRAMESIZE);
     435
     436                        memcpy(&pbSeg[cbHdrs], &pbFrame[offPayload], cbPayload);
     437
     438                        slirp_input(pThis->pNATState->pSlirp, pbSeg, (int)(cbPayload + cbHdrs));
     439                    }
     440                    RTMemFree(pbSeg);
    431441                }
     442                else
     443                    AssertFailed();
    432444            }
    433445        }
    … …  
    721733
    722734    /* The first polling entry is for the control/wakeup pipe. */
     735    /** @todo r=bird: Either do this manually or use drvNAT_AddPollCb(), don't do
     736     *        both without a comment like HACK ALERT or something... (The causual
     737     *        reader would think drvNAT_AddPollCb has a different function than
     738     *        the code here.) */
    723739#ifdef RT_OS_WINDOWS
    724740    drvNAT_AddPollCb(pThis->ahWakeupSockPair[1], SLIRP_POLL_IN | SLIRP_POLL_HUP, pThis);
    … …  
    726742#else
    727743    unsigned int cPollNegRet = 0;
    728     RTHCINTPTR i64NativeReadPipe = RTPipeToNative(pThis->hPipeRead);
    729     Assert(i64NativeReadPipe < INT_MAX);
    730     drvNAT_AddPollCb(i64NativeReadPipe, SLIRP_POLL_IN | SLIRP_POLL_HUP, pThis);
    731     pThis->pNATState->polls[0].fd = i64NativeReadPipe;
     744    RTHCINTPTR const i64NativeReadPipe = RTPipeToNative(pThis->hPipeRead);
     745    int const        fdNativeReadPipe  = (int)i64NativeReadPipe;
     746    Assert(fdNativeReadPipe == i64NativeReadPipe); Assert(fdNativeReadPipe >= 0);
     747    drvNAT_AddPollCb(fdNativeReadPipe, SLIRP_POLL_IN | SLIRP_POLL_HUP, pThis);
     748    pThis->pNATState->polls[0].fd = fdNativeReadPipe;
    732749    pThis->pNATState->polls[0].events = POLLRDNORM | POLLPRI | POLLRDBAND;
    733750    pThis->pNATState->polls[0].revents = 0;
    … …  
    750767         * To prevent concurrent execution of sending/receiving threads
    751768         */
    752 
    753         int i32Timeout = DRVNAT_DEFAULT_TIMEOUT;
    754769        pThis->pNATState->nsock = 1;
    755770
    756         slirp_pollfds_fill(pThis->pNATState->pSlirp, &i32Timeout, drvNAT_AddPollCb /* SlirpAddPollCb */, pThis /* opaque */);
    757         drvNAT_UpdateTimeout(&i32Timeout, pThis);
     771        int cMsTimeout = DRVNAT_DEFAULT_TIMEOUT;
     772        slirp_pollfds_fill(pThis->pNATState->pSlirp, &cMsTimeout, drvNAT_AddPollCb /* SlirpAddPollCb */, pThis /* opaque */);
     773        cMsTimeout = drvNATTimersAdjustTimeoutDown(pThis, cMsTimeout);
    758774
    759775#ifdef RT_OS_WINDOWS
    760         int cChangedFDs = WSAPoll(pThis->pNATState->polls, pThis->pNATState->nsock, i32Timeout /* timeout */);
    761         /* Note: This must be called IMMEDIATELY after WSAPoll. */
    762         int error = WSAGetLastError();
     776        int cChangedFDs = WSAPoll(pThis->pNATState->polls, pThis->pNATState->nsock, cMsTimeout);
    763777#else
    764         int cChangedFDs = poll(pThis->pNATState->polls, pThis->pNATState->nsock, i32Timeout /* timeout */);
     778        int cChangedFDs = poll(pThis->pNATState->polls, pThis->pNATState->nsock, cMsTimeout);
    765779#endif
    766780        if (cChangedFDs < 0)
    767781        {
    768782#ifdef RT_OS_WINDOWS
    769             LogRel(("NAT: RTWinPoll returned error=%Rrc (cChangedFDs=%d)\n", error, cChangedFDs));
     783            int const iLastErr = WSAGetLastError(); /* (In debug builds LogRel translates to two RTLogLoggerExWeak calls.) */
     784            LogRel(("NAT: RTWinPoll returned error=%Rrc (cChangedFDs=%d)\n", iLastErr, cChangedFDs));
    770785            Log4(("NAT: NSOCK = %d\n", pThis->pNATState->nsock));
    771786#else
    … …  
    797812        if (pThis->pNATState->polls[0].revents & (POLLRDNORM|POLLPRI|POLLRDBAND))   /* POLLPRI won't be seen with WSAPoll. */
    798813        {
    799             char ch[1024];
     814            char achBuf[1024];
    800815            size_t cbRead;
    801816            uint64_t cbWakeupNotifs = ASMAtomicReadU64(&pThis->cbWakeupNotifs);
    802817#ifdef RT_OS_WINDOWS
    803             cbRead = recv(pThis->ahWakeupSockPair[1], &ch[0], RT_MIN(cbWakeupNotifs, 1024), NULL);
     818            /** @todo r=bird: This returns -1 (SOCKET_ERROR) on failure, so any kind of
     819             *        error return here and we'll bugger up cbWakeupNotifs! */
     820            cbRead = recv(pThis->ahWakeupSockPair[1], &achBuf[0], RT_MIN(cbWakeupNotifs, sizeof(achBuf)), NULL);
    804821#else
    805             RTPipeRead(pThis->hPipeRead, &ch[0], RT_MIN(cbWakeupNotifs, 1024), &cbRead);
     822            /** @todo r=bird: cbRead may in theory be used uninitialized here!  This
     823             *        isn't blocking, though, so we won't get stuck here if we mess up
     824             *         the count. */
     825            RTPipeRead(pThis->hPipeRead, &achBuf[0], RT_MIN(cbWakeupNotifs, sizeof(achBuf)), &cbRead);
    806826#endif
    807827            ASMAtomicSubU64(&pThis->cbWakeupNotifs, cbRead);
    … …  
    810830        /* process _all_ outstanding requests but don't wait */
    811831        RTReqQueueProcess(pThis->hSlirpReqQueue, 0);
    812         drvNAT_CheckTimeout(pThis);
     832        drvNATTimersRunExpired(pThis);
    813833    }
    814834
    … …  
    10761096 * Libslirp Utility Functions
    10771097 */
    1078 /**
    1079  * Update the timeout field in given list of Slirp timers.
    1080  *
    1081  * @param i32Timeout  Pointer to timeout value.
    1082  * @param opaque    Pointer to NAT State context.
    1083  *
    1084  * @thread  ?
    1085  */
    1086 static void drvNAT_UpdateTimeout(int *i32Timeout, void *opaque)
    1087 {
    1088     PDRVNAT pThis = (PDRVNAT)opaque;
    1089     Assert(pThis);
    1090 
    1091     int64_t currTime = drvNAT_ClockGetNsCb(pThis) / (1000 * 1000);
    1092     SlirpTimer *pCurrent = pThis->pNATState->pTimerHead;
     1098
     1099/**
     1100 * Reduce the given timeout to match the earliest timer deadline.
     1101 *
     1102 * @returns Updated cMsTimeout value.
     1103 * @param   pThis       Pointer to NAT State context.
     1104 * @param   cMsTimeout  The timeout to adjust, in milliseconds.
     1105 *
     1106 * @thread  pSlirpThread
     1107 */
     1108static int drvNATTimersAdjustTimeoutDown(PDRVNAT pThis, int cMsTimeout)
     1109{
     1110    /** @todo r=bird: This and a most other stuff would be easier if msExpire was
     1111     *                unsigned and we used UINT64_MAX for stopped timers.  */
     1112    /** @todo The timer code isn't thread safe, it assumes a single user thread
     1113     *        (pSlirpThread). */
     1114
     1115    /* Find the first (lowest) deadline. */
     1116    int64_t msDeadline = INT64_MAX;
     1117    for (SlirpTimer *pCurrent = pThis->pNATState->pTimerHead; pCurrent; pCurrent = pCurrent->next)
     1118        if (pCurrent->msExpire < msDeadline && pCurrent->msExpire > 0)
     1119            msDeadline = pCurrent->msExpire;
     1120
     1121    /* Adjust the timeout if there is a timer with a deadline. */
     1122    if (msDeadline < INT64_MAX)
     1123    {
     1124        int64_t const msNow = drvNAT_ClockGetNsCb(pThis) / RT_NS_1MS;
     1125        if (msNow < msDeadline)
     1126        {
     1127            int64_t cMilliesToDeadline = msDeadline - msNow;
     1128            if (cMilliesToDeadline < cMsTimeout)
     1129                cMsTimeout = (int)cMilliesToDeadline;
     1130        }
     1131        else
     1132            cMsTimeout = 0;
     1133    }
     1134
     1135    return cMsTimeout;
     1136}
     1137
     1138/**
     1139 * Run expired timers.
     1140 *
     1141 * @param   opaque  Pointer to NAT State context.
     1142 *
     1143 * @thread  pSlirpThread
     1144 */
     1145static void drvNATTimersRunExpired(PDRVNAT pThis)
     1146{
     1147    int64_t const msNow    = drvNAT_ClockGetNsCb(pThis) / RT_NS_1MS;
     1148    SlirpTimer   *pCurrent = pThis->pNATState->pTimerHead;
    10931149    while (pCurrent != NULL)
    10941150    {
    1095         if (pCurrent->uTimeExpire != 0)
     1151        SlirpTimer * const pNext = pCurrent->next; /* (in case the timer is destroyed from the callback) */
     1152        if (pCurrent->msExpire <= msNow && pCurrent->msExpire > 0)
    10961153        {
    1097             int64_t diff = pCurrent->uTimeExpire - currTime;
    1098 
    1099             if (diff < 0)
    1100                 diff = 0;
    1101 
    1102             if (diff < *i32Timeout)
    1103                 *i32Timeout = RT_MIN(diff, INT_MAX);
     1154            pCurrent->msExpire = 0;
     1155            pCurrent->pHandler(pCurrent->opaque);
    11041156        }
    1105 
    1106         pCurrent = pCurrent->next;
    1107     }
    1108 }
    1109 
    1110 /**
    1111  * Check if timeout has passed in given list of Slirp timers.
    1112  *
    1113  * @param   opaque  Pointer to NAT State context.
    1114  *
    1115  * @thread  ?
    1116  */
    1117 static void drvNAT_CheckTimeout(void *opaque)
    1118 {
    1119     PDRVNAT pThis = (PDRVNAT)opaque;
    1120     Assert(pThis);
    1121 
    1122     int64_t currTime = drvNAT_ClockGetNsCb(pThis) / (1000 * 1000);
    1123     SlirpTimer *pCurrent = pThis->pNATState->pTimerHead;
    1124     while (pCurrent != NULL)
    1125     {
    1126         if (pCurrent->uTimeExpire != 0)
    1127         {
    1128             int64_t diff = pCurrent->uTimeExpire - currTime;
    1129             if (diff <= 0)
    1130             {
    1131                 pCurrent->uTimeExpire = 0;
    1132                 pCurrent->pHandler(pCurrent->opaque);
    1133             }
    1134         }
    1135 
    1136         pCurrent = pCurrent->next;
     1157        pCurrent = pNext;
    11371158    }
    11381159}
    … …  
    11441165 *
    11451166 * @returns Integer representing host type poll events.
    1146  *
    1147  * @thread  ?
    1148  */
    1149 static short drvNAT_PollEventSlirpToHost(int iEvents) {
     1167 */
     1168static short drvNAT_PollEventSlirpToHost(int iEvents)
     1169{
    11501170    short iRet = 0;
    11511171#ifndef RT_OS_WINDOWS
    … …  
    12841304    Assert(pThis);
    12851305
    1286     SlirpTimer *pNewTimer = (SlirpTimer *)RTMemAlloc(sizeof(SlirpTimer));
    1287     if (!pNewTimer)
    1288         return NULL;
    1289 
    1290     pNewTimer->next = pThis->pNATState->pTimerHead;
    1291     pNewTimer->uTimeExpire = 0;
    1292     pNewTimer->pHandler = slirpTimeCb;
    1293     pNewTimer->opaque = cb_opaque;
    1294     pThis->pNATState->pTimerHead = pNewTimer;
    1295 
     1306    SlirpTimer * const pNewTimer = (SlirpTimer *)RTMemAlloc(sizeof(SlirpTimer));
     1307    if (pNewTimer)
     1308    {
     1309        pNewTimer->msExpire = 0;
     1310        pNewTimer->pHandler = slirpTimeCb;
     1311        pNewTimer->opaque = cb_opaque;
     1312        /** @todo r=bird: Not thread safe. Assumes pSlirpThread */
     1313        pNewTimer->next = pThis->pNATState->pTimerHead;
     1314        pThis->pNATState->pTimerHead = pNewTimer;
     1315    }
    12961316    return pNewTimer;
    12971317}
    … …  
    13001320 * Callback called by slirp to free a timer.
    13011321 *
    1302  * @param   pTimer  Pointer to slirpTimer object to be freed.
     1322 * @param   pvTimer Pointer to slirpTimer object to be freed.
     1323 * @param   pvUser  Pointer to NAT State context.
     1324 */
     1325static DECLCALLBACK(void) drvNAT_TimerFreeCb(void *pvTimer, void *pvUser)
     1326{
     1327    PDRVNAT const      pThis  = (PDRVNAT)pvUser;
     1328    SlirpTimer * const pTimer = (SlirpTimer *)pvTimer;
     1329    Assert(pThis);
     1330    /** @todo r=bird: Not thread safe. Assumes pSlirpThread */
     1331
     1332    SlirpTimer *pPrev    = NULL;
     1333    SlirpTimer *pCurrent = pThis->pNATState->pTimerHead;
     1334    while (pCurrent != NULL)
     1335    {
     1336        if (pCurrent == pTimer)
     1337        {
     1338            /* unlink it. */
     1339            if (!pPrev)
     1340                pThis->pNATState->pTimerHead = pCurrent->next;
     1341            else
     1342                pPrev->next                  = pCurrent->next;
     1343            pCurrent->next = NULL;
     1344            RTMemFree(pCurrent);
     1345            return;
     1346        }
     1347
     1348        /* advance */
     1349        pPrev = pCurrent;
     1350        pCurrent = pCurrent->next;
     1351    }
     1352    Assert(!pTimer);
     1353}
     1354
     1355/**
     1356 * Callback called by slirp to modify a timer.
     1357 *
     1358 * @param   pvTimer         Pointer to slirpTimer object to be modified.
     1359 * @param   msNewDeadlineTs The new absolute expiration time in milliseconds.
     1360 *                          Zero stops it.
     1361 * @param   pvUser          Pointer to NAT State context.
     1362 */
     1363static DECLCALLBACK(void) drvNAT_TimerModCb(void *pvTimer, int64_t msNewDeadlineTs, void *pvUser)
     1364{
     1365    SlirpTimer * const pTimer = (SlirpTimer *)pvTimer;
     1366    /** @todo r=bird: ASSUMES pSlirpThread, otherwise it may need to be woken up! */
     1367    pTimer->msExpire = msNewDeadlineTs;
     1368    RT_NOREF(pvUser);
     1369}
     1370
     1371/**
     1372 * Callback called by slirp when there is I/O that needs to happen.
     1373 *
    13031374 * @param   opaque  Pointer to NAT State context.
    13041375 */
    1305 static DECLCALLBACK(void) drvNAT_TimerFreeCb(void *pTimer, void *opaque)
     1376static DECLCALLBACK(void) drvNAT_NotifyCb(void *opaque)
    13061377{
    13071378    PDRVNAT pThis = (PDRVNAT)opaque;
    1308     Assert(pThis);
    1309     SlirpTimer *pCurrent = pThis->pNATState->pTimerHead;
    1310 
    1311     while (pCurrent != NULL)
    1312     {
    1313         if (pCurrent == (SlirpTimer *)pTimer)
    1314         {
    1315             SlirpTimer *pTmp = pCurrent->next;
    1316             RTMemFree(pCurrent);
    1317             pCurrent = pTmp;
    1318         }
    1319         else
    1320             pCurrent = pCurrent->next;
    1321     }
    1322 }
    1323 
    1324 /**
    1325  * Callback called by slirp to modify a timer.
    1326  *
    1327  * @param   pTimer      Pointer to slirpTimer object to be modified.
    1328  * @param   expireTime  Signed 64-bit integer representing the new expiry time.
    1329  * @param   opaque      Pointer to NAT State context.
    1330  */
    1331 static DECLCALLBACK(void) drvNAT_TimerModCb(void *pTimer, int64_t expireTime, void *opaque)
    1332 {
    1333     PDRVNAT pThis = (PDRVNAT)opaque;
    1334     Assert(pThis);
    1335 
    1336     RT_NOREF(pThis);
    1337 
    1338     ((SlirpTimer *)pTimer)->uTimeExpire = expireTime;
    1339 }
    1340 
    1341 /**
    1342  * Callback called by slirp when there is I/O that needs to happen.
    1343  *
    1344  * @param   opaque  Pointer to NAT State context.
    1345  */
    1346 static DECLCALLBACK(void) drvNAT_NotifyCb(void *opaque)
    1347 {
    1348     PDRVNAT pThis = (PDRVNAT)opaque;
    1349 
    1350     drvNATAsyncIoWakeup(pThis->pDrvIns, NULL);
     1379    drvNATNotifyNATThread(pThis, "drvNAT_NotifyCb");
    13511380}
    13521381
    … …  
    13571386{
    13581387    RT_NOREF(fd, opaque);
    1359     Log4(("Poll registered\n"));
     1388    Log4(("Poll registered: fd=%d\n", fd));
    13601389}
    13611390
    … …  
    13661395{
    13671396    RT_NOREF(fd, opaque);
    1368     Log4(("Poll unregistered\n"));
     1397    Log4(("Poll unregistered: fd=%d\n", fd));
    13691398}
    13701399
Note: See TracChangeset for help on using the changeset viewer.

© 2024 Oracle Support Privacy / Do Not Sell My Info Terms of Use Trademark Policy Automated Access Etiquette