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
