diff mbox series

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

Message ID MWHPR0501MB3884BF61E5A62A72DEC84913B09B0@MWHPR0501MB3884.namprd05.prod.outlook.com
State New
Headers show
Series [ovs-dev] Port patches to windows datapath: "conntrack: Fix conntrack new state" | expand

Commit Message

Rui Cao June 18, 2020, 1:49 p.m. UTC
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@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(-)




Thanks,
Rui

Comments

Rui Cao June 23, 2020, 12:22 a.m. UTC | #1
Hi,

Anyone could help review or submit the patch?


Thanks,
Rui
Yi-Hung Wei June 23, 2020, 4:33 p.m. UTC | #2
On Thu, Jun 18, 2020 at 7:05 AM Rui Cao <rcao@vmware.com> wrote:
>
> 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@vmware.com>
>

Thanks for porting this patch to windows datapath.  It looks good from
conntrack's perspective.  I do not have a windows environment to test
it tho.

Acked-by: Yi-Hung Wei <yihung.wei@gmail.com>
diff mbox series

Patch

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 */