Nithin,
I would like to point out something Saurabh pointed out to Ankur, some days ago:
> Something to be careful of in future. You should wait for the reviewers to
> give an actual 'Acked-by' for a patch. If its a private Ack, it would be a
> good idea to mention it and then put an Ack for them. For example, Nithin
> & Samuel reviewed your V1 patch and gave comments, but that doesn't mean
> an implicit ack. Original reviewers or others may still have comments for
> the V1/V2 patch. I noticed this happen earlier as well, so before it
> becomes a common practice I thought I would call it out.
In this particular case, I did not and will not ack these series of patches.
I have seen you have taken some of my suggestions, but sent them as separate,
later patches.
Such as this patch (patch 1), which has:
> UINT32 index[8];
while your patch 7 changes it to:
> UINT32 index[2];
I would ack a patch only if it has the correct / agreed upon form.
So please re-make the patch1, so it does not have "index[8]", if it's agreed
that we will not keep "index[8]".
Also, my suggestion was to use two separate variables, instead of an array,
each to have a specific name.
If you consider a better approach to keep it as an array, please do not
"Acked-by: Samuel Ghinet", it is possible I do not agree with your alternative.
Also, if you ignore a suggestion of mine, please do not say "Acked-by: Samuel
Ghinet"
Or when I suggest you add some doc comments, it is possible I do not find the
documentation clear enough, or complete.
So this patch goes: Not acked-by: Samuel Ghinet <sghi...@cloudbasesolutions.com>
Thanks,
Sam
________________________________________
Date: Fri, 29 Aug 2014 01:15:13 -0700
From: Nithin Raju <nit...@vmware.com>
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH 1/9 v2] datapath-windows: Data structures
and functions for dump state
Message-ID: <1409300121-13568-1-git-send-email-nit...@vmware.com>
Signed-off-by: Nithin Raju <nit...@vmware.com>
Acked-by: Ankur Sharma <ankursha...@vmware.com>
Acked-by: Samuel Ghinet <sghi...@cloudbasesolutions.com>
---
datapath-windows/ovsext/Datapath.h | 54 ++++++++++++++++++++++++++++++------
1 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/datapath-windows/ovsext/Datapath.h
b/datapath-windows/ovsext/Datapath.h
index 6d8a6db..0b303a2 100644
--- a/datapath-windows/ovsext/Datapath.h
+++ b/datapath-windows/ovsext/Datapath.h
@@ -28,6 +28,16 @@
#ifndef __OVS_DATAPATH_H_
#define __OVS_DATAPATH_H_ 1
+/*
+ * Structure of any message passed between userspace and kernel.
+ */
+typedef struct _OVS_MESSAGE {
+ NL_MSG_HDR nlMsg;
+ GENL_MSG_HDR genlMsg;
+ OVS_HDR ovsHdr;
+ /* Variable length nl_attrs follow. */
+} OVS_MESSAGE, *POVS_MESSAGE;
+
typedef struct _OVS_DEVICE_EXTENSION {
INT numberOpenInstance;
INT pidCount;
@@ -57,6 +67,12 @@ typedef struct _OVS_OPEN_INSTANCE {
* restriction might go away as the userspace code gets implemented.
*/
INT inUse;
+
+ struct {
+ POVS_MESSAGE ovsMsg; /* OVS message passed during dump start. */
+ UINT32 index[8]; /* markers to continue dump from. One or more
+ * of them may be used. */
+ } dumpState; /* data to support dump commands. */
} OVS_OPEN_INSTANCE, *POVS_OPEN_INSTANCE;
NDIS_STATUS OvsCreateDeviceObject(NDIS_HANDLE ovsExtDriverHandle);
@@ -67,15 +83,35 @@ POVS_OPEN_INSTANCE OvsGetOpenInstance(PFILE_OBJECT
fileObject,
NTSTATUS OvsCompleteIrpRequest(PIRP irp, ULONG_PTR infoPtr, NTSTATUS status);
-/*
- * Structure of any message passed between userspace and kernel.
- */
-typedef struct _OVS_MESSAGE {
- NL_MSG_HDR nlMsg;
- GENL_MSG_HDR genlMsg;
- struct ovs_header ovsHdr;
- /* Variable length nl_attrs follow. */
-} OVS_MESSAGE, *POVS_MESSAGE;
+static __inline NTSTATUS
+InitUserDumpState(POVS_OPEN_INSTANCE instance,
+ POVS_MESSAGE ovsMsg)
+{
+ /* Clear the dumpState from a previous dump sequence. */
+ ASSERT(instance->dumpState.ovsMsg == NULL);
+ ASSERT(ovsMsg);
+
+ instance->dumpState.ovsMsg = (POVS_MESSAGE) OvsAllocateMemory(
+ sizeof *instance->dumpState.ovsMsg);
+ if (instance->dumpState.ovsMsg == NULL) {
+ return STATUS_NO_MEMORY;
+ }
+ RtlCopyMemory(instance->dumpState.ovsMsg, ovsMsg,
+ sizeof *instance->dumpState.ovsMsg);
+ RtlZeroMemory(instance->dumpState.index,
+ sizeof instance->dumpState.index);
+
+ return STATUS_SUCCESS;
+}
+
+static __inline VOID
+FreeUserDumpState(POVS_OPEN_INSTANCE instance)
+{
+ if (instance->dumpState.ovsMsg != NULL) {
+ OvsFreeMemory(instance->dumpState.ovsMsg);
+ RtlZeroMemory(&instance->dumpState, sizeof instance->dumpState);
+ }
+}
#endif /* __OVS_DATAPATH_H_ */
--
1.7.4.1
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
