DPDK patches and discussions
 help / color / mirror / Atom feed
From: Olivier MATZ <olivier.matz@6wind.com>
To: "Wu, Jingjing" <jingjing.wu@intel.com>
Cc: "Xing, Beilei" <beilei.xing@intel.com>,
	"Richardson, Bruce" <bruce.richardson@intel.com>,
	"Zhang, Helin" <helin.zhang@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] i40e: pci probe fails when using one bogus sfp
Date: Thu, 15 Jun 2017 11:03:31 +0200	[thread overview]
Message-ID: <20170615110331.3c8ca067@glumotte.dev.6wind.com> (raw)
In-Reply-To: <9BB6961774997848B5B42BEC655768F810DA461C@SHSMSX103.ccr.corp.intel.com>

Hi,

On Tue, 13 Jun 2017 14:14:43 +0000, "Wu, Jingjing" <jingjing.wu@intel.com> wrote:
> > -----Original Message-----
> > From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> > Sent: Tuesday, June 13, 2017 4:28 PM
> > To: Wu, Jingjing <jingjing.wu@intel.com>
> > Cc: Xing, Beilei <beilei.xing@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>;
> > Zhang, Helin <helin.zhang@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.wu@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.

  reply	other threads:[~2017-06-15  9:03 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-08  9:29 Olivier Matz
2017-06-08 10:01 ` Bruce Richardson
2017-06-08 10:13   ` Olivier Matz
2017-06-12  8:45     ` Xing, Beilei
2017-06-12  9:45       ` Olivier Matz
2017-06-12 15:53         ` Wu, Jingjing
2017-06-12 16:25           ` Wu, Jingjing
2017-06-12 16:23         ` Wu, Jingjing
2017-06-13  8:27           ` Olivier MATZ
2017-06-13 14:14             ` Wu, Jingjing
2017-06-15  9:03               ` Olivier MATZ [this message]
2017-06-15  9:08                 ` [dpdk-dev] [PATCH] net/i40e: avoid PCI probing failure when using " Olivier Matz
2017-06-23  9:25                   ` Wu, Jingjing
2017-06-23 10:11                     ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170615110331.3c8ca067@glumotte.dev.6wind.com \
    --to=olivier.matz@6wind.com \
    --cc=beilei.xing@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=helin.zhang@intel.com \
    --cc=jingjing.wu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).