[ovs-dev] [PATCH 1/1] netflow: Modify netflow_flow_clear() to do netflow_expire__

[Anoob Soman]

Sorry, I should have made it clear in the commit message. In our testing, occasionally ovs_assert(!nf_flow->packet_count) would fail, with the following backtrace 

#0  0x00007f7d263ad265 in raise () from /lib64/libc.so.6
#1  0x00007f7d263aed10 in abort () from /lib64/libc.so.6
#2  0x00000000004a3b1e in ovs_abort_valist (err_no=<value optimized out>, format=<value optimized out>, args=<value optimized out>) at lib/util.c:245
#3  0x00000000004a8e78 in vlog_abort_valist (module_=<value optimized out>, message=0x4f9410 "%s: assertion %s failed in %s()", args=0x42e3cfe0) at lib/vlog.c:992
#4  0x00000000004a8f06 in vlog_abort (module=0x10a7, message=0x1192 <Address 0x1192 out of bounds>) at lib/vlog.c:1006
#5  0x00000000004a3dab in ovs_assert_failure (where=0x6 <Address 0x6 out of bounds>, function=0x80 <Address 0x80 out of bounds>, 
    condition=0xffffffffffffffff <Address 0xffffffffffffffff out of bounds>) at lib/util.c:68
#6  0x0000000000437db6 in netflow_flow_clear (nf=0x8722a0, flow=0x42e3ee20) at ofproto/netflow.c:299
#7  0x000000000042c6fb in revalidate_udumps (arg=0x7a4fe0) at ofproto/ofproto-dpif-upcall.c:1391
#8  udpif_revalidator (arg=0x7a4fe0) at ofproto/ofproto-dpif-upcall.c:724
#9  0x00007f7d2616873d in __free_tcb () from /lib64/libpthread.so.0

Though netflow_flow_clear() and netflow_expire() are always called together, there is a slight chance that the netflow stats are updated between the two calls. 

With my patch, netflow_expire() is dead code (no one calls it). But I have retained it for future use, if someone wants to expire netflow, without explicitly taking locks. 

Thanks,
Anoob.
________________________________________
From: Ben Pfaff [blp at nicira.com]
Sent: 20 May 2014 19:25
To: Anoob Soman
Cc: dev at openvswitch.org
Subject: Re: [ovs-dev] [PATCH 1/1] netflow: Modify netflow_flow_clear() to do netflow_expire__

On Tue, May 20, 2014 at 12:40:35PM +0100, Anoob Soman wrote:
> This avoids causing failed assert(), when netflow_flow_clear() and
> netflow_expire() are called together.
>
> Signed-off-by: Anoob Soman <anoob.soman at citrix.com>

I broke out the fix to the spelling of your name in AUTHORS (my fault,
sorry!) and pushed that.

Can you explain a little more about the remainder of the patch?  Does
the assertion trigger in practice (we have not received reports of
problems)?  Can you explain the code flow that triggers it?

Thanks,

Ben.