diff mbox series

[ovs-dev,ovs-dev,v2] datapath-windows, conntrack: Fix conntrack new state

Message ID MWHPR0501MB3884BA5F0FE5C569BB068702B0940@MWHPR0501MB3884.namprd05.prod.outlook.com
State Accepted
Headers show
Series [ovs-dev,ovs-dev,v2] datapath-windows, conntrack: Fix conntrack new state | expand

Commit Message

Rui Cao June 23, 2020, 6:46 a.m. UTC
On windows, if we send a connection setup packet in one direction
twice, it will make the connection to be in established state. The
same issue happened in Linux userspace conntrack module and has
been fixed.

This patch port the following previous fixes to windows datapath to
fix the 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(-)

--
2.25.1.windows.1

Comments

William Tu June 27, 2020, 11:50 p.m. UTC | #1
On Tue, Jun 23, 2020 at 06:46:22AM +0000, Rui Cao wrote:
> On windows, if we send a connection setup packet in one direction
> twice, it will make the connection to be in established state. The
> same issue happened in Linux userspace conntrack module and has
> been fixed.
> 
> This patch port the following previous fixes to windows datapath to
> fix the issue:
> - a867c010ee9183885ee9d3eb76a0005c075c4d2e
> - ac23d20fc90da3b1c9b2117d1e22102e99fba006
> 
> Signed-off-by: Rui Cao <rcao@vmware.com>
> ---

Thanks, applied to master.
Rui Cao June 29, 2020, 2:19 a.m. UTC | #2
Thanks William.  Could you help backport the patch to branch-2.13?


Thanks,
Rui
William Tu June 30, 2020, 9:11 p.m. UTC | #3
On Sun, Jun 28, 2020 at 7:19 PM Rui Cao <rcao@vmware.com> wrote:
>
> Thanks William.  Could you help backport the patch to branch-2.13?
done, thanks
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 */