[ovs-dev] [PATCH 9/9 v2] datapath-windows: refactor code to setup dump start state

Samuel Ghinet sghinet at cloudbasesolutions.com
Fri Aug 29 10:57:46 UTC 2014


Nithin,

I had expected you modify the original patch with my suggestions, not add a new patch on top of it, by which to refactor the original patch.
So this patch is: Not acked-by: Samuel Ghinet <sghinet at cloudbasesolutions.com>

Regards,
Sam
________________________________________
Date: Fri, 29 Aug 2014 01:15:21 -0700
From: Nithin Raju <nithin at vmware.com>
To: dev at openvswitch.org
Subject: [ovs-dev] [PATCH 9/9 v2] datapath-windows: refactor code to
        setup   dump start state
Message-ID: <1409300121-13568-9-git-send-email-nithin at vmware.com>

Per review comment, in this patch we refactor the code to create a
OvsSetupDumpStart() which can be leveraged by dump functions in the
future. I have not refactored the code that continues the dump
operation primarily since it is not final yet. Once the netlink set
APIs are in place, we can refactor that too.

Signed-off-by: Nithin Raju <nithin at vmware.com>
Acked-by: Ankur Sharma <ankursharma at vmware.com>
Acked-by: Samuel Ghinet <sghinet at cloudbasesolutions.com>
---
 datapath-windows/ovsext/Datapath.c |   66 ++++++++++++++++++++++--------------
 1 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
index 943e6f9..4a17914 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -110,8 +110,11 @@ static NTSTATUS OvsGetDpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,

 /* Netlink control family: this is a Windows specific family. */
 NETLINK_CMD nlControlFamilyCmdOps[] = {
-    { OVS_CTRL_CMD_WIN_GET_PID, OvsGetPidCmdHandler,
-      OVS_TRANSACTION_DEV_OP, FALSE }
+    { .cmd             = OVS_CTRL_CMD_WIN_GET_PID,
+      .handler         = OvsGetPidCmdHandler,
+      .supportedDevOp  = OVS_TRANSACTION_DEV_OP,
+      .validateDpIndex = FALSE
+    }
 };

 NETLINK_FAMILY nlControlFamilyOps = {
@@ -125,8 +128,11 @@ NETLINK_FAMILY nlControlFamilyOps = {

 /* Netlink datapath family. */
 NETLINK_CMD nlDatapathFamilyCmdOps[] = {
-    { OVS_DP_CMD_GET, OvsGetDpCmdHandler,
-      OVS_WRITE_DEV_OP | OVS_READ_DEV_OP, FALSE }
+    { .cmd             = OVS_DP_CMD_GET,
+      .handler         = OvsGetDpCmdHandler,
+      .supportedDevOp  = OVS_WRITE_DEV_OP | OVS_READ_DEV_OP,
+      .validateDpIndex = FALSE
+    }
 };

 NETLINK_FAMILY nlDatapathFamilyOps = {
@@ -182,6 +188,7 @@ static NTSTATUS ValidateNetlinkCmd(UINT32 devOp,
 static NTSTATUS InvokeNetlinkCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                                         NETLINK_FAMILY *nlFamilyOps,
                                         UINT32 *replyLen);
+static NTSTATUS OvsSetupDumpStart(POVS_USER_PARAMS_CONTEXT usrParamsCtx);


 /* Handles to the device object for communication with userspace. */
@@ -828,28 +835,8 @@ OvsGetDpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
         (POVS_OPEN_INSTANCE)usrParamsCtx->ovsInstance;

     if (usrParamsCtx->devOp == OVS_WRITE_DEV_OP) {
-        NTSTATUS status;
-
-        /* input buffer has been validated while validating write dev op. */
-        ASSERT(msgIn != NULL && usrParamsCtx->inputLength >= sizeof *msgIn);
-
-        /* A write operation that does not indicate dump start is invalid. */
-        if ((msgIn->nlMsg.nlmsgFlags & NLM_F_DUMP) != NLM_F_DUMP) {
-            return STATUS_INVALID_PARAMETER;
-        }
-        /* XXX: Handle other NLM_F_* flags in the future. */
-
-        /*
-         * 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.
-         */
-        if (instance->dumpState.ovsMsg != NULL) {
-            FreeUserDumpState(instance);
-        }
-        status = InitUserDumpState(instance, msgIn);
-        if (status != STATUS_SUCCESS) {
-            return STATUS_NO_MEMORY;
-        }
+        *replyLen = 0;
+        OvsSetupDumpStart(usrParamsCtx);
     } else {
         ASSERT(usrParamsCtx->devOp == OVS_READ_DEV_OP);

@@ -943,6 +930,33 @@ OvsGetDpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
     return STATUS_SUCCESS;
 }

+static NTSTATUS
+OvsSetupDumpStart(POVS_USER_PARAMS_CONTEXT usrParamsCtx)
+{
+    POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer;
+    POVS_OPEN_INSTANCE instance =
+        (POVS_OPEN_INSTANCE)usrParamsCtx->ovsInstance;
+
+    /* input buffer has been validated while validating write dev op. */
+    ASSERT(msgIn != NULL && usrParamsCtx->inputLength >= sizeof *msgIn);
+
+    /* A write operation that does not indicate dump start is invalid. */
+    if ((msgIn->nlMsg.nlmsgFlags & NLM_F_DUMP) != NLM_F_DUMP) {
+        return STATUS_INVALID_PARAMETER;
+    }
+    /* XXX: Handle other NLM_F_* flags in the future. */
+
+    /*
+     * 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.
+     */
+    if (instance->dumpState.ovsMsg != NULL) {
+        FreeUserDumpState(instance);
+    }
+
+    return InitUserDumpState(instance, msgIn);
+}
+
 /*
  * --------------------------------------------------------------------------
  *  Utility function to map the output buffer in an IRP. The buffer is assumed
--
1.7.4.1



More information about the dev mailing list