From c649f4e7173daa7379c4d8a39886c6b80fcd2ab0 Mon Sep 17 00:00:00 2001 From: Vadim Smirnov Date: Wed, 29 Jul 2026 20:48:11 +0200 Subject: [PATCH 1/3] perf: rewrite the Recalculate*Checksum inner loop over a wide one's-complement sum All four functions shared one loop: two bytes per iteration, byte loads, shift+mask, front-end bound at roughly 1.1-1.7 cycles/byte. On a VPN data path that recomputes L4 checksums on every packet in both directions, that loop was the single largest per-byte cost outside the cipher. inet_checksum::sum16_be accumulates 32-bit loads into four independent u64 accumulators - a u64 absorbs 2^32 such additions, so the loop needs no carry handling and no intrinsics, and builds unchanged for x86, x64 and ARM64. Measured on one out-of-order x64 core: 0.21-0.26 cycles/byte, 5.1-6.4x the old loop. The result is BIT-IDENTICAL: the Internet checksum is byte-order independent up to one byte swap of the folded result (RFC 1071 section 2(B)), and the 0x0000-versus-0xFFFF folding boundary cannot diverge because a folded sum is zero only for an all-zero span in both formulations. Proven by an exhaustive differential test against the old loop - 8 alignments x 3001 lengths plus boundary fills, under AddressSanitizer with allocations sized so any read past the span faults. Odd lengths are handled with a masked tail load. The old loop instead wrote a zero pad byte INTO the packet buffer one past the payload and read it back; nothing depended on that write, but it means raw capture bytes can differ from old builds in one receiver-ignored Ethernet-trailer byte on odd-L4 frames. RecalculateICMPChecksum also gains the length validation its TCP and UDP siblings received earlier: it computed ntohs(ip_len) - ip_hl*4 unchecked, so a forged or offload-mangled ip_len underflowed the length and drove the checksum loop - and previously the pad-byte write - far past m_IBuffer. Co-Authored-By: Claude Opus 5 --- ndisapi/inet_checksum.h | 107 ++++++++++++++++++++++++++++++++++++++++ ndisapi/ndisapi.cpp | 71 +++++++++++--------------- 2 files changed, 136 insertions(+), 42 deletions(-) create mode 100644 ndisapi/inet_checksum.h diff --git a/ndisapi/inet_checksum.h b/ndisapi/inet_checksum.h new file mode 100644 index 0000000..01c9ce1 --- /dev/null +++ b/ndisapi/inet_checksum.h @@ -0,0 +1,107 @@ +/*************************************************************************/ +/* Copyright (c) 2000-2026 NT KERNEL. */ +/* All Rights Reserved. */ +/* https://www.ntkernel.com */ +/* ndisrd@ntkernel.com */ +/* */ +/* Module Name: inet_checksum.h */ +/* */ +/* Description: Wide (RFC 1071) Internet checksum accumulation */ +/* */ +/* Environment: */ +/* User mode */ +/* */ +/*************************************************************************/ + +#pragma once + +#include +#include + +namespace inet_checksum +{ + /// + /// 16-bit one's-complement sum (NOT complemented) of a byte span, in the + /// big-endian word domain, i.e. exactly the value the byte-pair loop + /// sum += (buff[i] << 8) | buff[i + 1] accumulates and folds. + /// Callers add their pseudo-header words to it, fold, and complement, + /// unchanged. + /// + /// + /// The Internet checksum is byte-order independent up to one byte swap of + /// the folded result (RFC 1071 section 2(B)), so the hot loop accumulates + /// native little-endian 32-bit words and only the final 16 bits are + /// swapped. The invariant this rests on: for every span and every length + /// parity, folding the little-endian sum and swapping equals folding the + /// big-endian byte-pair sum (with a zero pad byte for odd lengths), and + /// the 0x0000-versus-0xFFFF folding boundary cannot diverge because a + /// folded sum is zero only for an all-zero argument in both formulations. + /// Any change here must keep a differential test against the byte-pair + /// formulation green across odd/even lengths and all start alignments. + /// + /// Accumulating 32-bit loads into 64-bit accumulators needs no carry + /// handling at all -- a uint64_t absorbs 2^32 such additions, and an IPv4 + /// payload is bounded far below that -- so the loop is portable across + /// x86, x64 and ARM64 with no intrinsics. Four independent accumulators + /// break the dependency chain that made the byte-pair loop front-end + /// bound; measured on one out-of-order x64 core, the cost drops from + /// 1.1-1.7 cycles/byte to roughly 0.21-0.26. + /// + /// An ODD length is handled with a masked tail load. The byte-pair loop + /// instead wrote a zero pad byte into the packet buffer one past the + /// payload before reading it back; this function never writes anywhere. + /// + /// Start of the span. No alignment requirement. + /// Span length in bytes. Zero yields zero. + /// The folded, uncomplemented sum, in [0x0000, 0xFFFF]. + inline uint16_t sum16_be(const unsigned char* data, const size_t length) noexcept + { + uint64_t s0 = 0, s1 = 0, s2 = 0, s3 = 0; + size_t i = 0; + + for (; i + 16 <= length; i += 16) + { + uint32_t w0, w1, w2, w3; + std::memcpy(&w0, data + i, 4); + std::memcpy(&w1, data + i + 4, 4); + std::memcpy(&w2, data + i + 8, 4); + std::memcpy(&w3, data + i + 12, 4); + s0 += w0; + s1 += w1; + s2 += w2; + s3 += w3; + } + + for (; i + 4 <= length; i += 4) + { + uint32_t w; + std::memcpy(&w, data + i, 4); + s0 += w; + } + + // Masked tail: up to three bytes. The 4-byte blocks above end on an + // even offset, so within this final word the byte at relative offset r + // belongs at little-endian lane r -- which is precisely what building + // the word with shifts 0/8/16 produces. Never reads past `length`. + if (i < length) + { + uint32_t w = 0; + unsigned shift = 0; + for (; i < length; ++i, shift += 8) + w |= static_cast(data[i]) << shift; + s0 += w; + } + + uint64_t sum = s0 + s1 + s2 + s3; + while (sum >> 32) + sum = (sum & 0xFFFFFFFFull) + (sum >> 32); + + uint32_t folded = static_cast(sum); + while (folded >> 16) + folded = (folded & 0xFFFF) + (folded >> 16); + + // Swap into the big-endian word domain the callers accumulate + // pseudo-header words in. + return static_cast(((folded & 0xFF) << 8) | (folded >> 8)); + } +} diff --git a/ndisapi/ndisapi.cpp b/ndisapi/ndisapi.cpp index 3896c3f..b62b13f 100644 --- a/ndisapi/ndisapi.cpp +++ b/ndisapi/ndisapi.cpp @@ -14,6 +14,7 @@ /*************************************************************************/ #include "precomp.h" +#include "inet_checksum.h" #if _MSC_VER >= 1800 && !defined(_USING_V110_SDK71_) #include @@ -3169,12 +3170,9 @@ void CNdisApi::RecalculateIPChecksum(PINTERMEDIATE_BUFFER pPacket) pIpHeader->ip_sum = 0; const PUCHAR buff = reinterpret_cast(pIpHeader); - // Calculate IP header checksum - for (unsigned int i = 0; i < pIpHeader->ip_hl * sizeof(DWORD); i += 2) - { - const unsigned short word16 = ((buff[i] << 8) & 0xFF00) + (buff[i + 1] & 0xFF); - sum += word16; - } + // Wide one's-complement sum over the header; bit-identical to the former + // byte-pair loop (see inet_checksum.h). + sum = inet_checksum::sum16_be(buff, pIpHeader->ip_hl * sizeof(DWORD)); // Keep only the last 16 bits of the 32-bit calculated sum and add the carries while (sum >> 16) @@ -3194,7 +3192,6 @@ void CNdisApi::RecalculateIPChecksum(PINTERMEDIATE_BUFFER pPacket) */ void CNdisApi::RecalculateICMPChecksum(PINTERMEDIATE_BUFFER pPacket) { - unsigned short padd = 0; unsigned int sum = 0; icmphdr_ptr pIcmpHeader; const iphdr_ptr pIpHeader = reinterpret_cast(&pPacket->m_IBuffer[sizeof(ether_header)]); @@ -3207,23 +3204,30 @@ void CNdisApi::RecalculateICMPChecksum(PINTERMEDIATE_BUFFER pPacket) else return; - const DWORD dwIcmpLen = ntohs(pIpHeader->ip_len) - pIpHeader->ip_hl * 4; + const DWORD dwIpHeaderLen = static_cast(pIpHeader->ip_hl) * 4; + const DWORD dwIpPacketLen = ntohs(pIpHeader->ip_len); - if ((dwIcmpLen / 2) * 2 != dwIcmpLen) + // Validate the lengths before the unsigned subtraction below, exactly as the + // TCP and UDP siblings do. A packet whose IP total-length field is smaller + // than its IP + ICMP headers would otherwise underflow dwIcmpLen to a huge + // value and the checksum would read far past m_IBuffer. This function was + // the one of the four that never received the guard. + if (dwIpHeaderLen < sizeof(iphdr) || + dwIpPacketLen < dwIpHeaderLen + sizeof(icmphdr) || + static_cast(sizeof(ether_header)) + dwIpPacketLen > pPacket->m_Length) { - padd = 1; - pPacket->m_IBuffer[dwIcmpLen + pIpHeader->ip_hl * 4 + static_cast(sizeof(ether_header))] = 0; + return; } + const DWORD dwIcmpLen = dwIpPacketLen - dwIpHeaderLen; + const PUCHAR buff = reinterpret_cast(pIcmpHeader); pIcmpHeader->checksum = 0; - // Make 16-bit words out of every two adjacent 8-bit words and calculate the sum of all 16-bit words - for (unsigned int i = 0; i < dwIcmpLen + padd; i = i + 2) - { - const unsigned short word16 = ((buff[i] << 8) & 0xFF00) + (buff[i + 1] & 0xFF); - sum = sum + static_cast(word16); - } + // Wide one's-complement sum; bit-identical to the former byte-pair loop and, + // unlike it, needs no zero pad byte written into the packet buffer for odd + // lengths (see inet_checksum.h). + sum = inet_checksum::sum16_be(buff, dwIcmpLen); // Keep only the last 16 bits of the 32-bit calculated sum and add the carries while (sum >> 16) @@ -3243,7 +3247,6 @@ void CNdisApi::RecalculateICMPChecksum(PINTERMEDIATE_BUFFER pPacket) void CNdisApi::RecalculateTCPChecksum(PINTERMEDIATE_BUFFER pPacket) { tcphdr_ptr pTcpHeader; - unsigned short padd = 0; unsigned int sum = 0; const iphdr_ptr pIpHeader = reinterpret_cast(&pPacket->m_IBuffer[sizeof(ether_header)]); @@ -3276,21 +3279,13 @@ void CNdisApi::RecalculateTCPChecksum(PINTERMEDIATE_BUFFER pPacket) const DWORD dwTcpLen = dwIpPacketLen - dwIpHeaderLen; - if ((dwTcpLen / 2) * 2 != dwTcpLen) - { - padd = 1; - pPacket->m_IBuffer[dwTcpLen + pIpHeader->ip_hl * 4 + static_cast(sizeof(ether_header))] = 0; - } - const PUCHAR buff = reinterpret_cast(pTcpHeader); pTcpHeader->th_sum = 0; - // Make 16-bit words out of every two adjacent 8-bit words and calculate the sum of all 16-bit words - for (unsigned int i = 0; i < dwTcpLen + padd; i = i + 2) - { - const unsigned short word16 = ((buff[i] << 8) & 0xFF00) + (buff[i + 1] & 0xFF); - sum = sum + static_cast(word16); - } + // Wide one's-complement sum over the TCP segment; bit-identical to the + // former byte-pair loop and, unlike it, needs no zero pad byte written into + // the packet buffer for odd lengths (see inet_checksum.h). + sum = inet_checksum::sum16_be(buff, dwTcpLen); // Add the TCP pseudo header which contains: // the IP source and destination addresses @@ -3319,7 +3314,6 @@ void CNdisApi::RecalculateTCPChecksum(PINTERMEDIATE_BUFFER pPacket) * the UDP packet. The calculated checksum is stored in the UDP header of the packet. */ void CNdisApi::RecalculateUDPChecksum(PINTERMEDIATE_BUFFER pPacket) { - unsigned short padd = 0; unsigned int sum = 0; const iphdr_ptr pIpHeader = reinterpret_cast(&pPacket->m_IBuffer[sizeof(ether_header)]); @@ -3351,20 +3345,13 @@ void CNdisApi::RecalculateUDPChecksum(PINTERMEDIATE_BUFFER pPacket) { const DWORD dwUdpLen = dwIpPacketLen - dwIpHeaderLen; - // Check if padding is needed - if ((dwUdpLen / 2) * 2 != dwUdpLen) { - padd = 1; - pPacket->m_IBuffer[dwUdpLen + pIpHeader->ip_hl * 4 + static_cast(sizeof(ether_header))] = 0; - } - const PUCHAR buff = reinterpret_cast(pUdpHeader); pUdpHeader->th_sum = 0; - // Calculate the sum of 16-bit words of the UDP packet - for (unsigned int i = 0; i < dwUdpLen + padd; i = i + 2) { - const unsigned short word16 = ((buff[i] << 8) & 0xFF00) + (buff[i + 1] & 0xFF); - sum = sum + static_cast(word16); - } + // Wide one's-complement sum over the UDP datagram; bit-identical to the + // former byte-pair loop and, unlike it, needs no zero pad byte written into + // the packet buffer for odd lengths (see inet_checksum.h). + sum = inet_checksum::sum16_be(buff, dwUdpLen); // Add the UDP pseudo-header to the sum sum = sum + ntohs(pIpHeader->ip_src.S_un.S_un_w.s_w1) + ntohs(pIpHeader->ip_src.S_un.S_un_w.s_w2); From 447f7ddaa983a7449d67226f68bf67d746586f5b Mon Sep 17 00:00:00 2001 From: Vadim Smirnov Date: Wed, 29 Jul 2026 21:28:25 +0200 Subject: [PATCH 2/3] compat: keep inet_checksum.h buildable by the legacy VC6/VS2012 projects ndisapi.cpp is compiled by ndisapi.vc6/ndisapi.dsp and the VS2012 projects as well as the modern ones, and the rest of the file guards its C++11 use behind _MSC_VER checks. The new header ignored that: requires VS2010 and `noexcept` VS2015. Pre-VS2010 toolchains now get typedefs onto the compiler's built-in __int64 plus 's memcpy, and pre-VS2015 an empty noexcept macro (INET_CHECKSUM_NOEXCEPT). The body already avoided the other VC6 traps -- no `ull` literal suffixes, no per-loop redeclaration of the index variable -- and the one 64-bit mask constant relies on promotion (`sum & 0xFFFFFFFFu` against a 64-bit operand) rather than a suffix. INET_CHECKSUM_FORCE_LEGACY_COMPAT forces a modern compiler down the legacy branch, which is how that branch is actually verified without a VC6 installation: the exhaustive differential test (8 alignments x 3001 lengths plus boundary fills, under AddressSanitizer) passes identically through both branches. What this does NOT prove is VC6's own parser accepting the file; the constructs used are all VC6-era (namespaces, inline, static_cast, __int64), but nothing here has been run through a real VC6. Co-Authored-By: Claude Opus 5 --- ndisapi/inet_checksum.h | 81 ++++++++++++++++++++++++++++++----------- 1 file changed, 59 insertions(+), 22 deletions(-) diff --git a/ndisapi/inet_checksum.h b/ndisapi/inet_checksum.h index 01c9ce1..00c6e3d 100644 --- a/ndisapi/inet_checksum.h +++ b/ndisapi/inet_checksum.h @@ -15,8 +15,45 @@ #pragma once +// This header is compiled by every project that builds ndisapi.cpp, including +// the legacy VC6/VS2012 ones (ndisapi.vc6/ndisapi.dsp), so it cannot assume +// C++11. arrived with VS2010 (_MSC_VER 1600) and `noexcept` with +// VS2015 (1900); older MSVC gets typedefs onto the compiler's built-in +// __int64 and an empty noexcept macro. The body itself avoids everything VC6 +// chokes on: no `ull` literal suffixes, no per-loop redeclaration of the +// index variable. +// +// INET_CHECKSUM_FORCE_LEGACY_COMPAT exists so a modern compiler can be forced +// down the legacy branch, which is how that branch is compile- and +// correctness-tested without a VC6 installation. It changes types and +// spelling only, never values. +#if (defined(_MSC_VER) && _MSC_VER < 1600) || defined(INET_CHECKSUM_FORCE_LEGACY_COMPAT) +#include +#include +#define INET_CHECKSUM_MEMCPY ::memcpy +namespace inet_checksum +{ + typedef unsigned __int64 sum_uint64; + typedef unsigned int sum_uint32; + typedef unsigned short sum_uint16; +} +#else #include #include +#define INET_CHECKSUM_MEMCPY ::std::memcpy +namespace inet_checksum +{ + typedef ::std::uint64_t sum_uint64; + typedef ::std::uint32_t sum_uint32; + typedef ::std::uint16_t sum_uint16; +} +#endif + +#if (defined(_MSC_VER) && _MSC_VER < 1900) || defined(INET_CHECKSUM_FORCE_LEGACY_COMPAT) +#define INET_CHECKSUM_NOEXCEPT +#else +#define INET_CHECKSUM_NOEXCEPT noexcept +#endif namespace inet_checksum { @@ -40,12 +77,12 @@ namespace inet_checksum /// formulation green across odd/even lengths and all start alignments. /// /// Accumulating 32-bit loads into 64-bit accumulators needs no carry - /// handling at all -- a uint64_t absorbs 2^32 such additions, and an IPv4 - /// payload is bounded far below that -- so the loop is portable across - /// x86, x64 and ARM64 with no intrinsics. Four independent accumulators - /// break the dependency chain that made the byte-pair loop front-end - /// bound; measured on one out-of-order x64 core, the cost drops from - /// 1.1-1.7 cycles/byte to roughly 0.21-0.26. + /// handling at all -- a 64-bit accumulator absorbs 2^32 such additions, + /// and an IPv4 payload is bounded far below that -- so the loop is + /// portable across x86, x64 and ARM64 with no intrinsics. Four + /// independent accumulators break the dependency chain that made the + /// byte-pair loop front-end bound; measured on one out-of-order x64 core, + /// the cost drops from 1.1-1.7 cycles/byte to roughly 0.21-0.26. /// /// An ODD length is handled with a masked tail load. The byte-pair loop /// instead wrote a zero pad byte into the packet buffer one past the @@ -54,18 +91,18 @@ namespace inet_checksum /// Start of the span. No alignment requirement. /// Span length in bytes. Zero yields zero. /// The folded, uncomplemented sum, in [0x0000, 0xFFFF]. - inline uint16_t sum16_be(const unsigned char* data, const size_t length) noexcept + inline sum_uint16 sum16_be(const unsigned char* data, const size_t length) INET_CHECKSUM_NOEXCEPT { - uint64_t s0 = 0, s1 = 0, s2 = 0, s3 = 0; + sum_uint64 s0 = 0, s1 = 0, s2 = 0, s3 = 0; size_t i = 0; for (; i + 16 <= length; i += 16) { - uint32_t w0, w1, w2, w3; - std::memcpy(&w0, data + i, 4); - std::memcpy(&w1, data + i + 4, 4); - std::memcpy(&w2, data + i + 8, 4); - std::memcpy(&w3, data + i + 12, 4); + sum_uint32 w0, w1, w2, w3; + INET_CHECKSUM_MEMCPY(&w0, data + i, 4); + INET_CHECKSUM_MEMCPY(&w1, data + i + 4, 4); + INET_CHECKSUM_MEMCPY(&w2, data + i + 8, 4); + INET_CHECKSUM_MEMCPY(&w3, data + i + 12, 4); s0 += w0; s1 += w1; s2 += w2; @@ -74,8 +111,8 @@ namespace inet_checksum for (; i + 4 <= length; i += 4) { - uint32_t w; - std::memcpy(&w, data + i, 4); + sum_uint32 w; + INET_CHECKSUM_MEMCPY(&w, data + i, 4); s0 += w; } @@ -85,23 +122,23 @@ namespace inet_checksum // the word with shifts 0/8/16 produces. Never reads past `length`. if (i < length) { - uint32_t w = 0; - unsigned shift = 0; + sum_uint32 w = 0; + unsigned int shift = 0; for (; i < length; ++i, shift += 8) - w |= static_cast(data[i]) << shift; + w |= static_cast(data[i]) << shift; s0 += w; } - uint64_t sum = s0 + s1 + s2 + s3; + sum_uint64 sum = s0 + s1 + s2 + s3; while (sum >> 32) - sum = (sum & 0xFFFFFFFFull) + (sum >> 32); + sum = (sum & 0xFFFFFFFFu) + (sum >> 32); - uint32_t folded = static_cast(sum); + sum_uint32 folded = static_cast(sum); while (folded >> 16) folded = (folded & 0xFFFF) + (folded >> 16); // Swap into the big-endian word domain the callers accumulate // pseudo-header words in. - return static_cast(((folded & 0xFF) << 8) | (folded >> 8)); + return static_cast(((folded & 0xFF) << 8) | (folded >> 8)); } } From fbe17d2a471e68a16fceb9dbd1772beab96290c0 Mon Sep 17 00:00:00 2001 From: Vadim Smirnov Date: Wed, 29 Jul 2026 21:36:50 +0200 Subject: [PATCH 3/3] compat: guarantee global size_t in the modern branch too sum16_be's signature uses unqualified size_t, which the legacy branch got from while the modern branch relied on injecting it into the global namespace - which the standard leaves unspecified ([support.c.headers]); only std::size_t is guaranteed there. Every MSVC/libstdc++/libc++ in practice does inject it, so this was latent, but the header claims self-sufficiency and should have it. now sits above the branch, covering both. Co-Authored-By: Claude Opus 5 --- ndisapi/inet_checksum.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ndisapi/inet_checksum.h b/ndisapi/inet_checksum.h index 00c6e3d..3ab0ebe 100644 --- a/ndisapi/inet_checksum.h +++ b/ndisapi/inet_checksum.h @@ -27,9 +27,13 @@ // down the legacy branch, which is how that branch is compile- and // correctness-tested without a VC6 installation. It changes types and // spelling only, never values. +// in both branches: sum16_be's signature uses unqualified size_t, +// and only the C header is guaranteed to place it in the global namespace -- +// / promise std::size_t, with global injection unspecified. +#include + #if (defined(_MSC_VER) && _MSC_VER < 1600) || defined(INET_CHECKSUM_FORCE_LEGACY_COMPAT) #include -#include #define INET_CHECKSUM_MEMCPY ::memcpy namespace inet_checksum {