[ovs-dev] [PATCH V2 3/3] ofproto-dpif-monitor: Move ofproto-dpif-monitor to a single thread.

[Alex Wang]

On Mon, Sep 30, 2013 at 3:16 PM, Ethan Jackson <ethan at nicira.com> wrote:

> Why do we need the bfd->last_tx change?  Should it be in a different
> patch with a commit message explaining it?
>


I'll send a separate patch for it.



> As discussed off list, the monitor_seq thing is a layering violation.
> Instead we should have a separate patch which causes time warp to wake
> up threads.
>


Yes, I'll send a separate patch for it.


I don't really like the function name monitor_handler().  It's not
> really handling anything in the same sense that the miss_handler is.
> I think we should rename the function interface_monitor(), and
> set_subprogram_name("interface_monitor") as well.
>
> It's probably cleaner to take the monitor_rwlock once and hold it over
> monitor_run() and monitor_wait().
>


Sure, I can modify the code.



> I don't really like how we're handling the thread creation and
> deletion here.   What if removed monitor_check and folded it's code
> into mport_update() after we've made the register/unregister calls?
> That way we could get rid of the rather add requirement to heap
> allocate the tid.  Also it would make the API more straight forward.
> One couldn't forget to start/stop the threads.
>


Yes, I originally worried about the behavior of creating a thread while
holding the xlate_rwlock & monitor_rwlock.  After check with Ben, I'm good
to modify it.



> Theres a redundant "o" at the end of one of changed unit test lines.
>


I'll remove it.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20131001/e839461f/attachment-0003.html>