From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Miller Subject: Re: [PATCH net] vxlan: add necessary locking on device removal Date: Tue, 16 Jul 2013 11:28:20 -0700 (PDT) Message-ID: <20130716.112820.260966128083756999.davem@davemloft.net> References: <20130713101818.0d4fa8e0@nehalam.linuxnetplumber.net> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: stephen@networkplumber.org, netdev@vger.kernel.org To: pshelar@nicira.com Return-path: Received: from shards.monkeyblade.net ([149.20.54.216]:38669 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932958Ab3GPS2W (ORCPT ); Tue, 16 Jul 2013 14:28:22 -0400 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: From: Pravin Shelar Date: Sat, 13 Jul 2013 12:21:52 -0700 > On Sat, Jul 13, 2013 at 10:18 AM, Stephen Hemminger > wrote: >> The socket management is now done in workqueue (outside of RTNL) >> and protected by vn->sock_lock. There were two possible bugs, first >> the vxlan device was removed from the VNI hash table per socket without >> holding lock. And there was a race when device is created and the workqueue >> could run after deletion. >> >> Signed-off-by: Stephen Hemminger >> >> --- a/drivers/net/vxlan.c 2013-07-08 16:31:50.080744429 -0700 >> +++ b/drivers/net/vxlan.c 2013-07-10 20:15:47.337653899 -0700 >> @@ -1767,9 +1767,15 @@ static int vxlan_newlink(struct net *net >> >> static void vxlan_dellink(struct net_device *dev, struct list_head *head) >> { >> + struct vxlan_net *vn = net_generic(dev_net(dev), vxlan_net_id); >> struct vxlan_dev *vxlan = netdev_priv(dev); >> >> + flush_workqueue(vxlan_wq); >> + > Doesn't this create dependency on sock_work thread while holding RTNL. > If so it can result in deadlock. What exact deadlock do you perceive? I don't see any code path in the sock_work handler (vxlan_sock_work) which takes the RTNL mutex. So we should be able to safely flush any pending sock_work jobs from vxlan_dellink(). The fact that vxlan_dellink() runs with the RTNL mutex shouldn't cause any issues.