[ovs-dev] [PATCH v1 3/6] datapath-windows: Calling OvsAddPidInstance and OvsDelPidInstance

[Eitan Eliahu]

Please find comments  inline

-----Original Message-----
From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Ankur Sharma
Sent: Monday, October 20, 2014 5:35 PM
To: dev at openvswitch.org
Subject: [ovs-dev] [PATCH v1 3/6] datapath-windows: Calling OvsAddPidInstance and OvsDelPidInstance

Signed-off-by: Ankur Sharma <ankursharma at vmware.com>
---
 datapath-windows/ovsext/User.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c index a8d9107..42473ab 100644
--- a/datapath-windows/ovsext/User.c
+++ b/datapath-windows/ovsext/User.c
@@ -136,9 +136,21 @@ OvsSubscribeDpIoctl(PVOID instanceP,
     POVS_USER_PACKET_QUEUE queue;
     POVS_OPEN_INSTANCE instance = (POVS_OPEN_INSTANCE)instanceP;
 
+    OvsAcquireCtrlLock();
+    if (!gOvsSwitchContext) {
+        OvsReleaseCtrlLock();
+        return STATUS_INVALID_PARAMETER;
+    }
+
[EITAN]
Must release the lock here
[EITAN]

     if (instance->packetQueue && !join) {
         /* unsubscribe */
         OvsCleanupPacketQueue(instance);
+
[EITAN]
It is ok for now (just for bringup purpose) to hold the user mode interface control lock. But, it created dependency between queues and defeats the purpose of having multiple queues.
A better way would be having in each entry (bucket) of the oid hash.
[EITAN]+        OvsAcquireCtrlLock();
+        /* Remove the instance from pidHashArray */
+        OvsDelPidInstance(gOvsSwitchContext, pid);
+        OvsReleaseCtrlLock();
+
     } else if (instance->packetQueue == NULL && join) {
         queue = (POVS_USER_PACKET_QUEUE) OvsAllocateMemory(sizeof *queue);
         if (queue == NULL) {
@@ -153,10 +165,18 @@ OvsSubscribeDpIoctl(PVOID instanceP,
         queue->instance = instance;
         instance->packetQueue = queue;
         NdisReleaseSpinLock(&queue->queueLock);
+
+        OvsAcquireCtrlLock();
+        /* Insert the instance to pidHashArray */
+        OvsAddPidInstance(gOvsSwitchContext, pid, instance);
+        OvsReleaseCtrlLock();
+
+
     } else {
         /* user mode should call only once for subscribe */
         return STATUS_INVALID_PARAMETER;
     }
+
     return STATUS_SUCCESS;
 }
 
--
1.9.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=yTvML8OxA42Jb6ViHe7fUXbvPVOYDPVq87w43doxtlY%3D%0A&m=hhlqlYVYz1uCQFwKRPXghvQ6kDR0QA3tHr6VfPyKhsA%3D%0A&s=6df15421408b8a7e136403306e5d10e8d3fd54d85ad507d7e54b30240cd4ab5b