Hi Yin, Thanks for the patches. I had some immediate review comments.
- OvsUpdateAddressAndPort does not have a return type - Refrain from changing other code that your patch does not require. Please send out separate incremental patch. - This is the correct way to define static inline functions - "static __inline NDIS_STATUS”. Don’t change this. Update your code to follow this instead. Please find the other comments inline. Thanks, Sairam On 5/8/17, 3:38 PM, "ovs-dev-boun...@openvswitch.org on behalf of Yin Lin" <ovs-dev-boun...@openvswitch.org on behalf of li...@vmware.com> wrote: >Signed-off-by: Yin Lin <li...@vmware.com> >--- > datapath-windows/automake.mk | 4 +- > datapath-windows/ovsext/Actions.c | 118 ++++++++++++++++++++- > datapath-windows/ovsext/Actions.h | 20 ++++ > datapath-windows/ovsext/Conntrack.c | 187 +++++++++++++++++++++------------ > datapath-windows/ovsext/Conntrack.h | 25 +++-- > datapath-windows/ovsext/ovsext.vcxproj | 2 + > 6 files changed, 273 insertions(+), 83 deletions(-) > >diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk >index 7177630..f1632cc 100644 >--- a/datapath-windows/automake.mk >+++ b/datapath-windows/automake.mk >@@ -16,9 +16,9 @@ EXTRA_DIST += \ > datapath-windows/ovsext/Conntrack-icmp.c \ > datapath-windows/ovsext/Conntrack-other.c \ > datapath-windows/ovsext/Conntrack-related.c \ >- datapath-windows/ovsext/Conntrack-nat.c \ >+ datapath-windows/ovsext/Conntrack-nat.c \ > datapath-windows/ovsext/Conntrack-tcp.c \ >- datapath-windows/ovsext/Conntrack-nat.h \ >+ datapath-windows/ovsext/Conntrack-nat.h \ > datapath-windows/ovsext/Conntrack.c \ > datapath-windows/ovsext/Conntrack.h \ > datapath-windows/ovsext/Datapath.c \ >diff --git a/datapath-windows/ovsext/Actions.c >b/datapath-windows/ovsext/Actions.c >index e2eae9a..d1938f3 100644 >--- a/datapath-windows/ovsext/Actions.c >+++ b/datapath-windows/ovsext/Actions.c >@@ -1380,7 +1380,7 @@ PUINT8 OvsGetHeaderBySize(OvsForwardingContext >*ovsFwdCtx, > * based on the specified key. > *---------------------------------------------------------------------------- > */ [Sai]: Please don’t change this. This is the correct - "static __inline NDIS_STATUS" >-static __inline NDIS_STATUS >+NDIS_STATUS > OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx, > const struct ovs_key_udp *udpAttr) > { >@@ -1427,7 +1427,7 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx, > * based on the specified key. > *---------------------------------------------------------------------------- > */ >-static __inline NDIS_STATUS >+NDIS_STATUS [Sai]: Please don’t change this. This is the correct - "static __inline NDIS_STATUS" > OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx, > const struct ovs_key_tcp *tcpAttr) > { >@@ -1465,12 +1465,124 @@ OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx, > > /* > *---------------------------------------------------------------------------- >+ * OvsUpdateAddressAndPort -- >+ * Updates the source/destination IP and port fields in >+ * ovsFwdCtx.curNbl inline based on the specified key. >+ *---------------------------------------------------------------------------- >+ */ [Sai]: Missing return type >+OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx, >+ UINT32 newAddr, UINT16 newPort, >+ BOOLEAN isSource, BOOLEAN isTx) >+{ >+ PUINT8 bufferStart; >+ UINT32 hdrSize; >+ OVS_PACKET_HDR_INFO *layers = &ovsFwdCtx->layers; >+ IPHdr *ipHdr; >+ TCPHdr *tcpHdr = NULL; >+ UDPHdr *udpHdr = NULL; >+ UINT32 *addrField = NULL; >+ UINT16 *portField = NULL; >+ UINT16 *checkField = NULL; >+ BOOLEAN l4Offload = FALSE; >+ NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO csumInfo; >+ >+ ASSERT(layers->value != 0); >+ >+ if (layers->isTcp || layers->isUdp) { >+ hdrSize = layers->l4Offset + >+ layers->isTcp ? sizeof (*tcpHdr) : sizeof (*udpHdr); >+ } else { >+ hdrSize = layers->l3Offset + sizeof (*ipHdr); >+ } >+ >+ bufferStart = OvsGetHeaderBySize(ovsFwdCtx, hdrSize); >+ if (!bufferStart) { >+ return NDIS_STATUS_RESOURCES; >+ } >+ >+ ipHdr = (IPHdr *)(bufferStart + layers->l3Offset); >+ >+ if (layers->isTcp) { >+ tcpHdr = (TCPHdr *)(bufferStart + layers->l4Offset); >+ } else if (layers->isUdp) { >+ udpHdr = (UDPHdr *)(bufferStart + layers->l4Offset); >+ } >+ >+ csumInfo.Value = NET_BUFFER_LIST_INFO(ovsFwdCtx->curNbl, >+ TcpIpChecksumNetBufferListInfo); >+ /* >+ * Adjust the IP header inline as dictated by the action, and also update >+ * the IP and the TCP checksum for the data modified. >+ * >+ * In the future, this could be optimized to make one call to >+ * ChecksumUpdate32(). Ignoring this for now, since for the most common >+ * case, we only update the TTL. >+ */ >+ >+ if (isSource) { >+ addrField = &ipHdr->saddr; >+ if (tcpHdr) { >+ portField = &tcpHdr->source; >+ checkField = &tcpHdr->check; >+ l4Offload = isTx ? (BOOLEAN)csumInfo.Transmit.TcpChecksum : >+ ((BOOLEAN)csumInfo.Receive.TcpChecksumSucceeded || >+ (BOOLEAN)csumInfo.Receive.TcpChecksumFailed); >+ } else if (udpHdr) { >+ portField = &udpHdr->source; >+ checkField = &udpHdr->check; >+ l4Offload = isTx ? (BOOLEAN)csumInfo.Transmit.UdpChecksum : >+ ((BOOLEAN)csumInfo.Receive.UdpChecksumSucceeded || >+ (BOOLEAN)csumInfo.Receive.UdpChecksumFailed); >+ } >+ } else { >+ addrField = &ipHdr->daddr; >+ if (tcpHdr) { >+ portField = &tcpHdr->dest; >+ checkField = &tcpHdr->check; >+ } else if (udpHdr) { >+ portField = &udpHdr->dest; >+ checkField = &udpHdr->check; >+ } >+ } >+ if (*addrField != newAddr) { >+ UINT32 oldAddr = *addrField; >+ if (checkField && *checkField != 0) { >+ if (l4Offload) { >+ /* Recompute IP pseudo checksum */ >+ *checkField = ~(*checkField); >+ *checkField = ChecksumUpdate32(*checkField, oldAddr, >+ newAddr); >+ *checkField = ~(*checkField); >+ } else { >+ *checkField = ChecksumUpdate32(*checkField, oldAddr, >+ newAddr); >+ } >+ } >+ if (ipHdr->check != 0) { >+ ipHdr->check = ChecksumUpdate32(ipHdr->check, oldAddr, >+ newAddr); >+ } >+ *addrField = newAddr; >+ } >+ >+ if (portField && *portField != newPort) { >+ if (checkField && !l4Offload) { >+ *checkField = ChecksumUpdate16(*checkField, *portField, >+ newPort); >+ } >+ *portField = newPort; >+ } >+ return NDIS_STATUS_SUCCESS; >+} >+ >+/* >+ *---------------------------------------------------------------------------- > * OvsUpdateIPv4Header -- > * Updates the IPv4 header in ovsFwdCtx.curNbl inline based on the > * specified key. > *---------------------------------------------------------------------------- > */ >-static __inline NDIS_STATUS >+NDIS_STATUS [Sai]: Same as above > OvsUpdateIPv4Header(OvsForwardingContext *ovsFwdCtx, > const struct ovs_key_ipv4 *ipAttr) > { >diff --git a/datapath-windows/ovsext/Actions.h >b/datapath-windows/ovsext/Actions.h >index 1ce6c20..fd050d5 100644 >--- a/datapath-windows/ovsext/Actions.h >+++ b/datapath-windows/ovsext/Actions.h >@@ -110,4 +110,24 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT switchContext, > UINT32 srcPortNo, > OVS_PACKET_HDR_INFO *layers); > >+PUINT8 OvsGetHeaderBySize(OvsForwardingContext *ovsFwdCtx, >+ UINT32 size); >+ >+NDIS_STATUS >+OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx, >+ const struct ovs_key_udp *udpAttr); >+ >+NDIS_STATUS >+OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx, >+ const struct ovs_key_tcp *tcpAttr); >+ >+NDIS_STATUS >+OvsUpdateIPv4Header(OvsForwardingContext *ovsFwdCtx, >+ const struct ovs_key_ipv4 *ipAttr); >+ >+NDIS_STATUS >+OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx, >+ UINT32 newAddr, UINT16 newPort, >+ BOOLEAN isSource, BOOLEAN isTx); >+ > #endif /* __ACTIONS_H_ */ >diff --git a/datapath-windows/ovsext/Conntrack.c >b/datapath-windows/ovsext/Conntrack.c >index 9824368..6e95f47 100644 >--- a/datapath-windows/ovsext/Conntrack.c >+++ b/datapath-windows/ovsext/Conntrack.c >@@ -19,6 +19,7 @@ > #include "Jhash.h" > #include "PacketParser.h" > #include "Event.h" >+#include "Conntrack-nat.h" > > #pragma warning(push) > #pragma warning(disable:4311) >@@ -27,7 +28,7 @@ > #define SEC_TO_UNIX_EPOCH 11644473600LL > #define SEC_TO_NANOSEC 1000000000LL > >-KSTART_ROUTINE ovsConntrackEntryCleaner; >+KSTART_ROUTINE OvsConntrackEntryCleaner; > static PLIST_ENTRY ovsConntrackTable; > static OVS_CT_THREAD_CTX ctThreadCtx; > static PNDIS_RW_LOCK_EX ovsConntrackLockObj; >@@ -72,7 +73,7 @@ OvsInitConntrack(POVS_SWITCH_CONTEXT context) > /* Init CT Cleaner Thread */ > KeInitializeEvent(&ctThreadCtx.event, NotificationEvent, FALSE); > status = PsCreateSystemThread(&threadHandle, SYNCHRONIZE, NULL, NULL, >- NULL, ovsConntrackEntryCleaner, >+ NULL, OvsConntrackEntryCleaner, > &ctThreadCtx); > > if (status != STATUS_SUCCESS) { >@@ -89,6 +90,13 @@ OvsInitConntrack(POVS_SWITCH_CONTEXT context) > &ctThreadCtx.threadObject, NULL); > ZwClose(threadHandle); > threadHandle = NULL; >+ >+ status = OvsNatInit(); >+ >+ if (status != STATUS_SUCCESS) { >+ OvsCleanupConntrack(); >+ return status; >+ } > return STATUS_SUCCESS; > } > >@@ -121,6 +129,7 @@ OvsCleanupConntrack(VOID) > > NdisFreeRWLock(ovsConntrackLockObj); > ovsConntrackLockObj = NULL; >+ OvsNatCleanup(); > } > > static __inline VOID >@@ -160,25 +169,42 @@ OvsPostCtEventEntry(POVS_CT_ENTRY entry, UINT8 type) > OvsPostCtEvent(&ctEventEntry); > } > >-static __inline VOID >-OvsCtAddEntry(POVS_CT_ENTRY entry, OvsConntrackKeyLookupCtx *ctx, UINT64 now) >+static __inline BOOLEAN >+OvsCtAddEntry(POVS_CT_ENTRY entry, OvsConntrackKeyLookupCtx *ctx, >+ PNAT_ACTION_INFO natInfo, UINT64 now) > { >- NdisMoveMemory(&entry->key, &ctx->key, sizeof (OVS_CT_KEY)); >- NdisMoveMemory(&entry->rev_key, &ctx->key, sizeof (OVS_CT_KEY)); >+ NdisMoveMemory(&entry->key, &ctx->key, sizeof(OVS_CT_KEY)); >+ NdisMoveMemory(&entry->rev_key, &ctx->key, sizeof(OVS_CT_KEY)); > OvsCtKeyReverse(&entry->rev_key); >+ >+ /* NatInfo is always initialized to be disabled, so that if NAT action >+ * fails, we will not end up deleting an non-existent NAT entry. >+ */ >+ if (natInfo != NULL && OvsIsForwardNat(natInfo->natAction)) { >+ entry->natInfo = *natInfo; >+ if (!OvsNatCtEntry(entry)) { >+ return FALSE; >+ } >+ ctx->hash = OvsHashCtKey(&entry->key); >+ } else { >+ entry->natInfo.natAction = natInfo->natAction; >+ } >+ > entry->timestampStart = now; > InsertHeadList(&ovsConntrackTable[ctx->hash & CT_HASH_TABLE_MASK], > &entry->link); > > ctTotalEntries++; >+ return TRUE; > } > > static __inline POVS_CT_ENTRY >-OvsCtEntryCreate(PNET_BUFFER_LIST curNbl, >+OvsCtEntryCreate(OvsForwardingContext *fwdCtx, > UINT8 ipProto, > UINT32 l4Offset, > OvsConntrackKeyLookupCtx *ctx, > OvsFlowKey *key, >+ PNAT_ACTION_INFO natInfo, > BOOLEAN commit, > UINT64 currentTime, > BOOLEAN *entryCreated) >@@ -186,6 +212,7 @@ OvsCtEntryCreate(PNET_BUFFER_LIST curNbl, > POVS_CT_ENTRY entry = NULL; > *entryCreated = FALSE; > UINT32 state = 0; >+ PNET_BUFFER_LIST curNbl = fwdCtx->curNbl; > switch (ipProto) > { > case IPPROTO_TCP: >@@ -213,12 +240,9 @@ OvsCtEntryCreate(PNET_BUFFER_LIST curNbl, > if (parentEntry != NULL) { > entry->parent = parentEntry; > } >- OvsCtAddEntry(entry, ctx, currentTime); > *entryCreated = TRUE; [Sai]: If you want to move out OvsCtAddEntry(entry, ctx, currentTime); please handle all the associated cases carefully. Eg: *entryCreated is not reset to FALSE. > } >- >- OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL); >- return entry; >+ break; > } > case IPPROTO_ICMP: > { >@@ -232,35 +256,31 @@ OvsCtEntryCreate(PNET_BUFFER_LIST curNbl, > state |= OVS_CS_F_NEW; > if (commit) { > entry = OvsConntrackCreateIcmpEntry(currentTime); >- if (!entry) { >- return NULL; >- } >- OvsCtAddEntry(entry, ctx, currentTime); > *entryCreated = TRUE; > } >- >- OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL); >- return entry; >+ break; > } > case IPPROTO_UDP: > { > state |= OVS_CS_F_NEW; > if (commit) { > entry = OvsConntrackCreateOtherEntry(currentTime); >- if (!entry) { >- return NULL; >- } >- OvsCtAddEntry(entry, ctx, currentTime); > *entryCreated = TRUE; > } >- >- OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL); >- return entry; >+ break; > } > default: > goto invalid; > } > >+ if (commit && !entry) { >+ return NULL; >+ } >+ if (entry && !OvsCtAddEntry(entry, ctx, natInfo, currentTime)) { >+ return NULL; >+ } >+ OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL); >+ return entry; > invalid: > state |= OVS_CS_F_INVALID; > OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL); >@@ -269,11 +289,11 @@ invalid: > > static enum CT_UPDATE_RES > OvsCtUpdateEntry(OVS_CT_ENTRY* entry, >- PNET_BUFFER_LIST nbl, >- UINT8 ipProto, >- UINT32 l4Offset, >- BOOLEAN reply, >- UINT64 now) >+ PNET_BUFFER_LIST nbl, >+ UINT8 ipProto, >+ UINT32 l4Offset, >+ BOOLEAN reply, >+ UINT64 now) > { > switch (ipProto) > { >@@ -299,6 +319,12 @@ OvsCtUpdateEntry(OVS_CT_ENTRY* entry, > static __inline VOID > OvsCtEntryDelete(POVS_CT_ENTRY entry) > { [Sai]: Entry is guaranteed to not be null when this function is called. If that’s not the case please handle it outside. >+ if (entry == NULL) { >+ return; >+ } >+ if (entry->natInfo.natAction) { >+ OvsNatDeleteKey(&entry->key); >+ } > OvsPostCtEventEntry(entry, OVS_EVENT_CT_DELETE); > RemoveEntryList(&entry->link); > OvsFreeMemoryWithTag(entry, OVS_CT_POOL_TAG); >@@ -308,10 +334,6 @@ OvsCtEntryDelete(POVS_CT_ENTRY entry) > static __inline BOOLEAN > OvsCtEntryExpired(POVS_CT_ENTRY entry) > { >- if (entry == NULL) { >- return TRUE; >- } >- > UINT64 currentTime; > NdisGetCurrentSystemTime((LARGE_INTEGER *)¤tTime); > return entry->expiration < currentTime; >@@ -346,7 +368,7 @@ OvsDetectCtPacket(OvsForwardingContext *fwdCtx, > return NDIS_STATUS_NOT_SUPPORTED; > } > >-static __inline BOOLEAN >+BOOLEAN > OvsCtKeyAreSame(OVS_CT_KEY ctxKey, OVS_CT_KEY entryKey) > { > return ((ctxKey.src.addr.ipv4 == entryKey.src.addr.ipv4) && >@@ -372,13 +394,14 @@ OvsCtIncrementCounters(POVS_CT_ENTRY entry, BOOLEAN >reply, PNET_BUFFER_LIST nbl) > } > } > >-static __inline POVS_CT_ENTRY >+POVS_CT_ENTRY > OvsCtLookup(OvsConntrackKeyLookupCtx *ctx) > { > PLIST_ENTRY link; > POVS_CT_ENTRY entry; > BOOLEAN reply = FALSE; > POVS_CT_ENTRY found = NULL; >+ OVS_CT_KEY key = ctx->key; > > if (!ctTotalEntries) { > return found; >@@ -387,13 +410,19 @@ OvsCtLookup(OvsConntrackKeyLookupCtx *ctx) > LIST_FORALL(&ovsConntrackTable[ctx->hash & CT_HASH_TABLE_MASK], link) { > entry = CONTAINING_RECORD(link, OVS_CT_ENTRY, link); > >- if (OvsCtKeyAreSame(ctx->key,entry->key)) { >+ if (OvsCtKeyAreSame(key,entry->key)) { > found = entry; > reply = FALSE; > break; > } > >- if (OvsCtKeyAreSame(ctx->key,entry->rev_key)) { >+ /* Reverse NAT must be performed before OvsCtLookup, so here >+ * we simply need to flip the src and dst in key and compare >+ * they are equal. Note that flipped key is not equal to >+ * rev_key due to NAT effect. >+ */ [Sai]: Please don’t reverse the original key. This key gets used for creating a new Entry later on. >+ OvsCtKeyReverse(&key); >+ if (OvsCtKeyAreSame(key, entry->key)) { > found = entry; > reply = TRUE; > break; >@@ -412,17 +441,18 @@ OvsCtLookup(OvsConntrackKeyLookupCtx *ctx) > return found; > } > >-static __inline UINT32 >-OvsExtractLookupCtxHash(OvsConntrackKeyLookupCtx *ctx) >+UINT32 >+OvsHashCtKey(const OVS_CT_KEY *key) > { >- UINT32 hsrc, hdst,hash; >- hsrc = OvsJhashBytes((UINT32*) &ctx->key.src, sizeof(ctx->key.src), 0); >- hdst = OvsJhashBytes((UINT32*) &ctx->key.dst, sizeof(ctx->key.dst), 0); >+ UINT32 hsrc, hdst, hash; >+ hsrc = OvsJhashBytes((UINT32*) &key->src, sizeof(key->src), 0); >+ hdst = OvsJhashBytes((UINT32*) &key->dst, sizeof(key->dst), 0); > hash = hsrc ^ hdst; /* TO identify reverse traffic */ >- return OvsJhashBytes((uint32_t *) &ctx->key.dst + 1, >- ((uint32_t *) (&ctx->key + 1) - >- (uint32_t *) (&ctx->key.dst + 1)), >+ hash = OvsJhashBytes((uint32_t *) &key->dst + 1, >+ ((uint32_t *) (key + 1) - >+ (uint32_t *) (&key->dst + 1)), > hash); >+ return hash; > } > > static UINT8 >@@ -453,6 +483,7 @@ OvsCtSetupLookupCtx(OvsFlowKey *flowKey, > PNET_BUFFER_LIST curNbl, > UINT32 l4Offset) > { >+ const OVS_NAT_ENTRY *natEntry; > ctx->key.zone = zone; > ctx->key.dl_type = flowKey->l2.dlType; > ctx->related = FALSE; >@@ -514,7 +545,14 @@ OvsCtSetupLookupCtx(OvsFlowKey *flowKey, > return NDIS_STATUS_INVALID_PACKET; > } > >- ctx->hash = OvsExtractLookupCtxHash(ctx); >+ natEntry = OvsNatLookup(&ctx->key, TRUE); >+ if (natEntry) { >+ /* Translate address first for reverse NAT */ >+ ctx->key = natEntry->ctEntry->key; >+ OvsCtKeyReverse(&ctx->key); >+ } >+ >+ ctx->hash = OvsHashCtKey(&ctx->key); > return NDIS_STATUS_SUCCESS; > } > >@@ -532,17 +570,19 @@ OvsDetectFtpPacket(OvsFlowKey *key) { > *---------------------------------------------------------------------------- > */ > static __inline POVS_CT_ENTRY >-OvsProcessConntrackEntry(PNET_BUFFER_LIST curNbl, >+OvsProcessConntrackEntry(OvsForwardingContext *fwdCtx, > UINT32 l4Offset, > OvsConntrackKeyLookupCtx *ctx, > OvsFlowKey *key, > UINT16 zone, >+ NAT_ACTION_INFO *natInfo, > BOOLEAN commit, > UINT64 currentTime, > BOOLEAN *entryCreated) > { > POVS_CT_ENTRY entry = ctx->entry; > UINT32 state = 0; >+ PNET_BUFFER_LIST curNbl = fwdCtx->curNbl; > *entryCreated = FALSE; > > /* If an entry was found, update the state based on TCP flags */ >@@ -569,8 +609,8 @@ OvsProcessConntrackEntry(PNET_BUFFER_LIST curNbl, > //Delete and update the Conntrack > OvsCtEntryDelete(ctx->entry); > ctx->entry = NULL; >- entry = OvsCtEntryCreate(curNbl, key->ipKey.nwProto, l4Offset, >- ctx, key, commit, currentTime, >+ entry = OvsCtEntryCreate(fwdCtx, key->ipKey.nwProto, l4Offset, >+ ctx, key, natInfo, commit, currentTime, > entryCreated); > if (!entry) { > return NULL; >@@ -637,7 +677,7 @@ OvsConntrackSetLabels(OvsFlowKey *key, > } > > static __inline NDIS_STATUS >-OvsCtExecute_(PNET_BUFFER_LIST curNbl, >+OvsCtExecute_(OvsForwardingContext *fwdCtx, > OvsFlowKey *key, > OVS_PACKET_HDR_INFO *layers, > BOOLEAN commit, >@@ -650,13 +690,12 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl, > { > NDIS_STATUS status = NDIS_STATUS_SUCCESS; > POVS_CT_ENTRY entry = NULL; >+ PNET_BUFFER_LIST curNbl = fwdCtx->curNbl; > OvsConntrackKeyLookupCtx ctx = { 0 }; > LOCK_STATE_EX lockState; > UINT64 currentTime; > NdisGetCurrentSystemTime((LARGE_INTEGER *) ¤tTime); > >- /* XXX: Not referenced for now */ >- UNREFERENCED_PARAMETER(natInfo); > > /* Retrieve the Conntrack Key related fields from packet */ > OvsCtSetupLookupCtx(key, zone, &ctx, curNbl, layers->l4Offset); >@@ -675,18 +714,31 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl, > > if (!entry) { > /* If no matching entry was found, create one and add New state */ >- entry = OvsCtEntryCreate(curNbl, key->ipKey.nwProto, >+ entry = OvsCtEntryCreate(fwdCtx, key->ipKey.nwProto, > layers->l4Offset, &ctx, >- key, commit, currentTime, >+ key, natInfo, commit, currentTime, > &entryCreated); > } else { > /* Process the entry and update CT flags */ > OvsCtIncrementCounters(entry, ctx.reply, curNbl); >- entry = OvsProcessConntrackEntry(curNbl, layers->l4Offset, &ctx, key, >- zone, commit, currentTime, >+ entry = OvsProcessConntrackEntry(fwdCtx, layers->l4Offset, &ctx, key, >+ zone, natInfo, commit, currentTime, > &entryCreated); > } > >+ /* >+ * Note that natInfo is not the same as entry->natInfo here. natInfo >+ * is decided by action in the openflow rule, entry->natInfo is decided >+ * when the entry is created. In the reverse NAT case, natInfo is >+ * 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) >+ { >+ OvsNatPacket(fwdCtx, entry, entry->natInfo.natAction, >+ key, ctx.reply); >+ } >+ > if (entry && mark) { > OvsConntrackSetMark(key, entry, mark->value, mark->mask); > } >@@ -735,10 +787,8 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx, > MD_LABELS *labels = NULL; > PCHAR helper = NULL; > NAT_ACTION_INFO natActionInfo; >- PNET_BUFFER_LIST curNbl = fwdCtx->curNbl; > OVS_PACKET_HDR_INFO *layers = &fwdCtx->layers; > PNET_BUFFER_LIST newNbl = NULL; >- NAT_ACTION_INFO natActionInfo; > NDIS_STATUS status; > > memset(&natActionInfo, 0, sizeof natActionInfo); >@@ -775,12 +825,12 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx, > BOOLEAN hasMaxIp = FALSE; > BOOLEAN hasMaxPort = FALSE; > NL_NESTED_FOR_EACH_UNSAFE (natAttr, left, ctAttr) { >- enum ovs_nat_attr sub_type_nest = NlAttrType(natAttr); >- switch(sub_type_nest) { >+ enum ovs_nat_attr subtype = NlAttrType(natAttr); >+ switch(subtype) { > case OVS_NAT_ATTR_SRC: > case OVS_NAT_ATTR_DST: > natActionInfo.natAction |= >- ((sub_type_nest == OVS_NAT_ATTR_SRC) >+ ((subtype == OVS_NAT_ATTR_SRC) > ? NAT_ACTION_SRC : NAT_ACTION_DST); > break; > case OVS_NAT_ATTR_IP_MIN: >@@ -844,19 +894,19 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx, > commit = TRUE; > } > /* If newNbl is not allocated, use the current Nbl*/ >- status = OvsCtExecute_(newNbl != NULL ? newNbl : curNbl, key, layers, >+ status = OvsCtExecute_(fwdCtx, key, layers, > commit, force, zone, mark, labels, helper, > &natActionInfo); > return status; > } > > /* > *---------------------------------------------------------------------------- >- * ovsConntrackEntryCleaner >+ * OvsConntrackEntryCleaner > * Runs periodically and cleans up the connection tracker > *---------------------------------------------------------------------------- > */ > VOID >-ovsConntrackEntryCleaner(PVOID data) >+OvsConntrackEntryCleaner(PVOID data) > { > > POVS_CT_THREAD_CTX context = (POVS_CT_THREAD_CTX)data; >@@ -873,15 +923,13 @@ ovsConntrackEntryCleaner(PVOID data) > } > > /* Set the timeout for the thread and cleanup */ >- UINT64 currentTime, threadSleepTimeout; >- NdisGetCurrentSystemTime((LARGE_INTEGER *)¤tTime); >- threadSleepTimeout = currentTime + CT_CLEANUP_INTERVAL; >+ INT64 threadSleepTimeout = -CT_CLEANUP_INTERVAL; > > if (ctTotalEntries) { > 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->expiration < currentTime) { >+ if (entry && OvsCtEntryExpired(entry)) { > OvsCtEntryDelete(entry); > } > } >@@ -921,6 +969,7 @@ OvsCtFlush(UINT16 zone) > } > } > >+ OvsNatFlush(zone); > NdisReleaseRWLock(ovsConntrackLockObj, &lockState); > return NDIS_STATUS_SUCCESS; > } >diff --git a/datapath-windows/ovsext/Conntrack.h >b/datapath-windows/ovsext/Conntrack.h >index 1ad289f..fcdd96e 100644 >--- a/datapath-windows/ovsext/Conntrack.h >+++ b/datapath-windows/ovsext/Conntrack.h >@@ -18,8 +18,9 @@ > #define __OVS_CONNTRACK_H_ 1 > > #include "precomp.h" >-#include "Flow.h" >+#include "Actions.h" > #include "Debug.h" >+#include "Flow.h" > #include "Actions.h" > #include <stddef.h> > >@@ -87,6 +88,14 @@ typedef struct _OVS_CT_KEY { > UINT64 byteCount; > } OVS_CT_KEY, *POVS_CT_KEY; > >+typedef struct _NAT_ACTION_INFO { >+ struct ct_addr minAddr; >+ struct ct_addr maxAddr; >+ uint16_t minPort; >+ uint16_t maxPort; >+ uint16_t natAction; >+} NAT_ACTION_INFO, *PNAT_ACTION_INFO; >+ > typedef struct OVS_CT_ENTRY { > OVS_CT_KEY key; > OVS_CT_KEY rev_key; >@@ -95,6 +104,7 @@ typedef struct OVS_CT_ENTRY { > UINT32 mark; > UINT64 timestampStart; > struct ovs_key_ct_labels labels; >+ NAT_ACTION_INFO natInfo; > PVOID parent; /* Points to main connection */ > } OVS_CT_ENTRY, *POVS_CT_ENTRY; > >@@ -119,14 +129,6 @@ typedef struct OvsConntrackKeyLookupCtx { > BOOLEAN related; > } OvsConntrackKeyLookupCtx; > >-typedef struct _NAT_ACTION_INFO { >- struct ct_addr minAddr; >- struct ct_addr maxAddr; >- uint16_t minPort; >- uint16_t maxPort; >- uint16_t natAction; >-} NAT_ACTION_INFO, *PNAT_ACTION_INFO; >- > #define CT_HASH_TABLE_SIZE ((UINT32)1 << 10) > #define CT_HASH_TABLE_MASK (CT_HASH_TABLE_SIZE - 1) > #define CT_INTERVAL_SEC 10000000LL //1s >@@ -225,4 +227,9 @@ NDIS_STATUS OvsCtHandleFtp(PNET_BUFFER_LIST curNbl, > POVS_CT_ENTRY entry, > 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_ */ >diff --git a/datapath-windows/ovsext/ovsext.vcxproj >b/datapath-windows/ovsext/ovsext.vcxproj >index ecfc0b8..69dfb56 100644 >--- a/datapath-windows/ovsext/ovsext.vcxproj >+++ b/datapath-windows/ovsext/ovsext.vcxproj >@@ -103,6 +103,7 @@ > <ClInclude Include="Actions.h" /> > <ClInclude Include="Atomic.h" /> > <ClInclude Include="BufferMgmt.h" /> >+ <ClInclude Include="Conntrack-nat.h" /> > <ClInclude Include="Conntrack.h" /> > <ClInclude Include="Datapath.h" /> > <ClInclude Include="Debug.h" /> >@@ -257,6 +258,7 @@ > <ItemGroup> > <ClCompile Include="Actions.c" /> > <ClCompile Include="BufferMgmt.c" /> >+ <ClCompile Include="Conntrack-nat.c" /> > <ClCompile Include="Conntrack-related.c" /> > <ClCompile Include="Conntrack-ftp.c" /> > <ClCompile Include="Conntrack-icmp.c" /> >-- >2.10.2.windows.1 > >_______________________________________________ >dev mailing list >d...@openvswitch.org >https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo&m=8gWG4n66FcEVbc_BOfr75-Ja18vlE-H8ODlm6rWW-Xc&s=qD4E5mj6_VLjSuPpTjVnWP323yyVSrMKUMOpgsK25XQ&e= > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev