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.
next prev parent 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).