netdev - Re: [PATCH net-next 1/1] tipc: remove interface state mirroring in bearer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20131209.203124.480496339513116139.davem@davemloft.net>
Date:	Mon, 09 Dec 2013 20:31:24 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	jon.maloy@...csson.com
Cc:	netdev@...r.kernel.org, paul.gortmaker@...driver.com,
	erik.hugne@...csson.com, ying.xue@...driver.com, maloy@...jonn.com,
	tipc-discussion@...ts.sourceforge.net
Subject: Re: [PATCH net-next 1/1] tipc: remove interface state mirroring in
 bearer

From: Jon Maloy <jon.maloy@...csson.com>
Date: Fri,  6 Dec 2013 10:08:00 -0500

> From: Erik Hugne <erik.hugne@...csson.com>
> 
> struct 'tipc_bearer' is a generic representation of the underlying
> media type, and exists in a one-to-one relationship to each interface
> TIPC is using. The struct contains a 'blocked' flag that mirrors the
> operational and execution state of the represented interface, and is
> updated through notification calls from the latter. The users of
> tipc_bearer are checking this flag before each attempt to send a
> packet via the interface.
> 
> This state mirroring serves no purpose in the current code base. TIPC
> links will not discover a media failure any faster through this
> mechanism, and in reality the flag only adds overhead at packet
> sending and reception.
> 
> Furthermore, the fact that the flag needs to be protected by a spinlock
> aggregated into tipc_bearer has turned out to cause a serious and
> completely unnecessary deadlock problem.
 ...
> I.e., del_timer_sync() on CPU0 never returns, because the timer handler
> on CPU1 is waiting for the bearer lock.
> 
> We eliminate the 'blocked' flag from struct tipc_bearer, along with all
> tests on this flag. This not only resolves the deadlock, but also
> simplifies and speeds up the data path execution of TIPC. It also fits
> well into our ongoing effort to make the locking policy simpler and
> more manageable.
> 
> An effect of this change is that we can get rid of functions such as
> tipc_bearer_blocked(), tipc_continue() and tipc_block_bearer().
> We replace the latter with a new function, tipc_reset_bearer(), which
> resets all links associated to the bearer immediately after an
> interface goes down.
> 
> A user might notice one slight change in link behaviour after this
> change. When an interface goes down, (e.g. through a NETDEV_DOWN
> event) all attached links will be reset immediately, instead of
> leaving it to each link to detect the failure through a timer-driven
> mechanism. We consider this an improvement, and see no obvious risks
> with the new behavior.
> 
> Signed-off-by: Erik Hugne <erik.hugne@...csson.com>
> Reviewed-by: Ying Xue <ying.xue@...driver.com>
> Reviewed-by: Paul Gortmaker <Paul.Gortmaker@...driver.com>
> Signed-off-by: Jon Maloy <jon.maloy@...csson.com>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html