| Message ID | 20170427010312.18249-1-joe@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, Apr 26, 2017 at 06:03:11PM -0700, Joe Stringer wrote: > There is a case where a flow is dumped from the kernel after the ukey is > already transitioned into an EVICTING/EVICTED/DELETED state, and the > revalidator thread attempts to shift that into UKEY_OPERATIONAL because > it was able to dump the flow from the datapath. This resulted in > triggering the assert in transition_ukey(). Detect this condition and > skip handling the flow (as it's already on its way out). > > Users report: > > Program terminated with signal SIGABRT, Aborted. > > raise () from /lib/x86_64-linux-gnu/libc.so.6 > > raise () from /lib/x86_64-linux-gnu/libc.so.6 > > abort () from /lib/x86_64-linux-gnu/libc.so.6 > > ovs_abort_valist > > vlog_abort_valist > > vlog_abort > > ovs_assert_failure > > transition_ukey (ukey=<optimized out>, dst=<optimized out>) > > at ofproto/ofproto-dpif-upcall.c:1674 > > revalidate (revalidator=0x1cb36c8) at ofproto/ofproto-dpif-upcall.c:2324 > > udpif_revalidator (arg=0x1cb36c8) at ofproto/ofproto-dpif-upcall.c:901 > > ovsthread_wrapper (aux_=<optimized out>) at lib/ovs-thread.c:348 > > start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0 > > clone () from /lib/x86_64-linux-gnu/libc.so.6 > > VMware-BZ: #1857694 > Signed-off-by: Joe Stringer <joe@ovn.org> Thank you for figuring this out. I doubt it was easy. Acked-by: Ben Pfaff <blp@ovn.org>
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 3b28f9a22939..ccf15a3c80b3 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -2323,8 +2323,13 @@ revalidate(struct revalidator *revalidator) continue; } - /* The flow is now confirmed to be in the datapath. */ - transition_ukey(ukey, UKEY_OPERATIONAL); + if (ukey->state <= UKEY_OPERATIONAL) { + /* The flow is now confirmed to be in the datapath. */ + transition_ukey(ukey, UKEY_OPERATIONAL); + } else { + ovs_mutex_unlock(&ukey->mutex); + continue; + } if (!used) { used = ukey->created;