kvm-guest-drivers-windows icon indicating copy to clipboard operation
kvm-guest-drivers-windows copied to clipboard

Tx no map - testing with DMAR with mapping disable for non-IP packets

Open ybendito opened this issue 6 months ago • 2 comments

ybendito avatar May 17 '25 19:05 ybendito

@YanVugenfirer The motivation of this PR is first of all to test it and get closer to the root cause. Will we enable DMAR for Win10 or not - this is for later decision. If we don't - this is also OK. Let it run, then we'll discuss it.

ybendito avatar May 18 '25 05:05 ybendito

@YanVugenfirer Also, there is an open question "where is the break-even" between mapping and copy. The mapping/unmapping operation is heavy and we definitely loose performance on short packets.

ybendito avatar May 18 '25 05:05 ybendito

/gemini summary

kostyanf14 avatar Jul 03 '25 12:07 kostyanf14

Summary of Changes

This pull request introduces a mechanism to conditionally skip DMA remapping (DMAR) for transmit (Tx) packets in the NetKVM driver, primarily for testing and performance evaluation. It implements logic to determine when mapping can be bypassed, opting instead to copy packet data for specific scenarios, such as short packets without complex offloads. Additionally, it enhances logging and provides utilities to manage network protocol bindings, facilitating the testing of this DMAR behavior.

Highlights

  • Conditional DMA Remapping Bypass: Implemented new logic within the CNBL::CheckMappingNeeded() method to determine if DMA remapping (DMAR) is necessary for outgoing packets. This decision is based on factors such as packet size (if it's smaller than the maximum size for in-descriptor data), the absence of Large Send Offload (LSO), Checksum Offload (CSO), and UDP Segmentation Offload (USO), and the bAnyLayout context flag. If mapping is deemed unnecessary, a m_SkipMapping flag is set.
  • Packet Copying for Unmapped Packets: Modified the CNBL::StartMapping() process to conditionally bypass the Scatter/Gather List (SGL) building. If CheckMappingNeeded() indicates that mapping is not required, the packet data is directly handled (implicitly copied into the descriptor's header area by CNB::CopyHeaders and CNB::FillDescriptorSGList), avoiding the overhead associated with SGL creation and DMA mapping. This also includes a new copiedTxPackets statistic for such cases.
  • DMAR Test Mode Activation: The DmaRemappingCompatible registry setting in netkvm-base.txt has been explicitly changed from INX_NETKVM_DMAREMAP to 2. This change likely enables a specific test or permissive mode for DMAR, allowing the driver to exercise the new conditional mapping logic without strict enforcement of DMAR by the system.
  • Enhanced Debugging and Testing Utilities: Added new command-line options (l for log, z for remove) to the ProtocolService utility. These options enable logging of interface table information to c:\netkvmp.log and, for the 'z' option, allow for the dynamic collection and removal of 'ndistest' protocols. This provides a robust mechanism for setting up and tearing down specific network test environments, crucial for validating the DMAR bypass behavior.
Changelog
  • NetKVM/Common/ParaNdis-TX.h
    • Added CheckMappingNeeded() and MappingNeeded() methods to CNBL for conditional mapping decisions.
    • Introduced a new m_SkipMapping boolean member to CNBL to store the mapping decision.
    • Modified CNB::GetSGLLength() to safely handle a potentially null m_SGL by returning 1.
    • Added MaxSizeForPacketData() method to CParaNdisTX to expose the maximum packet data size that can fit in a descriptor's header area.
  • NetKVM/Common/ParaNdis-VirtQueue.h
    • Added GetMaxSizeForPacketData() method to CTXVirtQueue to retrieve the maximum packet data size.
    • Introduced m_MaxSizeForPacketData member to CTXVirtQueue to store this size.
  • NetKVM/Common/ParaNdis_TX.cpp
    • Implemented the core logic for CNBL::CheckMappingNeeded() to determine if mapping can be skipped based on packet length, LSO, CSO, USO, and bAnyLayout.
    • Modified CNBL::StartMapping() to conditionally call NB->ScheduleBuildSGListForTx() or directly call NB->MappingDone(nullptr) based on the bMappingNeeded flag.
    • Adjusted CNB::FillDescriptorSGList() to handle cases where the entire packet is headers (i.e., copied) by returning SUCCESS and incrementing copiedTxPackets.
    • Modified CNB::CopyHeaders() to copy MaxSize of data if mapping is not needed, otherwise copy ETH_HEADER_SIZE.
    • Updated CNB::BindToDescriptor() to only return FAILURE if m_SGL is null and mapping was actually required (m_ParentNBL->MappingNeeded()).
    • Wrapped m_History cleanup in CNBL::~CNBL() with #if NBL_MAINTAIN_HISTORY.
  • NetKVM/Common/ParaNdis_VirtQueue.cpp
    • Initialized m_MaxSizeForPacketData in CTXVirtQueue::PrepareBuffers() using the MaxEthHeadersSize() of the first transmit descriptor.
  • NetKVM/ProtocolService/Log.h
    • Declared extern FILE *LogFile;.
    • Modified the Log macro to write output to LogFile if it's open, otherwise fall back to OutputDebugStringA.
  • NetKVM/ProtocolService/Names.h
    • Added bsCollectProtocols to the tBindingState enum.
  • NetKVM/ProtocolService/ProtocolService.cpp
    • Defined LogFile and ListOfTestProtocols global variables.
    • Implemented AddToList function to collect 'ndistest' protocols.
    • Modified CNetCfg::CheckBinding to handle the new bsCollectProtocols state by calling AddToList.
    • Updated CInterfaceTable::Dump to accept an optional tBindingState argument, allowing it to collect protocols.
    • Added new command-line options l (log) and z (remove) to main() for dumping interface tables to a log file and removing collected test protocols using netcfg -u.
  • NetKVM/netkvm-base.txt
    • Changed the DmaRemappingCompatible registry value from INX_NETKVM_DMAREMAP to 2.
Activity
  • On May 18, 2025, @ybendito clarified that the PR's motivation is to test DMAR and investigate the performance trade-off between mapping and copying for short packets, emphasizing that the decision to enable DMAR for Win10 is a later consideration.
  • On July 3, 2025, @kostyanf14 requested a summary of the pull request.

gemini-code-assist[bot] avatar Jul 03 '25 12:07 gemini-code-assist[bot]