Hck rss bsod work in progress #1333

ybendito · 2025-03-23T06:19:33Z

For comments and testing only.

Jedoku · 2025-03-31T04:51:42Z

NetKVM/Common/ParaNdis-TX.h

+class NBLHistory : public CNdisAllocatable<NBLHistory, 'NBLH'>
+{
+    DECLARE_CNDISLIST_ENTRY(NBLHistory);
+  public:


in this case there should be extra line before access specifiers

Jedoku · 2025-03-31T05:28:24Z

NetKVM/Common/ParaNdis-TX.h

+    DECLARE_CNDISLIST_ENTRY(NBLHistory);
+  public:
+    NBLHistory(LPCSTR Func, LPCSTR Title, LPCSTR ParameterMeaning, PVOID Parameter, LPCSTR ValueMeaning, ULONG Value);
+  protected:


@Jedoku is this behavior configurable? I ask because this does not make too much sense.

It's configured in .clang-format
EmptyLineBeforeAccessModifier: LogicalBlock
If we change it it will affect whole project

@Jedoku If there is an option "don't mind" this will not request any change, correct? Is it?

Leave option should work

Copilot

Copilot wasn't able to review any files in this pull request.

Files not reviewed (14)

NetKVM/Common/DebugData.h: Language not supported
NetKVM/Common/ParaNdis-TX.h: Language not supported
NetKVM/Common/ParaNdis-Util.h: Language not supported
NetKVM/Common/ParaNdis-VirtQueue.h: Language not supported
NetKVM/Common/ParaNdis_Debug.cpp: Language not supported
NetKVM/Common/ParaNdis_DebugHistory.h: Language not supported
NetKVM/Common/ParaNdis_TX.cpp: Language not supported
NetKVM/Common/ParaNdis_VirtQueue.cpp: Language not supported
NetKVM/Common/ndis56common.h: Language not supported
NetKVM/DebugTools/NetKVMDumpParser/NetKVMDumpParser.cpp: Language not supported
NetKVM/DebugTools/NetKVMDumpParser/NetKVMDumpParser.vcxproj: Language not supported
NetKVM/wlh/ParaNdis6_Driver.cpp: Language not supported
NetKVM/wlh/ParaNdis6_Impl.cpp: Language not supported
NetKVM/wlh/ParaNdis6_Oid.cpp: Language not supported

Signed-off-by: Yuri Benditovich <[email protected]>

When the device has one queue we do not need to analyze the hash and send NBLs one by one. Signed-off-by: Yuri Benditovich <[email protected]>

Signed-off-by: Yuri Benditovich <[email protected]>

This feature is disabled in production, we just fix the compilation problems when it is enabled. Signed-off-by: Yuri Benditovich <[email protected]>

Move the last TX completion timestamp from per-adapter context to per-queue context for better visibility. Signed-off-by: Yuri Benditovich <[email protected]>

Note that this is unsafe method and the result can be used only for information / tracking, not for decisions. Signed-off-by: Yuri Benditovich <[email protected]>

The new logic to support chains is placed under ifdef currently and the driver works in backward compatible mode when all the procedures related to chain information in the NBL are stubs. The new logic that takes in account NBL chain is: - Before we start mapping the NBL chain - all the CNBLs after first one have a reference to first CNBL in the chain. At this point the first CNBL has a counter of NBLs that follow it in the chain - The check whether the NBL is 'done' now modified. The mandatory condition, as before, is that all the NBs of the NBL are completed. Additional condition will be enabled in future when we define NBL_CHAINS. Then all the NBLs related to the same chain become 'done' at the same moment, when all of them are done. Signed-off-by: Yuri Benditovich <[email protected]>

Signed-off-by: Yuri Benditovich <[email protected]>

Connection is src:dest IP addresses and ports, so for all the packets sent to specific connection the hash should be the same. We collect NBL chain till all the indirection indices are the same, then flush the chain to the TX queue and start a new chain. Signed-off-by: Yuri Benditovich <[email protected]>

Signed-off-by: Yuri Benditovich <[email protected]>

If defined (by default disabled by compilation) we keep the NBL history in the CNBL as a list of events. Especially useful for stuck NBL that has not completed. From the dump we'll be able to read the history. Signed-off-by: Yuri Benditovich <[email protected]>

Signed-off-by: Yuri Benditovich <[email protected]>

This is useful for logging of chained NBLs. Signed-off-by: Yuri Benditovich <[email protected]>

If one NB failed, the entire NBL is not send, so we need to reflect this in TX errors. Also adding printout in such a case. Signed-off-by: Yuri Benditovich <[email protected]>

Chain NBLs from the list of completed CNBLs as they appear in the list and do not invert them. Later we're going to add NBL chains from the 'first in chain' CNBLs. Signed-off-by: Yuri Benditovich <[email protected]>

Keep the order of NBLs wherever possible, i.e. push them at the end of waiting list and then process waiting list and remove from it all the completed NBLs, so that earlier NBLs are removed first. Signed-off-by: Yuri Benditovich <[email protected]>

Now we enable NBL chain support. Deliver entire NBL chain if one was provided in Send() call. Each CNBL created for each 'next' NBL (following to the first one) on completion of all it's NBs detaches it's NBL and attaches it to the m_NBL of the first one. Such a way when all the CNBLs related to the same chain are ready, only first one has m_NBL (with all the rest of NBLs chained to it). For all the rest of CNBLs the m_NBL is NULL. The order of NBLs in the chain might be slightly different than original one (due to NB completions running simultaneously in different flows), for that the chain is tested and repaired if needed before it is indicated up. Signed-off-by: Yuri Benditovich <[email protected]>

Signed-off-by: Yuri Benditovich <[email protected]>

ybendito · 2025-04-09T08:53:20Z

@kostyanf14 Win2022x64:

DVL folder not found, probably because there is no DVL file in the package. Is it expected?
Strict property validation: do we need to update something in our env or submit the update to the MS repo? 2025 test passed.

YanVugenfirer · 2025-04-09T08:54:13Z

NetKVM/Common/ParaNdis_TX.cpp

@@ -727,6 +727,11 @@ bool CParaNdisTX::SendMapped(bool IsInterrupt, CRawCNBLList &toWaitingList)
            }
        }
    }
