[ovs-dev] [PATCH] Port patches to windows datapath: "conntrack: Fix conntrack new state"

Rui Cao rcao at vmware.com
Thu Jun 18 13:49:27 UTC 2020


Hi,

Recenty we found that two previous patches which fix the TCP conntrack new state issue in userspace are also needed by windows datapath.

Here's the details of the issue: https://github.com/openvswitch/ovs-issues/issues/188

I port these two patches to windows datapath and verified it locally, could you help review the patch:
https://github.com/openvswitch/ovs/pull/320
[https://avatars3.githubusercontent.com/u/7143863?s=400&v=4]<https://github.com/openvswitch/ovs/pull/320>
Port patches to windows: "conntrack: Fix conntrack new state" by ruicao93 · Pull Request #320 · openvswitch/ovs<https://github.com/openvswitch/ovs/pull/320>
Port following commits to windows driver to fix the TCP conntrack new state issue on windows: a867c01 ac23d20 @YiHungWei Fixes openvswitch/ovs-issues#188
github.com


Port patches to windows: "conntrack: Fix conntrack new state"

Port following commits to windows to fix the conntrack new state issue:
- a867c010ee9183885ee9d3eb76a0005c075c4d2e
- ac23d20fc90da3b1c9b2117d1e22102e99fba006

Signed-off-by: Rui Cao <rcao at vmware.com>

-------------------------------

 datapath-windows/ovsext/Conntrack-other.c |  4 +++-
 datapath-windows/ovsext/Conntrack-tcp.c   | 14 ++++++++++----
 datapath-windows/ovsext/Conntrack.c       |  3 +++
 datapath-windows/ovsext/Conntrack.h       |  1 +
 4 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack-other.c b/datapath-windows/ovsext/Conntrack-other.c
index 962cc8ac6..8580415a6 100644
--- a/datapath-windows/ovsext/Conntrack-other.c
+++ b/datapath-windows/ovsext/Conntrack-other.c
@@ -49,17 +49,19 @@ OvsConntrackUpdateOtherEntry(OVS_CT_ENTRY *conn_,
 {
     ASSERT(conn_);
     struct conn_other *conn = OvsCastConntrackEntryToOtherEntry(conn_);
+    enum CT_UPDATE_RES ret = CT_UPDATE_VALID;

     if (reply && conn->state != OTHERS_BIDIR) {
         conn->state = OTHERS_BIDIR;
     } else if (conn->state == OTHERS_FIRST) {
         conn->state = OTHERS_MULTIPLE;
+        ret = CT_UPDATE_VALID_NEW;
     }

     OvsConntrackUpdateExpiration(&conn->up, now,
                                  other_timeouts[conn->state]);

-    return CT_UPDATE_VALID;
+    return ret;
 }

 OVS_CT_ENTRY *
diff --git a/datapath-windows/ovsext/Conntrack-tcp.c b/datapath-windows/ovsext/Conntrack-tcp.c
index eda42ac82..a468c3e6b 100644
--- a/datapath-windows/ovsext/Conntrack-tcp.c
+++ b/datapath-windows/ovsext/Conntrack-tcp.c
@@ -213,11 +213,17 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_,
         return CT_UPDATE_INVALID;
     }

-    if (((tcp_flags & (TCP_SYN|TCP_ACK)) == TCP_SYN)
-            && dst->state >= CT_DPIF_TCPS_FIN_WAIT_2
+    if ((tcp_flags & (TCP_SYN|TCP_ACK)) == TCP_SYN) {
+        if (dst->state >= CT_DPIF_TCPS_FIN_WAIT_2
             && src->state >= CT_DPIF_TCPS_FIN_WAIT_2) {
-        src->state = dst->state = CT_DPIF_TCPS_CLOSED;
-        return CT_UPDATE_NEW;
+            src->state = dst->state = CT_DPIF_TCPS_CLOSED;
+            return CT_UPDATE_NEW;
+        } else if (src->state <= CT_DPIF_TCPS_SYN_SENT) {
+            src->state = CT_DPIF_TCPS_SYN_SENT;
+            OvsConntrackUpdateExpiration(&conn->up, now,
+                                         30 * CT_INTERVAL_SEC);
+            return CT_UPDATE_VALID_NEW;
+        }
     }

     if (src->wscale & CT_WSCALE_FLAG
diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c
index ba5611697..55917c43f 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -753,6 +753,9 @@ OvsProcessConntrackEntry(OvsForwardingContext *fwdCtx,
                 return NULL;
             }
             break;
+        case CT_UPDATE_VALID_NEW:
+            state |= OVS_CS_F_NEW;
+            break;
         }
     }
     if (entry) {
diff --git a/datapath-windows/ovsext/Conntrack.h b/datapath-windows/ovsext/Conntrack.h
index bc6580d70..b0932186a 100644
--- a/datapath-windows/ovsext/Conntrack.h
+++ b/datapath-windows/ovsext/Conntrack.h
@@ -56,6 +56,7 @@ typedef enum CT_UPDATE_RES {
     CT_UPDATE_INVALID,
     CT_UPDATE_VALID,
     CT_UPDATE_NEW,
+    CT_UPDATE_VALID_NEW,
 } CT_UPDATE_RES;

 /* Metadata mark for masked write to conntrack mark */



Thanks,
Rui


More information about the dev mailing list