[ovs-dev] [PATCH v3] datapath-windows: Optimize conntrack performance

Anand Kumar kumaranand at vmware.com
Mon Jun 18 05:36:46 UTC 2018


Abandoning this change. I have couple of patches to optimize conntrack, will be sending it out as a series.

Thanks,
Anand Kumar

On 6/11/18, 11:39 AM, "Anand Kumar" <kumaranand at vmware.com> wrote:

    - Use spinlock instead of read/write lock for conntrack entry.
    - Use NdisInterlockedXX wrapper api's instead of InterlockedXX
    - Update 'ctTotalRelatedEntries' using interlocked functions
    - Refactor conntrack code to make it simpler to read.
    
    Testing:
    Evaluated TCP performance using iperf3.
    
    Before optimization:
    Native: 6.0Gbps
    OVS: 5.1-5.75Gbps
    OVS with conntrack enabled: 3.9-4.0Gbps
    
    After optimization:
    Native: 6.0Gbps
    OVS: 5.1-5.75Gbps
    OVS with conntrack enabled:: 4.2-4.4Gbps
    
    Tested by loading/unloading driver with driver verifier enabled
    
    Signed-off-by: Anand Kumar <kumaranand at vmware.com>
    ---
    v1->v2: Update commit message
    v2->v3: Address review comments
    ---
     datapath-windows/ovsext/Conntrack-nat.c     |   7 +-
     datapath-windows/ovsext/Conntrack-related.c |  17 ++--
     datapath-windows/ovsext/Conntrack.c         | 130 +++++++++++++---------------
     datapath-windows/ovsext/Conntrack.h         |  20 ++++-
     4 files changed, 89 insertions(+), 85 deletions(-)
    
    diff --git a/datapath-windows/ovsext/Conntrack-nat.c b/datapath-windows/ovsext/Conntrack-nat.c
    index 316c946..da1814f 100644
    --- a/datapath-windows/ovsext/Conntrack-nat.c
    +++ b/datapath-windows/ovsext/Conntrack-nat.c
    @@ -167,16 +167,13 @@ OvsNatPacket(OvsForwardingContext *ovsFwdCtx,
     {
         UINT32 natFlag;
         const struct ct_endpoint* endpoint;
    -    LOCK_STATE_EX lockState;
    -    /* XXX: Move conntrack locks out of NAT after implementing lock in NAT. */
    -    NdisAcquireRWLockRead(entry->lock, &lockState, 0);
    +
         /* When it is NAT, only entry->rev_key contains NATTED address;
            When it is unNAT, only entry->key contains the UNNATTED address;*/
         const OVS_CT_KEY *ctKey = reverse ? &entry->key : &entry->rev_key;
         BOOLEAN isSrcNat;
     
         if (!(natAction & (NAT_ACTION_SRC | NAT_ACTION_DST))) {
    -        NdisReleaseRWLock(entry->lock, &lockState);
             return;
         }
         isSrcNat = (((natAction & NAT_ACTION_SRC) && !reverse) ||
    @@ -206,7 +203,6 @@ OvsNatPacket(OvsForwardingContext *ovsFwdCtx,
             }
         } else if (ctKey->dl_type == htons(ETH_TYPE_IPV6)){
             // XXX: IPv6 packet not supported yet.
    -        NdisReleaseRWLock(entry->lock, &lockState);
             return;
         }
         if (natAction & (NAT_ACTION_SRC_PORT | NAT_ACTION_DST_PORT)) {
    @@ -220,7 +216,6 @@ OvsNatPacket(OvsForwardingContext *ovsFwdCtx,
                 }
             }
         }
    -    NdisReleaseRWLock(entry->lock, &lockState);
     }
     
     
    diff --git a/datapath-windows/ovsext/Conntrack-related.c b/datapath-windows/ovsext/Conntrack-related.c
    index ec4b536..b798137 100644
    --- a/datapath-windows/ovsext/Conntrack-related.c
    +++ b/datapath-windows/ovsext/Conntrack-related.c
    @@ -18,7 +18,7 @@
     #include "Jhash.h"
     
     static PLIST_ENTRY ovsCtRelatedTable; /* Holds related entries */
    -static UINT64 ctTotalRelatedEntries;
    +static ULONG ctTotalRelatedEntries;
     static OVS_CT_THREAD_CTX ctRelThreadCtx;
     static PNDIS_RW_LOCK_EX ovsCtRelatedLockObj;
     extern POVS_SWITCH_CONTEXT gOvsSwitchContext;
    @@ -75,13 +75,11 @@ OvsCtRelatedLookup(OVS_CT_KEY key, UINT64 currentTime)
         POVS_CT_REL_ENTRY entry;
         LOCK_STATE_EX lockState;
     
    -    NdisAcquireRWLockRead(ovsCtRelatedLockObj, &lockState, 0);
    -
         if (!ctTotalRelatedEntries) {
    -        NdisReleaseRWLock(ovsCtRelatedLockObj, &lockState);
             return NULL;
         }
     
    +    NdisAcquireRWLockRead(ovsCtRelatedLockObj, &lockState, 0);
         for (int i = 0; i < CT_HASH_TABLE_SIZE; i++) {
             /* XXX - Scan the table based on the hash instead */
             LIST_FORALL_SAFE(&ovsCtRelatedTable[i], link, next) {
    @@ -103,7 +101,7 @@ OvsCtRelatedEntryDelete(POVS_CT_REL_ENTRY entry)
     {
         RemoveEntryList(&entry->link);
         OvsFreeMemoryWithTag(entry, OVS_CT_POOL_TAG);
    -    ctTotalRelatedEntries--;
    +    NdisInterlockedDecrement((PLONG)&ctTotalRelatedEntries);
     }
     
     NDIS_STATUS
    @@ -139,7 +137,7 @@ OvsCtRelatedEntryCreate(UINT8 ipProto,
         NdisAcquireRWLockWrite(ovsCtRelatedLockObj, &lockState, 0);
         InsertHeadList(&ovsCtRelatedTable[hash & CT_HASH_TABLE_MASK],
                        &entry->link);
    -    ctTotalRelatedEntries++;
    +    NdisInterlockedIncrement((PLONG)&ctTotalRelatedEntries);
         NdisReleaseRWLock(ovsCtRelatedLockObj, &lockState);
     
         return NDIS_STATUS_SUCCESS;
    @@ -150,11 +148,10 @@ OvsCtRelatedFlush()
     {
         PLIST_ENTRY link, next;
         POVS_CT_REL_ENTRY entry;
    -
         LOCK_STATE_EX lockState;
    -    NdisAcquireRWLockWrite(ovsCtRelatedLockObj, &lockState, 0);
     
         if (ctTotalRelatedEntries) {
    +        NdisAcquireRWLockWrite(ovsCtRelatedLockObj, &lockState, 0);
             for (int i = 0; i < CT_HASH_TABLE_SIZE; i++) {
                 LIST_FORALL_SAFE(&ovsCtRelatedTable[i], link, next) {
                     entry = CONTAINING_RECORD(link, OVS_CT_REL_ENTRY, link);
    @@ -189,9 +186,8 @@ OvsCtRelatedEntryCleaner(PVOID data)
                 /* Lock has been freed by 'OvsCleanupCtRelated()' */
                 break;
             }
    -        NdisAcquireRWLockWrite(ovsCtRelatedLockObj, &lockState, 0);
    +
             if (context->exit) {
    -            NdisReleaseRWLock(ovsCtRelatedLockObj, &lockState);
                 break;
             }
     
    @@ -201,6 +197,7 @@ OvsCtRelatedEntryCleaner(PVOID data)
             threadSleepTimeout = currentTime + CT_CLEANUP_INTERVAL;
     
             if (ctTotalRelatedEntries) {
    +            NdisAcquireRWLockWrite(ovsCtRelatedLockObj, &lockState, 0);
                 for (int i = 0; i < CT_HASH_TABLE_SIZE; i++) {
                     LIST_FORALL_SAFE(&ovsCtRelatedTable[i], link, next) {
                         entry = CONTAINING_RECORD(link, OVS_CT_REL_ENTRY, link);
    diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c
    index add1491..da812e5 100644
    --- a/datapath-windows/ovsext/Conntrack.c
    +++ b/datapath-windows/ovsext/Conntrack.c
    @@ -34,7 +34,7 @@ static OVS_CT_THREAD_CTX ctThreadCtx;
     static PNDIS_RW_LOCK_EX *ovsCtBucketLock = NULL;
     static PNDIS_RW_LOCK_EX ovsCtNatLockObj;
     extern POVS_SWITCH_CONTEXT gOvsSwitchContext;
    -static LONG ctTotalEntries;
    +static ULONG ctTotalEntries;
     
     static __inline OvsCtFlush(UINT16 zone, struct ovs_key_ct_tuple_ipv4 *tuple);
     static __inline NDIS_STATUS
    @@ -208,7 +208,6 @@ OvsPostCtEventEntry(POVS_CT_ENTRY entry, UINT8 type)
     {
         OVS_CT_EVENT_ENTRY ctEventEntry = {0};
         NdisMoveMemory(&ctEventEntry.entry, entry, sizeof(OVS_CT_ENTRY));
    -    ctEventEntry.entry.lock = NULL;
         ctEventEntry.entry.parent = NULL;
         ctEventEntry.type = type;
         OvsPostCtEvent(&ctEventEntry);
    @@ -217,8 +216,8 @@ OvsPostCtEventEntry(POVS_CT_ENTRY entry, UINT8 type)
     static __inline VOID
     OvsCtIncrementCounters(POVS_CT_ENTRY entry, BOOLEAN reply, PNET_BUFFER_LIST nbl)
     {
    -    LOCK_STATE_EX lockState;
    -    NdisAcquireRWLockWrite(entry->lock, &lockState, 0);
    +    KIRQL irql = KeGetCurrentIrql();
    +    OVS_CT_ACQUIRE_SPIN_LOCK(&(entry->lock), irql);
         if (reply) {
             entry->rev_key.byteCount+= OvsPacketLenNBL(nbl);
             entry->rev_key.packetCount++;
    @@ -226,11 +225,11 @@ OvsCtIncrementCounters(POVS_CT_ENTRY entry, BOOLEAN reply, PNET_BUFFER_LIST nbl)
             entry->key.byteCount += OvsPacketLenNBL(nbl);
             entry->key.packetCount++;
         }
    -    NdisReleaseRWLock(entry->lock, &lockState);
    +    OVS_CT_RELEASE_SPIN_LOCK(&(entry->lock), irql);
     }
     
     static __inline BOOLEAN
    -OvsCtAddEntry(POVS_SWITCH_CONTEXT switchContext, POVS_CT_ENTRY entry,
    +OvsCtAddEntry(POVS_CT_ENTRY entry,
                   OvsConntrackKeyLookupCtx *ctx,
                   PNAT_ACTION_INFO natInfo, UINT64 now)
     {
    @@ -261,18 +260,14 @@ OvsCtAddEntry(POVS_SWITCH_CONTEXT switchContext, POVS_CT_ENTRY entry,
         }
     
         entry->timestampStart = now;
    -    entry->lock = NdisAllocateRWLock(switchContext->NdisFilterHandle);
    -    if (entry->lock == NULL) {
    -        OVS_LOG_ERROR("NdisAllocateRWLock failed : Insufficient resources");
    -        return FALSE;
    -    }
    +    NdisAllocateSpinLock(&(entry->lock));
         UINT32 bucketIdx = ctx->hash & CT_HASH_TABLE_MASK;
         entry->bucketLockRef = ovsCtBucketLock[bucketIdx];
         NdisAcquireRWLockWrite(ovsCtBucketLock[bucketIdx], &lockState, 0);
         InsertHeadList(&ovsConntrackTable[bucketIdx],
                        &entry->link);
     
    -    InterlockedIncrement((LONG volatile *)&ctTotalEntries);
    +    NdisInterlockedIncrement((PLONG)&ctTotalEntries);
         NdisReleaseRWLock(ovsCtBucketLock[bucketIdx], &lockState);
         return TRUE;
     }
    @@ -351,8 +346,7 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
         if (state != OVS_CS_F_INVALID && commit) {
             if (entry) {
                 entry->parent = parentEntry;
    -            if (OvsCtAddEntry(fwdCtx->switchContext, entry, ctx,
    -                              natInfo, currentTime)) {
    +            if (OvsCtAddEntry(entry, ctx, natInfo, currentTime)) {
                     *entryCreated = TRUE;
                 } else {
                     /* Unable to add entry to the list */
    @@ -382,8 +376,8 @@ OvsCtUpdateEntry(OVS_CT_ENTRY* entry,
                      UINT64 now)
     {
         CT_UPDATE_RES status;
    -    LOCK_STATE_EX lockState;
    -    NdisAcquireRWLockWrite(entry->lock, &lockState, 0);
    +    KIRQL irql = KeGetCurrentIrql();
    +    OVS_CT_ACQUIRE_SPIN_LOCK(&(entry->lock), irql);
         switch (ipProto) {
         case IPPROTO_TCP:
         {
    @@ -407,7 +401,7 @@ OvsCtUpdateEntry(OVS_CT_ENTRY* entry,
             status = CT_UPDATE_INVALID;
             break;
         }
    -    NdisReleaseRWLock(entry->lock, &lockState);
    +    OVS_CT_RELEASE_SPIN_LOCK(&(entry->lock), irql);
         return status;
     }
     
    @@ -431,8 +425,8 @@ OvsCtEntryDelete(POVS_CT_ENTRY entry, BOOLEAN forceDelete)
         if (entry == NULL) {
             return;
         }
    -    LOCK_STATE_EX lockState;
    -    NdisAcquireRWLockWrite(entry->lock, &lockState, 0);
    +    KIRQL irql = KeGetCurrentIrql();
    +    OVS_CT_ACQUIRE_SPIN_LOCK(&(entry->lock), irql);
         if (forceDelete || OvsCtEntryExpired(entry)) {
             if (entry->natInfo.natAction) {
                 LOCK_STATE_EX lockStateNat;
    @@ -442,13 +436,13 @@ OvsCtEntryDelete(POVS_CT_ENTRY entry, BOOLEAN forceDelete)
             }
             OvsPostCtEventEntry(entry, OVS_EVENT_CT_DELETE);
             RemoveEntryList(&entry->link);
    -        NdisReleaseRWLock(entry->lock, &lockState);
    -        NdisFreeRWLock(entry->lock);
    +        OVS_CT_RELEASE_SPIN_LOCK(&(entry->lock), irql);
    +        NdisFreeSpinLock(&(entry->lock));
             OvsFreeMemoryWithTag(entry, OVS_CT_POOL_TAG);
    -        InterlockedDecrement((LONG volatile*)&ctTotalEntries);
    +        NdisInterlockedDecrement((PLONG)&ctTotalEntries);
             return;
         }
    -    NdisReleaseRWLock(entry->lock, &lockState);
    +    OVS_CT_RELEASE_SPIN_LOCK(&(entry->lock), irql);
     }
     
     static __inline NDIS_STATUS
    @@ -499,7 +493,7 @@ OvsCtLookup(OvsConntrackKeyLookupCtx *ctx)
         POVS_CT_ENTRY entry;
         BOOLEAN reply = FALSE;
         POVS_CT_ENTRY found = NULL;
    -    LOCK_STATE_EX lockState, lockStateTable;
    +    LOCK_STATE_EX lockStateTable;
         UINT32 bucketIdx;
         /* Reverse NAT must be performed before OvsCtLookup, so here
          * we simply need to flip the src and dst in key and compare
    @@ -512,11 +506,14 @@ OvsCtLookup(OvsConntrackKeyLookupCtx *ctx)
         if (!ctTotalEntries) {
             return found;
         }
    +
    +    KIRQL irql = KeGetCurrentIrql();
         bucketIdx = ctx->hash & CT_HASH_TABLE_MASK;
         NdisAcquireRWLockRead(ovsCtBucketLock[bucketIdx], &lockStateTable, 0);
         LIST_FORALL(&ovsConntrackTable[bucketIdx], link) {
             entry = CONTAINING_RECORD(link, OVS_CT_ENTRY, link);
    -        NdisAcquireRWLockRead(entry->lock, &lockState, 0);
    +        OVS_CT_ACQUIRE_SPIN_LOCK(&(entry->lock), irql);
    +
             if (OvsCtKeyAreSame(ctx->key, entry->key)) {
                 found = entry;
                 reply = FALSE;
    @@ -533,10 +530,10 @@ OvsCtLookup(OvsConntrackKeyLookupCtx *ctx)
                 } else {
                     ctx->reply = reply;
                 }
    -            NdisReleaseRWLock(entry->lock, &lockState);
    +            OVS_CT_RELEASE_SPIN_LOCK(&(entry->lock), irql);
                 break;
             }
    -        NdisReleaseRWLock(entry->lock, &lockState);
    +        OVS_CT_RELEASE_SPIN_LOCK(&(entry->lock), irql);
         }
     
         NdisReleaseRWLock(ovsCtBucketLock[bucketIdx], &lockStateTable);
    @@ -689,7 +686,7 @@ OvsProcessConntrackEntry(OvsForwardingContext *fwdCtx,
         POVS_CT_ENTRY entry = ctx->entry;
         UINT32 state = 0;
         PNET_BUFFER_LIST curNbl = fwdCtx->curNbl;
    -    LOCK_STATE_EX lockState, lockStateTable;
    +    LOCK_STATE_EX lockStateTable;
         PNDIS_RW_LOCK_EX bucketLockRef = NULL;
         *entryCreated = FALSE;
     
    @@ -730,7 +727,8 @@ OvsProcessConntrackEntry(OvsForwardingContext *fwdCtx,
             }
         }
         if (entry) {
    -        NdisAcquireRWLockRead(entry->lock, &lockState, 0);
    +        KIRQL irql = KeGetCurrentIrql();
    +        OVS_CT_ACQUIRE_SPIN_LOCK(&(entry->lock), irql);
             if (key->ipKey.nwProto == IPPROTO_TCP) {
                 /* Update the related bit if there is a parent */
                 if (entry->parent) {
    @@ -748,7 +746,7 @@ OvsProcessConntrackEntry(OvsForwardingContext *fwdCtx,
             /* Copy mark and label from entry into flowKey. If actions specify
                different mark and label, update the flowKey. */
             OvsCtUpdateFlowKey(key, state, zone, entry->mark, &entry->labels);
    -        NdisReleaseRWLock(entry->lock, &lockState);
    +        OVS_CT_RELEASE_SPIN_LOCK(&(entry->lock), irql);
         } else {
             OvsCtUpdateFlowKey(key, state, zone, 0, NULL);
         }
    @@ -802,8 +800,6 @@ OvsCtSetMarkLabel(OvsFlowKey *key,
                            MD_LABELS *labels,
                            BOOLEAN *triggerUpdateEvent)
     {
    -    LOCK_STATE_EX lockState;
    -    NdisAcquireRWLockWrite(entry->lock, &lockState, 0);
         if (mark) {
             OvsConntrackSetMark(key, entry, mark->value, mark->mask,
                                 triggerUpdateEvent);
    @@ -813,7 +809,6 @@ OvsCtSetMarkLabel(OvsFlowKey *key,
             OvsConntrackSetLabels(key, entry, &labels->value, &labels->mask,
                                   triggerUpdateEvent);
         }
    -    NdisReleaseRWLock(entry->lock, &lockState);
     }
     
     /*
    @@ -854,10 +849,11 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
     {
         NDIS_STATUS status = NDIS_STATUS_SUCCESS;
         BOOLEAN triggerUpdateEvent = FALSE;
    +    BOOLEAN entryCreated = FALSE;
         POVS_CT_ENTRY entry = NULL;
         PNET_BUFFER_LIST curNbl = fwdCtx->curNbl;
         OvsConntrackKeyLookupCtx ctx = { 0 };
    -    LOCK_STATE_EX lockState, lockStateTable;
    +    LOCK_STATE_EX lockStateTable;
         UINT64 currentTime;
         NdisGetCurrentSystemTime((LARGE_INTEGER *) &currentTime);
     
    @@ -866,10 +862,9 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
     
         /* Lookup Conntrack entries for a matching entry */
         entry = OvsCtLookup(&ctx);
    -    BOOLEAN entryCreated = FALSE;
     
         /* Delete entry in reverse direction if 'force' is specified */
    -    if (entry && force && ctx.reply) {
    +    if (force && ctx.reply && entry) {
             PNDIS_RW_LOCK_EX bucketLockRef = entry->bucketLockRef;
             NdisAcquireRWLockWrite(bucketLockRef, &lockStateTable, 0);
             OvsCtEntryDelete(entry, TRUE);
    @@ -877,34 +872,33 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
             entry = NULL;
         }
     
    -    if (!entry && commit && ctTotalEntries >= CT_MAX_ENTRIES) {
    -        /* Don't proceed with processing if the max limit has been hit.
    -         * This blocks only new entries from being created and doesn't
    -         * affect existing connections.
    +    if (entry) {
    +        /* Increment stats for the entry if it wasn't tracked previously or
    +         * if they are on different zones
              */
    -        OVS_LOG_ERROR("Conntrack Limit hit: %lu", ctTotalEntries);
    -        return NDIS_STATUS_RESOURCES;
    -    }
    -
    -    /* Increment stats for the entry if it wasn't tracked previously or
    -     * if they are on different zones
    -     */
    -    if (entry && (entry->key.zone != key->ct.zone ||
    -           (!(key->ct.state & OVS_CS_F_TRACKED)))) {
    -        OvsCtIncrementCounters(entry, ctx.reply, curNbl);
    -    }
    +        if ((entry->key.zone != key->ct.zone ||
    +               (!(key->ct.state & OVS_CS_F_TRACKED)))) {
    +            OvsCtIncrementCounters(entry, ctx.reply, curNbl);
    +        }
    +        /* Process the entry and update CT flags */
    +        entry = OvsProcessConntrackEntry(fwdCtx, layers->l4Offset, &ctx, key,
    +                                         zone, natInfo, commit, currentTime,
    +                                         &entryCreated);
     
    -    if (!entry) {
    +    } else {
    +        if (commit && ctTotalEntries >= CT_MAX_ENTRIES) {
    +            /* Don't proceed with processing if the max limit has been hit.
    +             * This blocks only new entries from being created and doesn't
    +             * affect existing connections.
    +             */
    +            OVS_LOG_ERROR("Conntrack Limit hit: %lu", ctTotalEntries);
    +            return NDIS_STATUS_RESOURCES;
    +        }
             /* If no matching entry was found, create one and add New state */
             entry = OvsCtEntryCreate(fwdCtx, key->ipKey.nwProto,
                                      layers->l4Offset, &ctx,
                                      key, natInfo, commit, currentTime,
                                      &entryCreated);
    -    } else {
    -        /* Process the entry and update CT flags */
    -        entry = OvsProcessConntrackEntry(fwdCtx, layers->l4Offset, &ctx, key,
    -                                         zone, natInfo, commit, currentTime,
    -                                         &entryCreated);
         }
     
         if (entry == NULL) {
    @@ -918,6 +912,8 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
          * NAT_ACTION_REVERSE, yet entry->natInfo is NAT_ACTION_SRC or
          * NAT_ACTION_DST without NAT_ACTION_REVERSE
          */
    +    KIRQL irql = KeGetCurrentIrql();
    +    OVS_CT_ACQUIRE_SPIN_LOCK(&(entry->lock), irql);
         if (natInfo->natAction != NAT_ACTION_NONE)
         {
             LOCK_STATE_EX lockStateNat;
    @@ -939,15 +935,14 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
                 OVS_LOG_ERROR("Error while parsing the FTP packet");
             }
         }
    -    NdisAcquireRWLockRead(entry->lock, &lockState, 0);
    +
         /* Add original tuple information to flow Key */
         if (entry->key.dl_type == ntohs(ETH_TYPE_IPV4)) {
             if (entry->parent != NULL) {
                 POVS_CT_ENTRY parent = entry->parent;
    -            LOCK_STATE_EX lockStateParent;
    -            NdisAcquireRWLockRead(parent->lock, &lockStateParent, 0);
    +            OVS_CT_ACQUIRE_SPIN_LOCK(&(parent->lock), irql);
                 OvsCtUpdateTuple(key, &parent->key);
    -            NdisReleaseRWLock(parent->lock, &lockStateParent);
    +            OVS_CT_RELEASE_SPIN_LOCK(&(parent->lock), irql);
             } else {
                 OvsCtUpdateTuple(key, &entry->key);
             }
    @@ -955,13 +950,11 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
     
         if (entryCreated) {
             OvsPostCtEventEntry(entry, OVS_EVENT_CT_NEW);
    -    }
    -    if (postUpdateEvent && !entryCreated && triggerUpdateEvent) {
    +    } else if (postUpdateEvent && triggerUpdateEvent) {
             OvsPostCtEventEntry(entry, OVS_EVENT_CT_UPDATE);
         }
     
    -    NdisReleaseRWLock(entry->lock, &lockState);
    -
    +    OVS_CT_RELEASE_SPIN_LOCK(&(entry->lock), irql);
         return status;
     }
     
    @@ -1738,8 +1731,9 @@ OvsCtDumpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
         UINT32 inIndex = instance->dumpState.index[1];
         UINT32 i = CT_HASH_TABLE_SIZE;
         UINT32 outIndex = 0;
    +    KIRQL irql = KeGetCurrentIrql();
    +    LOCK_STATE_EX lockStateTable;
     
    -    LOCK_STATE_EX lockState, lockStateTable;
         if (ctTotalEntries) {
             for (i = inBucket; i < CT_HASH_TABLE_SIZE; i++) {
                 PLIST_ENTRY head, link;
    @@ -1756,7 +1750,7 @@ OvsCtDumpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                      */
                     if (outIndex >= inIndex) {
                         entry = CONTAINING_RECORD(link, OVS_CT_ENTRY, link);
    -                    NdisAcquireRWLockRead(entry->lock, &lockState, 0);
    +                    OVS_CT_ACQUIRE_SPIN_LOCK(&(entry->lock), irql);
                         rc = OvsCreateNlMsgFromCtEntry(entry,
                                                        usrParamsCtx->outputBuffer,
                                                        usrParamsCtx->outputLength,
    @@ -1765,7 +1759,7 @@ OvsCtDumpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                                                        msgIn->nlMsg.nlmsgPid,
                                                        msgIn->nfGenMsg.version,
                                                        0);
    -                    NdisReleaseRWLock(entry->lock, &lockState);
    +                    OVS_CT_RELEASE_SPIN_LOCK(&(entry->lock), irql);
                         if (rc != NDIS_STATUS_SUCCESS) {
                             NdisReleaseRWLock(ovsCtBucketLock[i], &lockStateTable);
                             return STATUS_UNSUCCESSFUL;
    diff --git a/datapath-windows/ovsext/Conntrack.h b/datapath-windows/ovsext/Conntrack.h
    index 3be309e..2dfd8fb 100644
    --- a/datapath-windows/ovsext/Conntrack.h
    +++ b/datapath-windows/ovsext/Conntrack.h
    @@ -29,6 +29,24 @@
     #endif
     #define OVS_DBG_MOD OVS_DBG_CONTRK
     
    +#define OVS_CT_ACQUIRE_SPIN_LOCK(_pLock, _dispatchLevel)                     \
    +{                                                                            \
    +    if (_dispatchLevel) {                                                    \
    +        NdisDprAcquireSpinLock(_pLock);                                      \
    +    } else {                                                                 \
    +        NdisAcquireSpinLock(_pLock);                                         \
    +    }                                                                        \
    +}
    +
    +#define OVS_CT_RELEASE_SPIN_LOCK(_pLock, _dispatchLevel)                     \
    +{                                                                            \
    +    if (_dispatchLevel) {                                                    \
    +        NdisDprReleaseSpinLock(_pLock);                                      \
    +    } else {                                                                 \
    +        NdisReleaseSpinLock(_pLock);                                         \
    +    }                                                                        \
    +}
    +
     struct ct_addr {
         union {
             ovs_be32 ipv4;
    @@ -101,7 +119,7 @@ typedef struct _NAT_ACTION_INFO {
     typedef struct OVS_CT_ENTRY {
         /* Reference to ovsCtBucketLock of ovsConntrackTable.*/
         PNDIS_RW_LOCK_EX bucketLockRef;
    -    PNDIS_RW_LOCK_EX lock;       /* Protects OVS_CT_ENTRY. */
    +    NDIS_SPIN_LOCK lock;       /* Protects OVS_CT_ENTRY. */
         OVS_CT_KEY  key;
         OVS_CT_KEY  rev_key;
         UINT64      expiration;
    -- 
    2.9.3.windows.1
    
    



More information about the dev mailing list