On 02/12/2018 12:58 PM, Han Zhou wrote:

On Mon, Feb 12, 2018 at 6:16 AM, Mark Michelson <mmich...@redhat.com <mailto:mmich...@redhat.com>> wrote:
 > On 02/09/2018 07:36 PM, Han Zhou wrote:
 >> Looks a great tool! Just some minor comments:
>> On Fri, Feb 9, 2018 at 3:00 PM, Mark Michelson <mmich...@redhat.com <mailto:mmich...@redhat.com> <mailto:mmich...@redhat.com <mailto:mmich...@redhat.com>>> wrote:
 >>  >
>>  > This modifies ovn-controller to measure the amount of time it takes to >>  > detect a change in the southbound database, generate the resulting flow
 >>  > table, and write the flow table down to vswitchd.
>> The comment is a little inaccurate. The change cannot guarantee measurement of "write the flow table down to vswitchd", since sending the flow-adding messages to vswitchd could be performed in multiple iterations of the main loop, depending on the buffering status of vconn_send(). If it can't be completed in one round, the message sendings will continue in next loops in ofctrl_run().
> Thanks for the review Han. I attempted to account for this in the code by only ending the sample in the case that ofctrl_can_put() returns true. This way, we might start a measurement in one loop iteration, but the measurement might not end until a later iteration if ofctrl_can_put() returns false. Do you have suggestions for other factors that I should take into account for determining when to end a measurement?

I think one way to address this, if we really want to, is to compare ofctrl_get_cur_cfg() with get_nb_cfg(ctx.ovnsb_idl). If they are equal, it means ovs-vswitchd caught up with SB with all required flows installed. I suggest to have separate measurements, one for main loop execution time, one for SB change handling up until flows installed to OVS. They could be separate patches. What do you think?

That sounds like a good approach. Thanks for the feedback!
dev mailing list

Reply via email to