[ovs-dev] [PATCH v4 1/3] datapath-windows: Refactor conntrack code.
Anand Kumar
kumaranand at vmware.com
Mon Jan 29 18:27:59 UTC 2018
Some of the functions and code are refactored
so that new conntrack lock can be implemented
Signed-off-by: Anand Kumar <kumaranand at vmware.com>
Acked-by: Alin Gabriel Serdean <aserdean at ovn.org>
---
datapath-windows/ovsext/Conntrack-nat.c | 11 +-
datapath-windows/ovsext/Conntrack.c | 174 ++++++++++++++++++--------------
datapath-windows/ovsext/Conntrack.h | 4 -
3 files changed, 103 insertions(+), 86 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(&ovsNatTable[i]);
InitializeListHead(&ovsUnNatTable[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 169ec4f..3cde836 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 OvsCtFlush(UINT16 zone, struct ovs_key_ct_tuple_ipv4 *tuple);
static __inline NDIS_STATUS
@@ -212,7 +212,7 @@ OvsCtAddEntry(POVS_CT_ENTRY entry, OvsConntrackKeyLookupCtx *ctx,
InsertHeadList(&ovsConntrackTable[ctx->hash & CT_HASH_TABLE_MASK],
&entry->link);
- ctTotalEntries++;
+ InterlockedIncrement((LONG volatile *)&ctTotalEntries);
return TRUE;
}
@@ -235,11 +235,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:
{
@@ -283,6 +278,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;
@@ -315,6 +315,7 @@ OvsCtUpdateEntry(OVS_CT_ENTRY* entry,
BOOLEAN reply,
UINT64 now)
{
+ CT_UPDATE_RES status;
switch (ipProto) {
case IPPROTO_TCP:
{
@@ -322,32 +323,23 @@ OvsCtUpdateEntry(OVS_CT_ENTRY* entry,
const TCPHdr *tcp;
tcp = OvsGetTcp(nbl, l4Offset, &tcpStorage);
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(&entry->key);
+ status = CT_UPDATE_INVALID;
+ break;
}
- OvsPostCtEventEntry(entry, OVS_EVENT_CT_DELETE);
- RemoveEntryList(&entry->link);
- OvsFreeMemoryWithTag(entry, OVS_CT_POOL_TAG);
- ctTotalEntries--;
+ return status;
}
static __inline BOOLEAN
@@ -358,6 +350,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(&entry->key);
+ }
+ OvsPostCtEventEntry(entry, OVS_EVENT_CT_DELETE);
+ RemoveEntryList(&entry->link);
+ OvsFreeMemoryWithTag(entry, OVS_CT_POOL_TAG);
+ InterlockedDecrement((LONG volatile*)&ctTotalEntries);
+ return;
+ }
+}
+
static __inline NDIS_STATUS
OvsDetectCtPacket(OvsForwardingContext *fwdCtx,
OvsFlowKey *key,
@@ -425,21 +435,20 @@ OvsCtLookup(OvsConntrackKeyLookupCtx *ctx)
if (OvsCtKeyAreSame(ctx->key, entry->key)) {
found = entry;
reply = FALSE;
- break;
}
- if (OvsCtKeyAreSame(revCtxKey, entry->key)) {
+ if (!found && OvsCtKeyAreSame(revCtxKey, entry->key)) {
found = entry;
reply = TRUE;
- break;
}
- }
- if (found) {
- if (OvsCtEntryExpired(found)) {
- found = NULL;
- } else {
- ctx->reply = reply;
+ if (found) {
+ if (OvsCtEntryExpired(found)) {
+ found = NULL;
+ } else {
+ ctx->reply = reply;
+ }
+ break;
}
}
@@ -613,7 +622,7 @@ OvsProcessConntrackEntry(OvsForwardingContext *fwdCtx,
break;
case CT_UPDATE_NEW:
//Delete and update the Conntrack
- OvsCtEntryDelete(ctx->entry);
+ OvsCtEntryDelete(ctx->entry, TRUE);
ctx->entry = NULL;
entry = OvsCtEntryCreate(fwdCtx, key->ipKey.nwProto, l4Offset,
ctx, key, natInfo, commit, currentTime,
@@ -689,6 +698,41 @@ OvsConntrackSetLabels(OvsFlowKey *key,
sizeof(struct ovs_key_ct_labels));
}
+static void
+OvsCtSetMarkLabel(OvsFlowKey *key,
+ POVS_CT_ENTRY entry,
+ MD_MARK *mark,
+ MD_LABELS *labels,
+ BOOLEAN *triggerUpdateEvent)
+{
+ if (mark) {
+ OvsConntrackSetMark(key, entry, mark->value, mark->mask,
+ triggerUpdateEvent);
+ }
+
+ if (labels) {
+ OvsConntrackSetLabels(key, entry, &labels->value, &labels->mask,
+ triggerUpdateEvent);
+ }
+}
+
+static __inline void
+OvsCtUpdateTuple(OvsFlowKey *key, OVS_CT_KEY *ctKey)
+{
+ key->ct.tuple_ipv4.ipv4_src = ctKey->src.addr.ipv4_aligned;
+ key->ct.tuple_ipv4.ipv4_dst = ctKey->dst.addr.ipv4_aligned;
+ key->ct.tuple_ipv4.ipv4_proto = ctKey->nw_proto;
+
+ /* Orig tuple Port is overloaded to take in ICMP-Type & Code */
+ /* This mimics the behavior in lib/conntrack.c*/
+ key->ct.tuple_ipv4.src_port = ctKey->nw_proto != IPPROTO_ICMP ?
+ ctKey->src.port :
+ htons(ctKey->src.icmp_type);
+ key->ct.tuple_ipv4.dst_port = ctKey->nw_proto != IPPROTO_ICMP ?
+ ctKey->dst.port :
+ htons(ctKey->src.icmp_code);
+}
+
static __inline NDIS_STATUS
OvsCtExecute_(OvsForwardingContext *fwdCtx,
OvsFlowKey *key,
@@ -723,7 +767,7 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
/* Delete entry in reverse direction if 'force' is specified */
if (entry && force && ctx.reply) {
- OvsCtEntryDelete(entry);
+ OvsCtEntryDelete(entry, TRUE);
entry = NULL;
}
@@ -757,6 +801,9 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
&entryCreated);
}
+ if (entry == NULL) {
+ return status;
+ }
/*
* Note that natInfo is not the same as entry->natInfo here. natInfo
* is decided by action in the openflow rule, entry->natInfo is decided
@@ -764,23 +811,15 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
* NAT_ACTION_REVERSE, yet entry->natInfo is NAT_ACTION_SRC or
* NAT_ACTION_DST without NAT_ACTION_REVERSE
*/
- if (entry && natInfo->natAction != NAT_ACTION_NONE)
+ if (natInfo->natAction != NAT_ACTION_NONE)
{
OvsNatPacket(fwdCtx, entry, entry->natInfo.natAction,
key, ctx.reply);
}
- if (entry && mark) {
- OvsConntrackSetMark(key, entry, mark->value, mark->mask,
- &triggerUpdateEvent);
- }
-
- if (entry && labels) {
- OvsConntrackSetLabels(key, entry, &labels->value, &labels->mask,
- &triggerUpdateEvent);
- }
+ OvsCtSetMarkLabel(key, entry, mark, labels, &triggerUpdateEvent);
- if (entry && OvsDetectFtpPacket(key)) {
+ if (OvsDetectFtpPacket(key)) {
/* FTP parser will always be loaded */
UNREFERENCED_PARAMETER(helper);
@@ -792,33 +831,19 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
}
/* Add original tuple information to flow Key */
- if (entry && entry->key.dl_type == ntohs(ETH_TYPE_IPV4)) {
- OVS_CT_KEY *ctKey;
+ if (entry->key.dl_type == ntohs(ETH_TYPE_IPV4)) {
if (entry->parent != NULL) {
POVS_CT_ENTRY parent = entry->parent;
- ctKey = &parent->key;
+ OvsCtUpdateTuple(key, &parent->key);
} else {
- ctKey = &entry->key;
+ OvsCtUpdateTuple(key, &entry->key);
}
-
- key->ct.tuple_ipv4.ipv4_src = ctKey->src.addr.ipv4_aligned;
- key->ct.tuple_ipv4.ipv4_dst = ctKey->dst.addr.ipv4_aligned;
- key->ct.tuple_ipv4.ipv4_proto = ctKey->nw_proto;
-
- /* Orig tuple Port is overloaded to take in ICMP-Type & Code */
- /* This mimics the behavior in lib/conntrack.c*/
- key->ct.tuple_ipv4.src_port = ctKey->nw_proto != IPPROTO_ICMP ?
- ctKey->src.port :
- htons(ctKey->src.icmp_type);
- key->ct.tuple_ipv4.dst_port = ctKey->nw_proto != IPPROTO_ICMP ?
- ctKey->dst.port :
- htons(ctKey->src.icmp_code);
}
- if (entryCreated && entry) {
+ if (entryCreated) {
OvsPostCtEventEntry(entry, OVS_EVENT_CT_NEW);
}
- if (postUpdateEvent && entry && !entryCreated && triggerUpdateEvent) {
+ if (postUpdateEvent && !entryCreated && triggerUpdateEvent) {
OvsPostCtEventEntry(entry, OVS_EVENT_CT_UPDATE);
}
@@ -1003,9 +1028,7 @@ OvsConntrackEntryCleaner(PVOID data)
for (int i = 0; i < CT_HASH_TABLE_SIZE; i++) {
LIST_FORALL_SAFE(&ovsConntrackTable[i], link, next) {
entry = CONTAINING_RECORD(link, OVS_CT_ENTRY, link);
- if (entry && OvsCtEntryExpired(entry)) {
- OvsCtEntryDelete(entry);
- }
+ OvsCtEntryDelete(entry, FALSE);
}
}
}
@@ -1033,7 +1056,7 @@ OvsCtFlush(UINT16 zone, struct ovs_key_ct_tuple_ipv4 *tuple)
NdisAcquireRWLockWrite(ovsConntrackLockObj, &lockState, 0);
if (ctTotalEntries) {
- for (int i = 0; i < CT_HASH_TABLE_SIZE; i++) {
+ for (UINT32 i = 0; i < CT_HASH_TABLE_SIZE; i++) {
LIST_FORALL_SAFE(&ovsConntrackTable[i], link, next) {
entry = CONTAINING_RECORD(link, OVS_CT_ENTRY, link);
if (tuple) {
@@ -1044,17 +1067,17 @@ OvsCtFlush(UINT16 zone, struct ovs_key_ct_tuple_ipv4 *tuple)
tuple->src_port == entry->key.src.port &&
tuple->dst_port == entry->key.dst.port &&
(zone ? entry->key.zone == zone: TRUE)) {
- OvsCtEntryDelete(entry);
+ OvsCtEntryDelete(entry, TRUE);
} else if (tuple->ipv4_src == entry->key.src.addr.ipv4_aligned &&
tuple->ipv4_dst == entry->key.dst.addr.ipv4_aligned &&
tuple->ipv4_proto == entry->key.nw_proto &&
tuple->src_port == entry->key.src.icmp_type &&
tuple->dst_port == entry->key.src.icmp_code &&
(zone ? entry->key.zone == zone: TRUE)) {
- OvsCtEntryDelete(entry);
+ OvsCtEntryDelete(entry, TRUE);
}
} else if (!zone || zone == entry->key.zone) {
- OvsCtEntryDelete(entry);
+ OvsCtEntryDelete(entry, TRUE);
}
}
}
@@ -1434,6 +1457,7 @@ OvsCreateNlMsgFromCtEntry(POVS_CT_ENTRY entry,
UINT16 nlmsgFlags = NLM_F_CREATE;
NdisGetCurrentSystemTime((LARGE_INTEGER *)¤tTime);
UINT8 nfgenFamily = 0;
+
if (entry->key.dl_type == htons(ETH_TYPE_IPV4)) {
nfgenFamily = AF_INET;
} else if (entry->key.dl_type == htons(ETH_TYPE_IPV6)) {
diff --git a/datapath-windows/ovsext/Conntrack.h b/datapath-windows/ovsext/Conntrack.h
index ef1fe21..35075db 100644
--- a/datapath-windows/ovsext/Conntrack.h
+++ b/datapath-windows/ovsext/Conntrack.h
@@ -228,8 +228,4 @@ NDIS_STATUS OvsCtHandleFtp(PNET_BUFFER_LIST curNbl,
BOOLEAN request);
UINT32 OvsHashCtKey(const OVS_CT_KEY *key);
-BOOLEAN OvsCtKeyAreSame(OVS_CT_KEY ctxKey, OVS_CT_KEY entryKey);
-POVS_CT_ENTRY OvsCtLookup(OvsConntrackKeyLookupCtx *ctx);
-
-
#endif /* __OVS_CONNTRACK_H_ */
--
2.9.3.windows.1
More information about the dev
mailing list