vbox: avoid blocking nic_ep thread

during receive the nic_ep may block as long as the guest does not provide
another receive network descriptor. In the meantime, all Genode signals
regarding the network interface, e.g. tx, will be postponed, which may
effect the throughput.

Instead use the nic_ep for rx packets unblocking. Add an notification mechanism
to the e1000 vbox network model, to notify us as soon as the guest added new
receive descriptors in the model.

Issue #5146
This commit is contained in:
Alexander Boettcher 2024-01-31 17:31:23 +01:00 committed by Christian Helmuth
parent e1e87657c7
commit 23078154cd
6 changed files with 173 additions and 19 deletions

View File

@ -1 +1 @@
9d3de38fdb7c95517c4ce76dbd090a147fa7b79c
4df67ce24bce24be1c0171bfb9eef96a42e95c6f

View File

@ -1 +1 @@
a1376cc9dbcced9ff29f37da20ddc54363e24f3d
4f4c05a80b5767a0132c333a352fada6ba8965a8

View File

@ -71,6 +71,8 @@ typedef struct DRVNIC
PPDMDRVINS pDrvIns;
/** Transmit lock used by pfnBeginXmit/pfnEndXmit. */
RTCRITSECT XmitLock;
/** Receive lock used by nic_ep and EMT-0..X */
RTCRITSECT RecvLock;
/** Nic::Session client wrapper. */
Nic_client *nic_client;
} DRVNIC, *PDRVNIC;
@ -120,31 +122,57 @@ class Nic_client
PDRVNIC _drvnic;
void _handle_rx_packet_avail()
{
int const rc = RTCritSectEnter(&_drvnic->RecvLock);
AssertRC(rc);
_handle_rx_packet_avail_unlocked();
RTCritSectLeave(&_drvnic->RecvLock);
}
void _handle_rx_packet_avail_unlocked()
{
auto &rx = *_nic.rx();
bool progress = false;
bool unready = false;
while (rx.packet_avail() && rx.ready_to_ack()) {
Libc::with_libc([&] () {
RTMSINTERVAL const wait_ms = 0; /* try once without blocking */
int rc = _down_net->pfnWaitReceiveAvail(_down_net, wait_ms);
if (rc != VINF_SUCCESS)
unready = true;
});
/* network model of VBox can't accept a new packet at moment */
if (unready)
break;
auto const rx_packet = rx.try_get_packet();
if (!rx_packet.size())
if (!rx_packet.size()) {
RTLogPrintf("unexpected - should not happen - size 0\n");
break;
}
char *rx_content = rx.packet_content(rx_packet);
if (!rx_content)
if (!rx_content) {
RTLogPrintf("unexpected - should not happen - no content\n");
break;
}
Libc::with_libc([&] () {
int rc = _down_net->pfnWaitReceiveAvail(_down_net, RT_INDEFINITE_WAIT);
if (RT_FAILURE(rc)) return;
int rc = _down_net->pfnReceive(_down_net, rx_content,
rx_packet.size());
rc = _down_net->pfnReceive(_down_net, rx_content, rx_packet.size());
AssertRC(rc);
if (rx.try_ack_packet(rx_packet))
if (rc == VINF_SUCCESS && rx.try_ack_packet(rx_packet))
progress = true;
else
RTLogPrintf("unexpected - should not happen - ack packet\n");
});
}
@ -352,6 +380,16 @@ class Nic_client
_tx_wakeup = false;
_nic.tx()->wakeup();
}
void rx_resume()
{
int const rc = RTCritSectEnter(&_drvnic->RecvLock);
AssertRC(rc);
_handle_rx_packet_avail_unlocked();
RTCritSectLeave(&_drvnic->RecvLock);
}
};
@ -604,10 +642,23 @@ static DECLCALLBACK(void) drvNicDestruct(PPDMDRVINS pDrvIns)
/* wait until the recv thread exits */
destruct_blockade().block();
if (nic_client)
destroy(vmm_heap(), nic_client);
if (RTCritSectIsInitialized(&pThis->XmitLock))
RTCritSectDelete(&pThis->XmitLock);
destroy(vmm_heap(), nic_client);
if (RTCritSectIsInitialized(&pThis->RecvLock))
RTCritSectDelete(&pThis->RecvLock);
}
static DECLCALLBACK(void) drvNicNetworkUp_ReceiveReady(PPDMINETWORKUP pInterface)
{
PDRVNIC const pThis = PDMINETWORKUP_2_DRVNIC(pInterface);
Nic_client * const nic_client = pThis->nic_client;
nic_client->rx_resume();
}
@ -636,10 +687,12 @@ static DECLCALLBACK(int) drvNicConstruct(PPDMDRVINS pDrvIns, PCFGMNODE pCfg, uin
pThis->INetworkUp.pfnEndXmit = drvNicNetworkUp_EndXmit;
pThis->INetworkUp.pfnSetPromiscuousMode = drvNicNetworkUp_SetPromiscuousMode;
pThis->INetworkUp.pfnNotifyLinkChanged = drvNicNetworkUp_NotifyLinkChanged;
pThis->INetworkUp.pfnReceiveReady = drvNicNetworkUp_ReceiveReady;
/* INetworkConfig - used on Genode to request Mac address of nic_session */
pThis->INetworkConfig.pfnGetMac = drvGetMac;
RTCritSectInit(&pThis->XmitLock);
RTCritSectInit(&pThis->RecvLock);
/*
* Check that no-one is attached to us.

View File

@ -4,6 +4,19 @@ diff --git a/src/app/virtualbox/src/VBox/Devices/Network/DevE1000.cpp b/src/app/
index aa3eb87..fc2660a 100644
--- a/src/app/virtualbox/src/VBox/Devices/Network/DevE1000.cpp
+++ b/src/app/virtualbox/src/VBox/Devices/Network/DevE1000.cpp
@@ -1614,6 +1614,12 @@
static void e1kWakeupReceive(PPDMDEVINS pDevIns)
{
PE1KSTATE pThis = PDMINS_2_DATA(pDevIns, PE1KSTATE);
+
+ if (pThis->pDrvR3 && pThis->pDrvR3->pfnReceiveReady) {
+ pThis->pDrvR3->pfnReceiveReady(pThis->pDrvR3);
+ return;
+ }
+
if ( pThis->fMaybeOutOfSpace
&& pThis->hEventMoreRxDescAvail != NIL_RTSEMEVENT)
{
@@ -7629,7 +7629,7 @@
return PDMDEV_SET_ERROR(pDevIns, rc,
N_("Configuration error: Failed to get the value of 'EthernetCRC'"));
@ -94,3 +107,14 @@ index 481267e..80f4af9 100644
if (rc == VINF_NAT_DNS)
{
#ifdef RT_OS_LINUX
--- a/src/app/virtualbox/include/VBox/vmm/pdmnetifs.h
+++ b/src/app/virtualbox/include/VBox/vmm/pdmnetifs.h
@@ -322,6 +322,8 @@
/** @todo Add a callback that informs the driver chain about MAC address changes if we ever implement that. */
+ DECLR3CALLBACKMEMBER(void, pfnReceiveReady,(PPDMINETWORKUP pInterface));
+
} PDMINETWORKUP;
/** Ring-0 edition of PDMINETWORKUP. */

