[ovs-dev] [PATCH V2] bridge: Allow users to configure statistics update to OVSDB.

[Alex Wang]

On Thu, May 1, 2014 at 7:21 PM, Joe Stringer <joestringer at nicira.com> wrote:

> On 30 April 2014 12:46, Alex Wang <alexw at nicira.com> wrote:
>>
>> +dnl set the stats update interval to 100K ms, the following 'recv'
>> should not be updated.
>> +AT_CHECK([ovs-vsctl set O . other_config:stats-update-interval=100000])
>> +for i in `seq 0 50`; do ovs-appctl time/warp 1000; done
>> +for i in `seq 1 5`; do
>> +    AT_CHECK([ovs-appctl netdev-dummy/receive p1
>> 'eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
>> +done
>> +
>> +dnl advance the clock by 100K ms, the previous 'recv' should be updated.
>> +for i in `seq 0 100`; do ovs-appctl time/warp 1000; done
>> +OVS_VSCTL_CHECK_RX_PKT([p1], [6])
>>
>
> Does there need to be an OVS_VSCTL_CHECK_RX_PKT() command in between these
> two snippets? I don't see a check to show that it's not updated during the
> time 5000ms -> 100000ms after the setting changed.  (only a check to make
> sure it is updated after 100000ms).
>


I think it is not necessary to check in every iteration for the change.
 Also, it may introduce race if we check the timing too closely.

So, I'm more inclined to add an additional check in the middle. like below:

diff --git a/tests/ovs-vswitchd.at b/tests/ovs-vswitchd.at
index 17db549..5cbdf07 100644
--- a/tests/ovs-vswitchd.at
+++ b/tests/ovs-vswitchd.at
@@ -54,7 +54,9 @@ for i in `seq 1 5`; do
 done

 dnl advance the clock by 100K ms, the previous 'recv' should be updated.
-for i in `seq 0 100`; do ovs-appctl time/warp 1000; done
+for i in `seq 0 49`; do ovs-appctl time/warp 1000; done
+OVS_VSCTL_CHECK_RX_PKT([p1], [1])
+for i in `seq 0 49`; do ovs-appctl time/warp 1000; done
 OVS_VSCTL_CHECK_RX_PKT([p1], [6])



>  +      <column name="other_config" key="stats-update-interval"
>> +              type='{"type": "integer", "minInteger": 5000}'>
>> +        <p>
>> +          Period of statistics update to database, in milliseconds.
>>
>
> Perhaps a bit more natural to say "Interval for updating statistics to the
> database, in milliseconds".
>


Thx, I'll fold this in,



>
>  +        Key-value pairs that report port statistics.  The update period
>> +        is controlled by <ref column="other_config"
>> +        key="stats-update-interval"/> of <code>Open_vSwitch</code> table.
>>
>
> "of" -> "in the". (Also for interface and mirror)
>


Thx, I'll fold this in,



>
>
> Acked-by: Joe Stringer <joestringer at nicira.com>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140502/b9cf12cf/attachment-0005.html>