| Message ID | MWHPR0501MB3884BF61E5A62A72DEC84913B09B0@MWHPR0501MB3884.namprd05.prod.outlook.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] Port patches to windows datapath: "conntrack: Fix conntrack new state" | expand |
Hi, Anyone could help review or submit the patch? Thanks, Rui
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 --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 */
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