Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
NetKVM: CX: Use separate DPC 
This commit separates the CX path handeling to a standalone DPC, the
motive to do this is that sometimes we can miss Control interrupts due to
the way it is implemented today.

The issue is disscussed in great detail here:
#281

Signed-off-by: Sameeh Jubran <sameeh@daynix.com>
  • Loading branch information
@sameehj @YanVugenfirer
sameehj authored and YanVugenfirer committed Sep 17, 2018
1 parent fa1cf3d commit 861e272
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 37 deletions.
6 changes: 6 additions & 0 deletions NetKVM/Common/ParaNdis-CX.cpp
View file
Expand Up @@ -35,12 +35,18 @@ bool CParaNdisCX::Create(PPARANDIS_ADAPTER Context, UINT DeviceQueueIndex)
m_Context->m_CxStateMachine.Start();

CreatePath();
InitDPC();

return m_VirtQueue.Create(DeviceQueueIndex,
&m_Context->IODevice,
m_Context->MiniportHandle);
}

void CParaNdisCX::InitDPC()
{
KeInitializeDpc(&m_DPC, MiniportMSIInterruptCXDpc, m_Context);
}

BOOLEAN CParaNdisCX::SendControlMessage(
UCHAR cls,
UCHAR cmd,
Expand Down
4 changes: 4 additions & 0 deletions NetKVM/Common/ParaNdis-CX.h
View file
Expand Up @@ -12,6 +12,8 @@ class CParaNdisCX : public CParaNdisTemplatePath<CVirtQueue>, public CPlacementA

virtual NDIS_STATUS SetupMessageIndex(u16 vector);

void InitDPC();

BOOLEAN CParaNdisCX::SendControlMessage(
UCHAR cls,
UCHAR cmd,
Expand All @@ -22,6 +24,8 @@ class CParaNdisCX : public CParaNdisTemplatePath<CVirtQueue>, public CPlacementA
int levelIfOK
);

KDPC m_DPC;

protected:
tCompletePhysicalAddress m_ControlData;
};
68 changes: 42 additions & 26 deletions NetKVM/Common/ParaNdis-Common.cpp
View file
Expand Up @@ -1849,7 +1849,40 @@ void ParaNdis_ReuseRxNBLs(PNET_BUFFER_LIST pNBL)
}
}

bool ParaNdis_DPCWorkBody(PARANDIS_ADAPTER *pContext, ULONG ulMaxPacketsToIndicate)
void ParaNdis_CXDPCWorkBody(PARANDIS_ADAPTER *pContext)
{
InterlockedIncrement(&pContext->counterDPCInside);
if (pContext->bEnableInterruptHandlingDPC && pContext->CXPath.WasInterruptReported())
{
UINT8 status = 0;
status = ReadDeviceStatus(pContext);

if (virtio_is_feature_enabled(pContext->u64HostFeatures, VIRTIO_F_VERSION_1) &&
(status & VIRTIO_CONFIG_S_NEEDS_RESET))
{
DPrintf(0, "Received VIRTIO_CONFIG_S_NEEDS_RESET event");
pContext->m_StateMachine.NotifyDeviceNeedsReset();
pContext->bDeviceNeedsReset = TRUE;
}

ReadLinkState(pContext);
if (pContext->bLinkDetectSupported)
{
ReadLinkState(pContext);
ParaNdis_SynchronizeLinkState(pContext);
}
if (pContext->bGuestAnnounceSupported && pContext->bGuestAnnounced)
{
ParaNdis_SendGratuitousArpPacket(pContext);
pContext->CXPath.SendControlMessage(VIRTIO_NET_CTRL_ANNOUNCE, VIRTIO_NET_CTRL_ANNOUNCE_ACK, NULL, 0, NULL, 0, 0);
pContext->bGuestAnnounced = FALSE;
}
pContext->CXPath.ClearInterruptReport();
}
InterlockedDecrement(&pContext->counterDPCInside);
}

