Hi Sorin, Thank you for addressing my comments. Can we please use a flag which indicates that the a dump process is in progress rather than setting the whole buffer to zero? Besides of that, everything is good. Thanks, Eitan
-----Original Message----- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis Sent: Friday, July 03, 2015 8:37 AM To: dev@openvswitch.org Subject: [ovs-dev] [PATCH] datapath-windows: Modified dump start message memory representation Changed the dump start message memory representation to be static. Signed-off-by: Sorin Vinturis <svintu...@cloudbasesolutions.com> --- datapath-windows/ovsext/Datapath.c | 53 +++++++++++++++++--------------------- datapath-windows/ovsext/Datapath.h | 8 +++--- datapath-windows/ovsext/Flow.c | 4 +-- datapath-windows/ovsext/Util.h | 5 ++++ datapath-windows/ovsext/Vport.c | 9 ++++--- 5 files changed, 40 insertions(+), 39 deletions(-) diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c index 4af909c..603008d 100644 --- a/datapath-windows/ovsext/Datapath.c +++ b/datapath-windows/ovsext/Datapath.c @@ -347,21 +347,16 @@ NDIS_SPIN_LOCK ovsCtrlLockObj; PNDIS_SPIN_LOCK gOvsCtrlLock; NTSTATUS -InitUserDumpState(POVS_OPEN_INSTANCE instance, - POVS_MESSAGE ovsMsg) +SetUserDumpState(POVS_OPEN_INSTANCE instance, + POVS_MESSAGE ovsMsg) { /* Clear the dumpState from a previous dump sequence. */ - ASSERT(instance->dumpState.ovsMsg == NULL); + ASSERT(OvsBufferIsEmpty((char*)&instance->dumpState.ovsMsg, + sizeof(instance->dumpState.ovsMsg))); ASSERT(ovsMsg); - instance->dumpState.ovsMsg = - (POVS_MESSAGE)OvsAllocateMemoryWithTag(sizeof(OVS_MESSAGE), - OVS_DATAPATH_POOL_TAG); - if (instance->dumpState.ovsMsg == NULL) { - return STATUS_NO_MEMORY; - } - RtlCopyMemory(instance->dumpState.ovsMsg, ovsMsg, - sizeof *instance->dumpState.ovsMsg); + RtlCopyMemory(&instance->dumpState.ovsMsg, ovsMsg, + sizeof instance->dumpState.ovsMsg); RtlZeroMemory(instance->dumpState.index, sizeof instance->dumpState.index); @@ -369,13 +364,9 @@ InitUserDumpState(POVS_OPEN_INSTANCE instance, } VOID -FreeUserDumpState(POVS_OPEN_INSTANCE instance) +ClearUserDumpState(POVS_OPEN_INSTANCE instance) { - if (instance->dumpState.ovsMsg != NULL) { - OvsFreeMemoryWithTag(instance->dumpState.ovsMsg, - OVS_DATAPATH_POOL_TAG); - RtlZeroMemory(&instance->dumpState, sizeof instance->dumpState); - } + RtlZeroMemory(&instance->dumpState, sizeof instance->dumpState); } VOID @@ -588,7 +579,6 @@ OvsRemoveOpenInstance(PFILE_OBJECT fileObject) OvsReleaseCtrlLock(); ASSERT(instance->eventQueue == NULL); ASSERT (instance->packetQueue == NULL); - FreeUserDumpState(instance); OvsFreeMemoryWithTag(instance, OVS_DATAPATH_POOL_TAG); } @@ -839,12 +829,14 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject, * * In the absence of a dump start, return 0 bytes. */ - if (instance->dumpState.ovsMsg == NULL) { + if (OvsBufferIsEmpty((char*)&instance->dumpState.ovsMsg, + sizeof(instance->dumpState.ovsMsg))) { replyLen = 0; status = STATUS_SUCCESS; goto done; } - RtlCopyMemory(&ovsMsgReadOp, instance->dumpState.ovsMsg, + + RtlCopyMemory(&ovsMsgReadOp, &instance->dumpState.ovsMsg, sizeof (ovsMsgReadOp)); /* Create an NL message for consumption. */ @@ -1263,11 +1255,12 @@ HandleGetDpDump(POVS_USER_PARAMS_CONTEXT usrParamsCtx, } else { NL_BUFFER nlBuf; NTSTATUS status; - POVS_MESSAGE msgIn = instance->dumpState.ovsMsg; + POVS_MESSAGE msgIn = &instance->dumpState.ovsMsg; ASSERT(usrParamsCtx->devOp == OVS_READ_DEV_OP); - if (instance->dumpState.ovsMsg == NULL) { + if (OvsBufferIsEmpty((char*)&instance->dumpState.ovsMsg, + sizeof(instance->dumpState.ovsMsg))) { ASSERT(FALSE); return STATUS_INVALID_DEVICE_STATE; } @@ -1285,7 +1278,7 @@ HandleGetDpDump(POVS_USER_PARAMS_CONTEXT usrParamsCtx, if (status != STATUS_SUCCESS) { *replyLen = 0; - FreeUserDumpState(instance); + ClearUserDumpState(instance); return status; } @@ -1293,8 +1286,8 @@ HandleGetDpDump(POVS_USER_PARAMS_CONTEXT usrParamsCtx, instance->dumpState.index[0] = 1; *replyLen = msgOut->nlMsg.nlmsgLen; - /* Free up the dump state, since there's no more data to continue. */ - FreeUserDumpState(instance); + /* Clear the dump state, since there's no more data to continue. */ + ClearUserDumpState(instance); } return STATUS_SUCCESS; @@ -1428,9 +1421,9 @@ OvsSetupDumpStart(POVS_USER_PARAMS_CONTEXT usrParamsCtx) * This operation should be setting up the dump state. If there's any * previous state, clear it up so as to set it up afresh. */ - FreeUserDumpState(instance); + ClearUserDumpState(instance); - return InitUserDumpState(instance, msgIn); + return SetUserDumpState(instance, msgIn); } @@ -1566,7 +1559,8 @@ OvsReadEventCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, ASSERT(usrParamsCtx->devOp == OVS_READ_DEV_OP); /* Should never read events with a dump socket */ - ASSERT(instance->dumpState.ovsMsg == NULL); + ASSERT(OvsBufferIsEmpty((char*)&instance->dumpState.ovsMsg, + sizeof(instance->dumpState.ovsMsg))); /* Must have an event queue */ ASSERT(instance->eventQueue != NULL); @@ -1614,7 +1608,8 @@ OvsReadPacketCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, ASSERT(usrParamsCtx->devOp == OVS_READ_DEV_OP); /* Should never read events with a dump socket */ - ASSERT(instance->dumpState.ovsMsg == NULL); + ASSERT(OvsBufferIsEmpty((char*)&instance->dumpState.ovsMsg, + sizeof(instance->dumpState.ovsMsg))); /* Must have an packet queue */ ASSERT(instance->packetQueue != NULL); diff --git a/datapath-windows/ovsext/Datapath.h b/datapath-windows/ovsext/Datapath.h index 2c61d82..f41357f 100644 --- a/datapath-windows/ovsext/Datapath.h +++ b/datapath-windows/ovsext/Datapath.h @@ -53,7 +53,7 @@ typedef struct _OVS_OPEN_INSTANCE { UINT32 pid; struct { - POVS_MESSAGE ovsMsg; /* OVS message passed during dump start. */ + OVS_MESSAGE ovsMsg; /* OVS message passed during dump start. */ UINT32 index[2]; /* markers to continue dump from. One or more * of them may be used. Eg. in flow dump, the * markers can store the row and the column @@ -113,10 +113,10 @@ InitUserParamsCtx(PIRP irp, usrParamsCtx->outputLength = outputLength; } -NTSTATUS InitUserDumpState(POVS_OPEN_INSTANCE instance, - POVS_MESSAGE ovsMsg); +NTSTATUS SetUserDumpState(POVS_OPEN_INSTANCE instance, + POVS_MESSAGE ovsMsg); -VOID FreeUserDumpState(POVS_OPEN_INSTANCE instance); +VOID ClearUserDumpState(POVS_OPEN_INSTANCE instance); NTSTATUS OvsSetupDumpStart(POVS_USER_PARAMS_CONTEXT usrParamsCtx); diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c index 69b546a..4971a25 100644 --- a/datapath-windows/ovsext/Flow.c +++ b/datapath-windows/ovsext/Flow.c @@ -556,7 +556,7 @@ _FlowNlDumpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, goto done; } - POVS_MESSAGE msgIn = instance->dumpState.ovsMsg; + POVS_MESSAGE msgIn = &instance->dumpState.ovsMsg; PNL_MSG_HDR nlMsgHdr = &(msgIn->nlMsg); PGENL_MSG_HDR genlMsgHdr = &(msgIn->genlMsg); POVS_HDR ovsHdr = &(msgIn->ovsHdr); @@ -612,7 +612,7 @@ _FlowNlDumpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, NlMsgAlignSize(nlMsgOutHdr); *replyLen += NlMsgSize(nlMsgOutHdr); - FreeUserDumpState(instance); + ClearUserDumpState(instance); break; } else { BOOLEAN ok; diff --git a/datapath-windows/ovsext/Util.h b/datapath-windows/ovsext/Util.h index e3f9ede..1baa8e0 100644 --- a/datapath-windows/ovsext/Util.h +++ b/datapath-windows/ovsext/Util.h @@ -90,4 +90,9 @@ VOID OvsAppendList(PLIST_ENTRY dst, PLIST_ENTRY src); BOOLEAN OvsCompareString(PVOID string1, PVOID string2); +__inline BOOLEAN OvsBufferIsEmpty(char *buf, size_t size) { + return (buf[0] == 0 && !memcmp(buf, buf + 1, size - 1)); } + #endif /* __UTIL_H_ */ diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c index 4315464..646139c 100644 --- a/datapath-windows/ovsext/Vport.c +++ b/datapath-windows/ovsext/Vport.c @@ -1848,7 +1848,8 @@ OvsGetVportDumpNext(POVS_USER_PARAMS_CONTEXT usrParamsCtx, ASSERT(usrParamsCtx->devOp == OVS_READ_DEV_OP); - if (instance->dumpState.ovsMsg == NULL) { + if (OvsBufferIsEmpty((char*)&instance->dumpState.ovsMsg, + sizeof(instance->dumpState.ovsMsg))) { ASSERT(FALSE); return STATUS_INVALID_DEVICE_STATE; } @@ -1856,7 +1857,7 @@ OvsGetVportDumpNext(POVS_USER_PARAMS_CONTEXT usrParamsCtx, /* Output buffer has been validated while validating read dev op. */ ASSERT(usrParamsCtx->outputBuffer != NULL); - msgIn = instance->dumpState.ovsMsg; + msgIn = &instance->dumpState.ovsMsg; /* * XXX: when we implement OVS_DP_ATTR_USER_FEATURES in datapath, @@ -1935,8 +1936,8 @@ OvsGetVportDumpNext(POVS_USER_PARAMS_CONTEXT usrParamsCtx, * it's dump done */ *replyLen = 0; - /* Free up the dump state, since there's no more data to continue. */ - FreeUserDumpState(instance); + /* Clear the dump state, since there's no more data to continue. */ + ClearUserDumpState(instance); } return STATUS_SUCCESS; -- 1.9.0.msysgit.0 _______________________________________________ dev mailing list dev@openvswitch.org https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=Vq3YbkPiJV3h0Q6PsQokNHXGG38vwhGdYGQey1ZVSbE&s=zI97YCLM60jW34_JqX7d-_bS4OXGarPIsQuqpB7Avew&e= _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
