netdev - Re: [PATCH 3/3] ixgbe: synchronize the link_speed and link

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <7EED4F91-2DE5-407F-BA6A-681ED57650E4@intel.com>
Date:	Wed, 30 Dec 2015 19:02:54 +0000
From:	"Rustad, Mark D" <mark.d.rustad@...el.com>
To:	"zyjzyj2000@...il.com" <zyjzyj2000@...il.com>
CC:	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
	"Nelson, Shannon" <shannon.nelson@...el.com>,
	"Wyborny, Carolyn" <carolyn.wyborny@...el.com>,
	"Skidmore, Donald C" <donald.c.skidmore@...el.com>,
	"Allan, Bruce W" <bruce.w.allan@...el.com>,
	"Ronciak, John" <john.ronciak@...el.com>,
	"Williams, Mitch A" <mitch.a.williams@...el.com>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"e1000-devel@...ts.sourceforge.net" 
	<e1000-devel@...ts.sourceforge.net>,
	"Viswanathan, Ven (Wind River)" <venkat.viswanathan@...driver.com>,
	"Shteinbock, Boris (Wind River)" <boris.shteinbock@...driver.com>,
	"Bourg, Vincent (Wind River)" <vincent.bourg@...driver.com>
Subject: Re: [PATCH 3/3] ixgbe: synchronize the link_speed and link_up of a
 slave interface

zyjzyj2000@...il.com wrote:

> From: Zhu Yanjun <yanjun.zhu@...driver.com>
> 
> According to the suggestion from Rustad, Mark D, this behavior perhaps
> is more related to the copper phy. But to make fiber phy more robust,
> to all the interfaces as a slave interface, the link_speed and link_up
> is synchronized.
> 
> Signed-off-by: Zhu Yanjun <yanjun.zhu@...driver.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 1bb6056..ce47639 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -6441,10 +6441,12 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)
> 	 * a bonding driver in 802.3ad mode. When X540 NIC acts as an
> 	 * independent interface, it is not necessary to synchronize link_up
> 	 * and link_speed.
> -	 * In the end, not continue if (X540 NIC && SLAVE && link_speed UNKNOWN)
> +	 * According to the suggestion from Rustad, Mark D, this behavior
> +	 * perhaps is related to the copper phy. To make fiber phy more robust,
> +	 * To all the interfaces as a slave, the link_speed is checked.
> +	 * In the end, not continue if (SLAVE && link_speed UNKNOWN)

There is no need to make reference to my suggestion in the comment, especially since that is in the commit message. Please simplify your comment above to be something like:
	 * For all slave interfaces, wait for the link_speed to be known.

> 	 */
> -	if ((hw->mac.type == ixgbe_mac_X540) &&
> -	    (netdev->flags & IFF_SLAVE))
> +	if (netdev->flags & IFF_SLAVE)
> 		if (link_speed == IXGBE_LINK_SPEED_UNKNOWN)
> 			return;

The above would be better as:
	if ((netdev->flags & IFF_SLAVE) &&
	    link_speed == IXGBE_LINK_SPEED_UNKNOWN)
		return;

Please do not send a series of patches - it just adds needless confusion and is a bisect hazard. Just send a single patch with the desired change as a V5.

--
Mark Rustad, Networking Division, Intel Corporation

Download attachment "signature.asc" of type "application/pgp-signature" (842 bytes)