[ovs-dev] [PATCH] revalidator: Fix ukey stats cache updating.

Alex Wang alexw at nicira.com
Fri Apr 25 22:25:51 UTC 2014


Acked-by: Alex Wang <alexw at nicira.com>


And applied,


On Fri, Apr 25, 2014 at 3:12 PM, Alex Wang <alexw at nicira.com> wrote:

> 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 <alexw at 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 <alexw at 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 <joestringer at 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 <joestringer at 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
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140425/d91c7706/attachment-0005.html>


More information about the dev mailing list