On Mon, Mar 14, 2011 at 11:48:17AM -0700, Andrew Evans wrote: > On Mar 14, 2011, at 10:11 AM, Ben Pfaff wrote: > > I think that the function-level comment on > > reconnect_get_last_connect_elapsed() is incorrect: if 'fsm' connected, > > then the number of milliseconds since 'fsm' was last connected is 0, > > but this function won't necessarily return that. Maybe "the number of > > milliseconds since 'fsm' last successfully connected to its peer" or > > something else that indicates that it's the edge disconnected -> > > connected that we're tracking? > > > > Same feedback for reconnect_get_last_disconnect_elapsed(). > > I agree, that should be explained more clearly. How about this?
Looks good. Thank you. > > In update_remote_row(), I would have expected the key/value pairs for > > sec_since_connect and sec_since_disconnect to be omitted entirely, > > instead of being set to empty values, if there have been no > > connections or disconnections. > > I agree. Pete says he can live with it either way, but he thinks > it's "more self-documenting" if the keys always exist. I think he > has a point. So which way do you want to go, then? I don't care enough to bother arguing strenuously either way, so just pick whatever you think is best. > > Did you carefully look through the updated test results to make sure > > that they make sense? I did for an earlier version of this patch, but > > I didn't yet for this version. If you haven't, let me know, and I > > will. > > I did pore over them carefully, since I made pretty major changes to > the way connect and disconnect events get reported. I spent quite a > bit of time making sure that the results were what I expected. It > certainly wouldn't hurt to have someone else double-check, > though. In general I made the reconnect tests not report information > about connect or disconnect times unless a connect or disconnect > event had occurred in that cycle, or if the times weren't what were > expected (last time plus delta). I'm happy enough just hearing that you were careful. I'm satisfied with this patch now. Thanks again. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
