Skip to content

Commit

Permalink
netkvm: use per-queue max RX buffers instead of global
Browse files Browse the repository at this point in the history
Global value of max RX buffers, shared among queues, is
bad idea. It is especially bad when memory allocation
during queue initialization may fail, for example in case
of driver running under driver verifier. Changing the
global field to per-queue field.

Signed-off-by: Yuri Benditovich <[email protected]>
  • Loading branch information
ybendito committed Dec 1, 2024
1 parent 9991311 commit 359de33
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 18 deletions.
8 changes: 5 additions & 3 deletions NetKVM/Common/ParaNdis-RX.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,16 @@ class CParaNdisRX : public CParaNdisTemplatePath<CVirtQueue>, public CNdisAlloca
void KickRXRing();

PARANDIS_RECEIVE_QUEUE &UnclassifiedPacketsQueue() { return m_UnclassifiedPacketsQueue; }

UINT GetFreeRxBuffers() const { return m_NetNofReceiveBuffers; }
private:
/* list of Rx buffers available for data (under VIRTIO management) */
LIST_ENTRY m_NetReceiveBuffers;
UINT m_NetNofReceiveBuffers;
UINT m_NetNofReceiveBuffers = 0;
UINT m_NetMaxReceiveBuffers = 0;
UINT m_MinRxBufferLimit;

UINT m_nReusedRxBuffersCounter, m_nReusedRxBuffersLimit = 0;
UINT m_nReusedRxBuffersCounter = 0;
UINT m_nReusedRxBuffersLimit = 0;

bool m_Reinsert = true;

Expand Down
2 changes: 1 addition & 1 deletion NetKVM/Common/ParaNdis_Common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ static void ReadNicConfiguration(PARANDIS_ADAPTER *pContext, PUCHAR pNewMACAddre
virtioDebugLevel = pConfiguration->debugLevel.ulValue;
pContext->physicalMediaType = (NDIS_PHYSICAL_MEDIUM)pConfiguration->PhysicalMediaType.ulValue;
pContext->maxFreeTxDescriptors = pConfiguration->TxCapacity.ulValue;
pContext->NetMaxReceiveBuffers = pConfiguration->RxCapacity.ulValue;
pContext->maxRxBufferPerQueue = pConfiguration->RxCapacity.ulValue;
pContext->uNumberOfHandledRXPacketsInDPC = pConfiguration->NumberOfHandledRXPacketsInDPC.ulValue;
pContext->bDoSupportPriority = pConfiguration->PrioritySupport.ulValue != 0;
pContext->Offload.flagsValue = 0;
Expand Down
10 changes: 9 additions & 1 deletion NetKVM/Common/ParaNdis_Oid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,16 @@ NDIS_STATUS ParaNdis_OidQueryCommon(PARANDIS_ADAPTER *pContext, tOidDesc *pOid)
break;
}
case OID_GEN_RECEIVE_BUFFER_SPACE:
SETINFO(ul, pContext->MaxPacketSize.nMaxFullSizeOsRx * pContext->NetMaxReceiveBuffers);
{
ULONG totalRxBuffer = 0;
for (UINT i = 0; i < pContext->nPathBundles; i++)
{
totalRxBuffer += pContext->pPathBundles[i].rxPath.GetFreeRxBuffers();
}

SETINFO(ul, pContext->MaxPacketSize.nMaxFullSizeOsRx * totalRxBuffer);
break;
}
case OID_GEN_RECEIVE_BLOCK_SIZE:
SETINFO(ul, pContext->MaxPacketSize.nMaxFullSizeOsRx);
break;
Expand Down
22 changes: 11 additions & 11 deletions NetKVM/Common/ParaNdis_RX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ static void ParaNdis_FreeRxBufferDescriptor(PARANDIS_ADAPTER *pContext, pRxNetDe
NdisFreeMemory(p, 0, 0);
}

