Disregard this patch. I have sent out a v4 which addresses another review comment.
On 6/2/16, 5:12 PM, "Sairam Venugopal" <vsai...@vmware.com> wrote: >Enable support for UDP and ICMP in the connection tracking module on >Hyper-V. > >Signed-off-by: Sairam Venugopal <vsai...@vmware.com> >--- > datapath-windows/automake.mk | 1 + > datapath-windows/ovsext/Conntrack-other.c | 82 ++++++++++++++ > datapath-windows/ovsext/Conntrack.c | 177 >++++++++++++++++++++++-------- > datapath-windows/ovsext/Conntrack.h | 6 +- > datapath-windows/ovsext/ovsext.vcxproj | 1 + > 5 files changed, 222 insertions(+), 45 deletions(-) > create mode 100644 datapath-windows/ovsext/Conntrack-other.c > >diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk >index c9af806..668cf2c 100644 >--- a/datapath-windows/automake.mk >+++ b/datapath-windows/automake.mk >@@ -13,6 +13,7 @@ EXTRA_DIST += \ > datapath-windows/ovsext/Atomic.h \ > datapath-windows/ovsext/BufferMgmt.c \ > datapath-windows/ovsext/BufferMgmt.h \ >+ datapath-windows/ovsext/Conntrack-other.c \ > datapath-windows/ovsext/Conntrack-tcp.c \ > datapath-windows/ovsext/Conntrack.c \ > datapath-windows/ovsext/Conntrack.h \ >diff --git a/datapath-windows/ovsext/Conntrack-other.c >b/datapath-windows/ovsext/Conntrack-other.c >new file mode 100644 >index 0000000..60bc76c >--- /dev/null >+++ b/datapath-windows/ovsext/Conntrack-other.c >@@ -0,0 +1,82 @@ >+/* >+ * Copyright (c) 2015, 2016 VMware, Inc. >+ * >+ * Licensed under the Apache License, Version 2.0 (the "License"); >+ * you may not use this file except in compliance with the License. >+ * You may obtain a copy of the License at: >+ * >+ * http://www.apache.org/licenses/LICENSE-2.0 >+ * >+ * Unless required by applicable law or agreed to in writing, software >+ * distributed under the License is distributed on an "AS IS" BASIS, >+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >implied. >+ * See the License for the specific language governing permissions and >+ * limitations under the License. >+ */ >+ >+#include "Conntrack.h" >+#include <stddef.h> >+ >+enum other_state { >+ OTHERS_FIRST, >+ OTHERS_MULTIPLE, >+ OTHERS_BIDIR, >+}; >+ >+struct conn_other { >+ struct OVS_CT_ENTRY up; >+ enum other_state state; >+}; >+ >+static const long long other_timeouts[] = { >+ [OTHERS_FIRST] = 60 * 10000000, >+ [OTHERS_MULTIPLE] = 60 * 10000000, >+ [OTHERS_BIDIR] = 30 * 10000000, >+}; >+ >+static __inline struct conn_other* >+OvsCastConntrackEntryToOtherEntry(OVS_CT_ENTRY *conn) >+{ >+ ASSERT(conn); >+ return CONTAINER_OF(conn, struct conn_other, up); >+} >+ >+static __inline VOID >+OvsConntrackUpdateExpiration(struct conn_other *conn, long long now) >+{ >+ ASSERT(conn); >+ conn->up.expiration = now + other_timeouts[conn->state]; >+} >+ >+enum ct_update_res >+OvsConntrackUpdateOtherEntry(OVS_CT_ENTRY *conn_, >+ BOOLEAN reply, >+ UINT64 now) >+{ >+ ASSERT(conn_); >+ struct conn_other *conn = OvsCastConntrackEntryToOtherEntry(conn_); >+ >+ if (reply && conn->state != OTHERS_BIDIR) { >+ conn->state = OTHERS_BIDIR; >+ } else if (conn->state == OTHERS_FIRST) { >+ conn->state = OTHERS_MULTIPLE; >+ } >+ >+ OvsConntrackUpdateExpiration(conn, now); >+ >+ return CT_UPDATE_VALID; >+} >+ >+OVS_CT_ENTRY * >+OvsConntrackCreateOtherEntry(UINT64 now) >+{ >+ struct conn_other *conn; >+ conn = OvsAllocateMemoryWithTag(sizeof(struct conn_other), >+ OVS_CT_POOL_TAG); >+ /* XXX Handle memory allocation error (by returning a status) */ >+ ASSERT(conn); >+ conn->up = (OVS_CT_ENTRY) {0}; >+ conn->state = OTHERS_FIRST; >+ OvsConntrackUpdateExpiration(conn, now); >+ return &conn->up; >+} >diff --git a/datapath-windows/ovsext/Conntrack.c >b/datapath-windows/ovsext/Conntrack.c >index 544fd51..e285a62 100644 >--- a/datapath-windows/ovsext/Conntrack.c >+++ b/datapath-windows/ovsext/Conntrack.c >@@ -146,9 +146,20 @@ OvsCtUpdateFlowKey(struct OvsFlowKey *key, > } > } > >+static __inline VOID >+OvsCtAddEntry(POVS_CT_ENTRY entry, OvsConntrackKeyLookupCtx *ctx) >+{ >+ NdisMoveMemory(&entry->key, &ctx->key, sizeof (OVS_CT_KEY)); >+ NdisMoveMemory(&entry->rev_key, &ctx->key, sizeof (OVS_CT_KEY)); >+ OvsCtKeyReverse(&entry->rev_key); >+ InsertHeadList(&ovsConntrackTable[ctx->hash & CT_HASH_TABLE_MASK], >+ &entry->link); >+} >+ > static __inline POVS_CT_ENTRY >-OvsCtEntryCreate(const TCPHdr *tcp, >- PNET_BUFFER_LIST curNbl, >+OvsCtEntryCreate(PNET_BUFFER_LIST curNbl, >+ UINT8 ipProto, >+ UINT32 l4Offset, > OvsConntrackKeyLookupCtx *ctx, > OvsFlowKey *key, > BOOLEAN commit, >@@ -156,26 +167,74 @@ OvsCtEntryCreate(const TCPHdr *tcp, > { > POVS_CT_ENTRY entry = NULL; > UINT32 state = 0; >- if (!OvsConntrackValidateTcpPacket(tcp)) { >- state |= OVS_CS_F_INVALID; >- OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL); >- return entry; >- } >+ switch (ipProto) >+ { >+ case IPPROTO_TCP: >+ { >+ TCPHdr tcpStorage; >+ const TCPHdr *tcp; >+ tcp = OvsGetTcp(curNbl, l4Offset, &tcpStorage); >+ if (!OvsConntrackValidateTcpPacket(tcp)) { >+ goto invalid; >+ } >+ >+ state |= OVS_CS_F_NEW; >+ if (commit) { >+ entry = OvsConntrackCreateTcpEntry(tcp, curNbl, >currentTime); >+ OvsCtAddEntry(entry, ctx); >+ } >+ >+ OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL); >+ return entry; >+ } >+ case IPPROTO_ICMP: >+ case IPPROTO_UDP: >+ state |= OVS_CS_F_NEW; >+ if (commit) { >+ entry = OvsConntrackCreateOtherEntry(currentTime); >+ OvsCtAddEntry(entry, ctx); >+ } > >- state |= OVS_CS_F_NEW; >- if (commit) { >- entry = OvsConntrackCreateTcpEntry(tcp, curNbl, currentTime); >- NdisMoveMemory(&entry->key, &ctx->key, sizeof (OVS_CT_KEY)); >- NdisMoveMemory(&entry->rev_key, &ctx->key, sizeof (OVS_CT_KEY)); >- OvsCtKeyReverse(&entry->rev_key); >- InsertHeadList(&ovsConntrackTable[ctx->hash & >CT_HASH_TABLE_MASK], >- &entry->link); >+ OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL); >+ return entry; >+ default: >+ goto invalid; > } > >+invalid: >+ state |= OVS_CS_F_INVALID; > OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL); > return entry; > } > >+static enum CT_UPDATE_RES >+OvsCtUpdateEntry(OVS_CT_ENTRY* entry, >+ PNET_BUFFER_LIST nbl, >+ UINT8 ipProto, >+ UINT32 l4Offset, >+ BOOLEAN reply, >+ UINT64 now) >+{ >+ switch (ipProto) >+ { >+ case IPPROTO_TCP: >+ { >+ TCPHdr tcpStorage; >+ const TCPHdr *tcp; >+ tcp = OvsGetTcp(nbl, l4Offset, &tcpStorage); >+ if (!tcp) { >+ return CT_UPDATE_INVALID; >+ } >+ return OvsConntrackUpdateTcpEntry(entry, tcp, nbl, reply, >now); >+ } >+ case IPPROTO_ICMP: >+ case IPPROTO_UDP: >+ return OvsConntrackUpdateOtherEntry(entry, reply, now); >+ default: >+ return CT_UPDATE_INVALID; >+ } >+} >+ > static __inline VOID > OvsCtEntryDelete(POVS_CT_ENTRY entry) > { >@@ -204,10 +263,12 @@ OvsDetectCtPacket(OvsFlowKey *key) > if (key->ipKey.nwFrag != OVS_FRAG_TYPE_NONE) { > return NDIS_STATUS_NOT_SUPPORTED; > } >- if (key->ipKey.nwProto != IPPROTO_TCP) { >- return NDIS_STATUS_NOT_SUPPORTED; >+ if (key->ipKey.nwProto == IPPROTO_TCP >+ || key->ipKey.nwProto == IPPROTO_UDP >+ || key->ipKey.nwProto == IPPROTO_ICMP) { >+ return NDIS_STATUS_SUCCESS; > } >- return NDIS_STATUS_SUCCESS; >+ return NDIS_STATUS_NOT_SUPPORTED; > case ETH_TYPE_IPV6: > return NDIS_STATUS_NOT_SUPPORTED; > } >@@ -265,16 +326,31 @@ OvsCtLookup(OvsConntrackKeyLookupCtx *ctx) > return found; > } > >-static __inline VOID >-OvsCtSetupLookupCtx(OvsFlowKey *flowKey, >- UINT16 zone, >- OvsConntrackKeyLookupCtx *ctx) >+static __inline UINT32 >+OvsExtractLookupCtxHash(OvsConntrackKeyLookupCtx *ctx) > { > 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); >+ 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); >+} > >+static __inline NDIS_STATUS >+OvsCtSetupLookupCtx(OvsFlowKey *flowKey, >+ UINT16 zone, >+ OvsConntrackKeyLookupCtx *ctx, >+ PNET_BUFFER_LIST curNbl, >+ UINT32 l4Offset) >+{ > ctx->key.zone = zone; > ctx->key.dl_type = flowKey->l2.dlType; >+ ctx->related = FALSE; > >+ /* Extract L3 and L4*/ > if (flowKey->l2.dlType == htons(ETH_TYPE_IPV4)) { > ctx->key.src.addr.ipv4 = flowKey->ipKey.nwSrc; > ctx->key.dst.addr.ipv4 = flowKey->ipKey.nwDst; >@@ -282,6 +358,26 @@ OvsCtSetupLookupCtx(OvsFlowKey *flowKey, > > ctx->key.src.port = flowKey->ipKey.l4.tpSrc; > ctx->key.dst.port = flowKey->ipKey.l4.tpDst; >+ if (flowKey->ipKey.nwProto == IPPROTO_ICMP) { >+ ICMPHdr icmpStorage; >+ const ICMPHdr *icmp; >+ icmp = OvsGetIcmp(curNbl, l4Offset, &icmpStorage); >+ ctx->key.src.port = ctx->key.dst.port = icmp->fields.echo.id; >+ >+ /* Related bit is set when ICMP has an error */ >+ /* XXX parse out the appropriate src and dst from inner pkt >*/ >+ switch (icmp->type) { >+ case ICMP4_DEST_UNREACH: >+ case ICMP4_TIME_EXCEEDED: >+ case ICMP4_PARAM_PROB: >+ case ICMP4_SOURCE_QUENCH: >+ case ICMP4_REDIRECT: { >+ ctx->related = TRUE; >+ } >+ default: >+ ctx->related = FALSE; >+ } >+ } > } else if (flowKey->l2.dlType == htons(ETH_TYPE_IPV6)) { > ctx->key.src.addr.ipv6 = flowKey->ipv6Key.ipv6Src; > ctx->key.dst.addr.ipv6 = flowKey->ipv6Key.ipv6Dst; >@@ -289,18 +385,13 @@ OvsCtSetupLookupCtx(OvsFlowKey *flowKey, > > ctx->key.src.port = flowKey->ipv6Key.l4.tpSrc; > ctx->key.dst.port = flowKey->ipv6Key.l4.tpDst; >+ /* XXX Handle ICMPv6 errors*/ >+ } else { >+ return NDIS_STATUS_INVALID_PACKET; > } > >- /* Related bit is set for ICMP and FTP (Not supported)*/ >- ctx->related = FALSE; >- >- hsrc = OvsJhashBytes((UINT32*) &ctx->key.src, sizeof(ctx->key.src), >0); >- hdst = OvsJhashBytes((UINT32*) &ctx->key.dst, sizeof(ctx->key.dst), >0); >- hash = hsrc ^ hdst; /* TO identify reverse traffic */ >- ctx->hash = OvsJhashBytes((uint32_t *) &ctx->key.dst + 1, >- ((uint32_t *) (&ctx->key + 1) - >- (uint32_t *) (&ctx->key.dst + 1)), >- hash); >+ ctx->hash = OvsExtractLookupCtxHash(ctx); >+ return NDIS_STATUS_SUCCESS; > } > > /* >@@ -311,7 +402,7 @@ OvsCtSetupLookupCtx(OvsFlowKey *flowKey, > */ > static __inline POVS_CT_ENTRY > OvsProcessConntrackEntry(PNET_BUFFER_LIST curNbl, >- const TCPHdr *tcp, >+ UINT32 l4Offset, > OvsConntrackKeyLookupCtx *ctx, > OvsFlowKey *key, > UINT16 zone, >@@ -329,8 +420,8 @@ OvsProcessConntrackEntry(PNET_BUFFER_LIST curNbl, > } > } else { > CT_UPDATE_RES result; >- result = OvsConntrackUpdateTcpEntry(entry, tcp, curNbl, >- ctx->reply, currentTime); >+ result = OvsCtUpdateEntry(entry, curNbl, key->ipKey.nwProto, >+ l4Offset, ctx->reply, currentTime); > switch (result) { > case CT_UPDATE_VALID: > state |= OVS_CS_F_ESTABLISHED; >@@ -345,8 +436,8 @@ OvsProcessConntrackEntry(PNET_BUFFER_LIST curNbl, > //Delete and update the Conntrack > OvsCtEntryDelete(ctx->entry); > ctx->entry = NULL; >- entry = OvsCtEntryCreate(tcp, curNbl, ctx, key, >- commit, currentTime); >+ entry = OvsCtEntryCreate(curNbl, key->ipKey.nwProto, >l4Offset, >+ ctx, key, commit, currentTime); > break; > } > } >@@ -401,15 +492,12 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl, > NDIS_STATUS status = NDIS_STATUS_SUCCESS; > POVS_CT_ENTRY entry = NULL; > OvsConntrackKeyLookupCtx ctx = { 0 }; >- TCPHdr tcpStorage; >- UINT64 currentTime; > LOCK_STATE_EX lockState; >- const TCPHdr *tcp; >- tcp = OvsGetTcp(curNbl, layers->l4Offset, &tcpStorage); >+ UINT64 currentTime; > NdisGetCurrentSystemTime((LARGE_INTEGER *) ¤tTime); > > /* Retrieve the Conntrack Key related fields from packet */ >- OvsCtSetupLookupCtx(key, zone, &ctx); >+ OvsCtSetupLookupCtx(key, zone, &ctx, curNbl, layers->l4Offset); > > NdisAcquireRWLockWrite(ovsConntrackLockObj, &lockState, 0); > >@@ -418,11 +506,12 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl, > > if (!entry) { > /* If no matching entry was found, create one and add New state >*/ >- entry = OvsCtEntryCreate(tcp, curNbl, &ctx, >+ entry = OvsCtEntryCreate(curNbl, key->ipKey.nwProto, >+ layers->l4Offset, &ctx, > key, commit, currentTime); > } else { > /* Process the entry and update CT flags */ >- entry = OvsProcessConntrackEntry(curNbl, tcp, &ctx, key, >+ entry = OvsProcessConntrackEntry(curNbl, layers->l4Offset, &ctx, >key, > zone, commit, currentTime); > } > >diff --git a/datapath-windows/ovsext/Conntrack.h >b/datapath-windows/ovsext/Conntrack.h >index a754544..cdbf75b 100644 >--- a/datapath-windows/ovsext/Conntrack.h >+++ b/datapath-windows/ovsext/Conntrack.h >@@ -99,9 +99,13 @@ BOOLEAN OvsConntrackValidateTcpPacket(const TCPHdr >*tcp); > OVS_CT_ENTRY * OvsConntrackCreateTcpEntry(const TCPHdr *tcp, > PNET_BUFFER_LIST nbl, > UINT64 now); >+OVS_CT_ENTRY * OvsConntrackCreateOtherEntry(UINT64 now); > enum CT_UPDATE_RES OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_, > const TCPHdr *tcp, > PNET_BUFFER_LIST nbl, > BOOLEAN reply, > UINT64 now); >-#endif /* __OVS_CONNTRACK_H_ */ >\ No newline at end of file >+enum ct_update_res OvsConntrackUpdateOtherEntry(OVS_CT_ENTRY *conn_, >+ BOOLEAN reply, >+ UINT64 now); >+#endif /* __OVS_CONNTRACK_H_ */ >diff --git a/datapath-windows/ovsext/ovsext.vcxproj >b/datapath-windows/ovsext/ovsext.vcxproj >index 0356ddf..0ad4c58 100644 >--- a/datapath-windows/ovsext/ovsext.vcxproj >+++ b/datapath-windows/ovsext/ovsext.vcxproj >@@ -176,6 +176,7 @@ > <ItemGroup> > <ClCompile Include="Actions.c" /> > <ClCompile Include="BufferMgmt.c" /> >+ <ClCompile Include="Conntrack-other.c" /> > <ClCompile Include="Conntrack-tcp.c" /> > <ClCompile Include="Conntrack.c" /> > <ClCompile Include="Debug.c" /> >-- >2.5.0.windows.1 > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