View File

@ -74,6 +74,8 @@ typedef struct DRVNIC
PPDMDRVINS pDrvIns;
/** Transmit lock used by pfnBeginXmit/pfnEndXmit. */
RTCRITSECT XmitLock;
/** Receive lock used by nic_ep and EMT-0..X */
RTCRITSECT RecvLock;
/** Nic::Session client wrapper. */
Nic_client *nic_client;
} DRVNIC, *PDRVNIC;
@ -136,31 +138,57 @@ class Nic_client
PDRVNIC _drvnic;
void _handle_rx_packet_avail()
{
int const rc = RTCritSectEnter(&_drvnic->RecvLock);
AssertRC(rc);
_handle_rx_packet_avail_unlocked();
RTCritSectLeave(&_drvnic->RecvLock);
}
void _handle_rx_packet_avail_unlocked()
{
auto &rx = *_nic.rx();
bool progress = false;
bool unready = false;
while (rx.packet_avail() && rx.ready_to_ack()) {
Libc::with_libc([&] () {
RTMSINTERVAL const wait_ms = 0; /* try once without blocking */
int rc = _down_net->pfnWaitReceiveAvail(_down_net, wait_ms);
if (rc != VINF_SUCCESS)
unready = true;
});
/* network model of VBox can't accept a new packet at moment */
if (unready)
break;
auto const rx_packet = rx.try_get_packet();
if (!rx_packet.size())
if (!rx_packet.size()) {
RTLogPrintf("unexpected - should not happen - size 0\n");
break;
}
char *rx_content = rx.packet_content(rx_packet);
if (!rx_content)
if (!rx_content) {
RTLogPrintf("unexpected - should not happen - no content\n");
break;
}
Libc::with_libc([&] () {
int rc = _down_net->pfnWaitReceiveAvail(_down_net, RT_INDEFINITE_WAIT);
if (RT_FAILURE(rc)) return;
int rc = _down_net->pfnReceive(_down_net, rx_content,
rx_packet.size());
rc = _down_net->pfnReceive(_down_net, rx_content, rx_packet.size());
AssertRC(rc);
if (rx.try_ack_packet(rx_packet))
if (rc == VINF_SUCCESS && rx.try_ack_packet(rx_packet))
progress = true;
else
RTLogPrintf("unexpected - should not happen - ack packet\n");
});
}
@ -367,6 +395,16 @@ class Nic_client
_tx_wakeup = false;
_nic.tx()->wakeup();
}
void rx_resume()
{
int const rc = RTCritSectEnter(&_drvnic->RecvLock);
AssertRC(rc);
_handle_rx_packet_avail_unlocked();
RTCritSectLeave(&_drvnic->RecvLock);
}
};
@ -619,6 +657,18 @@ static DECLCALLBACK(void) drvNicDestruct(PPDMDRVINS pDrvIns)
if (RTCritSectIsInitialized(&pThis->XmitLock))
RTCritSectDelete(&pThis->XmitLock);
if (RTCritSectIsInitialized(&pThis->RecvLock))
RTCritSectDelete(&pThis->RecvLock);
}
static DECLCALLBACK(void) drvNicNetworkUp_ReceiveReady(PPDMINETWORKUP pInterface)
{
PDRVNIC const pThis = PDMINETWORKUP_2_DRVNIC(pInterface);
Nic_client * const nic_client = pThis->nic_client;
nic_client->rx_resume();
}
@ -647,10 +697,12 @@ static DECLCALLBACK(int) drvNicConstruct(PPDMDRVINS pDrvIns, PCFGMNODE pCfg, uin
pThis->INetworkUp.pfnEndXmit = drvNicNetworkUp_EndXmit;
pThis->INetworkUp.pfnSetPromiscuousMode = drvNicNetworkUp_SetPromiscuousMode;
pThis->INetworkUp.pfnNotifyLinkChanged = drvNicNetworkUp_NotifyLinkChanged;
pThis->INetworkUp.pfnReceiveReady = drvNicNetworkUp_ReceiveReady;
/* INetworkConfig - used on Genode to request Mac address of nic_session */
pThis->INetworkConfig.pfnGetMac = drvGetMac;
RTCritSectInit(&pThis->XmitLock);
RTCritSectInit(&pThis->RecvLock);
/*
* Check that no-one is attached to us.

View File

@ -2,6 +2,20 @@ diff --git a/src/virtualbox6/src/VBox/Devices/Network/DevE1000.cpp b/src/virtual
index 2c2e366..b45855b 100644
--- a/src/virtualbox6/src/VBox/Devices/Network/DevE1000.cpp
+++ b/src/virtualbox6/src/VBox/Devices/Network/DevE1000.cpp
@@ -1768,6 +1768,13 @@
*/
static void e1kWakeupReceive(PPDMDEVINS pDevIns, PE1KSTATE pThis)
{
+ PE1KSTATECC pThisCC = PDMDEVINS_2_DATA_CC(pDevIns, PE1KSTATECC);
+
+ if (pThisCC->pDrvR3 && pThisCC->pDrvR3->pfnReceiveReady) {
+ pThisCC->pDrvR3->pfnReceiveReady(pThisCC->pDrvR3);
+ return;
+ }
+
if ( pThis->fMaybeOutOfSpace
&& pThis->hEventMoreRxDescAvail != NIL_SUPSEMEVENT)
{
@@ -8001,7 +8001,7 @@
return PDMDEV_SET_ERROR(pDevIns, rc,
N_("Configuration error: Failed to get the value of 'EthernetCRC'"));
@ -93,3 +107,14 @@ index bb69ca7..6d75de0 100644
pThisCC->pDrv = PDMIBASE_QUERY_INTERFACE(pThisCC->pDrvBase, PDMINETWORKUP);
AssertMsgReturn(pThisCC->pDrv, ("Failed to obtain the PDMINETWORKUP interface!\n"),
VERR_PDM_MISSING_INTERFACE_BELOW);
--- a/src/virtualbox6/include/VBox/vmm/pdmnetifs.h
+++ b/src/virtualbox6/include/VBox/vmm/pdmnetifs.h
@@ -325,6 +325,8 @@
/** @todo Add a callback that informs the driver chain about MAC address changes if we ever implement that. */
+ DECLR3CALLBACKMEMBER(void, pfnReceiveReady,(PPDMINETWORKUP pInterface));
+
} PDMINETWORKUP;
/** Ring-0 edition of PDMINETWORKUP. */