From mboxrd@z Thu Jan 1 00:00:00 1970 From: Petr Machata Subject: Re: [PATCH net-next] net: bridge: Lock before br_fdb_find() Date: Thu, 31 May 2018 01:03:05 +0300 Message-ID: References: <20180530.124215.898586103229215718.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain Cc: , , To: David Miller Return-path: Received: from mail-eopbgr40044.outbound.protection.outlook.com ([40.107.4.44]:24474 "EHLO EUR03-DB5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753484AbeE3WD0 (ORCPT ); Wed, 30 May 2018 18:03:26 -0400 In-Reply-To: <20180530.124215.898586103229215718.davem@davemloft.net> (David Miller's message of "Wed, 30 May 2018 12:42:15 -0400") Sender: netdev-owner@vger.kernel.org List-ID: David Miller writes: > 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. OK, I'll take another look at this. Thanks, Petr