CParaNdisRX::CParaNdisRX() : m_nReusedRxBuffersCounter(0), m_NetNofReceiveBuffers(0)
CParaNdisRX::CParaNdisRX()
{
InitializeListHead(&m_NetReceiveBuffers);
}
Expand All @@ -85,6 +85,7 @@ bool CParaNdisRX::Create(PPARANDIS_ADAPTER Context, UINT DeviceQueueIndex)
{
m_Context = Context;
m_queueIndex = (u16)DeviceQueueIndex;
m_NetMaxReceiveBuffers = Context->maxRxBufferPerQueue;

if (!m_VirtQueue.Create(DeviceQueueIndex,
&m_Context->IODevice,
Expand All @@ -96,7 +97,7 @@ bool CParaNdisRX::Create(PPARANDIS_ADAPTER Context, UINT DeviceQueueIndex)

PrepareReceiveBuffers();

m_nReusedRxBuffersLimit = m_Context->NetMaxReceiveBuffers / 4 + 1;
m_nReusedRxBuffersLimit = m_NetMaxReceiveBuffers / 4 + 1;

CreatePath();

Expand All @@ -109,7 +110,7 @@ int CParaNdisRX::PrepareReceiveBuffers()
UINT i;
DEBUG_ENTRY(4);

for (i = 0; i < m_Context->NetMaxReceiveBuffers; ++i)
for (i = 0; i < m_NetMaxReceiveBuffers; ++i)
{
pRxNetDescriptor pBuffersDescriptor = CreateRxDescriptorOnInit();
if (!pBuffersDescriptor) break;
Expand All @@ -126,10 +127,9 @@ int CParaNdisRX::PrepareReceiveBuffers()

m_NetNofReceiveBuffers++;
}
/* TODO - NetMaxReceiveBuffers should take into account all queues */
m_Context->NetMaxReceiveBuffers = m_NetNofReceiveBuffers;
m_NetMaxReceiveBuffers = m_NetNofReceiveBuffers;
m_MinRxBufferLimit = m_NetNofReceiveBuffers * m_Context->MinRxBufferPercent / 100;
DPrintf(0, "[%s] MaxReceiveBuffers %d, m_MinRxBufferLimit %u\n", __FUNCTION__, m_Context->NetMaxReceiveBuffers, m_MinRxBufferLimit);
DPrintf(0, "[%s] m_NetMaxReceiveBuffers %d, m_MinRxBufferLimit %u\n", __FUNCTION__, m_NetMaxReceiveBuffers, m_MinRxBufferLimit);
if (m_Context->extraStatistics.minFreeRxBuffers == 0 || m_Context->extraStatistics.minFreeRxBuffers > m_NetNofReceiveBuffers)
{
m_Context->extraStatistics.minFreeRxBuffers = m_NetNofReceiveBuffers;
Expand Down Expand Up @@ -240,10 +240,10 @@ void CParaNdisRX::ReuseReceiveBufferNoLock(pRxNetDescriptor pBuffersDescriptor)
InsertTailList(&m_NetReceiveBuffers, &pBuffersDescriptor->listEntry);
m_NetNofReceiveBuffers++;

if (m_NetNofReceiveBuffers > m_Context->NetMaxReceiveBuffers)
if (m_NetNofReceiveBuffers > m_NetMaxReceiveBuffers)
{
DPrintf(0, " Error: NetNofReceiveBuffers > NetMaxReceiveBuffers(%d>%d)\n",
m_NetNofReceiveBuffers, m_Context->NetMaxReceiveBuffers);
DPrintf(0, " Error: m_NetNofReceiveBuffers > m_NetMaxReceiveBuffers (%d>%d)\n",
m_NetNofReceiveBuffers, m_NetMaxReceiveBuffers);
}

/* TODO - nReusedRXBuffers per queue or per context ?*/
Expand All @@ -258,7 +258,7 @@ void CParaNdisRX::ReuseReceiveBufferNoLock(pRxNetDescriptor pBuffersDescriptor)
/* TODO - NetMaxReceiveBuffers per queue or per context ?*/
DPrintf(0, "FAILED TO REUSE THE BUFFER!!!!\n");
ParaNdis_FreeRxBufferDescriptor(m_Context, pBuffersDescriptor);
m_Context->NetMaxReceiveBuffers--;
m_NetMaxReceiveBuffers--;
}
}

Expand Down Expand Up @@ -538,7 +538,7 @@ void CParaNdisRX::PopulateQueue()
/* TODO - NetMaxReceiveBuffers should take into account all queues */
DPrintf(0, "FAILED TO REUSE THE BUFFER!!!!\n");
ParaNdis_FreeRxBufferDescriptor(m_Context, pBufferDescriptor);
m_Context->NetMaxReceiveBuffers--;
m_NetMaxReceiveBuffers--;
}
}
m_Reinsert = true;
Expand Down
2 changes: 1 addition & 1 deletion NetKVM/Common/ndis56common.h
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ struct _PARANDIS_ADAPTER : public CNdisAllocatable<_PARANDIS_ADAPTER, 'DCTX'>
/* initial number of free Tx descriptor(from cfg) - max number of available Tx descriptors */
UINT maxFreeTxDescriptors = 0;
/* total of Rx buffer in turnaround */
UINT NetMaxReceiveBuffers = 0;
UINT maxRxBufferPerQueue = 0;
UINT nPnpEventIndex = 0;
NDIS_DEVICE_PNP_EVENT PnpEvents[16] = {};
tOffloadSettings Offload = {};
Expand Down
2 changes: 1 addition & 1 deletion NetKVM/wlh/ParaNdis6_Oid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ static NDIS_STATUS ParaNdis_OidQuery(PARANDIS_ADAPTER *pContext, tOidDesc *pOid)
pInfo = &u.WmiConfig;
ulSize = sizeof(u.WmiConfig);
u.WmiConfig.NumOfQueues = pContext->nPathBundles;
u.WmiConfig.RxQueueSize = pContext->NetMaxReceiveBuffers;
u.WmiConfig.RxQueueSize = pContext->maxRxBufferPerQueue;
u.WmiConfig.TxQueueSize = pContext->pPathBundles[0].txPath.GetActualQueueSize();
u.WmiConfig.RscEnabledv4 = pContext->ReportedOffloadConfiguration.Rsc.IPv4.Enabled;
u.WmiConfig.RscEnabledv6 = pContext->ReportedOffloadConfiguration.Rsc.IPv6.Enabled;
Expand Down

0 comments on commit 359de33

Please sign in to comment.