Message ID | 20180618053729.2372-4-kumaranand@vmware.com |
---|---|
State | Superseded |
Headers | show |
Series | Optimize conntrack performance | expand |
This patch should be combined with the patch where NAT lock is removed from CT. Keeping this separate will cause the previous patches in this series to break NAT functionality. Thanks, Shashank On 06/17/2018 10:37 PM, Anand Kumar wrote: > The 'ovsNatTable' and 'ovsUnNatTable' tables are shared > between cleanup threads and packet processing thread. > In order to protect these two tables use a spinlock. > > Also introduce counters to track number of nat entries. > > Signed-off-by: Anand Kumar <kumaranand@vmware.com> > --- > datapath-windows/ovsext/Conntrack-nat.c | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/datapath-windows/ovsext/Conntrack-nat.c b/datapath-windows/ovsext/Conntrack-nat.c > index da1814f..11057e6 100644 > --- a/datapath-windows/ovsext/Conntrack-nat.c > +++ b/datapath-windows/ovsext/Conntrack-nat.c > @@ -3,7 +3,8 @@ > > PLIST_ENTRY ovsNatTable = NULL; > PLIST_ENTRY ovsUnNatTable = NULL; > - > +static NDIS_SPIN_LOCK ovsCtNatLock; > +static ULONG ovsNatEntries; > /* > *--------------------------------------------------------------------------- > * OvsHashNatKey > @@ -109,6 +110,8 @@ NTSTATUS OvsNatInit() > InitializeListHead(&ovsUnNatTable[i]); > } > > + NdisAllocateSpinLock(&ovsCtNatLock); > + ovsNatEntries = 0; > return STATUS_SUCCESS; > } > > @@ -121,6 +124,11 @@ NTSTATUS OvsNatInit() > VOID OvsNatFlush(UINT16 zone) > { > PLIST_ENTRY link, next; > + if (!ovsNatEntries) { > + return; > + } > + > + NdisAcquireSpinLock(&ovsCtNatLock); > for (int i = 0; i < NAT_HASH_TABLE_SIZE; i++) { > LIST_FORALL_SAFE(&ovsNatTable[i], link, next) { > POVS_NAT_ENTRY entry = > @@ -131,6 +139,7 @@ VOID OvsNatFlush(UINT16 zone) > } > } > } > + NdisReleaseSpinLock(&ovsCtNatLock); > } > > /* > @@ -144,10 +153,14 @@ VOID OvsNatCleanup() > if (ovsNatTable == NULL) { > return; > } > + > + NdisAcquireSpinLock(&ovsCtNatLock); > OvsFreeMemoryWithTag(ovsNatTable, OVS_CT_POOL_TAG); > OvsFreeMemoryWithTag(ovsUnNatTable, OVS_CT_POOL_TAG); > ovsNatTable = NULL; > ovsUnNatTable = NULL; > + NdisReleaseSpinLock(&ovsCtNatLock); > + NdisFreeSpinLock(&ovsCtNatLock); > } > > /* > @@ -250,10 +263,13 @@ static UINT32 OvsNatHashRange(const OVS_CT_ENTRY *entry, UINT32 basis) > VOID > OvsNatAddEntry(OVS_NAT_ENTRY* entry) > { > + NdisAcquireSpinLock(&ovsCtNatLock); > InsertHeadList(OvsNatGetBucket(&entry->key, FALSE), > &entry->link); > InsertHeadList(OvsNatGetBucket(&entry->value, TRUE), > &entry->reverseLink); > + NdisReleaseSpinLock(&ovsCtNatLock); > + NdisInterlockedIncrement((PLONG)&ovsNatEntries); > } > > /* > @@ -399,21 +415,29 @@ OvsNatLookup(const OVS_CT_KEY *ctKey, BOOLEAN reverse) > PLIST_ENTRY link; > POVS_NAT_ENTRY entry; > > + if (!ovsNatEntries) { > + return NULL; > + } > + > + NdisAcquireSpinLock(&ovsCtNatLock); > LIST_FORALL(OvsNatGetBucket(ctKey, reverse), link) { > if (reverse) { > entry = CONTAINING_RECORD(link, OVS_NAT_ENTRY, reverseLink); > > if (OvsNatKeyAreSame(ctKey, &entry->value)) { > + NdisReleaseSpinLock(&ovsCtNatLock); > return entry; > } > } else { > entry = CONTAINING_RECORD(link, OVS_NAT_ENTRY, link); > > if (OvsNatKeyAreSame(ctKey, &entry->key)) { > + NdisReleaseSpinLock(&ovsCtNatLock); > return entry; > } > } > } > + NdisReleaseSpinLock(&ovsCtNatLock); > return NULL; > } > > @@ -432,6 +456,7 @@ OvsNatDeleteEntry(POVS_NAT_ENTRY entry) > RemoveEntryList(&entry->link); > RemoveEntryList(&entry->reverseLink); > OvsFreeMemoryWithTag(entry, OVS_CT_POOL_TAG); > + NdisInterlockedDecrement((PLONG)&ovsNatEntries); > } > > /*
Hi Shashank, I will address this in next version of the patch. Thanks, Anand Kumar On 6/18/18, 2:36 PM, "Shashank Ram" <rams@vmware.com> wrote: This patch should be combined with the patch where NAT lock is removed from CT. Keeping this separate will cause the previous patches in this series to break NAT functionality. Thanks, Shashank On 06/17/2018 10:37 PM, Anand Kumar wrote: > The 'ovsNatTable' and 'ovsUnNatTable' tables are shared > between cleanup threads and packet processing thread. > In order to protect these two tables use a spinlock. > > Also introduce counters to track number of nat entries. > > Signed-off-by: Anand Kumar <kumaranand@vmware.com> > --- > datapath-windows/ovsext/Conntrack-nat.c | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/datapath-windows/ovsext/Conntrack-nat.c b/datapath-windows/ovsext/Conntrack-nat.c > index da1814f..11057e6 100644 > --- a/datapath-windows/ovsext/Conntrack-nat.c > +++ b/datapath-windows/ovsext/Conntrack-nat.c > @@ -3,7 +3,8 @@ > > PLIST_ENTRY ovsNatTable = NULL; > PLIST_ENTRY ovsUnNatTable = NULL; > - > +static NDIS_SPIN_LOCK ovsCtNatLock; > +static ULONG ovsNatEntries; > /* > *--------------------------------------------------------------------------- > * OvsHashNatKey > @@ -109,6 +110,8 @@ NTSTATUS OvsNatInit() > InitializeListHead(&ovsUnNatTable[i]); > } > > + NdisAllocateSpinLock(&ovsCtNatLock); > + ovsNatEntries = 0; > return STATUS_SUCCESS; > } > > @@ -121,6 +124,11 @@ NTSTATUS OvsNatInit() > VOID OvsNatFlush(UINT16 zone) > { > PLIST_ENTRY link, next; > + if (!ovsNatEntries) { > + return; > + } > + > + NdisAcquireSpinLock(&ovsCtNatLock); > for (int i = 0; i < NAT_HASH_TABLE_SIZE; i++) { > LIST_FORALL_SAFE(&ovsNatTable[i], link, next) { > POVS_NAT_ENTRY entry = > @@ -131,6 +139,7 @@ VOID OvsNatFlush(UINT16 zone) > } > } > } > + NdisReleaseSpinLock(&ovsCtNatLock); > } > > /* > @@ -144,10 +153,14 @@ VOID OvsNatCleanup() > if (ovsNatTable == NULL) { > return; > } > + > + NdisAcquireSpinLock(&ovsCtNatLock); > OvsFreeMemoryWithTag(ovsNatTable, OVS_CT_POOL_TAG); > OvsFreeMemoryWithTag(ovsUnNatTable, OVS_CT_POOL_TAG); > ovsNatTable = NULL; > ovsUnNatTable = NULL; > + NdisReleaseSpinLock(&ovsCtNatLock); > + NdisFreeSpinLock(&ovsCtNatLock); > } > > /* > @@ -250,10 +263,13 @@ static UINT32 OvsNatHashRange(const OVS_CT_ENTRY *entry, UINT32 basis) > VOID > OvsNatAddEntry(OVS_NAT_ENTRY* entry) > { > + NdisAcquireSpinLock(&ovsCtNatLock); > InsertHeadList(OvsNatGetBucket(&entry->key, FALSE), > &entry->link); > InsertHeadList(OvsNatGetBucket(&entry->value, TRUE), > &entry->reverseLink); > + NdisReleaseSpinLock(&ovsCtNatLock); > + NdisInterlockedIncrement((PLONG)&ovsNatEntries); > } > > /* > @@ -399,21 +415,29 @@ OvsNatLookup(const OVS_CT_KEY *ctKey, BOOLEAN reverse) > PLIST_ENTRY link; > POVS_NAT_ENTRY entry; > > + if (!ovsNatEntries) { > + return NULL; > + } > + > + NdisAcquireSpinLock(&ovsCtNatLock); > LIST_FORALL(OvsNatGetBucket(ctKey, reverse), link) { > if (reverse) { > entry = CONTAINING_RECORD(link, OVS_NAT_ENTRY, reverseLink); > > if (OvsNatKeyAreSame(ctKey, &entry->value)) { > + NdisReleaseSpinLock(&ovsCtNatLock); > return entry; > } > } else { > entry = CONTAINING_RECORD(link, OVS_NAT_ENTRY, link); > > if (OvsNatKeyAreSame(ctKey, &entry->key)) { > + NdisReleaseSpinLock(&ovsCtNatLock); > return entry; > } > } > } > + NdisReleaseSpinLock(&ovsCtNatLock); > return NULL; > } > > @@ -432,6 +456,7 @@ OvsNatDeleteEntry(POVS_NAT_ENTRY entry) > RemoveEntryList(&entry->link); > RemoveEntryList(&entry->reverseLink); > OvsFreeMemoryWithTag(entry, OVS_CT_POOL_TAG); > + NdisInterlockedDecrement((PLONG)&ovsNatEntries); > } > > /*
diff --git a/datapath-windows/ovsext/Conntrack-nat.c b/datapath-windows/ovsext/Conntrack-nat.c index da1814f..11057e6 100644 --- a/datapath-windows/ovsext/Conntrack-nat.c +++ b/datapath-windows/ovsext/Conntrack-nat.c @@ -3,7 +3,8 @@ PLIST_ENTRY ovsNatTable = NULL; PLIST_ENTRY ovsUnNatTable = NULL; - +static NDIS_SPIN_LOCK ovsCtNatLock; +static ULONG ovsNatEntries; /* *--------------------------------------------------------------------------- * OvsHashNatKey @@ -109,6 +110,8 @@ NTSTATUS OvsNatInit() InitializeListHead(&ovsUnNatTable[i]); } + NdisAllocateSpinLock(&ovsCtNatLock); + ovsNatEntries = 0; return STATUS_SUCCESS; } @@ -121,6 +124,11 @@ NTSTATUS OvsNatInit() VOID OvsNatFlush(UINT16 zone) { PLIST_ENTRY link, next; + if (!ovsNatEntries) { + return; + } + + NdisAcquireSpinLock(&ovsCtNatLock); for (int i = 0; i < NAT_HASH_TABLE_SIZE; i++) { LIST_FORALL_SAFE(&ovsNatTable[i], link, next) { POVS_NAT_ENTRY entry = @@ -131,6 +139,7 @@ VOID OvsNatFlush(UINT16 zone) } } } + NdisReleaseSpinLock(&ovsCtNatLock); } /* @@ -144,10 +153,14 @@ VOID OvsNatCleanup() if (ovsNatTable == NULL) { return; } + + NdisAcquireSpinLock(&ovsCtNatLock); OvsFreeMemoryWithTag(ovsNatTable, OVS_CT_POOL_TAG); OvsFreeMemoryWithTag(ovsUnNatTable, OVS_CT_POOL_TAG); ovsNatTable = NULL; ovsUnNatTable = NULL; + NdisReleaseSpinLock(&ovsCtNatLock); + NdisFreeSpinLock(&ovsCtNatLock); } /* @@ -250,10 +263,13 @@ static UINT32 OvsNatHashRange(const OVS_CT_ENTRY *entry, UINT32 basis) VOID OvsNatAddEntry(OVS_NAT_ENTRY* entry) { + NdisAcquireSpinLock(&ovsCtNatLock); InsertHeadList(OvsNatGetBucket(&entry->key, FALSE), &entry->link); InsertHeadList(OvsNatGetBucket(&entry->value, TRUE), &entry->reverseLink); + NdisReleaseSpinLock(&ovsCtNatLock); + NdisInterlockedIncrement((PLONG)&ovsNatEntries); } /* @@ -399,21 +415,29 @@ OvsNatLookup(const OVS_CT_KEY *ctKey, BOOLEAN reverse) PLIST_ENTRY link; POVS_NAT_ENTRY entry; + if (!ovsNatEntries) { + return NULL; + } + + NdisAcquireSpinLock(&ovsCtNatLock); LIST_FORALL(OvsNatGetBucket(ctKey, reverse), link) { if (reverse) { entry = CONTAINING_RECORD(link, OVS_NAT_ENTRY, reverseLink); if (OvsNatKeyAreSame(ctKey, &entry->value)) { + NdisReleaseSpinLock(&ovsCtNatLock); return entry; } } else { entry = CONTAINING_RECORD(link, OVS_NAT_ENTRY, link); if (OvsNatKeyAreSame(ctKey, &entry->key)) { + NdisReleaseSpinLock(&ovsCtNatLock); return entry; } } } + NdisReleaseSpinLock(&ovsCtNatLock); return NULL; } @@ -432,6 +456,7 @@ OvsNatDeleteEntry(POVS_NAT_ENTRY entry) RemoveEntryList(&entry->link); RemoveEntryList(&entry->reverseLink); OvsFreeMemoryWithTag(entry, OVS_CT_POOL_TAG); + NdisInterlockedDecrement((PLONG)&ovsNatEntries); } /*
The 'ovsNatTable' and 'ovsUnNatTable' tables are shared between cleanup threads and packet processing thread. In order to protect these two tables use a spinlock. Also introduce counters to track number of nat entries. Signed-off-by: Anand Kumar <kumaranand@vmware.com> --- datapath-windows/ovsext/Conntrack-nat.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)