[ovs-dev] [PATCH] dpif-netdev: Fix rare flow add race condition.

[Jarno Rajahalme]

Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>

Thanks for the fix and happy new year!

  Jarno

On Jan 3, 2015, at 11:53 AM, Ethan Jackson <ethan at nicira.com> wrote:

> Before this patch, dp_netdev_flow_add() inserted newly minted flows in
> the "flow_table" cmap before inserting them into the per core "dpcls"
> classifier.  Since dpcls_insert() initializes 'flow->cr.mask', there's
> a brief window where the flow is accessible from the cmap, but has a
> bogus mask value.
> 
> In my testing, under rare instances (i.e. once every 20 minutes with a
> very specific flow table and traffic pattern), revalidators core dump
> when they call dpif_netdev_flow_dump_next(), which accesses this bogus
> mask value from dp_netdev_flow_to_dpif_flow().
> 
> By inserting into the per core classifier before the cmap, all the
> values are guaranteed to be initialized during flow dumps.  With this
> patch, I can no longer reproduce the crash.
> 
> Signed-off-by: Ethan Jackson <ethan at nicira.com>
> ---
> lib/dpif-netdev.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 6158bf9..166ba4b 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -1743,12 +1743,12 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,
>     ovs_refcount_init(&flow->ref_cnt);
>     ovsrcu_set(&flow->actions, dp_netdev_actions_create(actions, actions_len));
> 
> -    cmap_insert(&pmd->flow_table,
> -                CONST_CAST(struct cmap_node *, &flow->node),
> -                dp_netdev_flow_hash(&flow->ufid));
>     netdev_flow_key_init_masked(&flow->cr.flow, &match->flow, &mask);
>     dpcls_insert(&pmd->cls, &flow->cr, &mask);
> 
> +    cmap_insert(&pmd->flow_table, CONST_CAST(struct cmap_node *, &flow->node),
> +                dp_netdev_flow_hash(&flow->ufid));
> +
>     if (OVS_UNLIKELY(VLOG_IS_DBG_ENABLED())) {
>         struct match match;
>         struct ds ds = DS_EMPTY_INITIALIZER;
> -- 
> 1.9.1
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev