[ovs-dev] [PATCH] datapath-windows: Modified dump start message memory representation
Eitan Eliahu
eliahue at vmware.com
Mon Jul 13 18:54:20 UTC 2015
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-bounces at openvswitch.org] On Behalf Of Sorin Vinturis
Sent: Friday, July 03, 2015 8:37 AM
To: dev at 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 <svinturis at 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 at 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=
More information about the dev
mailing list