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.zh...@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.y...@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.m...@6wind.com] > Sent: Friday, January 13, 2017 9:25 PM > To: Olivier MATZ <olivier.m...@dev.6wind.com> > Cc: Rowden, Aaron F <aaron.f.row...@intel.com>; Christos Ricudis > <ricudis.chris...@gmail.com>; Zhang, Helin <helin.zh...@intel.com>; > dev@dpdk.org; Wu, Jingjing <jingjing...@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.m...@dev.6wind.com> wrote: > > Hi, > > > > On Wed, 11 Jan 2017 20:51:58 +0000, "Rowden, Aaron F" > > <aaron.f.row...@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.zh...@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