| Message ID | 20171114184821.124-3-kumaranand@vmware.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Alin Gabriel Serdean |
| Headers | show |
| Series | datapath-windows: New lock implementation in conntrack | expand |
Acked-by: Alin Gabriel Serdean <aserdean@ovn.org> > -----Original Message----- > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev- > bounces@openvswitch.org] On Behalf Of Anand Kumar > Sent: Tuesday, November 14, 2017 8:48 PM > To: dev@openvswitch.org > Subject: [ovs-dev] [PATCH v1 2/3] datapath-windows: Add a global level RW > lock for NAT > > Currently NAT module relies on the existing conntrack lock. > This patch provides a basic lock implementation for NAT module in conntrack. > > Signed-off-by: Anand Kumar <kumaranand@vmware.com> > --- > datapath-windows/ovsext/Conntrack.c | 36 > ++++++++++++++++++++++++++++++++++-- > 1 file changed, 34 insertions(+), 2 deletions(-) > > diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath- > windows/ovsext/Conntrack.c > index 48d4abf..ba0dc88 100644 > --- a/datapath-windows/ovsext/Conntrack.c > +++ b/datapath-windows/ovsext/Conntrack.c > @@ -32,6 +32,7 @@ KSTART_ROUTINE OvsConntrackEntryCleaner; static > PLIST_ENTRY ovsConntrackTable; static OVS_CT_THREAD_CTX ctThreadCtx; > static PNDIS_RW_LOCK_EX ovsConntrackLockObj; > +static PNDIS_RW_LOCK_EX ovsCtNatLockObj; > extern POVS_SWITCH_CONTEXT gOvsSwitchContext; static LONG > ctTotalEntries; > > @@ -56,6 +57,13 @@ OvsInitConntrack(POVS_SWITCH_CONTEXT context) > return STATUS_INSUFFICIENT_RESOURCES; > } > > + ovsCtNatLockObj = NdisAllocateRWLock(context->NdisFilterHandle); > + if (ovsCtNatLockObj == NULL) { > + NdisFreeRWLock(ovsConntrackLockObj); > + ovsConntrackLockObj = NULL; > + return STATUS_INSUFFICIENT_RESOURCES; > + } > + > /* Init the Hash Buffer */ > ovsConntrackTable = OvsAllocateMemoryWithTag(sizeof(LIST_ENTRY) > * CT_HASH_TABLE_SIZE, @@ -63,6 +71,8 @@ > OvsInitConntrack(POVS_SWITCH_CONTEXT context) > if (ovsConntrackTable == NULL) { > NdisFreeRWLock(ovsConntrackLockObj); > ovsConntrackLockObj = NULL; > + NdisFreeRWLock(ovsCtNatLockObj); > + ovsCtNatLockObj = NULL; > return STATUS_INSUFFICIENT_RESOURCES; > } > > @@ -80,6 +90,9 @@ OvsInitConntrack(POVS_SWITCH_CONTEXT context) > NdisFreeRWLock(ovsConntrackLockObj); > ovsConntrackLockObj = NULL; > > + NdisFreeRWLock(ovsCtNatLockObj); > + ovsCtNatLockObj = NULL; > + > OvsFreeMemoryWithTag(ovsConntrackTable, OVS_CT_POOL_TAG); > ovsConntrackTable = NULL; > > @@ -109,7 +122,7 @@ OvsInitConntrack(POVS_SWITCH_CONTEXT context) > VOID > OvsCleanupConntrack(VOID) > { > - LOCK_STATE_EX lockState; > + LOCK_STATE_EX lockState, lockStateNat; > NdisAcquireRWLockWrite(ovsConntrackLockObj, &lockState, 0); > ctThreadCtx.exit = 1; > KeSetEvent(&ctThreadCtx.event, 0, FALSE); @@ -129,7 +142,11 @@ > OvsCleanupConntrack(VOID) > > NdisFreeRWLock(ovsConntrackLockObj); > ovsConntrackLockObj = NULL; > + NdisAcquireRWLockWrite(ovsCtNatLockObj, &lockStateNat, 0); > OvsNatCleanup(); > + NdisReleaseRWLock(ovsCtNatLockObj, &lockStateNat); > + NdisFreeRWLock(ovsCtNatLockObj); > + ovsCtNatLockObj = NULL; > } > > static __inline VOID > @@ -195,15 +212,19 @@ OvsCtAddEntry(POVS_CT_ENTRY entry, > OvsConntrackKeyLookupCtx *ctx, > if (natInfo == NULL) { > entry->natInfo.natAction = NAT_ACTION_NONE; > } else { > + LOCK_STATE_EX lockStateNat; > + NdisAcquireRWLockWrite(ovsCtNatLockObj, &lockStateNat, 0); > if (OvsIsForwardNat(natInfo->natAction)) { > entry->natInfo = *natInfo; > if (!OvsNatTranslateCtEntry(entry)) { > + NdisReleaseRWLock(ovsCtNatLockObj, &lockStateNat); > return FALSE; > } > ctx->hash = OvsHashCtKey(&entry->key); > } else { > entry->natInfo.natAction = natInfo->natAction; > } > + NdisReleaseRWLock(ovsCtNatLockObj, &lockStateNat); > } > > entry->timestampStart = now; > @@ -356,7 +377,10 @@ OvsCtEntryDelete(POVS_CT_ENTRY entry, BOOLEAN > forceDelete) > } > if (forceDelete || OvsCtEntryExpired(entry)) { > if (entry->natInfo.natAction) { > + LOCK_STATE_EX lockStateNat; > + NdisAcquireRWLockWrite(ovsCtNatLockObj, &lockStateNat, 0); > OvsNatDeleteKey(&entry->key); > + NdisReleaseRWLock(ovsCtNatLockObj, &lockStateNat); > } > OvsPostCtEventEntry(entry, OVS_EVENT_CT_DELETE); > RemoveEntryList(&entry->link); > @@ -558,7 +582,10 @@ OvsCtSetupLookupCtx(OvsFlowKey *flowKey, > return NDIS_STATUS_INVALID_PACKET; > } > > + LOCK_STATE_EX lockStateNat; > + NdisAcquireRWLockRead(ovsCtNatLockObj, &lockStateNat, 0); > natEntry = OvsNatLookup(&ctx->key, TRUE); > + NdisReleaseRWLock(ovsCtNatLockObj, &lockStateNat); > if (natEntry) { > /* Translate address first for reverse NAT */ > ctx->key = natEntry->ctEntry->key; @@ -811,8 +838,11 @@ > OvsCtExecute_(OvsForwardingContext *fwdCtx, > */ > if (natInfo->natAction != NAT_ACTION_NONE) > { > + LOCK_STATE_EX lockStateNat; > + NdisAcquireRWLockWrite(ovsCtNatLockObj, &lockStateNat, 0); > OvsNatPacket(fwdCtx, entry, entry->natInfo.natAction, > key, ctx.reply); > + NdisReleaseRWLock(ovsCtNatLockObj, &lockStateNat); > } > > OvsCtSetMarkLabel(key, entry, mark, labels, &triggerUpdateEvent); @@ - > 1050,7 +1080,7 @@ OvsCtFlush(UINT16 zone) > PLIST_ENTRY link, next; > POVS_CT_ENTRY entry; > > - LOCK_STATE_EX lockState; > + LOCK_STATE_EX lockState, lockStateNat; > NdisAcquireRWLockWrite(ovsConntrackLockObj, &lockState, 0); > > if (ctTotalEntries) { > @@ -1064,7 +1094,9 @@ OvsCtFlush(UINT16 zone) > } > } > > + NdisAcquireRWLockWrite(ovsCtNatLockObj, &lockStateNat, 0); > OvsNatFlush(zone); > + NdisReleaseRWLock(ovsCtNatLockObj, &lockStateNat); > NdisReleaseRWLock(ovsConntrackLockObj, &lockState); > return NDIS_STATUS_SUCCESS; > } > -- > 2.9.3.windows.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c index 48d4abf..ba0dc88 100644 --- a/datapath-windows/ovsext/Conntrack.c +++ b/datapath-windows/ovsext/Conntrack.c @@ -32,6 +32,7 @@ KSTART_ROUTINE OvsConntrackEntryCleaner; static PLIST_ENTRY ovsConntrackTable; static OVS_CT_THREAD_CTX ctThreadCtx; static PNDIS_RW_LOCK_EX ovsConntrackLockObj; +static PNDIS_RW_LOCK_EX ovsCtNatLockObj; extern POVS_SWITCH_CONTEXT gOvsSwitchContext; static LONG ctTotalEntries; @@ -56,6 +57,13 @@ OvsInitConntrack(POVS_SWITCH_CONTEXT context) return STATUS_INSUFFICIENT_RESOURCES; } + ovsCtNatLockObj = NdisAllocateRWLock(context->NdisFilterHandle); + if (ovsCtNatLockObj == NULL) { + NdisFreeRWLock(ovsConntrackLockObj); + ovsConntrackLockObj = NULL; + return STATUS_INSUFFICIENT_RESOURCES; + } + /* Init the Hash Buffer */ ovsConntrackTable = OvsAllocateMemoryWithTag(sizeof(LIST_ENTRY) * CT_HASH_TABLE_SIZE, @@ -63,6 +71,8 @@ OvsInitConntrack(POVS_SWITCH_CONTEXT context) if (ovsConntrackTable == NULL) { NdisFreeRWLock(ovsConntrackLockObj); ovsConntrackLockObj = NULL; + NdisFreeRWLock(ovsCtNatLockObj); + ovsCtNatLockObj = NULL; return STATUS_INSUFFICIENT_RESOURCES; } @@ -80,6 +90,9 @@ OvsInitConntrack(POVS_SWITCH_CONTEXT context) NdisFreeRWLock(ovsConntrackLockObj); ovsConntrackLockObj = NULL; + NdisFreeRWLock(ovsCtNatLockObj); + ovsCtNatLockObj = NULL; + OvsFreeMemoryWithTag(ovsConntrackTable, OVS_CT_POOL_TAG); ovsConntrackTable = NULL; @@ -109,7 +122,7 @@ OvsInitConntrack(POVS_SWITCH_CONTEXT context) VOID OvsCleanupConntrack(VOID) { - LOCK_STATE_EX lockState; + LOCK_STATE_EX lockState, lockStateNat; NdisAcquireRWLockWrite(ovsConntrackLockObj, &lockState, 0); ctThreadCtx.exit = 1; KeSetEvent(&ctThreadCtx.event, 0, FALSE); @@ -129,7 +142,11 @@ OvsCleanupConntrack(VOID) NdisFreeRWLock(ovsConntrackLockObj); ovsConntrackLockObj = NULL; + NdisAcquireRWLockWrite(ovsCtNatLockObj, &lockStateNat, 0); OvsNatCleanup(); + NdisReleaseRWLock(ovsCtNatLockObj, &lockStateNat); + NdisFreeRWLock(ovsCtNatLockObj); + ovsCtNatLockObj = NULL; } static __inline VOID @@ -195,15 +212,19 @@ OvsCtAddEntry(POVS_CT_ENTRY entry, OvsConntrackKeyLookupCtx *ctx, if (natInfo == NULL) { entry->natInfo.natAction = NAT_ACTION_NONE; } else { + LOCK_STATE_EX lockStateNat; + NdisAcquireRWLockWrite(ovsCtNatLockObj, &lockStateNat, 0); if (OvsIsForwardNat(natInfo->natAction)) { entry->natInfo = *natInfo; if (!OvsNatTranslateCtEntry(entry)) { + NdisReleaseRWLock(ovsCtNatLockObj, &lockStateNat); return FALSE; } ctx->hash = OvsHashCtKey(&entry->key); } else { entry->natInfo.natAction = natInfo->natAction; } + NdisReleaseRWLock(ovsCtNatLockObj, &lockStateNat); } entry->timestampStart = now; @@ -356,7 +377,10 @@ OvsCtEntryDelete(POVS_CT_ENTRY entry, BOOLEAN forceDelete) } if (forceDelete || OvsCtEntryExpired(entry)) { if (entry->natInfo.natAction) { + LOCK_STATE_EX lockStateNat; + NdisAcquireRWLockWrite(ovsCtNatLockObj, &lockStateNat, 0); OvsNatDeleteKey(&entry->key); + NdisReleaseRWLock(ovsCtNatLockObj, &lockStateNat); } OvsPostCtEventEntry(entry, OVS_EVENT_CT_DELETE); RemoveEntryList(&entry->link); @@ -558,7 +582,10 @@ OvsCtSetupLookupCtx(OvsFlowKey *flowKey, return NDIS_STATUS_INVALID_PACKET; } + LOCK_STATE_EX lockStateNat; + NdisAcquireRWLockRead(ovsCtNatLockObj, &lockStateNat, 0); natEntry = OvsNatLookup(&ctx->key, TRUE); + NdisReleaseRWLock(ovsCtNatLockObj, &lockStateNat); if (natEntry) { /* Translate address first for reverse NAT */ ctx->key = natEntry->ctEntry->key; @@ -811,8 +838,11 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx, */ if (natInfo->natAction != NAT_ACTION_NONE) { + LOCK_STATE_EX lockStateNat; + NdisAcquireRWLockWrite(ovsCtNatLockObj, &lockStateNat, 0); OvsNatPacket(fwdCtx, entry, entry->natInfo.natAction, key, ctx.reply); + NdisReleaseRWLock(ovsCtNatLockObj, &lockStateNat); } OvsCtSetMarkLabel(key, entry, mark, labels, &triggerUpdateEvent); @@ -1050,7 +1080,7 @@ OvsCtFlush(UINT16 zone) PLIST_ENTRY link, next; POVS_CT_ENTRY entry; - LOCK_STATE_EX lockState; + LOCK_STATE_EX lockState, lockStateNat; NdisAcquireRWLockWrite(ovsConntrackLockObj, &lockState, 0); if (ctTotalEntries) { @@ -1064,7 +1094,9 @@ OvsCtFlush(UINT16 zone) } } + NdisAcquireRWLockWrite(ovsCtNatLockObj, &lockStateNat, 0); OvsNatFlush(zone); + NdisReleaseRWLock(ovsCtNatLockObj, &lockStateNat); NdisReleaseRWLock(ovsConntrackLockObj, &lockState); return NDIS_STATUS_SUCCESS; }
Currently NAT module relies on the existing conntrack lock. This patch provides a basic lock implementation for NAT module in conntrack. Signed-off-by: Anand Kumar <kumaranand@vmware.com> --- datapath-windows/ovsext/Conntrack.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-)