netdev - RE: [net-next-2.6 PATCH] ixgbe: fix for link failure on SFP+ DA cables

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]

Date:	Thu, 2 Dec 2010 09:28:18 -0800
From:	"Skidmore, Donald C" <donald.c.skidmore@...el.com>
To:	Joe Perches <joe@...ches.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
CC:	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>,
	"bphilips@...ell.com" <bphilips@...ell.com>
Subject: RE: [net-next-2.6 PATCH] ixgbe: fix for link failure on SFP+ DA
 cables


>-----Original Message-----
>From: Joe Perches [mailto:joe@...ches.com]
>Sent: Wednesday, December 01, 2010 11:42 PM
>To: Kirsher, Jeffrey T
>Cc: davem@...emloft.net; netdev@...r.kernel.org; gospo@...hat.com;
>bphilips@...ell.com; Skidmore, Donald C
>Subject: Re: [net-next-2.6 PATCH] ixgbe: fix for link failure on SFP+ DA
>cables
>
>On Wed, 2010-12-01 at 22:59 -0800, Jeff Kirsher wrote:
>> This patch helps prevent FW/SW semaphore collision from leading
>> to link establishment failure.  The collision might mess up the
>> PHY registers so we reset the PHY.  However there are SFI/KR areas
>> in the PHY that are not reset with a Reset_AN so we need to change
>> LMS to reset it.  Also wait until AN state machine is AN_GOOD
>[]
>>  drivers/net/ixgbe/ixgbe_82599.c |   28 +++++++++++++++++++++++++---
>> diff --git a/drivers/net/ixgbe/ixgbe_82599.c
>b/drivers/net/ixgbe/ixgbe_82599.c
>[]
>> @@ -116,14 +118,34 @@ static s32 ixgbe_setup_sfp_modules_82599(struct
>ixgbe_hw *hw)
>>  			IXGBE_WRITE_FLUSH(hw);
>>  			hw->eeprom.ops.read(hw, ++data_offset, &data_value);
>>  		}
>> -		/* Now restart DSP by setting Restart_AN */
>> -		IXGBE_WRITE_REG(hw, IXGBE_AUTOC,
>> -		    (IXGBE_READ_REG(hw, IXGBE_AUTOC) |
>IXGBE_AUTOC_AN_RESTART));
>>
>>  		/* Release the semaphore */
>>  		ixgbe_release_swfw_sync(hw, IXGBE_GSSR_MAC_CSR_SM);
>>  		/* Delay obtaining semaphore again to allow FW access */
>>  		msleep(hw->eeprom.semaphore_delay);
>
>might these msleeps be usleep_range?
>
>[]
>
>> +		/* Wait for AN to leave state 0 */
>> +		for (i = 0; i < 10; i++) {
>> +			msleep(4);
>
>Perhaps usleep_range?
>
>Maybe all the msleep uses < 20ms in the intel drivers?
>
>$ grep -nPr --include=*.[ch] "msleep\s*\(\s*1?\d\s*\)" drivers/net/ig*
>drivers/net/ixg* drivers/net/e100* | wc -l
>123
>
>Maybe something like doubling the sleep value
>for the upper bound range?
>
>Here's a little script that does that.
>
>$ grep -nPrl --include=*.[ch] "msleep\s*\(\s*1?\d\s*\)" \
>	drivers/net/ig* drivers/net/ixg* drivers/net/e100* | \
>xargs perl -p -i -e 's/msleep\s*\(\s*(1?\d)\s*\)/"usleep_range\(${1}000, "
>. scalar($1) * 2 . "000\)"/ge'
>

Good point about the msleeps.  We do have a plan to change all the <20 ones to usleep_range I just haven't gotten around to make the patch yet.  I just wasn't positive on what our upper range would need to be for each case and wanted to look at each individually to make sure.  Your suggestion may well end up being what we end up trying (doubling the upper bound). :)

I kept msleeps in this patch as I wanted to be consistent when doing the usleep_range changeover and the longer delay really doesn't hurt here as the sleep value is really only the minimum needed.  I do promises that the other patch is coming soon. :)

Thanks,
-Don
--
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