[ovs-dev] [PATCH 3/4 v4] datapath-windows: add a context structure for user parameters

Ankur Sharma ankursharma at vmware.com
Fri Aug 29 22:47:49 UTC 2014


In this patch we add a context structure for collecting all the parameters
passed from usersapce in one place. The idea is to reduce the number of
parameters being passed to the netlink command handler functions.

It can be argued that not all functions require all the arguments, but this
approach keeps the code clean, IMO.

Signed-off-by: Nithin Raju <nithin at vmware.com>
Signed-off-by: Ankur Sharma <ankursharma at vmware.com>
Acked-by: Ankur Sharma <ankursharma at vmware.com>
---
Please attribute this change to Nithin Raju <nithin at vmware.com>
---
 datapath-windows/ovsext/Datapath.c | 72 +++++++++++---------------------------
 datapath-windows/ovsext/Datapath.h | 38 ++++++++++++++++++++
 2 files changed, 59 insertions(+), 51 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
index 3c2f27d..270a68d 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -64,9 +64,7 @@
  * Handler for a given netlink command. Not all the parameters are used by all
  * the handlers.
  */
-typedef NTSTATUS (*NetlinkCmdHandler)(PIRP irp, PFILE_OBJECT fileObject,
-                                      PVOID inputBuffer, UINT32 inputLength,
-                                      PVOID outputBuffer, UINT32 outputLength,
+typedef NTSTATUS (*NetlinkCmdHandler)(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                                       UINT32 *replyLen);
 
 typedef struct _NETLINK_CMD {
@@ -97,9 +95,7 @@ typedef struct _NETLINK_FAMILY {
 #define OVS_TRANSACTION_DEV_OP   (1 << 2)
 
 /* Handlers for the various netlink commands. */
-static NTSTATUS OvsGetPidCmdHandler(PIRP irp, PFILE_OBJECT fileObject,
-                                    PVOID inputBuffer, UINT32 inputLength,
-                                    PVOID outputBuffer, UINT32 outputLength,
+static NTSTATUS OvsGetPidCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                                     UINT32 *replyLen);
 
 /*
@@ -180,15 +176,8 @@ ValidateNetlinkCmd(UINT32 devOp,
                    POVS_MESSAGE ovsMsg,
                    NETLINK_FAMILY *nlFamilyOps);
 static NTSTATUS
-InvokeNetlinkCmdHandler(PIRP irp,
-                        PFILE_OBJECT fileObject,
-                        UINT32 devOp,
-                        POVS_MESSAGE ovsMsg,
-                        NETLINK_FAMILY *nlFamily,
-                        PVOID inputBuffer,
-                        UINT32 inputLength,
-                        PVOID outputBuffer,
-                        UINT32 outputLength,
+InvokeNetlinkCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
+                        NETLINK_FAMILY *nlFamilyOps,
                         UINT32 *replyLen);
 
 
@@ -541,6 +530,7 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
     OVS_MESSAGE ovsMsgReadOp;
     POVS_MESSAGE ovsMsg;
     NETLINK_FAMILY *nlFamilyOps;
+    OVS_USER_PARAMS_CONTEXT usrParamsCtx;
 
 #ifdef DBG
     POVS_DEVICE_EXTENSION ovsExt =
@@ -675,11 +665,12 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
         }
     }
 
-    status = InvokeNetlinkCmdHandler(irp, fileObject, devOp,
-                                     ovsMsg, nlFamilyOps,
-                                     inputBuffer, inputBufferLen,
-                                     outputBuffer, outputBufferLen,
-                                     &replyLen);
+    InitUserParamsCtx(irp, instance, devOp, ovsMsg,
+                      inputBuffer, inputBufferLen,
+                      outputBuffer, outputBufferLen,
+                      &usrParamsCtx);
+
+    status = InvokeNetlinkCmdHandler(&usrParamsCtx, nlFamilyOps, &replyLen);
 
 done:
     KeMemoryBarrier();
@@ -743,28 +734,16 @@ done:
  * --------------------------------------------------------------------------
  */
 static NTSTATUS
-InvokeNetlinkCmdHandler(PIRP irp,
-                        PFILE_OBJECT fileObject,
-                        UINT32 devOp,
-                        OVS_MESSAGE *ovsMsg,
+InvokeNetlinkCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                         NETLINK_FAMILY *nlFamilyOps,
-                        PVOID inputBuffer,
-                        UINT32 inputLength,
-                        PVOID outputBuffer,
-                        UINT32 outputLength,
                         UINT32 *replyLen)
 {
     NTSTATUS status = STATUS_INVALID_PARAMETER;
     UINT16 i;
 
-    UNREFERENCED_PARAMETER(devOp);
-
     for (i = 0; i < nlFamilyOps->opsCount; i++) {
-        if (nlFamilyOps->cmds[i].cmd == ovsMsg->genlMsg.cmd) {
-            status = nlFamilyOps->cmds[i].handler(irp, fileObject,
-                                                inputBuffer, inputLength,
-                                                outputBuffer, outputLength,
-                                                replyLen);
+        if (nlFamilyOps->cmds[i].cmd == usrParamsCtx->ovsMsg->genlMsg.cmd) {
+            status = nlFamilyOps->cmds[i].handler(usrParamsCtx, replyLen);
             break;
         }
     }
@@ -785,24 +764,15 @@ InvokeNetlinkCmdHandler(PIRP irp,
  * --------------------------------------------------------------------------
  */
 static NTSTATUS
-OvsGetPidCmdHandler(PIRP irp,
-                    PFILE_OBJECT fileObject,
-                    PVOID inputBuffer,
-                    UINT32 inputLength,
-                    PVOID outputBuffer,
-                    UINT32 outputLength,
+OvsGetPidCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                     UINT32 *replyLen)
 {
-    UNREFERENCED_PARAMETER(irp);
-    UNREFERENCED_PARAMETER(fileObject);
-    UNREFERENCED_PARAMETER(inputBuffer);
-    UNREFERENCED_PARAMETER(inputLength);
-
-    POVS_MESSAGE msgIn = (POVS_MESSAGE)inputBuffer;
-    POVS_MESSAGE msgOut = (POVS_MESSAGE)outputBuffer;
+    POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer;
+    POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx->outputBuffer;
 
-    if (outputLength >= sizeof *msgOut) {
-        POVS_OPEN_INSTANCE instance = (POVS_OPEN_INSTANCE)fileObject->FsContext;
+    if (usrParamsCtx->outputLength >= sizeof *msgOut) {
+        POVS_OPEN_INSTANCE instance =
+            (POVS_OPEN_INSTANCE)usrParamsCtx->ovsInstance;
 
         RtlZeroMemory(msgOut, sizeof *msgOut);
         msgOut->nlMsg.nlmsgSeq = msgIn->nlMsg.nlmsgSeq;
@@ -813,7 +783,7 @@ OvsGetPidCmdHandler(PIRP irp,
         return STATUS_NDIS_INVALID_LENGTH;
     }
 
-    return NDIS_STATUS_SUCCESS;
+    return STATUS_SUCCESS;
 }
 
 
diff --git a/datapath-windows/ovsext/Datapath.h b/datapath-windows/ovsext/Datapath.h
index a8faeff..4e6c9b1 100644
--- a/datapath-windows/ovsext/Datapath.h
+++ b/datapath-windows/ovsext/Datapath.h
@@ -85,6 +85,44 @@ POVS_OPEN_INSTANCE OvsGetOpenInstance(PFILE_OBJECT fileObject,
 
 NTSTATUS OvsCompleteIrpRequest(PIRP irp, ULONG_PTR infoPtr, NTSTATUS status);
 
+
+/*
+ * Utility structure and functions to collect in one place all the parameters
+ * passed during a call from userspace.
+ */
+typedef struct _OVS_USER_PARAMS_CONTEXT {
+    PIRP irp;            /* The IRP used for the userspace call. */
+    POVS_OPEN_INSTANCE ovsInstance; /* Private data of the device handle. */
+    UINT32 devOp;        /* Device operation of the userspace call. */
+    POVS_MESSAGE ovsMsg; /* OVS message that userspace passed down. */
+    PVOID inputBuffer;   /* Input data specified by userspace. Maybe NULL. */
+    UINT32 inputLength;  /* Length of input buffer. */
+    PVOID outputBuffer;  /* Output buffer specified by userspace for reading
+                          * data. Maybe NULL. */
+    UINT32 outputLength; /* Length of output buffer. */
+} OVS_USER_PARAMS_CONTEXT, *POVS_USER_PARAMS_CONTEXT;
+
+static __inline VOID
+InitUserParamsCtx(PIRP irp,
+                  POVS_OPEN_INSTANCE ovsInstance,
+                  UINT32 devOp,
+                  POVS_MESSAGE ovsMsg,
+                  PVOID inputBuffer,
+                  UINT32 inputLength,
+                  PVOID outputBuffer,
+                  UINT32 outputLength,
+                  POVS_USER_PARAMS_CONTEXT usrParamsCtx)
+{
+    usrParamsCtx->irp = irp;
+    usrParamsCtx->ovsInstance = ovsInstance;
+    usrParamsCtx->devOp = devOp;
+    usrParamsCtx->ovsMsg = ovsMsg;
+    usrParamsCtx->inputBuffer = inputBuffer;
+    usrParamsCtx->inputLength = inputLength;
+    usrParamsCtx->outputBuffer = outputBuffer;
+    usrParamsCtx->outputLength = outputLength;
+}
+
 static __inline NTSTATUS
 InitUserDumpState(POVS_OPEN_INSTANCE instance,
                   POVS_MESSAGE ovsMsg)
-- 
1.8.3.2




More information about the dev mailing list