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.

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

Reply via email to