+    else
+    {
+        // carrier loss or removal


It should be on a "link loss".

YanVugenfirer · 2025-04-09T08:56:44Z

NetKVM/wlh/ParaNdis6_Driver.cpp

@@ -446,7 +446,7 @@ static VOID ParaNdis6_SendNetBufferLists(NDIS_HANDLE miniportAdapterContext,
    }
 #ifdef PARANDIS_SUPPORT_RSS
    CNdisPassiveReadAutoLock autoLock(pContext->RSSParameters.rwLock);
-    if (pContext->RSS2QueueMap != nullptr)
+    if (pContext->nPathBundles > 1 && pContext->RSS2QueueMap != nullptr)


YanVugenfirer · 2025-04-09T08:58:34Z

NetKVM/Common/ParaNdis-TX.h

@@ -3,6 +3,7 @@
 #include "ParaNdis-AbstractPath.h"
 #include "ParaNdis_GuestAnnounce.h"
 #include "ParaNdis_LockFreeQueue.h"
+#include "ParaNdis_DebugHistory.h"


Please submit this as a separate PR. It can already be merged and make future review easier.

Thanks!

YanVugenfirer · 2025-04-09T09:00:58Z

NetKVM/Common/ParaNdis_VirtQueue.cpp

@@ -324,7 +324,7 @@ UINT CTXVirtQueue::ReleaseTransmitBuffers(CRawCNBList &listDone)
    }
    if (i)
    {
-        NdisGetCurrentSystemTime(&m_Context->LastTxCompletionTimeStamp);
+        UpdateTimestamp(m_LastTxCompletionTimestamp);


Also, can be submitted as a separate PR (or parts of refactoring/ ready for merge commits).

YanVugenfirer · 2025-04-09T09:11:36Z

NetKVM/DebugTools/NetKVMDumpParser/NetKVMDumpParser.cpp

@@ -41,7 +41,7 @@

 FILE *outf = stdout;

-typedef CString(*TimeStampConversionFunc)(LONGLONG BaseTime, LARGE_INTEGER TimeStamp);
+typedef CString (*TimeStampConversionFunc)(LONGLONG BaseTime, LARGE_INTEGER TimeStamp);


Please send it as part of a separate PR already

YanVugenfirer · 2025-04-09T09:13:02Z

NetKVM/Common/ParaNdis_TX.cpp

        NBLHolder->SetStatus(NDIS_STATUS_FAILURE);
        // this is ok if this CNBL is not first one
+        m_Context->Statistics.ifOutErrors += NBLHolder->NumberOfBuffers();


BTW: For the acadenic interest, do you see this in practice? When?

No, just to handle the error path and give some visibility in case it happens.

kostyanf14 · 2025-04-09T09:24:13Z

1. DVL folder not found, probably because there is no DVL file in the package. Is it expected?

Not expected, some build bug.

Build - Performing SDV checks for NetKVM-VS2015.vcxproj...
Microsoft (R) Build Engine version 16.11.2+f32259642 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

Build started 4/7/2025 3:54:57 PM.
     1>Project "C:\jenkins\workspace\build-virtio-win-EWDK-24H2-21H2\NetKVM\NetKVM-VS2015.vcxproj" on node 1 (sdv target(s)).
     1>sdv:
         staticdv /check /devenv
         SDV: c:\ewdk11\Program Files\Windows Kits\10\TOOLS\SDV
         SMV: c:\ewdk11\Program Files\Windows Kits\10\TOOLS\SDV\smv
         SDVAP: c:\ewdk11\Program Files\Windows Kits\10\TOOLS\SDV\smv\analysisplugins\sdv
         Build environment: msbuild
         [INFO] Cleaning ... 
         [INFO] Setting interceptor platform to x64
         [INFO] Setting platform to x86_amd64
         [INFO] Validating XML against schema: c:\ewdk11\Program Files\Windows Kits\10\TOOLS\SDV\smv\bin\Config.xsd
         [INFO] Running local scheduler with 12 threads
         [INFO] Driver type found: ndis
         [INFO] Currently reading and validating XML settings from c:\ewdk11\Program Files\Windows Kits\10\TOOLS\SDV\data\ndis\sdv-default.xml
         
         [INFO] 1 of 2 jobs remaining. Avg(s): 25.00. Std.Dev(s): 0.00
         [INFO] 1 of 3 jobs remaining. Avg(s): 179.50. Std.Dev(s): 154.50
         Scan ...Done
         
         [INFO] 0 of 3 jobs remaining. Avg(s): 119.67. Std.Dev(s): 151.90
         
         Building ...Done
         [INFO] Using plugin SdvPlugin.SmvSdv for analysis.
         [INFO] Running analysis on 3 precondition(s) & 100 rule(s) ...
         [INFO] Checking preconditions...
         
         [INFO] 3 of 7 jobs remaining. Avg(s): 92.75. Std.Dev(s): 139.57
         [INFO] 3 of 8 jobs remaining. Avg(s): 76.80. Std.Dev(s): 128.84
         
         Unhandled Exception: System.IndexOutOfRangeException: Index was outside the bounds of the array.
            at System.Collections.Generic.HashSet`1.AddIfNotPresent(T value)
            at System.Collections.Generic.HashSet`1.Add(T item)
            at SdvPlugin.SdvTelemetry.LogStageStart(String stageName)
            at SdvPlugin.SmvSdv.PreAction(SMVAction action)
            at SmvLibrary.Utility.ExecuteAction(SMVAction action, Boolean fromWorker, Boolean workerUseDb, String workerTaskId)
            at SmvLibrary.LocalSMVActionScheduler.Execute()
            at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
            at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
            at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
            at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
            at System.Threading.ThreadHelper.ThreadStart()
         [INFO] 3 of 9 jobs remaining. Avg(s): 66.17. Std.Dev(s): 120.00
     1>c:\ewdk11\Program Files\Windows Kits\10\build\windowsdriver.Sdv.targets(136,9): error MSB3073: The command "staticdv /check /devenv" exited with code 255. [C:\jenkins\workspace\build-virtio-win-EWDK-24H2-21H2\NetKVM\NetKVM-VS2015.vcxproj]
     1>Done Building Project "C:\jenkins\workspace\build-virtio-win-EWDK-24H2-21H2\NetKVM\NetKVM-VS2015.vcxproj" (sdv target(s)) -- FAILED.

2. Strict property validation: do we need to update something in our env or submit the update to the MS repo? 2025 test passed.

need to update our env.

YanVugenfirer · 2025-04-09T09:31:11Z

NetKVM/DebugTools/NetKVMDumpParser/NetKVMDumpParser.cpp

@@ -941,7 +1070,9 @@ int ParseDumpFile(int argc, TCHAR *argv[])
    }
    else
    {
-        PRINT("%s", SessionStatusValues[0].name);


Also can be submited for merge already.

YanVugenfirer · 2025-04-09T09:32:17Z

NetKVM/DebugTools/NetKVMDumpParser/NetKVMDumpParser.vcxproj

@@ -18,14 +18,14 @@
  <Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props" />
  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|Win32'" Label="Configuration">
    <ConfigurationType>Application</ConfigurationType>
-    <PlatformToolset>v141</PlatformToolset>
+    <PlatformToolset>v142</PlatformToolset>


Can be submitted already. Shouldn't trigger HCCK-CI.

YanVugenfirer · 2025-04-21T14:22:42Z

@ybendito Can we close this PR?

ybendito requested a review from YanVugenfirer as a code owner March 23, 2025 06:19

ybendito force-pushed the hck_rss_bsod_work branch 7 times, most recently from 0f6cab4 to 7896d63 Compare March 26, 2025 18:28

ybendito force-pushed the hck_rss_bsod_work branch 6 times, most recently from 484fb1a to dc5a7cc Compare March 30, 2025 17:34

Jedoku reviewed Mar 31, 2025

View reviewed changes

kostyanf14 requested a review from Copilot March 31, 2025 11:02

Copilot AI reviewed Mar 31, 2025

View reviewed changes

ybendito added 12 commits April 7, 2025 21:10

netkvm: move all NBLs drop to separate procedure

8f385f5

Signed-off-by: Yuri Benditovich <[email protected]>

netkvm: drop all the NBLs on carrier loss

37aed71

Signed-off-by: Yuri Benditovich <[email protected]>

netkvm: simplify TX code for case of single queue

8fa55ef

When the device has one queue we do not need to analyze the hash and send NBLs one by one. Signed-off-by: Yuri Benditovich <[email protected]>

netkvm: add debug printout in send path

f8aa83d

Signed-off-by: Yuri Benditovich <[email protected]>

netkvm: fix broken compilation with history log

42722f7

This feature is disabled in production, we just fix the compilation problems when it is enabled. Signed-off-by: Yuri Benditovich <[email protected]>

netkvm: track last tx completion time per queue

c850c5c

Move the last TX completion timestamp from per-adapter context to per-queue context for better visibility. Signed-off-by: Yuri Benditovich <[email protected]>

netkvm: add public method for getting ref count

2dbfe47

Note that this is unsafe method and the result can be used only for information / tracking, not for decisions. Signed-off-by: Yuri Benditovich <[email protected]>

netkvm: remove unneeded procedure

c21810c

Signed-off-by: Yuri Benditovich <[email protected]>

netkvm: add history logs for NBL-related operations

4f54424

Signed-off-by: Yuri Benditovich <[email protected]>

netkvm-dumpparser: fix build and update

1a0a550

Signed-off-by: Yuri Benditovich <[email protected]>

ybendito added 2 commits April 7, 2025 22:02

netkvm-dumpparser: parse history data only

0cb024d

Signed-off-by: Yuri Benditovich <[email protected]>

not for delivery: use performance counter for timestamps

f378d0f

Signed-off-by: Yuri Benditovich <[email protected]>

ybendito force-pushed the hck_rss_bsod_work branch from dc5a7cc to a8a6e3a Compare April 7, 2025 19:16

ybendito added 12 commits April 7, 2025 22:29

wip Die

f7f42c9

Signed-off-by: Yuri Benditovich <[email protected]>

netkvm: add CNBL getter for m_NBL

6dd6d41

Signed-off-by: Yuri Benditovich <[email protected]>

netkvm: emit debug history with queue index

8a28663

Signed-off-by: Yuri Benditovich <[email protected]>

netkvm: emit debug history for chained NBLs with queue index

69af2e6

Signed-off-by: Yuri Benditovich <[email protected]>

netkvm: remember serial number in chain inside CNBL

27de6a4

This is useful for logging of chained NBLs. Signed-off-by: Yuri Benditovich <[email protected]>

netkvm: take care of mapping failure case

653784e

If one NB failed, the entire NBL is not send, so we need to reflect this in TX errors. Also adding printout in such a case. Signed-off-by: Yuri Benditovich <[email protected]>

netkvm: do not invert order of NBLs on completion

3805b40

Chain NBLs from the list of completed CNBLs as they appear in the list and do not invert them. Later we're going to add NBL chains from the 'first in chain' CNBLs. Signed-off-by: Yuri Benditovich <[email protected]>

wip fire dpc

e1e0236

Signed-off-by: Yuri Benditovich <[email protected]>

not sure - additional poll request

c2d4571

Signed-off-by: Yuri Benditovich <[email protected]>

ybendito force-pushed the hck_rss_bsod_work branch from 5aeca9f to c2d4571 Compare April 7, 2025 19:29

fix dump parser

3529450

Signed-off-by: Yuri Benditovich <[email protected]>

YanVugenfirer reviewed Apr 9, 2025

View reviewed changes

ybendito closed this Apr 22, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hck rss bsod work in progress #1333

Hck rss bsod work in progress #1333

ybendito commented Mar 23, 2025

Jedoku Mar 31, 2025 •

edited

Loading

Jedoku Mar 31, 2025

ybendito Apr 8, 2025

Jedoku Apr 9, 2025

ybendito Apr 9, 2025

Jedoku Apr 10, 2025

Copilot AI left a comment

ybendito commented Apr 9, 2025 •

edited

Loading

YanVugenfirer Apr 9, 2025

YanVugenfirer Apr 9, 2025

YanVugenfirer Apr 9, 2025

YanVugenfirer Apr 9, 2025

YanVugenfirer Apr 9, 2025

YanVugenfirer Apr 9, 2025

ybendito Apr 9, 2025

kostyanf14 commented Apr 9, 2025 •

edited

Loading

YanVugenfirer Apr 9, 2025

YanVugenfirer Apr 9, 2025

YanVugenfirer commented Apr 21, 2025

Hck rss bsod work in progress #1333

Hck rss bsod work in progress #1333

Conversation

ybendito commented Mar 23, 2025

Jedoku Mar 31, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Copilot AI left a comment

Choose a reason for hiding this comment

ybendito commented Apr 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kostyanf14 commented Apr 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YanVugenfirer commented Apr 21, 2025

Jedoku Mar 31, 2025 •

edited

Loading

ybendito commented Apr 9, 2025 •

edited

Loading

kostyanf14 commented Apr 9, 2025 •

edited

Loading