Hi Nithin,
Can you please revisit the locking sequence for the netdev handler?
It seems that OvsGetVportDumpNext() is called with the two locks, Dispatch and 
Control held, (can we add a comment on it in the function header?).
But, the Control lock is released at line 1214.
Thanks,
Eitan

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Nithin Raju
Sent: Wednesday, October 29, 2014 3:18 PM
To: [email protected]
Subject: [ovs-dev] [PATCH 2/3] datapath-windws: fix locking code in 
OvsGetNetdevCmdHandler()

We were acquiring locks twice, and forgotten to release a lock in an error case.

Signed-off-by: Nithin Raju <[email protected]>
---
 datapath-windows/ovsext/Vport.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c 
index b28336b..a46d49f 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -1201,7 +1201,6 @@ OvsGetExtInfoIoctl(POVS_VPORT_GET vportGet,
     if (vport == NULL || (vport->ovsState != OVS_STATE_CONNECTED &&
                           vport->ovsState != OVS_STATE_NIC_CREATED)) {
         NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState);
-        NdisReleaseSpinLock(gOvsCtrlLock);
         if (vportGet->portNo) {
             OVS_LOG_WARN("vport %u does not exist any more", vportGet->portNo);
         } else {
@@ -1293,7 +1292,6 @@ OvsGetNetdevCmdHandler(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
     NL_ERROR nlError = NL_ERROR_SUCCESS;
     OVS_VPORT_GET vportGet;
     OVS_VPORT_EXT_INFO info;
-    LOCK_STATE_EX lockState;
 
     static const NL_POLICY ovsNetdevPolicy[] = {
         [OVS_WIN_NETDEV_ATTR_NAME] = { .type = NL_A_STRING, @@ -1327,15 
+1325,12 @@ OvsGetNetdevCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
     RtlCopyMemory(&vportGet.name, NlAttrGet(netdevAttrs[OVS_VPORT_ATTR_NAME]),
                   NlAttrGetSize(netdevAttrs[OVS_VPORT_ATTR_NAME]));
 
-    NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState, 0);
     status = OvsGetExtInfoIoctl(&vportGet, &info);
     if (status == STATUS_DEVICE_DOES_NOT_EXIST) {
         nlError = NL_ERROR_NODEV;
-        NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState);
         OvsReleaseCtrlLock();
         goto cleanup;
     }
-    NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState);
 
     status = CreateNetlinkMesgForNetdev(&info, msgIn,
                  usrParamsCtx->outputBuffer, usrParamsCtx->outputLength,
--
1.7.4.1

_______________________________________________
dev mailing list
[email protected]
https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=yTvML8OxA42Jb6ViHe7fUXbvPVOYDPVq87w43doxtlY%3D%0A&m=%2BEYonzGTZSXOC0QpJNNcTX5iiu%2FP5HVj9N3n0wv0NHE%3D%0A&s=85eb05178b1183ec0384b302767774fe3d73a58987e62ffecf6dc8dbd231388e
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to