From patchwork Tue Nov 14 18:48:19 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anand Kumar X-Patchwork-Id: 837981 X-Patchwork-Delegate: aserdean@cloudbasesolutions.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ybxPN2tSCz9s7c for ; Wed, 15 Nov 2017 05:49:12 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id AE658A84; Tue, 14 Nov 2017 18:48:38 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id D3C30A81 for ; Tue, 14 Nov 2017 18:48:36 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from EX13-EDG-OU-001.vmware.com (ex13-edg-ou-001.vmware.com [208.91.0.189]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id ADC971AE for ; Tue, 14 Nov 2017 18:48:35 +0000 (UTC) Received: from sc9-mailhost2.vmware.com (10.113.161.72) by EX13-EDG-OU-001.vmware.com (10.113.208.155) with Microsoft SMTP Server id 15.0.1156.6; Tue, 14 Nov 2017 10:48:22 -0800 Received: from localhost.localdomain (win-anand1.prom.eng.vmware.com [10.33.78.106]) by sc9-mailhost2.vmware.com (Postfix) with ESMTP id 1D8BEB09C5; Tue, 14 Nov 2017 10:48:35 -0800 (PST) From: Anand Kumar To: Date: Tue, 14 Nov 2017 10:48:19 -0800 Message-ID: <20171114184821.124-2-kumaranand@vmware.com> X-Mailer: git-send-email 2.9.3.windows.1 In-Reply-To: <20171114184821.124-1-kumaranand@vmware.com> References: <20171114184821.124-1-kumaranand@vmware.com> MIME-Version: 1.0 Received-SPF: None (EX13-EDG-OU-001.vmware.com: kumaranand@vmware.com does not designate permitted sender hosts) X-Spam-Status: No, score=-2.3 required=5.0 tests=RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD autolearn=disabled version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH v1 1/3] datapath-windows: Refactor conntrack code. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org 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 --- datapath-windows/ovsext/Conntrack-nat.c | 11 +-- datapath-windows/ovsext/Conntrack.c | 170 ++++++++++++++++++-------------- datapath-windows/ovsext/Conntrack.h | 4 - 3 files changed, 101 insertions(+), 84 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 3203411..48d4abf 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 NDIS_STATUS OvsCtFlush(UINT16 zone); @@ -210,7 +210,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; } @@ -233,11 +233,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: { @@ -281,6 +276,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; @@ -313,6 +313,7 @@ OvsCtUpdateEntry(OVS_CT_ENTRY* entry, BOOLEAN reply, UINT64 now) { + CT_UPDATE_RES status; switch (ipProto) { case IPPROTO_TCP: { @@ -320,32 +321,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 @@ -356,6 +348,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, @@ -423,21 +433,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; } } @@ -611,7 +620,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, @@ -687,6 +696,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, @@ -721,7 +765,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; } @@ -755,6 +799,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 @@ -762,23 +809,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); @@ -790,33 +829,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); } @@ -1001,9 +1026,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); } } } @@ -1031,12 +1054,12 @@ OvsCtFlush(UINT16 zone) 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); /* zone is a non-zero value */ if (!zone || zone == entry->key.zone) - OvsCtEntryDelete(entry); + OvsCtEntryDelete(entry, TRUE); } } } @@ -1312,6 +1335,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 be5f34d..66957e8 100644 --- a/datapath-windows/ovsext/Conntrack.h +++ b/datapath-windows/ovsext/Conntrack.h @@ -231,8 +231,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_ */