On Tue, Feb 25, 2014 at 05:02:02PM -0800, Joe Stringer wrote:
> A flow's statistics could be partially lost in the following situation:
> * A flow is dumped from the datapath
> * Some traffic hits that flow
> * Revalidation fails for that flow
> 
> Previously, we would delete such a flow without fetching the latest
> statistics for it, causing the intermediate statistics to be lost. This
> patch fixes the bug by queueing the flow up for deletion and cleanup
> along with the other flows that will be deleted.
> 
> Signed-off-by: Joe Stringer <joestrin...@nicira.com>
> ---
>  ofproto/ofproto-dpif-upcall.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index e4f81a1..3478444 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -1534,8 +1534,9 @@ revalidate_udumps(struct revalidator *revalidator, 
> struct list *udumps)
>          ukey->mark = true;
>  
>          if (!revalidate_ukey(udpif, udump, ukey)) {
> -            dpif_flow_del(udpif->dpif, udump->key, udump->key_len, NULL);
> -            ukey_delete(revalidator, ukey);
> +            struct dump_op *dop = &ops[n_ops++];
> +
> +            dump_op_init(dop, udump->key, udump->key_len, ukey, udump);
>          }
>  
>          list_remove(&udump->list_node);

Is this safe?  In the other case where we do dump_op_init() in this
loop we "continue", which avoids the list_remove() and free() at the
end of the loop, and then do both of those in the secondary loop at
the end of the function.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to