Hi, On Tue, 13 Jun 2017 14:14:43 +0000, "Wu, Jingjing" <jingjing...@intel.com> wrote: > > -----Original Message----- > > From: Olivier MATZ [mailto:olivier.m...@6wind.com] > > Sent: Tuesday, June 13, 2017 4:28 PM > > To: Wu, Jingjing <jingjing...@intel.com> > > Cc: Xing, Beilei <beilei.x...@intel.com>; Richardson, Bruce > > <bruce.richard...@intel.com>; > > Zhang, Helin <helin.zh...@intel.com>; dev@dpdk.org > > Subject: Re: [dpdk-dev] i40e: pci probe fails when using one bogus sfp > > > > Hi Jingjing, > > > > On Mon, 12 Jun 2017 16:23:47 +0000, "Wu, Jingjing" <jingjing...@intel.com> > > wrote: > > > HI, Olivier > > > > > > > Thank you for your quick answer. > > > > > > > > Yes, the pci probing continues for the other ports even if one port > > > > failed (since v17.05, commit 10f6c93cea). > > > > > > > > But I find a bit strange to have this check about the SFP in the > > > > PCI probing function instead of having it the port initialization > > > > function. My understanding is the SFP check is not related to PCI > > > > probing. Is it consistent with other drivers? > > > > > > > Could your customer help to check what is the exactly error code is by > > > Checking the "hw->aq.asq_last_status" when eth_i40e_dev_init() fails. > > > > I'm afraid it won't be possible, since it's a random issue that is > > not reproducible. > > > > What do you think about adding a log in i40e_dev_sync_phy_type() to > > display the status value in case of failure? It would help for next > > times. I can submit a patch for that if you want. > > > Yes, It makes sense. Thanks for doing that. > > > > Yes, it seems better PHY init fails doesn't block PCI probe. Just > > > compared with i40e > > > Kernel version, PHY init fails doesn't block CPI probe. And there is > > > watchdog task to > > > Check the PHY status. But DPDK is polling mode, If PCI probe fails, PCI > > > probe continues, > > > then application need poll PHY status to support SFP change. > > > > From what I understand, i40e_dev_sync_phy_type() was added > > to know the PHY capabilities, which useful for instance for devinfo(). > > Indeed, devinfo() can be call before the port is started, so > > we need to have get the PHY capabilities before starting the port. > > > > I've done a quick patch that: > > - keeps the call to i40e_dev_sync_phy_type() in pci probing but > > continue in case of failure > > - add another call to i40e_dev_sync_phy_type() in port start > > function > > > I think this workaround would not introduce issue if SFP is ready on NIC > before Probe. > But I'm not sure if it can resolve the issue, because during probe, dozens of > initialization work > Is ongoing. I think we need to verify it PHY init during start phase works or > not. Maybe we > Need to do more than that? Not sure.
After discussion with Helin, I moved the call to i40e_dev_sync_phy_type() in dev_configure(). I'm sending it as a reply to this mail. I see if I can get my patch tested by the customer. > > > I think it would solve the issue without impacting the result > > of the devinfo() function. What would you think of a patch like > > this? > > > > > And I also checked ixgbe driver, it seems phy init is done at probe time. > > > In my opinion, dev_start and dev_stop is meaning ready for receiving and > > > transmitting > > > packets, it may not be suitable to put it in the start/stop phase. > > > > I'm wondering: what would/should occur if the SFP is unplugged and > > replugged while the port is running? > > > > I suppose we don't have any PCI notification when unplugging/plugging > > the SFP, so I'm not sure we should have this check at the PCI level, > > because the application does not know if the bus has to be probed again. > > > We can consider about unplugging/plugging as link event in DPDK. Now we > have LSC or user can polling link status. Maybe that is a way to go. Yes, I agree. Thanks for the confirmation.
