From c4d9f9319f1a7956b7b308f65ba57995c5d93c92 Mon Sep 17 00:00:00 2001 From: Anand Kumar Date: Mon, 29 Jan 2018 10:27:59 -0800 Subject: datapath-windows: Refactor conntrack code. Some of the functions and code are refactored so that new conntrack lock can be implemented Signed-off-by: Anand Kumar Acked-by: Alin Gabriel Serdean Signed-off-by: Alin Gabriel Serdean --- 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(-) (limited to 'datapath-windows') diff --git a/datapath-windows/ovsext/Conntrack-nat.c b/datapath-windows/ovsext/Conntrack-nat.c index c778f1213..7975770db 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 169ec4f31..3cde83624 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 ef1fe21c2..35075db4d 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_ */ -- cgit v1.2.1