Thanks for the review! Applied to master and branch-2.5
On 04/05/2016 11:28, "Jarno Rajahalme" <ja...@ovn.org> wrote:
>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
