It is even harder to reproduce the new failure, rarer than the flow-dumps
one. Just ran into this once in my life...
More importantly, the flow-dumps pkt counts are sync'ed in the failed case.
>From my understanding, this new issue, relates to reading stats from
netdev-dummy, which is different
area from this fix.
So, this change makes sense to me. I'll apply it first,
On Fri, Apr 25, 2014 at 8:10 AM, Alex Wang <al...@nicira.com> wrote:
> found this in the morning, will keep investigating,
>
>
> ./learn.at:362: (ovs-ofctl dump-ports br0 2; ovs-ofctl dump-ports br0 3)
> | sed 's/ (xid=0x[0-9a-fA-F]*)//'
>
> --- - 2014-04-25 01:25:43.022048477 -0700
>
> +++ /root/openvswitch/tests/testsuite.dir/at-groups/328/stdout 2014-04-25
> 01:25:43.017711899 -0700
>
> @@ -1,7 +1,7 @@
>
> OFPST_PORT reply: 1 ports
>
> port 2: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
>
> - tx pkts=2, bytes=120, drop=0, errs=0, coll=0
>
> + tx pkts=6, bytes=360, drop=0, errs=0, coll=0
>
> OFPST_PORT reply: 1 ports
>
> port 3: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
>
> - tx pkts=18, bytes=1080, drop=0, errs=0, coll=0
>
> + tx pkts=14, bytes=840, drop=0, errs=0, coll=0
>
>
> On Thu, Apr 24, 2014 at 10:34 PM, Alex Wang <al...@nicira.com> wrote:
>
>> i'll kick an overnight run on "learning action - self-modifying flow
>> with idle_timeout".~ (seq 10000). if it survives, tmr morning,
>> i'll ack it~~
>>
>>
>> On Thu, Apr 24, 2014 at 10:26 PM, Joe Stringer <joestrin...@nicira.com>wrote:
>>
>>> revalidate_ukey() had a bug where it would update the ukey->stats even
>>> if it decided not to push stats (as an optimisation). ukey->stats should
>>> only be updated when those stats are pushed.
>>>
>>> This bug would arise in the following situation:
>>> * A flow has been dumped before.
>>> * The flow needs to be revalidated.
>>> * The flow is low-throughput.
>>> * The flow has new statistics to push.
>>>
>>> Such cases rely on flow deletion to update the stats. However, that code
>>> pushes the delta between the ukey->stats and the final flow dump. If the
>>> ukey stats cache is updated without the stats being pushed, those stats
>>> would be lost.
>>>
>>> This caused intermittent testsuite failures on "learning action -
>>> self-modifying flow with idle_timeout". Introduced by 698ffe3623f1b630ae
>>> "revalidator: Only revalidate high-throughput flows."
>>>
>>> Bug #1238927.
>>>
>>> Signed-off-by: Joe Stringer <joestrin...@nicira.com>
>>> ---
>>> ofproto/ofproto-dpif-upcall.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/ofproto/ofproto-dpif-upcall.c
>>> b/ofproto/ofproto-dpif-upcall.c
>>> index 84afc56..5871772 100644
>>> --- a/ofproto/ofproto-dpif-upcall.c
>>> +++ b/ofproto/ofproto-dpif-upcall.c
>>> @@ -1245,7 +1245,6 @@ revalidate_ukey(struct udpif *udpif, struct
>>> udpif_key *ukey,
>>> push.n_bytes = stats->n_bytes > ukey->stats.n_bytes
>>> ? stats->n_bytes - ukey->stats.n_bytes
>>> : 0;
>>> - ukey->stats = *stats;
>>>
>>> if (!ukey->flow_exists) {
>>> /* Don't bother revalidating if the flow was already deleted. */
>>> @@ -1258,6 +1257,8 @@ revalidate_ukey(struct udpif *udpif, struct
>>> udpif_key *ukey,
>>> goto exit;
>>> }
>>>
>>> + /* We will push the stats, so update the ukey stats cache. */
>>> + ukey->stats = *stats;
>>> if (!push.n_packets && !udpif->need_revalidate) {
>>> ok = true;
>>> goto exit;
>>> --
>>> 1.7.10.4
>>>
>>>
>>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
