Resending email because ovn mail blocked the first one.

-----Original Message-----
From: Alin-Gabriel Serdean <[email protected]> 
Sent: Tuesday, November 8, 2022 8:01 PM
To: 'Wilson Peng' <[email protected]>; '[email protected]'
<[email protected]>
Subject: RE: [ovs-dev] [PATCH v1 1/1] datapath-windows: Check the condition
to reset pseudo header checksum on Rx side

Hi Wilson,

Thank you for the patch. Just a small nit rest looks good.

Please see below.

-----Original Message-----
From: dev <[email protected]> On Behalf Of Wilson Peng
Sent: Friday, October 21, 2022 7:34 AM
To: [email protected]
Subject: [ovs-dev] [PATCH v1 1/1] datapath-windows: Check the condition to
reset pseudo header checksum on Rx side

From: Wilson Peng <[email protected]>

If ovs node running on Windows is processing NAT action on the RX side,  it
will reset pseudo header checksum only if the L4 checksum is same as the
calculated pseudo header checksum before NAT action.

Without the fix, if the L4 header checksum is filled with a pseudo header
checksum (sourceip, dstip, protocol, tcppayloadlen+tcpheaderlen) OVS will
still do the checksum update(replace some IP and port and recalculate the
checksum). It will lead to incorrect L4 header checksum.

Reported-at:https://github.com/openvswitch/ovs-issues/issues/265
Signed-off-by: Wilson Peng <[email protected]>
---
 datapath-windows/ovsext/Actions.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c
b/datapath-windows/ovsext/Actions.c
index 2f44086b4..7d72a2276 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1514,6 +1514,8 @@ OvsUpdateAddressAndPort(OvsForwardingContext
*ovsFwdCtx,
     UINT16 *checkField = NULL;
     BOOLEAN l4Offload = FALSE;
     NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO csumInfo;
+    UINT16 PreNatPseudoChecksum = 0;
+    BOOLEAN PreNatChecksumIsPseudoChecksum = FALSE;
[AS] The name seems off a bit, how about preservePseudoChecksum ?
 
     ASSERT(layers->value != 0);
 
@@ -1550,6 +1552,10 @@ OvsUpdateAddressAndPort(OvsForwardingContext
*ovsFwdCtx,
      */
      /*Only tx direction the checksum value will be reset to be
PseudoChecksum*/
 
+    PreNatPseudoChecksum = IPPseudoChecksum(&ipHdr->saddr, &ipHdr->daddr,
+        tcpHdr ? IPPROTO_TCP : IPPROTO_UDP,
+        ntohs(ipHdr->tot_len) - ipHdr->ihl * 4);
+
     if (isSource) {
         addrField = &ipHdr->saddr;
         if (tcpHdr) {
@@ -1565,7 +1571,12 @@ OvsUpdateAddressAndPort(OvsForwardingContext
*ovsFwdCtx,
                         ((BOOLEAN)csumInfo.Receive.UdpChecksumSucceeded ||
                          (BOOLEAN)csumInfo.Receive.UdpChecksumFailed);
         }
-        if (isTx && l4Offload) {
+        if (!isTx && l4Offload) {
+            if (*checkField == PreNatPseudoChecksum) {
+                PreNatChecksumIsPseudoChecksum = TRUE;
+            }

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to