Hi Sairam, Thank you for reviewing recirculation patches. Please see my answers inline.
-Sorin -----Original Message----- From: Sairam Venugopal [mailto:vsai...@vmware.com] Sent: Friday, 26 February, 2016 21:51 To: Sorin Vinturis; dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH v3 1/6] datapath-windows: Added recirculation support. Hi Sorin, Thanks for the patch. Please check the comments inline. Thanks, Sairam On 2/22/16, 6:07 AM, "Sorin Vinturis" <svintu...@cloudbasesolutions.com> wrote: >Recirculation support for the OVS extension. > >Tested using PING and iperf with Driver Verifier enabled. > >Signed-off-by: Sorin Vinturis <svintu...@cloudbasesolutions.com> >Co-authored-by: Alin Gabriel Serdean <aserd...@cloudbasesolutions.com> >Reported-by: Sorin Vinturis <svintu...@cloudbasesolutions.com> >Reported-at: https://github.com/openvswitch/ovs-issues/issues/104 >--- >v2: Initialize flow key before using it. >v3: Synchronized access to deferred actions queue. >--- > datapath-windows/automake.mk | 3 + > datapath-windows/ovsext/Actions.c | 215 >++++++++++++++++++++++++++---- > datapath-windows/ovsext/Actions.h | 54 ++++++++ > datapath-windows/ovsext/Datapath.c | 4 + > datapath-windows/ovsext/DpInternal.h | 2 +- > datapath-windows/ovsext/Flow.c | 76 +++++++++-- > datapath-windows/ovsext/Flow.h | 5 +- > datapath-windows/ovsext/Netlink/Netlink.h | 11 ++ > datapath-windows/ovsext/PacketIO.c | 16 ++- > datapath-windows/ovsext/PacketIO.h | 10 -- > datapath-windows/ovsext/Recirc.c | 171 ++++++++++++++++++++++++ > datapath-windows/ovsext/Recirc.h | 82 ++++++++++++ > datapath-windows/ovsext/Tunnel.c | 15 ++- > datapath-windows/ovsext/User.c | 13 +- > datapath-windows/ovsext/ovsext.vcxproj | 3 + > 15 files changed, 612 insertions(+), 68 deletions(-) create mode > 100644 datapath-windows/ovsext/Actions.h create mode 100644 > datapath-windows/ovsext/Recirc.c create mode 100644 > datapath-windows/ovsext/Recirc.h > >diff --git a/datapath-windows/automake.mk >b/datapath-windows/automake.mk index f29f548..04fc97f 100644 >--- a/datapath-windows/automake.mk >+++ b/datapath-windows/automake.mk >@@ -9,6 +9,7 @@ EXTRA_DIST += \ > datapath-windows/misc/uninstall.cmd \ > datapath-windows/ovsext.sln \ > datapath-windows/ovsext/Actions.c \ >+ datapath-windows/ovsext/Actions.h \ > datapath-windows/ovsext/Atomic.h \ > datapath-windows/ovsext/BufferMgmt.c \ > datapath-windows/ovsext/BufferMgmt.h \ @@ -45,6 +46,8 @@ EXTRA_DIST >+= \ > datapath-windows/ovsext/PacketIO.h \ > datapath-windows/ovsext/PacketParser.c \ > datapath-windows/ovsext/PacketParser.h \ >+ datapath-windows/ovsext/Recirc.c \ >+ datapath-windows/ovsext/Recirc.h \ > datapath-windows/ovsext/Stt.c \ > datapath-windows/ovsext/Stt.h \ > datapath-windows/ovsext/Switch.c \ >diff --git a/datapath-windows/ovsext/Actions.c >b/datapath-windows/ovsext/Actions.c >index 5a04541..d3f18f2 100644 >--- a/datapath-windows/ovsext/Actions.c >+++ b/datapath-windows/ovsext/Actions.c >@@ -14,8 +14,7 @@ > * limitations under the License. > */ > >-#include "precomp.h" >- >+#include "Actions.h" > #include "Debug.h" > #include "Event.h" > #include "Flow.h" >@@ -24,6 +23,7 @@ > #include "NetProto.h" > #include "Offload.h" > #include "PacketIO.h" >+#include "Recirc.h" > #include "Stt.h" > #include "Switch.h" > #include "User.h" >@@ -35,6 +35,8 @@ > #endif > #define OVS_DBG_MOD OVS_DBG_ACTION > >+#define OVS_DEST_PORTS_ARRAY_MIN_SIZE 2 >+ > typedef struct _OVS_ACTION_STATS { > UINT64 rxGre; > UINT64 txGre; >@@ -66,7 +68,6 @@ OVS_ACTION_STATS ovsActionStats; > * exercised while adding new members to the structure - only add ones >that get > * used across multiple stages in the pipeline/get used in multiple >functions. > */ >-#define OVS_DEST_PORTS_ARRAY_MIN_SIZE 2 typedef struct >OvsForwardingContext { > POVS_SWITCH_CONTEXT switchContext; > /* The NBL currently used in the pipeline. */ @@ -99,7 +100,7 @@ >typedef struct OvsForwardingContext { > */ > OvsIPv4TunnelKey tunKey; > >- /* >+ /* > * Tunneling - Tx: > * To store the output port, when it is a tunneled port. We don't >foresee > * multiple tunneled ports as outport for any given NBL. >@@ -117,7 +118,6 @@ typedef struct OvsForwardingContext { > OVS_PACKET_HDR_INFO layers; > } OvsForwardingContext; > >- > /* > * >----------------------------------------------------------------------- >--- > * OvsInitForwardingCtx -- >@@ -564,10 +564,10 @@ OvsCompleteNBLForwardingCtx(OvsForwardingContext >*ovsFwdCtx, > static __inline NDIS_STATUS > OvsDoFlowLookupOutput(OvsForwardingContext *ovsFwdCtx) { >- OvsFlowKey key; >- OvsFlow *flow; >- UINT64 hash; >- NDIS_STATUS status; >+ OvsFlowKey key = { 0 }; >+ OvsFlow *flow = NULL; >+ UINT64 hash = 0; >+ NDIS_STATUS status = NDIS_STATUS_SUCCESS; > POVS_VPORT_ENTRY vport = > OvsFindVportByPortNo(ovsFwdCtx->switchContext, >ovsFwdCtx->srcVportNo); > if (vport == NULL || vport->ovsState != OVS_STATE_CONNECTED) { @@ >-595,11 +595,13 @@ OvsDoFlowLookupOutput(OvsForwardingContext >*ovsFwdCtx) > if (flow) { > OvsFlowUsed(flow, ovsFwdCtx->curNbl, &ovsFwdCtx->layers); > ovsFwdCtx->switchContext->datapath.hits++; >- status = OvsActionsExecute(ovsFwdCtx->switchContext, >- ovsFwdCtx->completionList, >ovsFwdCtx->curNbl, >- ovsFwdCtx->srcVportNo, >ovsFwdCtx->sendFlags, >- &key, &hash, &ovsFwdCtx->layers, >- flow->actions, flow->actionsLen); >+ status = OvsDoExecuteActions(ovsFwdCtx->switchContext, >+ ovsFwdCtx->completionList, >+ ovsFwdCtx->curNbl, >+ ovsFwdCtx->srcVportNo, >+ ovsFwdCtx->sendFlags, >+ &key, &hash, &ovsFwdCtx->layers, >+ flow->actions, flow->actionsLen); > ovsFwdCtx->curNbl = NULL; > } else { > LIST_ENTRY missedPackets; >@@ -1520,7 +1522,58 @@ OvsExecuteSetAction(OvsForwardingContext >*ovsFwdCtx, > > /* > * >----------------------------------------------------------------------- >--- >- * OvsActionsExecute -- >+ * OvsActionExecuteRecirc -- >+ * The function adds a deferred action to allow the current packet, >nbl, >+ * to re-enter datapath packet processing. >+ * >----------------------------------------------------------------------- >--- >+ */ >+NTSTATUS >+OvsActionExecuteRecirc(POVS_SWITCH_CONTEXT switchContext, >+ PNET_BUFFER_LIST nbl, >+ OvsFlowKey *key, >+ UINT64 *hashAction, >+ const PNL_ATTR actions, >+ int rem) >+{ >+ POVS_DEFERRED_ACTION deferredAction = NULL; >+ PNET_BUFFER_LIST newNbl = NULL; >+ UINT64 hash = 0; >+ >+ if (!NlAttrIsLast(actions, rem)) { >+ /* >+ * Recirc action is the not the last action of the action >+ list, >so we >+ * need to clone the packet. >+ */ >+ newNbl = OvsPartialCopyNBL(switchContext, nbl, >+ 0, 0, TRUE /*copy NBL info*/); >+ /* >+ * Skip the recirc action when out of memory, but continue on >with the >+ * rest of the action list. >+ */ >+ if (newNbl == NULL) { >+ ovsActionStats.noCopiedNbl++; >+ return STATUS_SUCCESS; >+ } >+ nbl = newNbl; >+ } >+ >+ hash = hashAction ? *hashAction : OvsHashFlow(key); >+ >+ deferredAction = OvsAddDeferredActions(nbl, key, hash, NULL); >+ if (deferredAction) { >+ deferredAction->key.recircId = NlAttrGetU32(actions); >+ } else { >+ if (newNbl) { >+ OvsCompleteNBL(switchContext, newNbl, TRUE); >+ } >+ } >+ >+ return STATUS_SUCCESS; >+} >+ >+/* >+ * >----------------------------------------------------------------------- >--- >+ * OvsDoExecuteActions -- > * Interpret and execute the specified 'actions' on the specifed >packet > * 'curNbl'. The expectation is that if the packet needs to be >dropped > * (completed) for some reason, it is added to 'completionList' so >that the >@@ -1537,16 +1590,16 @@ OvsExecuteSetAction(OvsForwardingContext >*ovsFwdCtx, > * >----------------------------------------------------------------------- >--- > */ > NDIS_STATUS >-OvsActionsExecute(POVS_SWITCH_CONTEXT switchContext, >- OvsCompletionList *completionList, >- PNET_BUFFER_LIST curNbl, >- UINT32 portNo, >- ULONG sendFlags, >- OvsFlowKey *key, >- UINT64 *hash, >- OVS_PACKET_HDR_INFO *layers, >- const PNL_ATTR actions, >- INT actionsLen) >+OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext, >+ OvsCompletionList *completionList, >+ PNET_BUFFER_LIST curNbl, >+ UINT32 portNo, >+ ULONG sendFlags, >+ OvsFlowKey *key, >+ UINT64 *hash, >+ OVS_PACKET_HDR_INFO *layers, >+ const PNL_ATTR actions, >+ INT actionsLen) > { > PNL_ATTR a; > INT rem; >@@ -1695,6 +1748,31 @@ OvsActionsExecute(POVS_SWITCH_CONTEXT >switchContext, > break; > } > >+ case OVS_ACTION_ATTR_RECIRC: >+ { >+ if (ovsFwdCtx.destPortsSizeOut > 0 || >+ ovsFwdCtx.tunnelTxNic >!= NULL >+ || ovsFwdCtx.tunnelRxNic != NULL) { >+ status = OvsOutputBeforeSetAction(&ovsFwdCtx); >+ if (status != NDIS_STATUS_SUCCESS) { >+ dropReason = L"OVS-adding destination failed"; >+ goto dropit; >+ } >+ } >+ >+ status = OvsActionExecuteRecirc(ovsFwdCtx.switchContext, >+ ovsFwdCtx.curNbl, key, hash, >+ (const PNL_ATTR)a, rem); >+ if (status != NDIS_STATUS_SUCCESS) { >+ dropReason = L"OVS-recirculation action failed"; >+ goto dropit; >+ } >+ >+ if (NlAttrIsLast(a, rem)) { >+ goto exit; >+ } >+ break; >+ } >+ > case OVS_ACTION_ATTR_USERSPACE: > { > PNL_ATTR userdataAttr; >@@ -1781,5 +1859,92 @@ dropit: > OvsCompleteNBLForwardingCtx(&ovsFwdCtx, dropReason); > } > >+exit: >+ return status; >+} >+ >+NDIS_STATUS >+OvsActionsExecute(POVS_SWITCH_CONTEXT switchContext, >+ OvsCompletionList *completionList, >+ PNET_BUFFER_LIST curNbl, >+ UINT32 portNo, >+ ULONG sendFlags, >+ OvsFlowKey *key, >+ UINT64 *hash, >+ OVS_PACKET_HDR_INFO *layers, >+ const PNL_ATTR actions, >+ INT actionsLen) >+{ >+ NDIS_STATUS status = STATUS_SUCCESS; >+ >+ status = OvsDoExecuteActions(switchContext, completionList, curNbl, >+ portNo, sendFlags, key, hash, layers, >+ actions, actionsLen); >+ >+ if (status == STATUS_SUCCESS) { >+ status = OvsProcessDeferredActions(switchContext, completionList, >+ portNo, sendFlags, layers); >+ } >+ >+ return status; >+} >+ >+NDIS_STATUS >+OvsDoRecircFlowLookupOutput(POVS_SWITCH_CONTEXT switchContext, >+ OvsCompletionList *completionList, >+ PNET_BUFFER_LIST curNbl, >+ OvsFlowKey *key, >+ UINT64 *hash) { >+ NDIS_STATUS status = NDIS_STATUS_SUCCESS; >+ OvsFlow *flow = NULL; >+ OvsForwardingContext ovsFwdCtx = { 0 }; >+ POVS_VPORT_ENTRY internalVport = switchContext->internalVport; >+ >+ ASSERT(switchContext->internalVport); >+ ASSERT(internalVport->nicState == NdisSwitchNicStateConnected); >+ >+ OvsInitForwardingCtx(&ovsFwdCtx, switchContext, curNbl, >+ internalVport->portNo, 0, >+ >NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(curNbl), >+ completionList, NULL, TRUE); >+ >+ flow = OvsLookupFlowRecirc(&ovsFwdCtx.switchContext->datapath, >+ key, hash); >+ if (flow) { >+ OvsFlowUsed(flow, ovsFwdCtx.curNbl, &ovsFwdCtx.layers); >+ ovsFwdCtx.switchContext->datapath.hits++; >+ status = OvsActionsExecute(ovsFwdCtx.switchContext, >+ ovsFwdCtx.completionList, >ovsFwdCtx.curNbl, >+ ovsFwdCtx.srcVportNo, >ovsFwdCtx.sendFlags, >+ key, hash, &ovsFwdCtx.layers, >+ flow->actions, flow->actionsLen); >+ ovsFwdCtx.curNbl = NULL; >+ } else { >+ LIST_ENTRY missedPackets; >+ UINT32 num = 0; >+ ovsFwdCtx.switchContext->datapath.misses++; >+ InitializeListHead(&missedPackets); Sai - &ovsFwdCtx.layers is not initialized. Call OvsExtractFlow(curNbl, internalVport->portNo, key, &ovsFwdCtx.layers, NULL); SV: OK. >+ status = OvsCreateAndAddPackets(NULL, 0, OVS_PACKET_CMD_MISS, >+ internalVport, key,ovsFwdCtx.curNbl, >+ FALSE, &ovsFwdCtx.layers, >+ ovsFwdCtx.switchContext, &missedPackets, >+ &num); Sai - Can you fix the alignment of the arguments to keep it consistent? SV: Sure. >+ if (num) { >+ OvsQueuePackets(&missedPackets, num); >+ } >+ if (status == NDIS_STATUS_SUCCESS) { >+ /* Complete the packet since it was copied to user buffer. */ >+ OvsCompleteNBLForwardingCtx(&ovsFwdCtx, >+ L"OVS-Dropped since packet was copied to userspace"); >+ ovsActionStats.flowMiss++; >+ status = NDIS_STATUS_SUCCESS; >+ } else { >+ OvsCompleteNBLForwardingCtx(&ovsFwdCtx, >+ L"OVS-Dropped due to failure to queue to userspace"); >+ status = NDIS_STATUS_FAILURE; >+ ovsActionStats.failedFlowMiss++; >+ } >+ } >+ > return status; > } >diff --git a/datapath-windows/ovsext/Actions.h >b/datapath-windows/ovsext/Actions.h >new file mode 100644 >index 0000000..01ad8d2 >--- /dev/null >+++ b/datapath-windows/ovsext/Actions.h >@@ -0,0 +1,54 @@ >+/* >+ * Copyright (c) 2015 Cloudbase Solutions Srl >+ * >+ * 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. >+ */ >+ >+#ifndef __ACTIONS_H_ >+#define __ACTIONS_H_ 1 >+ >+#include "Switch.h" >+#include "PacketIO.h" >+ >+NDIS_STATUS >+OvsActionsExecute(POVS_SWITCH_CONTEXT switchContext, >+ OvsCompletionList *completionList, >+ PNET_BUFFER_LIST curNbl, >+ UINT32 srcVportNo, >+ ULONG sendFlags, >+ OvsFlowKey *key, >+ UINT64 *hash, >+ OVS_PACKET_HDR_INFO *layers, >+ const PNL_ATTR actions, >+ int actionsLen); >+ >+NDIS_STATUS >+OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext, >+ OvsCompletionList *completionList, >+ PNET_BUFFER_LIST curNbl, >+ UINT32 srcVportNo, >+ ULONG sendFlags, >+ OvsFlowKey *key, >+ UINT64 *hash, >+ OVS_PACKET_HDR_INFO *layers, >+ const PNL_ATTR actions, >+ int actionsLen); >+ >+NDIS_STATUS >+OvsDoRecircFlowLookupOutput(POVS_SWITCH_CONTEXT switchContext, >+ OvsCompletionList *completionList, >+ PNET_BUFFER_LIST curNbl, >+ OvsFlowKey *key, >+ UINT64 *hash); >+ >+#endif /* __ACTIONS_H_ */ >diff --git a/datapath-windows/ovsext/Datapath.c >b/datapath-windows/ovsext/Datapath.c >index a9a218d..9e7d6a4 100644 >--- a/datapath-windows/ovsext/Datapath.c >+++ b/datapath-windows/ovsext/Datapath.c >@@ -346,6 +346,8 @@ extern POVS_SWITCH_CONTEXT gOvsSwitchContext; >NDIS_SPIN_LOCK ovsCtrlLockObj; PNDIS_SPIN_LOCK gOvsCtrlLock; > >+NDIS_SPIN_LOCK ovsRecircLockObj; >+ > NTSTATUS > InitUserDumpState(POVS_OPEN_INSTANCE instance, > POVS_MESSAGE ovsMsg) @@ -383,6 +385,7 @@ OvsInit() >{ > gOvsCtrlLock = &ovsCtrlLockObj; > NdisAllocateSpinLock(gOvsCtrlLock); >+ NdisAllocateSpinLock(&ovsRecircLockObj); > OvsInitEventQueue(); > } > >@@ -394,6 +397,7 @@ OvsCleanup() > NdisFreeSpinLock(gOvsCtrlLock); > gOvsCtrlLock = NULL; > } >+ NdisFreeSpinLock(&ovsRecircLockObj); > } > > VOID >diff --git a/datapath-windows/ovsext/DpInternal.h >b/datapath-windows/ovsext/DpInternal.h >index c094f32..845c132 100644 >--- a/datapath-windows/ovsext/DpInternal.h >+++ b/datapath-windows/ovsext/DpInternal.h >@@ -20,7 +20,6 @@ > #include <netioapi.h> > #define IFNAMSIZ IF_NAMESIZE > #include "../ovsext/Netlink/Netlink.h" >-#include "Mpls.h" > > #define OVS_DP_NUMBER ((uint32_t) 0) > >@@ -166,6 +165,7 @@ typedef __declspec(align(8)) struct OvsFlowKey { > Icmp6Key icmp6Key; /* size 72 */ > MplsKey mplsKey; /* size 8 */ > }; >+ UINT32 recircId; /* Recirculation ID. */ > } OvsFlowKey; > > #define OVS_WIN_TUNNEL_KEY_SIZE (sizeof (OvsIPv4TunnelKey)) diff --git >a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c index >5eec513..baf8422 100644 >--- a/datapath-windows/ovsext/Flow.c >+++ b/datapath-windows/ovsext/Flow.c >@@ -110,9 +110,7 @@ const NL_POLICY nlFlowPolicy[] = { > [OVS_FLOW_ATTR_PROBE] = {.type = NL_A_FLAG, .optional = TRUE} }; > >-/* For Parsing nested OVS_FLOW_ATTR_KEY attributes. >- * Some of the attributes like OVS_KEY_ATTR_RECIRC_ID >- * are not supported yet. */ >+/* For Parsing nested OVS_FLOW_ATTR_KEY attributes. */ > > const NL_POLICY nlFlowKeyPolicy[] = { > [OVS_KEY_ATTR_ENCAP] = {.type = NL_A_VAR_LEN, .optional = TRUE}, >@@ -252,7 +250,7 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT >usrParamsCtx, > UINT32 *replyLen) > { > NTSTATUS rc = STATUS_SUCCESS; >- BOOLEAN ok; >+ BOOLEAN ok = FALSE; > POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer; > POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx->outputBuffer; > PNL_MSG_HDR nlMsgHdr = &(msgIn->nlMsg); @@ -260,11 +258,11 @@ >OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT >usrParamsCtx, > POVS_HDR ovsHdr = &(msgIn->ovsHdr); > PNL_ATTR flowAttrs[__OVS_FLOW_ATTR_MAX]; > UINT32 attrOffset = NLMSG_HDRLEN + GENL_HDRLEN + OVS_HDRLEN; >- OvsFlowPut mappedFlow; >- OvsFlowStats stats; >- struct ovs_flow_stats replyStats; >+ OvsFlowPut mappedFlow = { 0 }; >+ OvsFlowStats stats = { 0 }; >+ struct ovs_flow_stats replyStats = { 0 }; > NL_ERROR nlError = NL_ERROR_SUCCESS; >- NL_BUFFER nlBuf; >+ NL_BUFFER nlBuf = { 0 }; > > RtlZeroMemory(&mappedFlow, sizeof(OvsFlowPut)); > RtlZeroMemory(&stats, sizeof(stats)); @@ -594,9 +592,9 @@ >_FlowNlDumpCmdHandler(POVS_USER_PARAMS_CONTEXT >usrParamsCtx, > UINT32 hdrOffset = 0; > > /* Get Next */ >- OvsFlowDumpOutput dumpOutput; >- OvsFlowDumpInput dumpInput; >- NL_BUFFER nlBuf; >+ OvsFlowDumpOutput dumpOutput = { 0 }; >+ OvsFlowDumpInput dumpInput = { 0 }; >+ NL_BUFFER nlBuf = { 0 }; > > NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, > usrParamsCtx->outputLength); @@ -826,6 +824,12 @@ >MapFlowKeyToNlKey(PNL_BUFFER nlBuf, > goto error_nested_start; > } > >+ if (!NlMsgPutTailU32(nlBuf, OVS_KEY_ATTR_RECIRC_ID, >+ flowKey->recircId)) { >+ rc = STATUS_UNSUCCESSFUL; >+ goto done; >+ } >+ > /* Ethernet header */ > RtlCopyMemory(&(ethKey.eth_src), flowKey->l2.dlSrc, ETH_ADDR_LEN); > RtlCopyMemory(&(ethKey.eth_dst), flowKey->l2.dlDst, ETH_ADDR_LEN); >@@ -1348,6 +1352,10 @@ _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs, { > _MapTunAttrToFlowPut(keyAttrs, tunnelAttrs, destKey); > >+ if (keyAttrs[OVS_KEY_ATTR_RECIRC_ID]) { >+ destKey->recircId = >NlAttrGetU32(keyAttrs[OVS_KEY_ATTR_RECIRC_ID]); >+ } >+ > /* ===== L2 headers ===== */ > destKey->l2.inPort = NlAttrGetU32(keyAttrs[OVS_KEY_ATTR_IN_PORT]); > >@@ -2072,6 +2080,9 @@ OvsLookupFlow(OVS_DATAPATH *datapath, > > if (!hashValid) { > *hash = OvsJhashBytes(start, size, 0); >+ if (key->recircId) { >+ *hash = OvsJhashWords((UINT32*)hash, 1, key->recircId); >+ } > } > > head = &datapath->flowTable[HASH_BUCKET(*hash)]; >@@ -2090,6 +2101,47 @@ OvsLookupFlow(OVS_DATAPATH *datapath, > return NULL; > } > >+/* >+ * >----------------------------------------------------------------------- >--- >-- >+ * OvsLookupFlowRecirc -- >+ * >+ * Find flow from flow table based on flow key and recircId, if >available. >+ * Caller should either hold portset handle or should >+ * have a flowRef in datapath or Acquired datapath. >+ * >+ * Results: >+ * Flow pointer if lookup successful. >+ * NULL if not exists. >+ * >----------------------------------------------------------------------- >--- >-- >+ */ >+OvsFlow * >+OvsLookupFlowRecirc(OVS_DATAPATH *datapath, >+ const OvsFlowKey *key, >+ UINT64 *hash) >+{ >+ OvsFlow* flow = NULL; >+ >+ if (!hash || !(*hash)) { >+ flow = OvsLookupFlow(datapath, key, hash, FALSE); >+ } else { >+ /* >+ * Pre and post recirculation flows usually have the same hash >+ * value. To avoid hash collisions, rehash the 'hash' with >+ * 'recircId'. >+ */ >+ if (key->recircId) { >+ *hash = OvsJhashWords((UINT32*)hash, 1, key->recircId); >+ } >+ >+ flow = OvsLookupFlow(datapath, key, hash, FALSE); >+ if (flow) { >+ flow->hash = *hash; >+ } >+ } >+ >+ return flow; >+} >+ > > /* > * >----------------------------------------------------------------------- >--- >-- >@@ -2239,6 +2291,8 @@ ReportFlowInfo(OvsFlow *flow, > } > } > >+ info->key.recircId = flow->key.recircId; >+ > return status; > } > >diff --git a/datapath-windows/ovsext/Flow.h >b/datapath-windows/ovsext/Flow.h index 74b9dfb..78bf7cc 100644 >--- a/datapath-windows/ovsext/Flow.h >+++ b/datapath-windows/ovsext/Flow.h >@@ -54,8 +54,11 @@ NDIS_STATUS OvsAllocateFlowTable(OVS_DATAPATH >*datapath, NDIS_STATUS OvsExtractFlow(const NET_BUFFER_LIST *pkt, >UINT32 inPort, > OvsFlowKey *flow, POVS_PACKET_HDR_INFO layers, > OvsIPv4TunnelKey *tunKey); -OvsFlow >*OvsLookupFlow(OVS_DATAPATH *datapath, const OvsFlowKey *key, >+OvsFlow* OvsLookupFlow(OVS_DATAPATH *datapath, const OvsFlowKey *key, > UINT64 *hash, BOOLEAN hashValid); >+OvsFlow* OvsLookupFlowRecirc(OVS_DATAPATH *datapath, >+ const OvsFlowKey *key, >+ UINT64 *hash); > UINT64 OvsHashFlow(const OvsFlowKey *key); VOID OvsFlowUsed(OvsFlow >*flow, const NET_BUFFER_LIST *pkt, > const POVS_PACKET_HDR_INFO layers); diff --git >a/datapath-windows/ovsext/Netlink/Netlink.h >b/datapath-windows/ovsext/Netlink/Netlink.h >index 99665fb..8f6a5be 100644 >--- a/datapath-windows/ovsext/Netlink/Netlink.h >+++ b/datapath-windows/ovsext/Netlink/Netlink.h >@@ -173,6 +173,17 @@ static __inline NlAttrTotalSize(UINT32 payloadSize) > return NLA_ALIGN(NlAttrSize(payloadSize)); > } > >+/* >+ * >----------------------------------------------------------------------- >--- >- >+ * Returns true if the last attribute is reached. >+ * >----------------------------------------------------------------------- >--- >- >+ */ >+BOOLEAN >+static __inline NlAttrIsLast(const PNL_ATTR nla, int rem) { >+ return nla->nlaLen == rem; >+} >+ > /* Netlink attribute validation */ > BOOLEAN NlAttrValidate(const PNL_ATTR, const PNL_POLICY); > >diff --git a/datapath-windows/ovsext/PacketIO.c >b/datapath-windows/ovsext/PacketIO.c >index cfbae34..a0ddc3d 100644 >--- a/datapath-windows/ovsext/PacketIO.c >+++ b/datapath-windows/ovsext/PacketIO.c >@@ -20,6 +20,8 @@ > */ > > #include "precomp.h" >+ >+#include "Actions.h" > #include "Switch.h" > #include "Vport.h" > #include "NetProto.h" >@@ -234,14 +236,14 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT >switchContext, > OvsInitCompletionList(&completionList, switchContext, >sendCompleteFlags); > > for (curNbl = netBufferLists; curNbl != NULL; curNbl = nextNbl) { >- POVS_VPORT_ENTRY vport; >- UINT32 portNo; >+ POVS_VPORT_ENTRY vport = NULL; >+ UINT32 portNo = 0; > OVS_DATAPATH *datapath = &switchContext->datapath; >- OVS_PACKET_HDR_INFO layers; >- OvsFlowKey key; >- UINT64 hash; >- PNET_BUFFER curNb; >- POVS_BUFFER_CONTEXT ctx; >+ OVS_PACKET_HDR_INFO layers = { 0 }; >+ OvsFlowKey key = { 0 }; >+ UINT64 hash = 0; >+ PNET_BUFFER curNb = NULL; >+ POVS_BUFFER_CONTEXT ctx = NULL; > > nextNbl = curNbl->Next; > curNbl->Next = NULL; >diff --git a/datapath-windows/ovsext/PacketIO.h >b/datapath-windows/ovsext/PacketIO.h >index 7247869..a7c1f76 100644 >--- a/datapath-windows/ovsext/PacketIO.h >+++ b/datapath-windows/ovsext/PacketIO.h >@@ -48,14 +48,4 @@ VOID OvsSendNBLIngress(POVS_SWITCH_CONTEXT >switchContext, > PNET_BUFFER_LIST netBufferLists, > ULONG sendFlags); > >-NDIS_STATUS OvsActionsExecute(POVS_SWITCH_CONTEXT switchContext, >- OvsCompletionList *completionList, >- PNET_BUFFER_LIST curNbl, UINT32 srcVportNo, >- ULONG sendFlags, OvsFlowKey *key, UINT64 >*hash, >- OVS_PACKET_HDR_INFO *layers, >- const PNL_ATTR actions, int actionsLen); >- >-VOID OvsLookupFlowOutput(POVS_SWITCH_CONTEXT switchContext, >- VOID *compList, PNET_BUFFER_LIST curNbl); >- > #endif /* __PACKETIO_H_ */ >diff --git a/datapath-windows/ovsext/Recirc.c >b/datapath-windows/ovsext/Recirc.c >new file mode 100644 >index 0000000..fa8db80 >--- /dev/null >+++ b/datapath-windows/ovsext/Recirc.c >@@ -0,0 +1,171 @@ >+/* >+ * Copyright (c) 2015 Cloudbase Solutions Srl >+ * >+ * 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 "Recirc.h" >+#include "Flow.h" >+#include "Jhash.h" >+ >+static OVS_DEFERRED_ACTION_QUEUE ovsDeferredActionsQueue = { 0 }; >+extern NDIS_SPIN_LOCK ovsRecircLockObj; >+ >+/* >+ * >----------------------------------------------------------------------- >--- >+ * OvsDeferredActionsQueueInit -- >+ * The function resets the queue to be ready for the next packet. >+ * >----------------------------------------------------------------------- >--- >+ */ >+static >+VOID >+OvsDeferredActionsQueueInit(POVS_DEFERRED_ACTION_QUEUE queue) { >+ queue->head = 0; >+ queue->tail = 0; >+} >+ >+/* >+ * >----------------------------------------------------------------------- >--- >+ * OvsDeferredActionsQueueIsEmpty -- >+ * The function verifies if the queue is empty. >+ * >----------------------------------------------------------------------- >--- >+ */ >+static >+BOOLEAN >+OvsDeferredActionsQueueIsEmpty(const POVS_DEFERRED_ACTION_QUEUE queue) >+{ Sai - This doesn¹t seem thread safe since OvsDeferredActionsQueueIsEmpty is called outside the lock SV: OvsDeferredActionsQueueIsEmpty is an internal function and is used only inside of a lock. >+ return (queue->head == queue->tail); } >+ >+/* >+ * >----------------------------------------------------------------------- >--- >+ * OvsDeferredActionsQueuePop -- >+ * The function pops the next queue element. >+ * >----------------------------------------------------------------------- >--- >+ */ >+static >+POVS_DEFERRED_ACTION >+OvsDeferredActionsQueuePop(POVS_DEFERRED_ACTION_QUEUE queue) { >+ POVS_DEFERRED_ACTION deferredAction = NULL; >+ >+ NdisAcquireSpinLock(&ovsRecircLockObj); >+ >+ if (OvsDeferredActionsQueueIsEmpty(queue)) { >+ /* Reset the queue for the next packet. */ >+ OvsDeferredActionsQueueInit(queue); >+ } else { >+ deferredAction = &queue->queue[queue->tail++]; >+ } >+ >+ NdisReleaseSpinLock(&ovsRecircLockObj); >+ >+ return deferredAction; >+} >+ >+/* >+ * >----------------------------------------------------------------------- >--- >+ * OvsDeferredActionsQueuePush -- >+ * The function pushes the current element in the deferred actions >queue. >+ * >----------------------------------------------------------------------- >--- >+ */ >+static >+POVS_DEFERRED_ACTION >+OvsDeferredActionsQueuePush(POVS_DEFERRED_ACTION_QUEUE queue) { >+ POVS_DEFERRED_ACTION deferredAction = NULL; >+ >+ NdisAcquireSpinLock(&ovsRecircLockObj); >+ >+ if (queue->head < DEFERRED_ACTION_QUEUE_SIZE) { >+ deferredAction = &queue->queue[queue->head++]; >+ } >+ >+ NdisReleaseSpinLock(&ovsRecircLockObj); >+ >+ return deferredAction; >+} >+ >+/* >+ * >----------------------------------------------------------------------- >--- >+ * OvsAddDeferredActions -- >+ * This function returns the new queue entry if the queue is not >already >+ * full. >+ * >+ * Note: >+ * The function is thread-safe. >+ * >----------------------------------------------------------------------- >--- >+ */ >+POVS_DEFERRED_ACTION >+OvsAddDeferredActions(PNET_BUFFER_LIST packet, >+ OvsFlowKey *key, >+ UINT64 hash, >+ const PNL_ATTR actions) { >+ POVS_DEFERRED_ACTION deferredAction = NULL; >+ >+ deferredAction = >OvsDeferredActionsQueuePush(&ovsDeferredActionsQueue); >+ if (deferredAction) { >+ deferredAction->packet = packet; >+ deferredAction->actions = actions; >+ deferredAction->key = *key; >+ deferredAction->hash = hash; >+ } >+ >+ return deferredAction; >+} >+ >+/* >+ * >----------------------------------------------------------------------- >--- >+ * OvsProcessDeferredActions -- >+ * This function processes all deferred actions contained in the >queue. >+ * >+ * Note: >+ * The function is thread-safe. >+ * >----------------------------------------------------------------------- >--- >+ */ >+NTSTATUS >+OvsProcessDeferredActions(POVS_SWITCH_CONTEXT switchContext, >+ OvsCompletionList *completionList, >+ UINT32 portNo, >+ ULONG sendFlags, >+ OVS_PACKET_HDR_INFO *layers) { >+ NTSTATUS status = STATUS_SUCCESS; >+ POVS_DEFERRED_ACTION_QUEUE queue = &ovsDeferredActionsQueue; >+ POVS_DEFERRED_ACTION deferredAction = NULL; >+ >+ /* Process all deferred actions. */ >+ while ((deferredAction = OvsDeferredActionsQueuePop(queue)) != >+ NULL) >{ >+ if (deferredAction->actions) { >+ status = OvsDoExecuteActions(switchContext, >+ completionList, >+ deferredAction->packet, >+ portNo, >+ sendFlags, >+ &deferredAction->key, >+ &deferredAction->hash, >+ layers, >+ deferredAction->actions, >+ >NlAttrGetSize(deferredAction->actions)); >+ } else { >+ status = OvsDoRecircFlowLookupOutput(switchContext, >+ completionList, >+ deferredAction->packet, >+ &deferredAction->key, >+ &deferredAction->hash); >+ } >+ } >+ >+ return status; >+} >diff --git a/datapath-windows/ovsext/Recirc.h >b/datapath-windows/ovsext/Recirc.h >new file mode 100644 >index 0000000..6b5653c >--- /dev/null >+++ b/datapath-windows/ovsext/Recirc.h >@@ -0,0 +1,82 @@ >+/* >+ * Copyright (c) 2015 Cloudbase Solutions Srl >+ * >+ * 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. >+ */ >+ >+#ifndef __RECIRC_H_ >+#define __RECIRC_H_ 1 >+ >+#include "Actions.h" >+ >+#define DEFERRED_ACTION_QUEUE_SIZE 10 >+ >+typedef struct _OVS_DEFERRED_ACTION { >+ PNET_BUFFER_LIST packet; >+ PNL_ATTR actions; >+ OvsFlowKey key; >+ UINT64 hash; >+} OVS_DEFERRED_ACTION, *POVS_DEFERRED_ACTION; >+ >+/* >+ * >----------------------------------------------------------------------- >--- >+ * '_OVS_DEFERRED_ACTION_QUEUE' is responsible for keeping track of >+ all >+ * deferred actions. The maximum number of deferred actions should not >exceed >+ * 'DEFERRED_ACTION_QUEUE_SIZE'. >+ * >+ * Note: >+ * Adding/removing a deferred action to/from the queue is only safe >while >+ * holding a proper lock. >+ * >----------------------------------------------------------------------- >--- >+ */ >+typedef struct _OVS_DEFERRED_ACTION_QUEUE { >+ UINT32 head; >+ UINT32 tail; >+ OVS_DEFERRED_ACTION queue[DEFERRED_ACTION_QUEUE_SIZE]; >+} OVS_DEFERRED_ACTION_QUEUE, *POVS_DEFERRED_ACTION_QUEUE; >+ >+/* >+ * >----------------------------------------------------------------------- >--- >+ * OvsProcessDeferredActions -- >+ * This function processes all deferred actions contained in the >queue. >+ * >+ * Note: >+ * The function is not thread-safe. It is the resposibility of the >caller >+ * to hold a proper lock. >+ * >----------------------------------------------------------------------- >--- >+ */ >+NTSTATUS >+OvsProcessDeferredActions(POVS_SWITCH_CONTEXT switchContext, >+ OvsCompletionList *completionList, >+ UINT32 portNo, >+ ULONG sendFlags, >+ OVS_PACKET_HDR_INFO *layers); >+ >+/* >+ * >----------------------------------------------------------------------- >--- >+ * OvsAddDeferredActions -- >+ * This function returns the new queue entry if the queue is not >already >+ * full. >+ * >+ * Note: >+ * The function is not thread-safe. It is the resposibility of the >caller >+ * to hold a proper lock. >+ * >----------------------------------------------------------------------- >--- >+ */ >+POVS_DEFERRED_ACTION >+OvsAddDeferredActions(PNET_BUFFER_LIST packet, >+ OvsFlowKey *key, >+ UINT64 hash, >+ const PNL_ATTR actions); >+ >+#endif /* __RECIRC_H_ */ >diff --git a/datapath-windows/ovsext/Tunnel.c >b/datapath-windows/ovsext/Tunnel.c >index eea4a84..e957aaf 100644 >--- a/datapath-windows/ovsext/Tunnel.c >+++ b/datapath-windows/ovsext/Tunnel.c >@@ -39,6 +39,7 @@ > #include "PacketIO.h" > #include "NetProto.h" > #include "Flow.h" >+#include "Actions.h" > > extern POVS_SWITCH_CONTEXT gOvsSwitchContext; > >@@ -258,13 +259,13 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl, > sendCompleteFlags); > > { >- POVS_VPORT_ENTRY vport; >- UINT32 portNo; >- OVS_PACKET_HDR_INFO layers; >- OvsFlowKey key; >- UINT64 hash; >- PNET_BUFFER curNb; >- OvsFlow *flow; >+ POVS_VPORT_ENTRY vport = NULL; >+ UINT32 portNo = 0; >+ OVS_PACKET_HDR_INFO layers = { 0 }; >+ OvsFlowKey key = { 0 }; >+ UINT64 hash = 0; >+ PNET_BUFFER curNb = NULL; >+ OvsFlow *flow = NULL; > > fwdDetail = NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(pNbl); > >diff --git a/datapath-windows/ovsext/User.c >b/datapath-windows/ovsext/User.c index e97f2b2..cadffda 100644 >--- a/datapath-windows/ovsext/User.c >+++ b/datapath-windows/ovsext/User.c >@@ -22,6 +22,7 @@ > > #include "precomp.h" > >+#include "Actions.h" > #include "Datapath.h" > #include "Debug.h" > #include "Event.h" >@@ -388,14 +389,14 @@ NTSTATUS > OvsExecuteDpIoctl(OvsPacketExecute *execute) { > NTSTATUS status = STATUS_SUCCESS; >- NTSTATUS ndisStatus; >+ NTSTATUS ndisStatus = STATUS_SUCCESS; > LOCK_STATE_EX lockState; >- PNET_BUFFER_LIST pNbl; >- PNL_ATTR actions; >+ PNET_BUFFER_LIST pNbl = NULL; >+ PNL_ATTR actions = NULL; > PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail; >- OvsFlowKey key; >- OVS_PACKET_HDR_INFO layers; >- POVS_VPORT_ENTRY vport; >+ OvsFlowKey key = { 0 }; >+ OVS_PACKET_HDR_INFO layers = { 0 }; >+ POVS_VPORT_ENTRY vport = NULL; > > if (execute->packetLen == 0) { > status = STATUS_INVALID_PARAMETER; diff --git >a/datapath-windows/ovsext/ovsext.vcxproj >b/datapath-windows/ovsext/ovsext.vcxproj >index 7b0ebcf..36187a6 100644 >--- a/datapath-windows/ovsext/ovsext.vcxproj >+++ b/datapath-windows/ovsext/ovsext.vcxproj >@@ -71,6 +71,7 @@ > </ImportGroup> > <ItemGroup Label="WrappedTaskItems"> > <ClInclude Include="..\include\OvsDpInterfaceExt.h" /> >+ <ClInclude Include="Actions.h" /> > <ClInclude Include="Atomic.h" /> > <ClInclude Include="BufferMgmt.h" /> > <ClInclude Include="Datapath.h" /> @@ -93,6 +94,7 @@ > <ClInclude Include="PacketIO.h" /> > <ClInclude Include="PacketParser.h" /> > <ClInclude Include="precomp.h" /> >+ <ClInclude Include="Recirc.h" /> > <ClInclude Include="resource.h" /> > <ClInclude Include="Stt.h" /> > <ClInclude Include="Switch.h" /> >@@ -189,6 +191,7 @@ > <PreCompiledHeader>Create</PreCompiledHeader> > ><PreCompiledHeaderOutputFile>$(IntDir)\precomp.h.pch</PreCompiledHeader >Out >putFile> > </ClCompile> >+ <ClCompile Include="Recirc.c" /> > <ClCompile Include="Stt.c" /> > <ClCompile Include="Switch.c" /> > <ClCompile Include="Tunnel.c" /> >-- >1.9.0.msysgit.0 > >_______________________________________________ >dev mailing list >dev@openvswitch.org >http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
