On 31.01.2019 11:48, Lilijun wrote: > This patch fix the dead lock when using dpdk userspace datapath. The > problem is described as follows: > 1) when add or delete port, the main thread will call > reconfigure_datapath() in the function dpif_netdev_run() > 2) Here the dp->port_mutex is locked. In dp_netdev_reload_pmd__(), it > will notify each pmd to reload. > 3) If pmd is doing packet upcall in fast_path_processing() and try to get > dp->port_mutex in > do_xlate_actions()-->tnl_route_lookup_flow()-->dpif_netdev_port_query_by_name(). > Here pmd get lock failed because the main thread has got the lock in > step 2. > 4) So the main thread was stuck for waiting pmd to reload done. Now we > got a dead lock. > > Here reload_affected_pmds() may not need to lock dp->port_mutex. So we > release the lock temporarily when calling reload_affected_pmds(). > > Signed-off-by: Lilijun <jerry.lili...@huawei.com>
Replying just to keep answers on a list/patchwork consistent. The deadlock caused by some local changes done by the user. Not possible in upstream master. See discussion for the previous version of this patch that is missing in patchwork for some reason: https://mail.openvswitch.org/pipermail/ovs-dev/2019-January/355756.html Best regards, Ilya Maximets. > --- > lib/dpif-netdev.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 0f57e3f8a..fc3bdae66 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -4813,8 +4813,11 @@ reconfigure_datapath(struct dp_netdev *dp) > > /* Reload affected pmd threads. We must wait for the pmd threads before > * reconfiguring the ports, because a port cannot be reconfigured while > - * it's being used. */ > + * it's being used. We need release dp->port_mutex to make sure that pmds > + * don't wait for getting the mutex when handling packet upcalls */ > + ovs_mutex_unlock(&dp->port_mutex); > reload_affected_pmds(dp); > + ovs_mutex_lock(&dp->port_mutex); > > /* Step 3: Reconfigure ports. */ > > @@ -4877,7 +4880,9 @@ reconfigure_datapath(struct dp_netdev *dp) > /* Reload affected pmd threads. We must wait for the pmd threads to > remove > * the old queues before readding them, otherwise a queue can be polled > by > * two threads at the same time. */ > + ovs_mutex_unlock(&dp->port_mutex); > reload_affected_pmds(dp); > + ovs_mutex_lock(&dp->port_mutex); > > /* Step 6: Add queues from scheduling, if they're not there already. */ > HMAP_FOR_EACH (port, node, &dp->ports) { > @@ -4909,7 +4914,9 @@ reconfigure_datapath(struct dp_netdev *dp) > } > > /* Reload affected pmd threads. */ > + ovs_mutex_unlock(&dp->port_mutex); > reload_affected_pmds(dp); > + ovs_mutex_lock(&dp->port_mutex); > > /* Check if PMD Auto LB is to be enabled */ > set_pmd_auto_lb(dp); > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
