On Mon, Sep 30, 2013 at 3:16 PM, Ethan Jackson <et...@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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
