[ovs-dev] [PATCH 2/2] datapath-windows: Fixes in OvsSetVportCmdHandler()

[Ankur Sharma]

Acked-by: Ankur Sharma <ankursharma at vmware.com>
________________________________________
From: dev <dev-bounces at openvswitch.org> on behalf of Nithin Raju <nithin at vmware.com>
Sent: Wednesday, October 15, 2014 2:08 PM
To: dev at openvswitch.org
Subject: [ovs-dev] [PATCH 2/2] datapath-windows: Fixes in       OvsSetVportCmdHandler()

In this patch, we make a few simple fixes based on reviewing the code.
The code as such is not tested. We'll be hitting the code path soon
and might make more fixes at that time.

Signed-off-by: Nithin Raju <nithin at vmware.com>
---
 datapath-windows/ovsext/Datapath.c |   40 ++++++++++++++++++++++++-----------
 1 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
index 076cd1a..c8e5b56 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -1699,6 +1699,11 @@ OvsComputeVportNo(POVS_SWITCH_CONTEXT switchContext)
     return OVS_DPPORT_NUMBER_INVALID;
 }

+/*
+ * --------------------------------------------------------------------------
+ *  Command Handler for 'OVS_VPORT_CMD_NEW'.
+ * --------------------------------------------------------------------------
+ */
 static NTSTATUS
 OvsNewVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                       UINT32 *replyLen)
@@ -1892,6 +1897,12 @@ Cleanup:
     return STATUS_SUCCESS;
 }

+
+/*
+ * --------------------------------------------------------------------------
+ *  Command Handler for 'OVS_VPORT_CMD_SET'.
+ * --------------------------------------------------------------------------
+ */
 static NTSTATUS
 OvsSetVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                       UINT32 *replyLen)
@@ -1939,8 +1950,8 @@ OvsSetVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
         OvsReleaseCtrlLock();
         return STATUS_INVALID_PARAMETER;
     }
-    OvsReleaseCtrlLock();

+    NdisAcquireRWLockWrite(gOvsSwitchContext->dispatchLock, &lockState, 0);
     if (vportAttrs[OVS_VPORT_ATTR_NAME] != NULL) {
         PSTR portName = NlAttrGet(vportAttrs[OVS_VPORT_ATTR_NAME]);
         UINT32 portNameLen = NlAttrGetSize(vportAttrs[OVS_VPORT_ATTR_NAME]);
@@ -1949,10 +1960,9 @@ OvsSetVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
         ASSERT(portName[portNameLen - 1] == '\0');

         vport = OvsFindVportByOvsName(gOvsSwitchContext, portName);
-    }
-    else if (vportAttrs[OVS_VPORT_ATTR_PORT_NO] != NULL) {
+    } else if (vportAttrs[OVS_VPORT_ATTR_PORT_NO] != NULL) {
         vport = OvsFindVportByPortNo(gOvsSwitchContext,
-            NlAttrGetU32(vportAttrs[OVS_VPORT_ATTR_PORT_NO]));
+                    NlAttrGetU32(vportAttrs[OVS_VPORT_ATTR_PORT_NO]));
     }

     if (!vport) {
@@ -1960,14 +1970,12 @@ OvsSetVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
         goto Cleanup;
     }

-    NdisAcquireRWLockWrite(gOvsSwitchContext->dispatchLock, &lockState, 0);
-
     /*
-    * XXX: when we implement OVS_DP_ATTR_USER_FEATURES in datapath,
-    * we'll need to check the OVS_DP_F_VPORT_PIDS flag: if it is set,
-    * it means we have an array of pids, instead of a single pid.
-    * ATM we assume we have one pid only.
-    */
+     * XXX: when we implement OVS_DP_ATTR_USER_FEATURES in datapath,
+     * we'll need to check the OVS_DP_F_VPORT_PIDS flag: if it is set,
+     * it means we have an array of pids, instead of a single pid.
+     * Currently, we support only one pid.
+     */
     if (vportAttrs[OVS_VPORT_ATTR_UPCALL_PID]) {
         vport->upcallPid = NlAttrGetU32(vportAttrs[OVS_VPORT_ATTR_UPCALL_PID]);
     }
@@ -1981,9 +1989,9 @@ OvsSetVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
     }

     if (vportAttrs[OVS_VPORT_ATTR_OPTIONS]) {
-        /* XXX: port options not implemented!*/
-        ASSERT(0);
+        OVS_LOG_ERROR("Vport options not supported");
         nlError = NL_ERROR_NOTSUPP;
+        goto Cleanup;
     }

     status = OvsCreateMsgFromVport(vport, msgIn, usrParamsCtx->outputBuffer,
@@ -1994,6 +2002,7 @@ OvsSetVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,

 Cleanup:
     NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState);
+    OvsReleaseCtrlLock();

     if (nlError != NL_ERROR_SUCCESS) {
         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
@@ -2006,6 +2015,11 @@ Cleanup:
     return STATUS_SUCCESS;
 }

+/*
+ * --------------------------------------------------------------------------
+ *  Command Handler for 'OVS_VPORT_CMD_DEL'.
+ * --------------------------------------------------------------------------
+ */
 static NTSTATUS
 OvsDeleteVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                          UINT32 *replyLen)
--
1.7.4.1

_______________________________________________
dev mailing list
dev at openvswitch.org
https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=f6EhnZ0ORGZNt5QbYmRaOxfWfx%2Bqd3KEiPf3%2FYaollU%3D%0A&m=mvOhPV9H4cdtssSAgqSNQX%2Fzl1iosMThLAewbqzGabw%3D%0A&s=5aad88e90c9d18d3f5e11ba5f166792a2ca20ff64ff63d5fc8d6bb7f6de15375