[dpdk-dev] i40e_aq_get_phy_capabilities() fails when using SFP+ with no link

[Wu, Jingjing]

Hi, Oliver

Let me 

Christos reports this issue is introduced by the  http://dpdk.org/ml/archives/dev/2016-September/047663.html
Which is the commit

commit edfb226f69bf68d514c9abae4b99b98aeb7c6a32
Author: Qi Zhang <qi.z.zhang at intel.com>
Date:   Tue Sep 27 09:37:21 2016 +0800

    net/i40e: use PHY type to check PHY capability

    Using device ID to check PHY capability is not extensible.

    Now we are using PHY type to detect PHY capability.

    All link speeds supported by the device are encoded into PHY type,
    and PHY type value can be read by admin queue "get_phy_capability"
    command at initialization stage.

While, your issue seems been introduce by
commit f4668a33efe5311301e10bc6e3709a2eccb446ad
Author: Qiming Yang <qiming.yang at intel.com>
Date:   Fri Nov 4 17:10:50 2016 +0800

    net/i40e: fix link status change interrupt

    Previously, link status interrupt in i40e is achieved by checking
    LINK_STAT_CHANGE_MASK in PFINT_ICR0 register which is provided only
    for diagnostic use. Instead, drivers need to get the link status
    change notification by using LSE (Link Status Event).

    This patch enables LSE and calls LSC callback when the event is
    received. This patch also removes the processing on
    LINK_STAT_CHANGE_MASK.


As my understanding, the first commit is try to get the phy type during initialization time,
it may cause error according to different PHYs.
But about the second one, I think the changes are only related to LSC (Link status change interrupt).
And I didn’t get the relationship between your issue and this patch.
Anyway, About the question " the consequences of reverting this patch", the answer is
It may affect the feature Link status change interrupt. Without the patch, there is an issue like:
When lsc is enabled, and link status change happens, interrupt is trigger, but the link status queried
by API rte_eth_link_get is not changed at all immediately. 


Thanks
Jingjing
> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz at 6wind.com]
> Sent: Friday, January 13, 2017 9:25 PM
> To: Olivier MATZ <olivier.matz at dev.6wind.com>
> Cc: Rowden, Aaron F <aaron.f.rowden at intel.com>; Christos Ricudis
> <ricudis.christos at gmail.com>; Zhang, Helin <helin.zhang at intel.com>;
> dev at dpdk.org; Wu, Jingjing <jingjing.wu at intel.com>
> Subject: Re: [dpdk-dev] i40e_aq_get_phy_capabilities() fails when using SFP+
> with no link
> 
> Hi,
> 
> On Thu, 12 Jan 2017 14:55:54 +0100, Olivier MATZ
> <olivier.matz at dev.6wind.com> wrote:
> > Hi,
> >
> > On Wed, 11 Jan 2017 20:51:58 +0000, "Rowden, Aaron F"
> > <aaron.f.rowden at intel.com> wrote:
> > > Hi Helin,
> > >
> > > I'm checking on this to see why it could be failing but I don’t
> > > think this is one part of formal validation. Intel modules are
> > > always what is recommended.
> > >
> > > Aaron
> > >
> > > > Hi Helin,
> > > >
> > > > > On 11 Jan 2017, at 09:08, Zhang, Helin <helin.zhang at intel.com>
> > > > > wrote:
> > > > >
> > > > > Hi Aaron
> > > > >
> > > > > Is the SFP+ (Finisar FTLX8571D3BCL) supported and validated by
> > > > > Intel? It seems there is some PHY issue in this case.
> > > >
> > > > As the original reporter of this issue, I will test with validated
> > > > SFP+s and will report on my testing.
> > > >
> > > > Shouldn’t unsupported SFP+s be blacklisted in the I40E driver?
> > > >
> >
> > Just to let you know that in my case the SFP are Intel ones.
> > Maybe it's a different issue.
> >
> > I see there are some i40e fixes in the net-next repo, I'll give a try
> > with this version.
> 
> The issue still exists in net-next.
> 
> I did a git bissect, and the commit that introduces the issue is:
> f4668a33efe5 ("net/i40e: fix link status change interrupt")  [1]
> 
> If I revert it (with some conflicts), the problem I described in [2] disappear.
> 
> Helin, Jinging, do you know what would be the consequences of reverting this
> patch?
> 
> Christos, I don't know if it also helps for yor issue. If no, sorry for having
> squatted your topic, the symptoms looked quite similar at first glance.
> 
> Thanks,
> Olivier
> 
> 
> [1] http://dpdk.org/browse/dpdk/commit/?id=f4668a33efe5
> [2] http://dpdk.org/ml/archives/dev/2017-January/054401.html