Re: [ovs-dev] [PATCH v1 1/3] datapath-windows: Refactor conntrack code.

[Alin Serdean]

Acked-by: Alin Gabriel Serdean <aserd...@ovn.org>

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Anand Kumar
> Sent: Tuesday, November 14, 2017 8:48 PM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH v1 1/3] datapath-windows: Refactor conntrack
> code.
> 
> Some of the functions and  code are refactored so that new conntrack lock
> can be implemented
> 
> Signed-off-by: Anand Kumar <kumaran...@vmware.com>
> ---
>  datapath-windows/ovsext/Conntrack-nat.c |  11 +--
>  datapath-windows/ovsext/Conntrack.c | 170 ++--
> 
>  datapath-windows/ovsext/Conntrack.h |   4 -
>  3 files changed, 101 insertions(+), 84 deletions(-)
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v1 1/3] datapath-windows: Refactor conntrack code.

[Anand Kumar]

Some of the functions and  code are refactored
so that new conntrack lock can be implemented

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/Conntrack-nat.c |  11 +--
 datapath-windows/ovsext/Conntrack.c | 170 ++--
 datapath-windows/ovsext/Conntrack.h |   4 -
 3 files changed, 101 insertions(+), 84 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack-nat.c 
b/datapath-windows/ovsext/Conntrack-nat.c
index c778f12..7975770 100644
--- a/datapath-windows/ovsext/Conntrack-nat.c
+++ b/datapath-windows/ovsext/Conntrack-nat.c
@@ -93,26 +93,23 @@ NTSTATUS OvsNatInit()
 sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
 OVS_CT_POOL_TAG);
 if (ovsNatTable == NULL) {
-goto failNoMem;
+return STATUS_INSUFFICIENT_RESOURCES;
 }
 
 ovsUnNatTable = OvsAllocateMemoryWithTag(
 sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
 OVS_CT_POOL_TAG);
 if (ovsUnNatTable == NULL) {
-goto freeNatTable;
+OvsFreeMemoryWithTag(ovsNatTable, OVS_CT_POOL_TAG);
+return STATUS_INSUFFICIENT_RESOURCES;
 }
 
 for (int i = 0; i < NAT_HASH_TABLE_SIZE; i++) {
 InitializeListHead([i]);
 InitializeListHead([i]);
 }
-return STATUS_SUCCESS;
 
-freeNatTable:
-OvsFreeMemoryWithTag(ovsNatTable, OVS_CT_POOL_TAG);
-failNoMem:
-return STATUS_INSUFFICIENT_RESOURCES;
+return STATUS_SUCCESS;
 }
 
 /*
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 3203411..48d4abf 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -33,7 +33,7 @@ static PLIST_ENTRY ovsConntrackTable;
 static OVS_CT_THREAD_CTX ctThreadCtx;
 static PNDIS_RW_LOCK_EX ovsConntrackLockObj;
 extern POVS_SWITCH_CONTEXT gOvsSwitchContext;
-static UINT64 ctTotalEntries;
+static LONG ctTotalEntries;
 
 static __inline NDIS_STATUS OvsCtFlush(UINT16 zone);
 
@@ -210,7 +210,7 @@ OvsCtAddEntry(POVS_CT_ENTRY entry, OvsConntrackKeyLookupCtx 
*ctx,
 InsertHeadList([ctx->hash & CT_HASH_TABLE_MASK],
>link);
 
-ctTotalEntries++;
+InterlockedIncrement((LONG volatile *));
 return TRUE;
 }
 
@@ -233,11 +233,6 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 *entryCreated = FALSE;
 state |= OVS_CS_F_NEW;
 
-parentEntry = OvsCtRelatedLookup(ctx->key, currentTime);
-if (parentEntry != NULL) {
-state |= OVS_CS_F_RELATED;
-}
-
 switch (ipProto) {
 case IPPROTO_TCP:
 {
@@ -281,6 +276,11 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 break;
 }
 
+parentEntry = OvsCtRelatedLookup(ctx->key, currentTime);
+if (parentEntry != NULL && state != OVS_CS_F_INVALID) {
+state |= OVS_CS_F_RELATED;
+}
+
 if (state != OVS_CS_F_INVALID && commit) {
 if (entry) {
 entry->parent = parentEntry;
@@ -313,6 +313,7 @@ OvsCtUpdateEntry(OVS_CT_ENTRY* entry,
  BOOLEAN reply,
  UINT64 now)
 {
+CT_UPDATE_RES status;
 switch (ipProto) {
 case IPPROTO_TCP:
 {
@@ -320,32 +321,23 @@ OvsCtUpdateEntry(OVS_CT_ENTRY* entry,
 const TCPHdr *tcp;
 tcp = OvsGetTcp(nbl, l4Offset, );
 if (!tcp) {
-return CT_UPDATE_INVALID;
+status =  CT_UPDATE_INVALID;
+break;
 }
-return OvsConntrackUpdateTcpEntry(entry, tcp, nbl, reply, now);
+status =  OvsConntrackUpdateTcpEntry(entry, tcp, nbl, reply, now);
+break;
 }
 case IPPROTO_ICMP:
-return OvsConntrackUpdateIcmpEntry(entry, reply, now);
+status =  OvsConntrackUpdateIcmpEntry(entry, reply, now);
+break;
 case IPPROTO_UDP:
-return OvsConntrackUpdateOtherEntry(entry, reply, now);
+status =  OvsConntrackUpdateOtherEntry(entry, reply, now);
+break;
 default:
-return CT_UPDATE_INVALID;
-}
-}
-
-static __inline VOID
-OvsCtEntryDelete(POVS_CT_ENTRY entry)
-{
-if (entry == NULL) {
-return;
-}
-if (entry->natInfo.natAction) {
-OvsNatDeleteKey(>key);
+status =  CT_UPDATE_INVALID;
+break;
 }
-OvsPostCtEventEntry(entry, OVS_EVENT_CT_DELETE);
-RemoveEntryList(>link);
-OvsFreeMemoryWithTag(entry, OVS_CT_POOL_TAG);
-ctTotalEntries--;
+return status;
 }
 
 static __inline BOOLEAN
@@ -356,6 +348,24 @@ OvsCtEntryExpired(POVS_CT_ENTRY entry)
 return entry->expiration < currentTime;
 }
 
+static __inline VOID
+OvsCtEntryDelete(POVS_CT_ENTRY entry, BOOLEAN forceDelete)
+{
+if (entry == NULL) {
+return;
+}
+if (forceDelete || OvsCtEntryExpired(entry)) {
+if (entry->natInfo.natAction) {
+OvsNatDeleteKey(>key);
+}
+OvsPostCtEventEntry(entry, OVS_EVENT_CT_DELETE);
+RemoveEntryList(>link);
+OvsFreeMemoryWithTag(entry, OVS_CT_POOL_TAG);
+InterlockedDecrement((LONG volatile*));