On Mon, Jun 23, 2014 at 11:44 AM, Ben Pfaff <b...@nicira.com> wrote: > On Fri, Jun 13, 2014 at 07:40:01AM -0700, Gurucharan Shetty wrote: >> After the change, both of them compile. test-netflow related >> unit tests pass. >> >> test-sflow related tests do not pass because >> of LOOPBACK_INTERFACE constraints for 'agent'. >> (It should be revisited later.) >> >> Signed-off-by: Gurucharan Shetty <gshe...@nicira.com> > > This patch kind of stands out to me when I look at it. It is awkward > to have to create an extra object (wevent) on Windows but not use it > anywhere except to pass to poll_fd_wait_event(). I didn't properly > understand until now that this was necessary. > > Looking through the tree, I think that there are two or three distinct > uses of wevents: > > 1. Some bits of code, like the ones added in this patch, don't > care about the wevent directly at all. Instead, they > really just want to wait on a socket. These pieces of code > still have to create wevents and pass them around and > eventually destroy them. This is the majority of use > cases. > > 2. Other bits of code are oriented around wevents and do use > them directly with calls to SetEvent(), ResetEvent, etc., > and do not associate them with sockets. One example is > daemon-windows.c. > > 3. fatal-signal.c appears to be a hybrid of these two cases, > but I think that this could be replaced by use of a latch. > > Cases #1 and #2 are different enough that I wonder whether poll_loop > should handle them differently. > > In case #1, the emphasis should be on convenience for the client. Can > we make poll_loop automatically create a wevent when we poll a socket? > (If this is expensive, then perhaps it could maintain a pool of > wevents.) Since sockets are different from fds on Windows, maybe it > would be best to have a function poll_loop_socket_wait() to make it > clear that a socket is being polled. I incorporated your suggestions with one difference. I did not rename the function to poll_loop_socket_wait(). As that would create a total of 3 functions for waiting on file descriptors. 1. Linux fds of pipes 2. Linux and Windows sockets 3. Windows events.
If you still prefer a separate function, I will re-spin. I sent in the patch for review. Once that looks good, I will post another version of this patch (tests: Port test-sflow and test-netflow to windows.) Thanks, Guru > > Case #2 is Windows-only and so we could use a Windows-specific poll > loop function to wait on those events. > > Does this make sense? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