bool ParaNdis_RXTXDPCWorkBody(PARANDIS_ADAPTER *pContext, ULONG ulMaxPacketsToIndicate)
{
bool stillRequiresProcessing = false;
UINT numOfPacketsToIndicate = min(ulMaxPacketsToIndicate, pContext->uNumberOfHandledRXPacketsInDPC);
Expand Down Expand Up @@ -1881,31 +1914,6 @@ bool ParaNdis_DPCWorkBody(PARANDIS_ADAPTER *pContext, ULONG ulMaxPacketsToIndica
{
stillRequiresProcessing = true;
}
if (pContext->CXPath.WasInterruptReported())
{
UINT8 status = 0;
status = ReadDeviceStatus(pContext);
if (virtio_is_feature_enabled(pContext->u64HostFeatures, VIRTIO_F_VERSION_1) &&
(status & VIRTIO_CONFIG_S_NEEDS_RESET))
{
DPrintf(0,"Received VIRTIO_CONFIG_S_NEEDS_RESET event");
pContext->m_StateMachine.NotifyDeviceNeedsReset();
pContext->bDeviceNeedsReset = TRUE;
}
ReadLinkState(pContext);
if (pContext->bLinkDetectSupported)
{
ReadLinkState(pContext);
ParaNdis_SynchronizeLinkState(pContext);
}
if (pContext->bGuestAnnounceSupported && pContext->bGuestAnnounced)
{
ParaNdis_SendGratuitousArpPacket(pContext);
pContext->CXPath.SendControlMessage(VIRTIO_NET_CTRL_ANNOUNCE, VIRTIO_NET_CTRL_ANNOUNCE_ACK, NULL, 0, NULL, 0, 0);
pContext->bGuestAnnounced = FALSE;
}
pContext->CXPath.ClearInterruptReport();
}

if (pathBundle != nullptr && pathBundle->txPath.DoPendingTasks(nullptr))
{
Expand All @@ -1917,6 +1925,14 @@ bool ParaNdis_DPCWorkBody(PARANDIS_ADAPTER *pContext, ULONG ulMaxPacketsToIndica
return stillRequiresProcessing;
}

bool ParaNdis_DPCWorkBody(PARANDIS_ADAPTER *pContext, ULONG ulMaxPacketsToIndicate)
{
bool stillRequiresProcessing = ParaNdis_RXTXDPCWorkBody(pContext, ulMaxPacketsToIndicate);
ParaNdis_CXDPCWorkBody(pContext);

return stillRequiresProcessing;
}

#ifdef PARANDIS_SUPPORT_RSS
VOID ParaNdis_ResetRxClassification(PARANDIS_ADAPTER *pContext)
{
Expand Down
13 changes: 13 additions & 0 deletions NetKVM/Common/ndis56common.h
View file
Expand Up @@ -540,6 +540,8 @@ bool ParaNdis_DPCWorkBody(
PARANDIS_ADAPTER *pContext,
ULONG ulMaxPacketsToIndicate);

void ParaNdis_CXDPCWorkBody(PARANDIS_ADAPTER *pContext);

void ParaNdis_ReuseRxNBLs(PNET_BUFFER_LIST pNBL);

#ifdef PARANDIS_SUPPORT_RSS
Expand Down Expand Up @@ -843,6 +845,17 @@ tTcpIpPacketParsingResult ParaNdis_CheckSumVerifyFlat(
return ParaNdis_CheckSumVerify(&SGBuffer, ulDataLength, 0, flags, verifyLength, caller);
}

VOID MiniportMSIInterruptCXDpc(
struct _KDPC *Dpc,
IN PVOID MiniportInterruptContext,
IN PVOID NdisReserved1,
IN PVOID NdisReserved2
);

bool ParaNdis_RXTXDPCWorkBody(PARANDIS_ADAPTER *pContext,
ULONG ulMaxPacketsToIndicate);


USHORT CheckSumCalculator(PVOID buffer, ULONG len);

tTcpIpPacketParsingResult ParaNdis_ReviewIPPacket(PVOID buffer, ULONG size, BOOLEAN verityLength, LPCSTR caller);
Expand Down
47 changes: 36 additions & 11 deletions NetKVM/wlh/ParaNdis6-Impl.cpp
View file
Expand Up @@ -300,9 +300,9 @@ static BOOLEAN MiniportMSIInterrupt(
PARANDIS_STORE_LAST_INTERRUPT_TIMESTAMP(pContext);

*TargetProcessors = 0;
*QueueDefaultInterruptDpc = FALSE;

if (!pContext->bDeviceInitialized) {
*QueueDefaultInterruptDpc = FALSE;
return TRUE;
}

Expand All @@ -313,21 +313,26 @@ static BOOLEAN MiniportMSIInterrupt(
path->DisableInterrupts();
path->ReportInterrupt();


#if NDIS_SUPPORT_NDIS620
if (path->DPCAffinity.Mask)
if (path->getMessageIndex() == pContext->CXPath.getMessageIndex())
{
NdisMQueueDpcEx(pContext->InterruptHandle, MessageId, &path->DPCAffinity, NULL);
*QueueDefaultInterruptDpc = FALSE;
KeInsertQueueDpc(&pContext->CXPath.m_DPC, NULL, NULL);
}
else
{
*QueueDefaultInterruptDpc = TRUE;
}
#if NDIS_SUPPORT_NDIS620
if (path->DPCAffinity.Mask)
{
NdisMQueueDpcEx(pContext->InterruptHandle, MessageId, &path->DPCAffinity, NULL);
}
else
{
*QueueDefaultInterruptDpc = TRUE;
}
#else
*TargetProcessors = (ULONG)path->DPCTargetProcessor;
*QueueDefaultInterruptDpc = TRUE;
*TargetProcessors = (ULONG)path->DPCTargetProcessor;
*QueueDefaultInterruptDpc = TRUE;
#endif
}

pContext->ulIrqReceived += 1;
return true;
Expand Down Expand Up @@ -390,6 +395,26 @@ static VOID MiniportInterruptDPC(
UNREFERENCED_PARAMETER(NdisReserved2);
}

/**********************************************************
A CX procedure for MSI DPC handling
Parameters:
KDPC * Dpc - The dpc structure for CX
IN ULONG MessageId - specific interrupt index
***********************************************************/
VOID MiniportMSIInterruptCXDpc(
struct _KDPC *Dpc,
IN PVOID MiniportInterruptContext,
IN PVOID NdisReserved1,
IN PVOID NdisReserved2
)
{
PARANDIS_ADAPTER *pContext = (PARANDIS_ADAPTER *)MiniportInterruptContext;
ParaNdis_CXDPCWorkBody(pContext);
UNREFERENCED_PARAMETER(Dpc);
UNREFERENCED_PARAMETER(NdisReserved1);
UNREFERENCED_PARAMETER(NdisReserved2);
}

/**********************************************************
NDIS-required procedure for MSI DPC handling
Parameters:
Expand All @@ -416,7 +441,7 @@ static VOID MiniportMSIInterruptDpc(
PNDIS_RECEIVE_THROTTLE_PARAMETERS RxThrottleParameters = (PNDIS_RECEIVE_THROTTLE_PARAMETERS)ReceiveThrottleParameters;

RxThrottleParameters->MoreNblsPending = 0;
requireDPCRescheduling = ParaNdis_DPCWorkBody(pContext, RxThrottleParameters->MaxNblsToIndicate);
requireDPCRescheduling = ParaNdis_RXTXDPCWorkBody(pContext, RxThrottleParameters->MaxNblsToIndicate);

if (requireDPCRescheduling)
{
Expand Down

0 comments on commit 861e272

Please sign in to comment.