[ovs-dev] [PATCH v4 3/4] datapath-windows: Implement locking in conntrack NAT.

Shashank Ram rams at vmware.com
Mon Jun 18 21:36:53 UTC 2018


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 at 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);
>   }
>   
>   /*



More information about the dev mailing list