Hi Sorin/Alin, I looked at the patch and had a few comments. Pls. find them inlined.
In general, I was a little confused about how we use the recirc ID and the hash. We can discuss more during the IRC meeting. Thanks, -- Nithin -----Original Message----- From: dev <dev-boun...@openvswitch.org> on behalf of Sorin Vinturis <svintu...@cloudbasesolutions.com> Date: Friday, March 18, 2016 at 7:58 AM To: "dev@openvswitch.org" <dev@openvswitch.org> Subject: [ovs-dev] [PATCH v6 2/6] datapath-windows: Added recirculation support. >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://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitc >h_ovs-2Dissues_issues_104&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNt >Xt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=OpMpvAAJDApIpx1rELD >Zz_PzL7S9hfkeHD2h-6_KTCg&s=oZxN8oCzUixc4GlkvE6n4m0RN2gf2XiL0OI-sVgXdus&e= >--- >v2: Initialize flow key before using it. >v3: Synchronized access to deferred actions queue. >v4: Address comments. >v5: update flow key and layers in the recirc action, queue/execute packet > per source port >v6: Added percpu deferred action queues. >--- >--- > datapath-windows/automake.mk | 3 + > datapath-windows/ovsext/Actions.c | 234 >+++++++++++++++++++++++++++--- > datapath-windows/ovsext/Actions.h | 56 +++++++ > datapath-windows/ovsext/Datapath.c | 3 + > datapath-windows/ovsext/DpInternal.h | 2 +- > datapath-windows/ovsext/Flow.c | 78 ++++++++-- > 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 | 224 >++++++++++++++++++++++++++++ > datapath-windows/ovsext/Recirc.h | 90 ++++++++++++ > datapath-windows/ovsext/Tunnel.c | 15 +- > datapath-windows/ovsext/User.c | 13 +- > datapath-windows/ovsext/Util.h | 1 + > datapath-windows/ovsext/ovsext.vcxproj | 3 + > 16 files changed, 696 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..333d487 100644 >--- a/datapath-windows/ovsext/Actions.c >+++ b/datapath-windows/ovsext/Actions.c >@@ -16,6 +16,7 @@ > > #include "precomp.h" > >+#include "Actions.h" > #include "Debug.h" > #include "Event.h" > #include "Flow.h" >@@ -24,6 +25,7 @@ > #include "NetProto.h" > #include "Offload.h" > #include "PacketIO.h" >+#include "Recirc.h" > #include "Stt.h" > #include "Switch.h" > #include "User.h" >@@ -35,6 +37,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 +70,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 +102,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 +120,6 @@ typedef struct OvsForwardingContext { > OVS_PACKET_HDR_INFO layers; > } OvsForwardingContext; > >- > /* > * >-------------------------------------------------------------------------- > * OvsInitForwardingCtx -- >@@ -564,10 +566,10 @@ OvsCompleteNBLForwardingCtx(OvsForwardingContext >*ovsFwdCtx, > static __inline NDIS_STATUS > OvsDoFlowLookupOutput(OvsForwardingContext *ovsFwdCtx) > { >- OvsFlowKey key; >- OvsFlow *flow; >- UINT64 hash; >- NDIS_STATUS status; Unnecessary initialization. >+ 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 +597,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,8 +1524,58 @@ OvsExecuteSetAction(OvsForwardingContext >*ovsFwdCtx, > > /* > * >-------------------------------------------------------------------------- >- * OvsActionsExecute -- >- * Interpret and execute the specified 'actions' on the specifed >packet >+ * OvsActionExecuteRecirc -- >+ * The function adds a deferred action to allow the current packet, >nbl, >+ * to re-enter datapath packet processing. >+ * >-------------------------------------------------------------------------- >+ */ Nit: OvsActionExecuteRecirc => OvsExecuteRecirc would probably be simpler and clearer name. Eg. OvsExecuteSetAction(): it executes the set action. >+NDIS_STATUS >+OvsActionExecuteRecirc(OvsFlowKey *key, >+ UINT64 *hashAction, >+ const PNL_ATTR actions, >+ int rem, >+ OvsForwardingContext ovsFwdCtx) There does not seem to be a good reason to pass ovsFwdCtx as a structure and not a pointer. >+{ >+ POVS_DEFERRED_ACTION deferredAction = NULL; >+ PNET_BUFFER_LIST newNbl = NULL; >+ UINT64 hash = 0; Nit: initialization not necessary. >+ >+ 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(ovsFwdCtx.switchContext, >ovsFwdCtx.curNbl, >+ 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 NDIS_STATUS_SUCCESS; >+ } >+ ovsFwdCtx.curNbl = newNbl; If you pass ŒovsFwdCtx' by pointer, you¹ll have to make sure that ovsFwdCxt->curNbl does not get updated. >+ } >+ >+ hash = hashAction ? *hashAction : OvsHashFlow(key); >+ >+ deferredAction = OvsAddDeferredActions(ovsFwdCtx.curNbl, key, hash, >NULL); >+ if (deferredAction) { >+ deferredAction->key.recircId = NlAttrGetU32(actions); >+ } else { >+ if (newNbl) { As stat for this: ovsActionsStats.deferredActionsFull++? >+ OvsCompleteNBL(ovsFwdCtx.switchContext, newNbl, TRUE); >+ } >+ } >+ >+ return NDIS_STATUS_SUCCESS; >+} >+ >+/* >+ * >-------------------------------------------------------------------------- >+ * OvsDoExecuteActions -- >+ * Interpret and execute the specified 'actions' on the specified >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 > * caller can complete the packet. If 'completionList' is NULL, the >NBL is >@@ -1537,16 +1591,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 +1749,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(key, hash, >+ (const PNL_ATTR)a, rem, >+ ovsFwdCtx); Nit: OvsActionExecuteRecirc => OvsExecuteRecirc or OvsProcessRecirc() or OvsQueueRecirc() Pass ŒovsFwdCtx¹ by pointer. >+ if (status != NDIS_STATUS_SUCCESS) { >+ dropReason = L"OVS-recirculation action failed"; >+ goto dropit; >+ } OvsActionExecuteRecirc() is not really returning failure under any conditions. Do we want to update that to be able to handle errors here? >+ >+ if (NlAttrIsLast(a, rem)) { >+ goto exit; >+ } Checking for last attribute and bailing out is not necessary. If it is the last attribute, the outer while loop will break out anwyay. In fact, with this code, you¹ll end up causing an NBL leak if OvsActionExecuteRecirc() did not enqueue the packet. >+ break; >+ } >+ > case OVS_ACTION_ATTR_USERSPACE: > { > PNL_ATTR userdataAttr; >@@ -1781,5 +1860,112 @@ dropit: > OvsCompleteNBLForwardingCtx(&ovsFwdCtx, dropReason); > } > >+exit: >+ return status; >+} >+ >+NDIS_STATUS Nit: pls add a header. At least a namesake :) >+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, Why do we need Œkey¹ and Œhash¹? These two can be local variables within OvsDoRecircFlowLookupOutput() >+ UINT32 srcPortNo, >+ OVS_PACKET_HDR_INFO *layers) Nit: pls add a header. At least a namesake :) Also, the function could just be renamed to OvsDoRecirc(). >+{ >+ NDIS_STATUS status = NDIS_STATUS_SUCCESS; >+ OvsFlow *flow = NULL; >+ OvsForwardingContext ovsFwdCtx = { 0 }; >+ ASSERT(layers); >+ >+ OvsInitForwardingCtx(&ovsFwdCtx, switchContext, curNbl, >+ srcPortNo, 0, >+ >NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(curNbl), >+ completionList, layers, TRUE); XXX: Pls. check for failure and call OvsCompleteNBLForwardingCtx(). I am not really sure if we need a ŒovsFwdCtx¹ here. The whole idea behind ŒovsFwdCtx¹ was to have a ³context² structure that corresponds to a packet, and pass it around from one function to another. Since we are not passing ŒovsFwdCtx¹ to any other function, I am not sure it is needed. > >+ status = OvsExtractFlow(ovsFwdCtx.curNbl, ovsFwdCtx.srcVportNo, key, >+ &ovsFwdCtx.layers, NULL); >+ if (status != NDIS_STATUS_SUCCESS) { >+ OvsCompleteNBLForwardingCtx(&ovsFwdCtx, >+ L"OVS-Dropped due to extract flow failure"); >+ status = NDIS_STATUS_FAILURE; >+ ovsActionStats.failedFlowMiss++; => ovsActionStats.failedFlowExtract++; The recircID is not being populated in the key as part of OvsExtractFlow(). My understand is that the flows would look something like this: key-foo, actions:bar,recirc(0x10) recirc_id(0x10),key_foo actions:2 We are not passing down the recirc id from the first flow to the second flow. >+ } >+ >+ 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); Where is the check for recursion? Why not call OvsDoActionsExecute() directly? >+ ovsFwdCtx.curNbl = NULL; >+ } else { >+ POVS_VPORT_ENTRY vport = NULL; >+ LIST_ENTRY missedPackets; >+ UINT32 num = 0; >+ >+ ovsFwdCtx.switchContext->datapath.misses++; >+ InitializeListHead(&missedPackets); >+ vport = OvsFindVportByPortNo(switchContext, srcPortNo); >+ if (vport == NULL || vport->ovsState != OVS_STATE_CONNECTED) { >+ OvsCompleteNBLForwardingCtx(&ovsFwdCtx, >+ L"OVS-Dropped due to port removal"); >+ ovsActionStats.noVport++; >+ return NDIS_STATUS_SUCCESS; >+ } >+ status = OvsCreateAndAddPackets(NULL, 0, OVS_PACKET_CMD_MISS, >+ vport, key, ovsFwdCtx.curNbl, >+ srcPortNo == >+ >switchContext->virtualExternalPortId, >+ &ovsFwdCtx.layers, >+ ovsFwdCtx.switchContext, >+ &missedPackets, &num); >+ 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..69a1f69 >--- /dev/null >+++ b/datapath-windows/ovsext/Actions.h >@@ -0,0 +1,56 @@ >+/* >+ * Copyright (c) 2016 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: >+ * >+ * >https://urldefense.proofpoint.com/v2/url?u=http-3A__www.apache.org_license >s_LICENSE-2D2.0&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=p >NHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=OpMpvAAJDApIpx1rELDZz_PzL7S9h >fkeHD2h-6_KTCg&s=S30nm7dlEorfpeMZh2t0GL8LoSf_KXweQSeVsbNOCtg&e= >+ * >+ * 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); There¹s already a declaration of OvsActionsExecute() in PacketIO.h. Maybe we want to clean that up. >+ >+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, >+ UINT32 srcPortNo, >+ OVS_PACKET_HDR_INFO *layers); >+ >+#endif /* __ACTIONS_H_ */ >diff --git a/datapath-windows/ovsext/Datapath.c >b/datapath-windows/ovsext/Datapath.c >index a9a218d..876d6bf 100644 >--- a/datapath-windows/ovsext/Datapath.c >+++ b/datapath-windows/ovsext/Datapath.c >@@ -30,6 +30,7 @@ > #include "Event.h" > #include "User.h" > #include "PacketIO.h" >+#include "Recirc.h" > #include "NetProto.h" > #include "Flow.h" > #include "User.h" >@@ -383,6 +384,7 @@ OvsInit() > { > gOvsCtrlLock = &ovsCtrlLockObj; > NdisAllocateSpinLock(gOvsCtrlLock); >+ OvsDeferredActionsQueueAlloc(); > OvsInitEventQueue(); > } > >@@ -394,6 +396,7 @@ OvsCleanup() > NdisFreeSpinLock(gOvsCtrlLock); > gOvsCtrlLock = NULL; > } >+ OvsDeferredActionsQueueFree(); > } > > 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. */ Where is the keylen bring incremented to make sure that recircId is part of the hash calculation? > } 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..2ceeb94 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 }; Unnecessary initialization. > > 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); Last argument to OvsLookupFlow() is ŒhashValid¹. If it is FALSE, hash gets recalculated. What is the point of passing in hash? >+ if (flow) { >+ flow->hash = *hash; Why do we keep updating the hash? I see that ŒrecircId¹ has been added to OvsFlowKey. Where are we updating the key->l2.keyLen to indicate that recircId should also be added to the hash during lookup? Also, do we need this extra variable Œhash¹? Looks like it¹s value is moot. >+ } >+ } >+ >+ return flow; >+} >+ > > /* > * >-------------------------------------------------------------------------- >-- >@@ -2219,7 +2271,7 @@ ReportFlowInfo(OvsFlow *flow, > if (getFlags & FLOW_GET_KEY) { > // always copy the tunnel key part > RtlCopyMemory(&info->key, &flow->key, >- flow->key.l2.keyLen + flow->key.l2.offset); >+ flow->key.l2.keyLen + flow->key.l2.offset); > } > > if (getFlags & FLOW_GET_STATS) { >@@ -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; Unnecessary initialization. > > 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..56b6200 >--- /dev/null >+++ b/datapath-windows/ovsext/Recirc.c >@@ -0,0 +1,224 @@ >+/* >+ * Copyright (c) 2016 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: >+ * >+ * >https://urldefense.proofpoint.com/v2/url?u=http-3A__www.apache.org_license >s_LICENSE-2D2.0&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=p >NHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=OpMpvAAJDApIpx1rELDZz_PzL7S9h >fkeHD2h-6_KTCg&s=S30nm7dlEorfpeMZh2t0GL8LoSf_KXweQSeVsbNOCtg&e= >+ * >+ * 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 POVS_DEFERRED_ACTION_QUEUE ovsDeferredActionQueues = NULL; >+ >+BOOLEAN >+OvsDeferredActionsQueueAlloc() >+{ >+ ovsDeferredActionQueues = >+ OvsAllocateMemoryPerCpu(sizeof(OVS_DEFERRED_ACTION_QUEUE), >+ OVS_RECIRC_POOL_TAG); >+ if (!ovsDeferredActionQueues) { >+ return FALSE; >+ } >+ return TRUE; >+} >+ >+VOID >+OvsDeferredActionsQueueFree() >+{ >+ OvsFreeMemoryWithTag(ovsDeferredActionQueues, >+ OVS_RECIRC_POOL_TAG); >+ ovsDeferredActionQueues = NULL; >+} >+ >+POVS_DEFERRED_ACTION_QUEUE >+OvsDeferredActionsQueueGet() >+{ >+ POVS_DEFERRED_ACTION_QUEUE queue = NULL; >+ ULONG index = 0; >+ KIRQL oldIrql = KeGetCurrentIrql(); >+ >+ if (oldIrql < DISPATCH_LEVEL) { >+ KeRaiseIrqlToDpcLevel(); >+ } >+ >+ index = KeGetCurrentProcessorNumber(); >+ queue = &ovsDeferredActionQueues[index]; >+ >+ if (oldIrql < DISPATCH_LEVEL) { >+ KeLowerIrql(oldIrql); >+ } >+ >+ return queue; >+} >+ >+/* >+ * >-------------------------------------------------------------------------- >+ * 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) >+{ >+ 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; >+ KIRQL oldIrql = KeGetCurrentIrql(); >+ >+ if (oldIrql < DISPATCH_LEVEL) { >+ KeRaiseIrqlToDpcLevel(); >+ } >+ >+ if (OvsDeferredActionsQueueIsEmpty(queue)) { >+ /* Reset the queue for the next packet. */ >+ OvsDeferredActionsQueueInit(queue); >+ } else { >+ deferredAction = &queue->queue[queue->tail++]; >+ } >+ >+ if (oldIrql < DISPATCH_LEVEL) { >+ KeLowerIrql(oldIrql); >+ } >+ >+ 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; >+ KIRQL oldIrql = KeGetCurrentIrql(); >+ >+ if (oldIrql < DISPATCH_LEVEL) { >+ KeRaiseIrqlToDpcLevel(); >+ } >+ >+ if (queue->head < DEFERRED_ACTION_QUEUE_SIZE) { >+ deferredAction = &queue->queue[queue->head++]; >+ } >+ >+ if (oldIrql < DISPATCH_LEVEL) { >+ KeLowerIrql(oldIrql); >+ } >+ >+ 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_QUEUE queue = OvsDeferredActionsQueueGet(); >+ POVS_DEFERRED_ACTION deferredAction = NULL; >+ >+ deferredAction = OvsDeferredActionsQueuePush(queue); >+ 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. >+ * >-------------------------------------------------------------------------- >+ */ >+NDIS_STATUS >+OvsProcessDeferredActions(POVS_SWITCH_CONTEXT switchContext, >+ OvsCompletionList *completionList, >+ UINT32 portNo, >+ ULONG sendFlags, >+ OVS_PACKET_HDR_INFO *layers) >+{ >+ NDIS_STATUS status = NDIS_STATUS_SUCCESS; >+ POVS_DEFERRED_ACTION_QUEUE queue = OvsDeferredActionsQueueGet(); >+ 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, >+ portNo, >+ layers); >+ } >+ } >+ >+ return status; >+} >diff --git a/datapath-windows/ovsext/Recirc.h >b/datapath-windows/ovsext/Recirc.h >new file mode 100644 >index 0000000..8e6d125 >--- /dev/null >+++ b/datapath-windows/ovsext/Recirc.h >@@ -0,0 +1,90 @@ >+/* >+ * Copyright (c) 2016 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: >+ * >+ * >https://urldefense.proofpoint.com/v2/url?u=http-3A__www.apache.org_license >s_LICENSE-2D2.0&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=p >NHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=OpMpvAAJDApIpx1rELDZz_PzL7S9h >fkeHD2h-6_KTCg&s=S30nm7dlEorfpeMZh2t0GL8LoSf_KXweQSeVsbNOCtg&e= >+ * >+ * 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; Nit: packet => nbl in alignment with the other data structures across the codebase. >+ PNL_ATTR actions; >+ OvsFlowKey key; I don¹t get the idea of using a hash. Maybe we can discuss this during the IRC meeting. >+ UINT64 hash; >+} OVS_DEFERRED_ACTION, *POVS_DEFERRED_ACTION; >+ >+/* >+ * >-------------------------------------------------------------------------- >+ * '_OVS_DEFERRED_ACTION_QUEUE' structure is responsible for keeping >track of >+ * all deferred actions. The maximum number of deferred actions should >not >+ * exceed 'DEFERRED_ACTION_QUEUE_SIZE'. >+ * >-------------------------------------------------------------------------- >+ */ >+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 >+ * corresponding to the current CPU. >+ * >-------------------------------------------------------------------------- >+ */ >+NDIS_STATUS >+OvsProcessDeferredActions(POVS_SWITCH_CONTEXT switchContext, >+ OvsCompletionList *completionList, >+ UINT32 portNo, >+ ULONG sendFlags, >+ OVS_PACKET_HDR_INFO *layers); >+ >+/* >+ * >-------------------------------------------------------------------------- >+ * OvsAddDeferredActions -- >+ * This function adds the deferred action to the current CPU queue >and >+ * returns the new queue entry if the queue is not already full. >+ * >-------------------------------------------------------------------------- >+ */ >+POVS_DEFERRED_ACTION >+OvsAddDeferredActions(PNET_BUFFER_LIST packet, >+ OvsFlowKey *key, >+ UINT64 hash, >+ const PNL_ATTR actions); >+ >+/* >+ * >-------------------------------------------------------------------------- >+ * OvsDeferredActionsQueueAlloc -- >+ * This function allocates an array of deferred action queues, one >for >+ * each CPU core. >+ * >-------------------------------------------------------------------------- >+ */ >+BOOLEAN >+OvsDeferredActionsQueueAlloc(); >+ >+/* >+ * >-------------------------------------------------------------------------- >+ * OvsDeferredActionsQueueFree -- >+ * This function frees the array of deferred action queues. >+ * >-------------------------------------------------------------------------- >+ */ >+VOID >+OvsDeferredActionsQueueFree(); >+ >+#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; Unnecessary initialization. > > 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; Unnecessary initialization. > > if (execute->packetLen == 0) { > status = STATUS_INVALID_PARAMETER; >diff --git a/datapath-windows/ovsext/Util.h >b/datapath-windows/ovsext/Util.h >index b2ec798..038754d 100644 >--- a/datapath-windows/ovsext/Util.h >+++ b/datapath-windows/ovsext/Util.h >@@ -36,6 +36,7 @@ > #define OVS_STT_POOL_TAG 'RSVO' > #define OVS_GRE_POOL_TAG 'GSVO' > #define OVS_TUNFLT_POOL_TAG 'WSVO' >+#define OVS_RECIRC_POOL_TAG 'CSVO' > > VOID *OvsAllocateMemory(size_t size); > VOID *OvsAllocateMemoryWithTag(size_t size, ULONG tag); >diff --git a/datapath-windows/ovsext/ovsext.vcxproj >b/datapath-windows/ovsext/ovsext.vcxproj >index e3aea97..af718f7 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" /> >@@ -193,6 +195,7 @@ > <PreCompiledHeader>Create</PreCompiledHeader> > ><PreCompiledHeaderOutputFile>$(IntDir)\precomp.h.pch</PreCompiledHeaderOut >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 >https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma >n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pN >HQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=OpMpvAAJDApIpx1rELDZz_PzL7S9hf >keHD2h-6_KTCg&s=B0JRbghGT_5kotOu2XQ_yegCP9Fj-DzzcfQ0dmmqKtU&e= _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
