[ovs-dev] [PATCH v2 2/6] datapath-windows: pid-instance hash table APIs.

Sorin Vinturis svinturis at cloudbasesolutions.com
Wed Oct 22 13:30:12 UTC 2014


Hi Ankur,

Please see my comments inline.

Thanks,
Sorin

-----Original Message-----
From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Ankur Sharma
Sent: Wednesday, October 22, 2014 3:25 AM
To: dev at openvswitch.org
Subject: [ovs-dev] [PATCH v2 2/6] datapath-windows: pid-instance hash table APIs.

In this patch we have added APIs for insert, delete and search APIs.

Signed-off-by: Ankur Sharma <ankursharma at vmware.com>
---
 datapath-windows/ovsext/User.c | 57 ++++++++++++++++++++++++++++++++++++++++++
 datapath-windows/ovsext/User.h | 10 ++++++++
 2 files changed, 67 insertions(+)

diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c index 3e6cda2..36787cd 100644
--- a/datapath-windows/ovsext/User.c
+++ b/datapath-windows/ovsext/User.c
@@ -32,6 +32,7 @@
 #include "NetProto.h"
 #include "Flow.h"
 #include "TunnelIntf.h"
+#include "Jhash.h"
 
 #ifdef OVS_DBG_MOD
 #undef OVS_DBG_MOD
@@ -598,6 +599,62 @@ OvsGetQueue(UINT32 pid)
     return NULL;
 }
 
+/*
+ * 
+-----------------------------------------------------------------------
+----
+ * Given a pid, returns the corresponding instance.
+ * gOvsCtrlLock must be acquired before calling this API.
+ * 
+-----------------------------------------------------------------------
+----
+ */
+POVS_OPEN_INSTANCE
+OvsGetPidInstance(POVS_SWITCH_CONTEXT switchContext, UINT32 pid) {
+    POVS_OPEN_INSTANCE instance;
+    PLIST_ENTRY head, link;
+    UINT32 hash = OvsJhashBytes((const VOID *)&pid, sizeof(pid),
+                                OVS_HASH_BASIS);
+    head = &(switchContext->pidHashArray[hash & OVS_PID_MASK]);
+    LIST_FORALL(head, link) {
+        instance = CONTAINING_RECORD(link, OVS_OPEN_INSTANCE, pidLink);
+        if (instance->pid == pid) {
+            return instance;
+        }
+    }
[Sorin]
The lines above searches through the pidHashArray for the requested pid, but fails to check the first element of the array.
E.g.:
+    head = &(switchContext->pidHashArray[hash & OVS_PID_MASK]);
+    LIST_FORALL(head, link) {
+        instance = CONTAINING_RECORD(link, OVS_OPEN_INSTANCE, pidLink);
+        if (instance->pid == pid) {
+            return instance;
+        }
To fix this, I propose the following:
     head = &(switchContext->pidHashArray[hash & OVS_PID_MASK]);
     link = head;
     do {
         instance = CONTAINING_RECORD(link, OVS_OPEN_INSTANCE, pidLink);
         if (instance->pid == pid) {
             return instance;
         }
         instance = NULL;
         link = link->Flink;
       } while (link != head);
[Sorin]
+    return NULL;
+}
+
+/*
+ * 
+-----------------------------------------------------------------------
+----
+ * Given a pid and an instance. This API adds instance to pidHashArray.
+ * gOvsCtrlLock must be acquired before calling this API.
+ * 
+-----------------------------------------------------------------------
+----
+ */
+VOID
+OvsAddPidInstance(POVS_SWITCH_CONTEXT switchContext, UINT32 pid,
+                  POVS_OPEN_INSTANCE instance) {
+    PLIST_ENTRY head;
+    UINT32 hash = OvsJhashBytes((const VOID *)&pid, sizeof(pid),
+                                OVS_HASH_BASIS);
+    head = &(switchContext->pidHashArray[hash & OVS_PID_MASK]);
+    InsertHeadList(head, &(instance->pidLink)); }
+
+/*
+ * 
+-----------------------------------------------------------------------
+----
+ * Given a pid and an instance. This API removes instance from pidHashArray.
+ * gOvsCtrlLock must be acquired before calling this API.
+ * 
+-----------------------------------------------------------------------
+----
+ */
+VOID
+OvsDelPidInstance(POVS_SWITCH_CONTEXT switchContext, UINT32 pid) {
+    POVS_OPEN_INSTANCE instance = OvsGetPidInstance(switchContext, 
+pid);
+
+    if (instance) {
+        RemoveEntryList(&(instance->pidLink));
+    }
+}
+
 VOID
 OvsQueuePackets(UINT32 queueId,
                 PLIST_ENTRY packetList, diff --git a/datapath-windows/ovsext/User.h b/datapath-windows/ovsext/User.h index 0c18e2f..47fb10b 100644
--- a/datapath-windows/ovsext/User.h
+++ b/datapath-windows/ovsext/User.h
@@ -108,4 +108,14 @@ NTSTATUS OvsWaitDpIoctl(PIRP irp, PFILE_OBJECT fileObject);  NTSTATUS OvsNlExecuteCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                                 UINT32 *replyLen);
 
+POVS_OPEN_INSTANCE
+OvsGetPidInstance(POVS_SWITCH_CONTEXT switchContext, UINT32 pid);
+
+VOID
+OvsAddPidInstance(POVS_SWITCH_CONTEXT switchContext, UINT32 pid,
+                  POVS_OPEN_INSTANCE instance);
+
+VOID
+OvsDelPidInstance(POVS_SWITCH_CONTEXT switchContext, UINT32 pid);
+
 #endif /* __USER_H_ */
--
1.9.1

_______________________________________________
dev mailing list
dev at openvswitch.org
http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list