[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87618083B2453E4A8714035B62D679924FF674A6@FMSMSX105.amr.corp.intel.com>
Date: Wed, 14 May 2014 18:22:20 +0000
From: "Tantilov, Emil S" <emil.s.tantilov@...el.com>
To: Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
"davem@...emloft.net" <davem@...emloft.net>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"gospo@...hat.com" <gospo@...hat.com>,
"sassmann@...hat.com" <sassmann@...hat.com>
Subject: RE: [net-next 01/11] ixgbe: fix detection of SFP+ capable interfaces
>-----Original Message-----
>From: Sergei Shtylyov
>[mailto:sergei.shtylyov@...entembedded.com]
>Sent: Wednesday, May 14, 2014 11:16 AM
>To: Tantilov, Emil S; Kirsher, Jeffrey T;
>davem@...emloft.net
>Cc: netdev@...r.kernel.org; gospo@...hat.com;
>sassmann@...hat.com
>Subject: Re: [net-next 01/11] ixgbe: fix detection of SFP+
>capable interfaces
>
>Hello.
>
>On 05/14/2014 08:59 PM, Tantilov, Emil S wrote:
>
>>>> From: Emil Tantilov <emil.s.tantilov@...el.com>
>
>>>> In cases where the driver is loaded while there are no
>SFP+ modules in
>>>> the cage the interface was not being detected as SFP
>capable. To account
>>>> for this the driver called identify_sfp in
>ixgbe_get_settings to make
>>>> sure the data is correct. However when there is no SFP+
>module in the cage
>>>> the driver waits for the I2C reads to time out which can
>take more than a
>>>> second and will cause issues with tools (like net-snmp)
>that may poll
>>>> for that information.
>
>>>> This patch resolves the issue by identifying 82599 based
>NIC with no PHY
>>>> type set as SFP capable which allows the driver to
>detect the SFP module
>>>> when the interface is brought up. As result of this we
>can also remove the
>>>> identify_sfp call from ixgbe_get_settings.
>
>>>> Signed-off-by: Emil Tantilov <emil.s.tantilov@...el.com>
>>>> Tested-by: Phil Schmitt <phillip.j.schmitt@...el.com>
>>>> Signed-off-by: Jeff Kirsher
><jeffrey.t.kirsher@...el.com>
>
>>> [...]
>
>>>> diff --git
>a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> index 8089ea9..e44c42a 100644
>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> @@ -4660,6 +4660,10 @@ static inline bool
>ixgbe_is_sfp(struct ixgbe_hw *hw)
>>>> case ixgbe_phy_nl:
>>>> if (hw->mac.type == ixgbe_mac_82598EB)
>>>> return true;
>>>> + /* ixgbe_phy_none is set when no SFP module is
>present */
>>>> + case ixgbe_phy_none:
>>>> + if (hw->mac.type == ixgbe_mac_82599EB)
>>>> + return true;
>
>>> Shouldn't it just be combined with the previous case?
>
>> How can you combine them? The previous case is for
>ixgbe_phy_nl and 82598 macs this patch is for ixgbe_phy_none
>and 82599.
>
> Ah, sorry, I've misread 82598EB as 82599EB.
Actually there is a problem with this patch due to this and the previous case falling through which can theoretically lead to a wrong return value.
Please do not apply this patch. I will submit ver 2.
Thanks for reviewing!
Emil
--
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
Powered by blists - more mailing lists