[ovs-dev] [PATCH] datapath-windows: cleanup dump state during instance cleanup

Samuel Ghinet sghinet at cloudbasesolutions.com
Thu Sep 18 12:16:36 UTC 2014


Thanks Eitan for clarifications!

I have checked the userspace code, there is no call for DuplicateHandle.
I am not sure whether we should put checks in kernel code now  for a possible future usage of this, or simply leave it to be handled later - if / when such a case will appear.

Sam
________________________________
From: Eitan Eliahu [eliahue at vmware.com]
Sent: Thursday, September 18, 2014 2:35 PM
To: Samuel Ghinet; Nithin Raju
Cc: dev at openvswitch.org
Subject: RE: [ovs-dev] [PATCH] datapath-windows: cleanup dump state during instance cleanup

No Sam, Duplication of handles is done through the calling DuplicateHandle((). I am not sure if OVS UM calls it. The general case if that one handle corresponds to a single file object.
Thanks,
Eitan

From: Samuel Ghinet [mailto:sghinet at cloudbasesolutions.com]
Sent: Thursday, September 18, 2014 4:27 AM
To: Eitan Eliahu; Nithin Raju
Cc: dev at openvswitch.org
Subject: RE: [ovs-dev] [PATCH] datapath-windows: cleanup dump state during instance cleanup

Hi Eitan,

By duplication of HANDLES, you mean assignment (like, "HANDLE a = CreateFile(params); HANDLE b = a;")?

We do have cases in the vswitchd where we keep multiple duplicates / instances of the same HANDLE, or where we create children processes that may inherit the parent handle?

Thanks,
Sam,
________________________________
From: Eitan Eliahu [eliahue at vmware.com]
Sent: Thursday, September 18, 2014 2:16 PM
To: Samuel Ghinet; Nithin Raju
Cc: dev at openvswitch.org<mailto:dev at openvswitch.org>
Subject: RE: [ovs-dev] [PATCH] datapath-windows: cleanup dump state during instance cleanup
Hi Sam,
As far as I understand when a process calls CreatFile the system will create a file handle and return it to the process. The file handle (in UM) is associated with a file object in (KM). Both objects are valid until the process closes the handle or terminates.
This means that if the device is opened multiple times the kernel will have multiple file  objects, each one corresponds to a file handle returned by CreateFile(). All these file objects are associated with the driver device object.
The exception is when a process duplicates a handle or when a child process  inherits a handle from a parent process. At this case the system holds multiple file handles which corresponds to a single file object.
I hope this answers your question.
Eitan

From: Samuel Ghinet [mailto:sghinet at cloudbasesolutions.com]
Sent: Thursday, September 18, 2014 3:53 AM
To: Eitan Eliahu; Nithin Raju
Cc: dev at openvswitch.org<mailto:dev at openvswitch.org>
Subject: RE: [ovs-dev] [PATCH] datapath-windows: cleanup dump state during instance cleanup

Eitan,


> In case that a handle is duplicated in user mode there will be multiple file handles per file object associated with the handle returned by the system when the device was opened.
As I remember when working with userspace HANDLEs and FILE_OBJECT ptrs in kernel, even though only one device was used and many file HANDLEs were opened for this same device, each HANDLE corresponded to one FILE_OBJECT ptr.
There was no case in which 2 or more userspace HANDLEs shared the same FILE_OBJECT ptr.

"there will be multiple file handles per file object" - Are you sure this is a real case?

Thanks,
Sam
________________________________
From: Eitan Eliahu [eliahue at vmware.com]
Sent: Monday, September 15, 2014 6:50 PM
To: Samuel Ghinet; Nithin Raju
Cc: dev at openvswitch.org<mailto:dev at openvswitch.org>
Subject: RE: [ovs-dev] [PATCH] datapath-windows: cleanup dump state during instance cleanup
Each time that a device is opened a new file object is created with a corresponding handle. In case that a handle is duplicated in user mode there will be multiple file handles per file object associated with the handle returned by the system when the device was opened.
We usually open the device per socket. I wouldn’t count on that that I/O on a handle and the closing of a handle would be from the same thread context.
Thanks,
Eitan

From: Samuel Ghinet [mailto:sghinet at cloudbasesolutions.com]
Sent: Sunday, September 14, 2014 8:04 PM
To: Eitan Eliahu; Nithin Raju
Cc: dev at openvswitch.org<mailto:dev at openvswitch.org>
Subject: RE: [ovs-dev] [PATCH] datapath-windows: cleanup dump state during instance cleanup

I have a question here,
is a file HANDLE normally used by only one userspace thread at a time (so that if you have multiple threads, each thread will have its own unique file HANDLEs), or each thread may use the file HANDLEs opened by other threads?

Thanks,
Sam
________________________________
From: Eitan Eliahu [eliahue at vmware.com]
Sent: Saturday, September 13, 2014 12:01 AM
To: Nithin Raju
Cc: Samuel Ghinet; dev at openvswitch.org<mailto:dev at openvswitch.org>
Subject: RE: [ovs-dev] [PATCH] datapath-windows: cleanup dump state during instance cleanup
My understating is the Cleanup callback is called when the file handle is closed by user mode. However, at that time you can still have outstanding I/O requests in the driver or even synchronous I/O requests sent to the driver from a context of a different user mode thread. We do complete the outstanding queue IRPs but I could see a race condition here.

Do you really need to allocate memory to hold the Dump OVS message? Please note, that the memory associated with the IRP is always available until we complete the IRP. I am not sure we need to create a copy of it. If you need to hold status variables (e.g. dump index across multiple I/O dump requests) you can add them to the device instance itself. Also, it would be nice that each Dump request would be self-contained. I know it requires some user mode change (store the dump index in the socket structure).

Thanks,
Eitan

From: Nithin Raju
Sent: Friday, September 12, 2014 1:02 PM
To: Eitan Eliahu
Cc: Samuel Ghinet; dev at openvswitch.org<mailto:dev at openvswitch.org>
Subject: Re: [ovs-dev] [PATCH] datapath-windows: cleanup dump state during instance cleanup

On Sep 12, 2014, at 12:46 PM, Eitan Eliahu <eliahue at vmware.com<mailto:eliahue at vmware.com>>
 wrote:

Sorry for coming late on this one.
We should free the dump state when the system calls the driver on cleanup as you did. But, the cleanup IOCTL can be (actually will be) executed from a different thread context. This means that we need to protect  dumpState.ovsMsg.

By the time IRP_MJ_CLEANUP has been called, IRP_MJ_CLOSE has already been called. So, my understanding is that there's no scope for a userspace process to be calling into the kernel. Pls. let me know if I'm missing anything.

Thanks,
-- Nithin



More information about the dev mailing list