Currently we have very minimal testing. I wanted to send the patch to get feedback and I got it. ;-)
I can hold off the push till we are ready for the make check and other unit testing efforts. You have some other suggestions? Thanks, Linda Sent from my iPad > On Dec 6, 2013, at 11:10 PM, Guolin Yang <[email protected]> wrote: > > is it ready for checkin? how much test did we do? > > >> On Fri, Dec 6, 2013 at 5:01 PM, Ethan Jackson <[email protected]> wrote: >> I'm not planning to review this patch, but could you please reformat >> the commit message in a more standard way? I.E. something like: >> >> poll-loop: <short description>\n >> \n >> <longer description> >> >> The current formatting doesn't play nicely with git send email. >> >> Ethan >> >> On Fri, Dec 6, 2013 at 4:41 PM, Linda Sun <[email protected]> wrote: >> > Can you elaborate which cases have duplicate fds? >> > >> > I've already used closesocket instead of close handle for sockets. I'll >> > send a separate patch with the stream related changes. E.g. Replace >> > read/write on socket with send/recv. >> > >> > Linda >> > ----- Original Message ----- >> > From: Eitan Eliahu <[email protected]> >> > To: Ben Pfaff <[email protected]> >> > Cc: [email protected], Linda Sun <[email protected]> >> > Sent: Fri, 06 Dec 2013 16:18:51 -0800 (PST) >> > Subject: Re: [ovs-dev] [PATCH] Use WaitForMultipleObjects for polling on >> > windows. This works on all kinds of objects, e.g. sockets, files, >> > especially ioctl calls to the kernel. One additional paramater is passed >> > down in poll_fd_wait() to help WaitForMultipleObjects. latch is signaled >> > with event, to be waited/polled by WaitForMultipleObjects() as well. >> > >> > >> > Ben and Linda, >> > It seems that the first two comments could be addressed (by removing NULL >> > events and duplicates handles. >> > >> > In regards to the third comments, if these handles are Sockets (which I >> > believe they are) these handles should never be closed by calling >> > CloseHandle(). For these socket handles closesocket() should be used and >> > therefore the resources associated with these handles will be freed and >> > WaitForMultipleObjects will return with an error. >> > >> > Thanks, >> > Eitan >> > >> > >> > >> > ----- Original Message ----- >> > From: "Ben Pfaff" <[email protected]> >> > To: "Linda Sun" <[email protected]> >> > Cc: [email protected] >> > Sent: Friday, December 6, 2013 3:57:03 PM >> > Subject: Re: [ovs-dev] [PATCH] Use WaitForMultipleObjects for polling on >> > windows. This works on all kinds of objects, e.g. sockets, files, >> > especially ioctl calls to the kernel. One additional paramater is passed >> > down in poll_fd_wait() to help WaitForMultipleObjects. latch is signaled >> > with event, to be waited/polled by WaitForMultipleObjects() as well. >> > >> > On Fri, Dec 06, 2013 at 02:47:29PM -0800, Linda Sun wrote: >> >> Signed-off-by: Linda Sun <[email protected]> >> > >> > Hi Linda. Thanks for sending this along so that we can start figuring >> > out how to work Windows into the poll loop. >> > >> > Looking at the documentation for WaitForMultipleObjects, I see a few >> > possible issues. >> > >> > One is that it looks like we add a lot of null pointers to the wevents >> > array (most of the poll_fd_wait() calls pass NULL). I don't see >> > documentation for what WaitForMultipleObjects does with null pointers. >> > >> > WaitForMultipleObjects also forbids duplicates in its handles array. >> > OVS calls poll with duplicates sometimes. >> > >> > WaitForMultipleObjects documentation says: "If one of these handles is >> > closed while the wait is still pending, the function's behavior is >> > undefined." OVS might sometimes close a file descriptor from one >> > thread that some other thread is waiting for, since Unix has >> > well-defined semantics for that. I'm not sure how to audit the code >> > for this, or what the consequences for getting it wrong are. >> > ("Undefined" sounds bad, but what happens in practice?) >> > >> > Thanks, >> > >> > Ben. >> > _______________________________________________ >> > 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=7pHyuyR919nucNKNtulWpl9q3u58pd6CW4vv29IF5NY%3D%0A&s=3708003efa1c031072b6af10a1c385a2328f4c5f5a6732040e544b86cb0194dd >> > _______________________________________________ >> > dev mailing list >> > [email protected] >> > http://openvswitch.org/mailman/listinfo/dev >> _______________________________________________ >> dev mailing list >> [email protected] >> http://openvswitch.org/mailman/listinfo/dev >
_______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
