From ecb9f3bf513a6c0d5fd940e6e7dc9d804e5e7073 Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Sat, 17 May 2025 09:36:49 +0300 Subject: [PATCH 1/6] wip protocol remove ndtest component Signed-off-by: Yuri Benditovich --- NetKVM/ProtocolService/Log.h | 4 +- NetKVM/ProtocolService/Names.h | 3 +- NetKVM/ProtocolService/ProtocolService.cpp | 58 +++++++++++++++++++++- 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/NetKVM/ProtocolService/Log.h b/NetKVM/ProtocolService/Log.h index cbfedd7bc..7961c5e9f 100644 --- a/NetKVM/ProtocolService/Log.h +++ b/NetKVM/ProtocolService/Log.h @@ -1,9 +1,11 @@ #pragma once +extern FILE *LogFile; + #define ELEMENTS_IN(a) sizeof(a) / sizeof(a[0]) #define Log(fmt, ...) \ { \ CStringA _s_; \ _s_.Format(fmt "\n", __VA_ARGS__); \ - OutputDebugStringA(_s_); \ + if (!LogFile) { OutputDebugStringA(_s_); } else { fwrite(_s_.GetString(), 1, _s_.GetLength(), LogFile); } \ } diff --git a/NetKVM/ProtocolService/Names.h b/NetKVM/ProtocolService/Names.h index c4fefe43a..87f0eebd2 100644 --- a/NetKVM/ProtocolService/Names.h +++ b/NetKVM/ProtocolService/Names.h @@ -71,5 +71,6 @@ typedef enum _tBindingState bsUnbindTcpip, bsBindTcpip, bsBindNone, - bsBindNoChange + bsBindNoChange, + bsCollectProtocols } tBindingState; diff --git a/NetKVM/ProtocolService/ProtocolService.cpp b/NetKVM/ProtocolService/ProtocolService.cpp index e7776288c..039421905 100644 --- a/NetKVM/ProtocolService/ProtocolService.cpp +++ b/NetKVM/ProtocolService/ProtocolService.cpp @@ -44,6 +44,27 @@ #define SERVICE_EXEFILE L"netkvmps.exe" #define SERVICE_ORGFILE L"netkvmp.exe" +FILE *LogFile; +CAtlArray ListOfTestProtocols; + +void AddToList(LPWSTR protocol) +{ + CStringA s; + s.Format("%S", protocol); + if (s.Find("ndistest") == 0) + { + auto &l = ListOfTestProtocols; + for (int i = 0; i < l.GetCount(); ++i) + { + if (!s.Compare(l[i])) + { + return; + } + } + l.Add(s); + } +} + class CNetCfg { public: @@ -228,6 +249,10 @@ class CNetCfg case bsBindNoChange: bShouldBeEnabled = enabled; break; + case bsCollectProtocols: + bShouldBeEnabled = enabled; + AddToList(upperId); + break; case bsBindAll: default: bShouldBeEnabled = true; @@ -353,9 +378,9 @@ class CInterfaceTable { CheckBinding(Index, NULL, true, State, false); } - void Dump() + void Dump(tBindingState st = bsBindNoChange) { - TraverseTable(INFINITE, NULL, false, bsBindNoChange, false, [](const MIB_IF_ROW2 &row) { + TraverseTable(INFINITE, NULL, false, st, false, [](const MIB_IF_ROW2 &row) { CMACString sMac(row.PhysicalAddress); auto &fl = row.InterfaceAndOperStatusFlags; Log("[%s] hw %d, paused %d, lp %d, %s", @@ -1255,6 +1280,35 @@ int __cdecl main(int argc, char **argv) CInterfaceTable t; t.Dump(); } + else if (!s.CompareNoCase("l") || !s.CompareNoCase("z")) + { + bool remove = !s.CompareNoCase("z"); + const char *fname = "c:\\netkvmp.log"; + fopen_s(&LogFile, fname, "a+b"); + if (LogFile) + { + printf("Dumping interface table to %s\n", fname); + CInterfaceTable t; + if (!remove) + { + t.Dump(); + } + else + { + t.Dump(bsCollectProtocols); + auto &l = ListOfTestProtocols; + for (int i = 0; i < l.GetCount(); ++i) + { + printf("removing %s\n", l[i].GetString()); + CStringA cmd = "netcfg -v -u "; + cmd += l[i]; + system(cmd); + } + } + fclose(LogFile); + LogFile = NULL; + } + } else { Usage(); From 1b75efb235b067959cde46daf955ca5c5cd8c1f0 Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Sat, 17 May 2025 09:39:42 +0300 Subject: [PATCH 2/6] return DMAR support to netkvm Win10 Signed-off-by: Yuri Benditovich --- NetKVM/netkvm-base.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NetKVM/netkvm-base.txt b/NetKVM/netkvm-base.txt index e6d52aa12..0935a68eb 100644 --- a/NetKVM/netkvm-base.txt +++ b/NetKVM/netkvm-base.txt @@ -250,7 +250,7 @@ HKR, , TypesSupported, 0x00010001, 7 HKR,,TextModeFlags,0x00010001, 0x0001 HKR,Parameters,DisableMSI,,"0" HKR,Parameters,EarlyDebug,,"3" -HKR,Parameters,DmaRemappingCompatible,0x00010001,INX_NETKVM_DMAREMAP +HKR,Parameters,DmaRemappingCompatible,0x00010001,2 [SourceDisksNames] From 168a3096c7e61e63117e95238171a5f28e26b2aa Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Sat, 17 May 2025 22:22:33 +0300 Subject: [PATCH 3/6] netkvm: add accesor for max usable data in TX headers area Each TX descriptor includes 4K block when we place the virtio header, ethernet header, VLAN/Priority tag and (when needed) the IP and TCP header that the driver can modify before submitting to the TX queue. The rest of the block the driver can use for packet data copy if needed. Keep the size of this free area and add getters to TX virtqueue and TX path objects. Signed-off-by: Yuri Benditovich --- NetKVM/Common/ParaNdis-TX.h | 5 ++++- NetKVM/Common/ParaNdis-VirtQueue.h | 6 ++++++ NetKVM/Common/ParaNdis_VirtQueue.cpp | 5 +++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/NetKVM/Common/ParaNdis-TX.h b/NetKVM/Common/ParaNdis-TX.h index d4dc50a5c..8fdfe8cdf 100644 --- a/NetKVM/Common/ParaNdis-TX.h +++ b/NetKVM/Common/ParaNdis-TX.h @@ -481,7 +481,10 @@ class CParaNdisTX : public CParaNdisTemplatePath, public CNdisAllo bool BorrowPages(CExtendedNBStorage *extraNBStorage, ULONG NeedPages); void ReturnPages(CExtendedNBStorage *extraNBStorage); void CheckStuckPackets(ULONG GraceTimeMillies); - + ULONG MaxSizeForPacketData() + { + return m_VirtQueue.GetMaxSizeForPacketData(); + } private: virtual void Notify(SMNotifications message) override; diff --git a/NetKVM/Common/ParaNdis-VirtQueue.h b/NetKVM/Common/ParaNdis-VirtQueue.h index f56e36af7..6d646fc42 100644 --- a/NetKVM/Common/ParaNdis-VirtQueue.h +++ b/NetKVM/Common/ParaNdis-VirtQueue.h @@ -365,6 +365,11 @@ class CTXVirtQueue : public CVirtQueue return m_TotalDescriptors; } + ULONG GetMaxSizeForPacketData() const + { + return m_MaxSizeForPacketData; + } + // TODO: Needs review void Shutdown(); @@ -375,6 +380,7 @@ class CTXVirtQueue : public CVirtQueue void FreeBuffers(); ULONG m_MaxBuffers = 0; ULONG m_HeaderSize = 0; + ULONG m_MaxSizeForPacketData = 0; void KickQueueOnOverflow(); void UpdateTXStats(const CNB &NB, CTXDescriptor &Descriptor); diff --git a/NetKVM/Common/ParaNdis_VirtQueue.cpp b/NetKVM/Common/ParaNdis_VirtQueue.cpp index 221b70682..4dfd78a6d 100644 --- a/NetKVM/Common/ParaNdis_VirtQueue.cpp +++ b/NetKVM/Common/ParaNdis_VirtQueue.cpp @@ -113,6 +113,11 @@ bool CTXVirtQueue::PrepareBuffers() } m_Descriptors.Push(TXDescr); + // save maximal size of packet than can be placed in the headera area + if (m_TotalDescriptors == 0) + { + m_MaxSizeForPacketData = TXDescr->HeadersAreaAccessor().MaxEthHeadersSize(); + } } m_FreeHWBuffers = m_TotalHWBuffers = m_TotalDescriptors; From ad5a43d792dd04ea9c64dba5f054c44ec71fd1f5 Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Sat, 17 May 2025 22:30:27 +0300 Subject: [PATCH 4/6] netkvm: add a flag to be able to do TX without mapping Nothing is changed yet. We just add the flag in the NBL that will allow us in future to skip the mapping if all the packet data can be copied to the headers block. We set this flag when the packet size allos that and when the packet is not LSO,CSO,USO,IP header checksum is not required and 'any layout' is accessible by the device. Typically this flag will be set for non-IP packets. Signed-off-by: Yuri Benditovich --- NetKVM/Common/ParaNdis-TX.h | 6 ++++++ NetKVM/Common/ParaNdis_TX.cpp | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/NetKVM/Common/ParaNdis-TX.h b/NetKVM/Common/ParaNdis-TX.h index 8fdfe8cdf..f026fd7a8 100644 --- a/NetKVM/Common/ParaNdis-TX.h +++ b/NetKVM/Common/ParaNdis-TX.h @@ -263,6 +263,7 @@ class CNBL : public CNdisAllocatableViaHelper, public CRefCountingObject, // TODO: Needs review void NBComplete(); bool IsSendDone(); + bool CheckMappingNeeded(); UCHAR ProtocolID() { @@ -338,6 +339,10 @@ class CNBL : public CNdisAllocatableViaHelper, public CRefCountingObject, { return m_NBL != nullptr; } + bool MappingNeeded() const + { + return !m_SkipMapping; + } ULONG NumberOfBuffers() const { return m_BuffersNumber; @@ -396,6 +401,7 @@ class CNBL : public CNdisAllocatableViaHelper, public CRefCountingObject, ULONG_PTR m_CNB_Storage[(sizeof(CNB) + sizeof(ULONG_PTR) - 1) / sizeof(ULONG_PTR)]; bool m_HaveFailedMappings = false; bool m_AllNBCompleted = false; + bool m_SkipMapping = false; CNdisList m_Buffers; diff --git a/NetKVM/Common/ParaNdis_TX.cpp b/NetKVM/Common/ParaNdis_TX.cpp index 11b5ab1b4..4748274c9 100644 --- a/NetKVM/Common/ParaNdis_TX.cpp +++ b/NetKVM/Common/ParaNdis_TX.cpp @@ -305,6 +305,18 @@ bool CNBL::ParseOffloads() return true; } +bool CNBL::CheckMappingNeeded() +{ + // checksums are excluded for simplicity + // if the packet is short, without LSO, CSO and USO, any layout is ok + // then no need to map it + // TODO: + driver verifier? + DMAR + bool bNoMap = m_MaxDataLength < m_ParentTXPath->MaxSizeForPacketData() && !IsLSO(); + bNoMap = bNoMap && !m_CsoInfo.Value && !IsUSO() && m_Context->bAnyLayout; + m_SkipMapping = bNoMap; + return !m_SkipMapping; +} + void CNBL::StartMapping() { CDpcIrqlRaiser OnDpc; From 313965f719a1d35f8bcc5b867476d4f16999a55f Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Sat, 17 May 2025 22:38:07 +0300 Subject: [PATCH 5/6] netkvm: work without packet mapping if allowed https://issues.redhat.com/browse/RHEL-78518 https://issues.redhat.com/browse/RHEL-82071 If the flag m_SkipMapping is set in the NBL, do not request creation of NBL for it. Copy entire packet into headers area in the descriptor and transmit the packet using single TXQ descriptor or two (if VLAN/Prio tagging is required). This will be applied mainly to non-IP packets. Such effective solves the problem of 1c_Mini6PerfSend test on Win10 when the DMAR is enabled. This minimizes the amount of work the driver verifier should do per packet. Signed-off-by: Yuri Benditovich --- NetKVM/Common/ParaNdis-TX.h | 2 +- NetKVM/Common/ParaNdis_TX.cpp | 27 ++++++++++++++++++++------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/NetKVM/Common/ParaNdis-TX.h b/NetKVM/Common/ParaNdis-TX.h index f026fd7a8..df7ff6016 100644 --- a/NetKVM/Common/ParaNdis-TX.h +++ b/NetKVM/Common/ParaNdis-TX.h @@ -70,7 +70,7 @@ class CNB : public CNdisAllocatableViaHelper ULONG GetSGLLength() const { - return m_SGL->NumberOfElements; + return m_SGL ? m_SGL->NumberOfElements : 1; } NBMappingStatus BindToDescriptor(CTXDescriptor &Descriptor); diff --git a/NetKVM/Common/ParaNdis_TX.cpp b/NetKVM/Common/ParaNdis_TX.cpp index 4748274c9..1c24372fb 100644 --- a/NetKVM/Common/ParaNdis_TX.cpp +++ b/NetKVM/Common/ParaNdis_TX.cpp @@ -320,15 +320,23 @@ bool CNBL::CheckMappingNeeded() void CNBL::StartMapping() { CDpcIrqlRaiser OnDpc; + bool bMappingNeeded = CheckMappingNeeded(); AddRef(); - m_Buffers.ForEach([this](CNB *NB) { - if (!NB->ScheduleBuildSGListForTx()) + m_Buffers.ForEach([&](CNB *NB) { + if (!bMappingNeeded) { - m_HaveFailedMappings = true; NB->MappingDone(nullptr); } + else + { + if (!NB->ScheduleBuildSGListForTx()) + { + m_HaveFailedMappings = true; + NB->MappingDone(nullptr); + } + } }); Release(); @@ -1236,7 +1244,12 @@ NBMappingStatus CNB::FillDescriptorSGList(CTXDescriptor &Descriptor, ULONG Parse { return NBMappingStatus::FAILURE; } - if (Descriptor.HasRoom(m_SGL->NumberOfElements)) + if (ParsedHeadersLength == GetDataLength()) + { + m_Context->extraStatistics.copiedTxPackets++; + return NBMappingStatus::SUCCESS; + } + if (m_SGL && Descriptor.HasRoom(m_SGL->NumberOfElements)) { return MapDataToVirtioSGL(Descriptor, ParsedHeadersLength + NET_BUFFER_DATA_OFFSET(m_NB)); } @@ -1305,8 +1318,8 @@ bool CNB::CopyHeaders(PVOID Destination, ULONG MaxSize, ULONG &HeadersLength, UL } else { - HeadersLength = ETH_HEADER_SIZE; - Copy(Destination, HeadersLength); + HeadersLength = m_ParentNBL->MappingNeeded() ? ETH_HEADER_SIZE : MaxSize; + HeadersLength = Copy(Destination, HeadersLength); } return (HeadersLength <= MaxSize); @@ -1365,7 +1378,7 @@ void CNB::Report(int level, bool Success) NBMappingStatus CNB::BindToDescriptor(CTXDescriptor &Descriptor) { - if (m_SGL == nullptr) + if (m_SGL == nullptr && m_ParentNBL->MappingNeeded()) { return NBMappingStatus::FAILURE; } From 787e6706548974e265ead348835f29d4efa6bd16 Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Sat, 17 May 2025 22:46:46 +0300 Subject: [PATCH 6/6] netkvm: exclude the entire NBL history list if not defined This does not matter for functionality but in the NBL destructor the verifier should not check the spinlock of history list, which is not used in any other place. Signed-off-by: Yuri Benditovich --- NetKVM/Common/ParaNdis-TX.h | 2 ++ NetKVM/Common/ParaNdis_TX.cpp | 2 ++ 2 files changed, 4 insertions(+) diff --git a/NetKVM/Common/ParaNdis-TX.h b/NetKVM/Common/ParaNdis-TX.h index df7ff6016..8cf9e63b5 100644 --- a/NetKVM/Common/ParaNdis-TX.h +++ b/NetKVM/Common/ParaNdis-TX.h @@ -423,7 +423,9 @@ class CNBL : public CNdisAllocatableViaHelper, public CRefCountingObject, #endif CAllocationHelper *m_NBAllocator; +#if NBL_MAINTAIN_HISTORY CHistoryList m_History; +#endif CChainOfNbls m_Chain = {}; diff --git a/NetKVM/Common/ParaNdis_TX.cpp b/NetKVM/Common/ParaNdis_TX.cpp index 1c24372fb..0541d8464 100644 --- a/NetKVM/Common/ParaNdis_TX.cpp +++ b/NetKVM/Common/ParaNdis_TX.cpp @@ -51,7 +51,9 @@ CNBL::~CNBL() } } +#if NBL_MAINTAIN_HISTORY m_History.ForEachDetached([this](NBLHistory *Entry) { NBLHistory::Destroy(Entry, m_Context->MiniportHandle); }); +#endif } bool CNBL::ParsePriority()