I think this was the crash that sunk us last time. We're in a really
good position at this point. The automated test runner is pretty
stable and I'm getting some really nice consistent numbers. I should
have some graphs shortly.
Also btw with the perf stuff. Since the NSDI deadline the datapath
classifier has been replaced by a completely new implementation. This
means it's pretty easy to disambiguate the function calls between
action internals, slow path classification, and fast path
classification. Later I'll send you an initial analysis of the
coalesced vs non-coalesced version and you can tell me what you think.
Ethan
On Sat, Jan 3, 2015 at 2:53 PM, Ethan Jackson <et...@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 <et...@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@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
