Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
> On May 3, 2016, at 6:10 PM, Daniele Di Proietto <diproiet...@vmware.com>
> wrote:
>
> After removing a flow from the dpcls classifier there might still be
> readers who have access to the flow, until the next grace period.
>
> Setting flow->cr.mask to NULL can cause concurrent readers to crash,
> so this commit avoids doing it.
>
> The crash can be reproduced, for example, by invoking an operation
> that cause datapath flows to be deleted (such as `ovs-appctl
> upcall/enable-megaflows`) while traffic is running.
>
> I think the assignment was intended just as a safety measure to catch
> race conditions, and it should be safe to remove.
>
> Here's a stack trace of a possible crash:
>
> Program terminated with signal SIGSEGV, Segmentation fault.
> #0 0x00000000005ecfe7 in dpcls_rule_matches_key (target=0x7f3afa3f79d0,
> rule=0x7f3ae8006190) at ../lib/dpif-netdev.c:4156
> 4156 if (OVS_UNLIKELY((value & *maskp++) != *keyp++)) {
> (gdb) bt
> #0 0x00000000005ecfe7 in dpcls_rule_matches_key (target=0x7f3afa3f79d0,
> rule=0x7f3ae8006190) at ../lib/dpif-netdev.c:4156
> #1 dpcls_lookup (cls=<optimized out>, keys=0x7f3afa3f6428,
> rules=0x7f3afa3f2e40, cnt=<optimized out>) at ../lib/dpif-netdev.c:4225
> #2 0x000000000040edc3 in fast_path_processing
> (pmd=pmd@entry=0x7f3afa3fc010, packets=packets@entry=0x7f3afa3fa420,
> cnt=cnt@entry=32, keys=keys@entry=0x7f3afa3f6428,
> batches=batches@entry=0x7f3afa3f4118,
> n_batches=n_batches@entry=0x7f3afa3fa3b0)
> at ../lib/dpif-netdev.c:3483
> #3 0x00000000005f0413 in dp_netdev_input__
> (pmd=pmd@entry=0x7f3afa3fc010, packets=packets@entry=0x7f3afa3fa420,
> cnt=<optimized out>, md_is_valid=md_is_valid@entry=false,
> port_no=<optimized out>) at ../lib/dpif-netdev.c:3625
> #4 0x00000000005f0501 in dp_netdev_input (port_no=<optimized out>,
> cnt=<optimized out>, packets=0x7f3afa3fa420, pmd=0x7f3afa3fc010) at
> ../lib/dpif-netdev.c:3642
> #5 dp_netdev_process_rxq_port (pmd=pmd@entry=0x7f3afa3fc010,
> rxq=<optimized out>, port=<optimized out>, port=<optimized out>) at
> ../lib/dpif-netdev.c:2574
> #6 0x00000000005f0807 in pmd_thread_main (f_=0x7f3afa3fc010) at
> ../lib/dpif-netdev.c:2693
> #7 0x000000000067d422 in ovsthread_wrapper (aux_=<optimized out>) at
> ../lib/ovs-thread.c:340
> #8 0x00007f3b40ec4182 in start_thread (arg=0x7f3afa3fb700) at
> pthread_create.c:312
> #9 0x00007f3b406e347d in clone () at
> ../sysdeps/unix/sysv/linux/x86_64/clone.S:111
>
> Fixes: 361d808dd9e4("flow: Split miniflow's map.")
> CC: Jarno Rajahalme <ja...@ovn.org>
> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
> ---
> lib/dpif-netdev.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 1e8a37c..d288f45 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -1444,8 +1444,6 @@ dp_netdev_pmd_remove_flow(struct dp_netdev_pmd_thread
> *pmd,
> struct cmap_node *node = CONST_CAST(struct cmap_node *, &flow->node);
>
> dpcls_remove(&pmd->cls, &flow->cr);
> - flow->cr.mask = NULL; /* Accessing rule's mask after this is not safe.
> */
> -
> cmap_remove(&pmd->flow_table, node, dp_netdev_flow_hash(&flow->ufid));
> flow->dead = true;
>
> --
> 2.1.4
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
