From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Miller Subject: Re: [PATCH net-next] net: bridge: Lock before br_fdb_find() Date: Wed, 30 May 2018 12:42:15 -0400 (EDT) Message-ID: <20180530.124215.898586103229215718.davem@davemloft.net> References: Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, bridge@lists.linux-foundation.org To: petrm@mellanox.com Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: bridge-bounces@lists.linux-foundation.org Errors-To: bridge-bounces@lists.linux-foundation.org List-Id: netdev.vger.kernel.org From: Petr Machata Date: Mon, 28 May 2018 17:44:16 +0200 > Callers of br_fdb_find() need to hold the hash lock, which > br_fdb_find_port() doesn't do. Add the missing lock/unlock > pair. > > Signed-off-by: Petr Machata If all of the these uses of br_fdb_find_port() are safe, then it should use the RCU fdb lookup variant. So I basically agree with Stephen that this locking doesn't make any sense. The lock is needed when you are going to add or delete an FDB entry. Here we are doing a lookup and returning a device pointer via the FDB entry found in the lookup. The RTNL assertion assures that the device returned won't disappear. If the device can disappear, the spinlock added by this patch doesn't change that at all.