From patchwork Fri Oct 22 09:05:13 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wilson Peng X-Patchwork-Id: 1544854 X-Patchwork-Delegate: aserdean@cloudbasesolutions.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HbJN72Sjqz9sRN for ; Fri, 22 Oct 2021 20:05:25 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id A5A93401FE; Fri, 22 Oct 2021 09:05:23 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id WkN3EYLH3f76; Fri, 22 Oct 2021 09:05:22 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp2.osuosl.org (Postfix) with ESMTPS id 6DDF4401B8; Fri, 22 Oct 2021 09:05:21 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 33A76C0020; Fri, 22 Oct 2021 09:05:21 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id EEF2EC001E for ; Fri, 22 Oct 2021 09:05:19 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id DCB416063B for ; Fri, 22 Oct 2021 09:05:19 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id O_7scPpYJ5I5 for ; Fri, 22 Oct 2021 09:05:19 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from EX13-EDG-OU-002.vmware.com (ex13-edg-ou-002.vmware.com [208.91.0.190]) by smtp3.osuosl.org (Postfix) with ESMTPS id 0FD4160614 for ; Fri, 22 Oct 2021 09:05:18 +0000 (UTC) Received: from sc9-mailhost3.vmware.com (10.113.161.73) by EX13-EDG-OU-002.vmware.com (10.113.208.156) with Microsoft SMTP Server id 15.0.1156.6; Fri, 22 Oct 2021 02:05:09 -0700 Received: from pweisong-a01.vmware.com.com (unknown [10.117.181.32]) by sc9-mailhost3.vmware.com (Postfix) with ESMTP id A8D3220135; Fri, 22 Oct 2021 02:05:15 -0700 (PDT) From: Wilson Peng To: Date: Fri, 22 Oct 2021 17:05:13 +0800 Message-ID: <20211022090513.91799-1-pweisong@vmware.com> X-Mailer: git-send-email 2.24.3 (Apple Git-128) MIME-Version: 1.0 Received-SPF: None (EX13-EDG-OU-002.vmware.com: pweisong@vmware.com does not designate permitted sender hosts) Subject: [ovs-dev] [PATCH v2 1/1] datapath-windows:Reset PseudoChecksum value only for TX direction offload case X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" While testing OVS-windows flows for the DNAT/SNAT action, the checksum in TCP header is set incorrectly when TCP offload is enabled by default. As a result,the packet will be dropped on the Windows VM when processing the packet from Linux VM which has included correct checksum at first. On the Windows VM, it has gone through two NAT actions and OVS Windows kernel will reset the checksum to PseudoChecksum and then it will lose the original correct checksum value which is set outside. Back to the Nat TCP/UDP checksum value reset logic,it should reset it TCP checksum To be PseudoChecksum value only on Tx direction for TCP Offload case. For the packet From the outside, OVS Windows Kernel does not need reset the TCP/UDP checksum as It should be the job of the received network driver to get out a correct checksum Value. >>>sample flow on default configuration on both Windows VM and Linux VM (src=192.168.252.1,dst=10.110.225.146)-->dnat/snat-> (src=169.254.169.253, Dst=10.176.26.107) Without the fix the return back packet(src=10.176.26.107, Dst=169.254.169.253) will have the correct TCP checksum. After the reverse NAT Actions, it will be changed to be packet (src=10.110.225.146, Dst=192.168.252.1) But with incorrect TCP checksum 0xa97a which is The PseudoChecksum. Related packet is put on the reported issue below. Reported-at:https://github.com/openvswitch/ovs-issues/issues/231 Signed-off-by: Wilson Peng --- datapath-windows/ovsext/Actions.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/Actions.c index 592cb7467..0c18c6254 100644 --- a/datapath-windows/ovsext/Actions.c +++ b/datapath-windows/ovsext/Actions.c @@ -1527,6 +1527,7 @@ OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx, csumInfo.Value = NET_BUFFER_LIST_INFO(ovsFwdCtx->curNbl, TcpIpChecksumNetBufferListInfo); + /* * Adjust the IP header inline as dictated by the action, and also update * the IP and the TCP checksum for the data modified. @@ -1535,6 +1536,7 @@ OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx, * ChecksumUpdate32(). Ignoring this for now, since for the most common * case, we only update the TTL. */ + /*Only tx direction the checksum value will be reset to be PseudoChecksum*/ if (isSource) { addrField = &ipHdr->saddr; @@ -1551,7 +1553,7 @@ OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx, ((BOOLEAN)csumInfo.Receive.UdpChecksumSucceeded || (BOOLEAN)csumInfo.Receive.UdpChecksumFailed); } - if (l4Offload) { + if (isTx && l4Offload) { *checkField = IPPseudoChecksum(&newAddr, &ipHdr->daddr, tcpHdr ? IPPROTO_TCP : IPPROTO_UDP, ntohs(ipHdr->tot_len) - ipHdr->ihl * 4); @@ -1572,7 +1574,7 @@ OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx, (BOOLEAN)csumInfo.Receive.UdpChecksumFailed); } - if (l4Offload) { + if (isTx && l4Offload) { *checkField = IPPseudoChecksum(&ipHdr->saddr, &newAddr, tcpHdr ? IPPROTO_TCP : IPPROTO_UDP, ntohs(ipHdr->tot_len) - ipHdr->ihl * 4); @@ -1581,7 +1583,7 @@ OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx, if (*addrField != newAddr) { UINT32 oldAddr = *addrField; - if (checkField && *checkField != 0 && !l4Offload) { + if ((checkField && *checkField != 0) && (!l4Offload || !isTx)) { /* Recompute total checksum. */ *checkField = ChecksumUpdate32(*checkField, oldAddr, newAddr); @@ -1590,11 +1592,12 @@ OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx, ipHdr->check = ChecksumUpdate32(ipHdr->check, oldAddr, newAddr); } + *addrField = newAddr; } if (portField && *portField != newPort) { - if (checkField && !l4Offload) { + if ((checkField) && (!l4Offload || !isTx)) { /* Recompute total checksum. */ *checkField = ChecksumUpdate16(*checkField, *portField, newPort);