[PATCH] [NetKVM] Fix checksum verification for pkts with csum==0xffff

[PATCH] [NetKVM] Fix checksum verification for pkts with csum==0xffff

[Dmitry Skorodumov]

Windows-host with disabled TX-checksum offloading may send
packets with tcp->checksum=0xffff. Most likely it is caused
by by Windows algorithm for computing incremental checksum
- see RFC 1624.

RFC1624 (sec.5) states that 0xffff and 0x0000 are equal, because
for example

0xCD7A + 0x3285 + 0xFFFF = 0xFFFF
0xCD7A + 0x3285 + 0x0000 = 0xFFFF;

Fix tcp/udp verification algorithm to check that checksum
of (transport-header + pseudo_hdr_csum) == 0
instead of recomputing checksum and comparing its value with
original value in transp_hdr->csum.

The problem could be reproduced on host linux network interface
without rx-offloading, because otherwise Guest-driver receives
from host packet with flag HDR_DATA_VALID
and VerifyTcpChecksum doesn't do any work.

---
 NetKVM/Common/sw-offload.c |   46 ++++++++++++++++++++++++++-----------------
 1 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/NetKVM/Common/sw-offload.c b/NetKVM/Common/sw-offload.c
index 24acfdc..c8e1a39 100644
--- a/NetKVM/Common/sw-offload.c
+++ b/NetKVM/Common/sw-offload.c
@@ -372,6 +372,7 @@ static __inline tTcpIpPacketParsingResult
 VerifyTcpChecksum( IPHeader *pIpHeader, ULONG len, tTcpIpPacketParsingResult known, ULONG whatToFix)
 {
 	USHORT  phcs;
+	USHORT checksum;
 	tTcpIpPacketParsingResult res = known;
 	TCPHeader *pTcpHeader = (TCPHeader *)RtlOffsetToPointer(pIpHeader, res.ipHeaderSize);
 	USHORT saved = pTcpHeader->tcp_xsum;
@@ -395,16 +396,21 @@ VerifyTcpChecksum( IPHeader *pIpHeader, ULONG len, tTcpIpPacketParsingResult kno
 			else if (res.xxpFull)
 			{
 				//USHORT ipFullLength = swap_short(pIpHeader->v4.ip_length);
-				pTcpHeader->tcp_xsum = phcs;
-				CalculateTcpChecksumGivenPseudoCS(pTcpHeader, xxpHeaderAndPayloadLen);
-				if (saved == pTcpHeader->tcp_xsum)
-					res.xxpCheckSum = ppresCSOK;
-
 				if (!(whatToFix & pcrFixXxpChecksum))
-					pTcpHeader->tcp_xsum = saved;
+				{
+					checksum = CheckSumCalculator(phcs, pTcpHeader, xxpHeaderAndPayloadLen);
+					if (checksum == 0)
+						res.xxpCheckSum = ppresCSOK;
+				}
 				else
+				{
+					pTcpHeader->tcp_xsum = phcs;
+					CalculateTcpChecksumGivenPseudoCS(pTcpHeader, xxpHeaderAndPayloadLen);
+					if (saved == pTcpHeader->tcp_xsum)
+						res.xxpCheckSum = ppresCSOK;
 					res.fixedXxpCS =
 						res.xxpCheckSum == ppresCSBad || res.xxpCheckSum == ppresPCSOK;
+				}
 			}
 			else if (whatToFix)
 			{
@@ -416,10 +422,9 @@ VerifyTcpChecksum( IPHeader *pIpHeader, ULONG len, tTcpIpPacketParsingResult kno
 			// we have correct PHCS and we do not need to fix anything
 			// there is a very small chance that it is also good TCP CS
 			// in such rare case we give a priority to TCP CS
-			CalculateTcpChecksumGivenPseudoCS(pTcpHeader, xxpHeaderAndPayloadLen);
-			if (saved == pTcpHeader->tcp_xsum)
+			checksum = CheckSumCalculator(phcs, pTcpHeader, xxpHeaderAndPayloadLen);
+			if (checksum == 0)
 				res.xxpCheckSum = ppresCSOK;
-			pTcpHeader->tcp_xsum = saved;
 		}
 	}
 	else
@@ -437,6 +442,7 @@ static __inline tTcpIpPacketParsingResult
 VerifyUdpChecksum( IPHeader *pIpHeader, ULONG len, tTcpIpPacketParsingResult known, ULONG whatToFix)
 {
 	USHORT  phcs;
+	USHORT checksum;
 	tTcpIpPacketParsingResult res = known;
 	UDPHeader *pUdpHeader = (UDPHeader *)RtlOffsetToPointer(pIpHeader, res.ipHeaderSize);
 	USHORT saved = pUdpHeader->udp_xsum;
@@ -459,16 +465,21 @@ VerifyUdpChecksum( IPHeader *pIpHeader, ULONG len, tTcpIpPacketParsingResult kno
 		{
 			if (res.xxpFull)
 			{
-				pUdpHeader->udp_xsum = phcs;
-				CalculateUdpChecksumGivenPseudoCS(pUdpHeader, xxpHeaderAndPayloadLen);
-				if (saved == pUdpHeader->udp_xsum)
-					res.xxpCheckSum = ppresCSOK;
-
 				if (!(whatToFix & pcrFixXxpChecksum))
-					pUdpHeader->udp_xsum = saved;
+				{
+					checksum = CheckSumCalculator(phcs, pUdpHeader, xxpHeaderAndPayloadLen);
+					if (checksum == 0)
+						res.xxpCheckSum = ppresCSOK;
+				}
 				else
+				{
+					pUdpHeader->udp_xsum = phcs;
+					CalculateUdpChecksumGivenPseudoCS(pUdpHeader, xxpHeaderAndPayloadLen);
+					if (saved == pUdpHeader->udp_xsum)
+						res.xxpCheckSum = ppresCSOK;
 					res.fixedXxpCS =
 						res.xxpCheckSum == ppresCSBad || res.xxpCheckSum == ppresPCSOK;
+				}
 			}
 			else
 				res.xxpCheckSum = ppresXxpIncomplete;
@@ -478,10 +489,9 @@ VerifyUdpChecksum( IPHeader *pIpHeader, ULONG len, tTcpIpPacketParsingResult kno
 			// we have correct PHCS and we do not need to fix anything
 			// there is a very small chance that it is also good UDP CS
 			// in such rare case we give a priority to UDP CS
-			CalculateUdpChecksumGivenPseudoCS(pUdpHeader, xxpHeaderAndPayloadLen);
-			if (saved == pUdpHeader->udp_xsum)
+			checksum = CheckSumCalculator(phcs, pUdpHeader, xxpHeaderAndPayloadLen);
+			if (checksum == 0)
 				res.xxpCheckSum = ppresCSOK;
-			pUdpHeader->udp_xsum = saved;
 		}
 	}
 	else
-- 
1.7.7.1.msysgit.0

Re: [PATCH] [NetKVM] Fix checksum verification for pkts with csum==0xffff

[Dmitry Skorodumov]

> Dmitry Skorodumov <sdmitry <at> parallels.com> writes:
> 
> Windows-host with disabled TX-checksum offloading may send
> packets with tcp->checksum=0xffff. Most likely it is caused
> by by Windows algorithm for computing incremental checksum
> - see RFC 1624.

Forgot to mention that packets with checksum 0xffff appears almost 100%
if run from windows-host to kvm-guest

netperf -H <guest_addr> -t TCP_RR

Dmitry