From: "Zhang, Helin" <helin.zhang@intel.com>
To: "Jankowski, Konrad" <konrad.jankowski@intel.com>,
"Zhang, Qi Z" <qi.z.zhang@intel.com>,
"Dai, Wei" <wei.dai@intel.com>,
"Xing, Beilei" <beilei.xing@intel.com>,
"Wu, Jingjing" <jingjing.wu@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] net/i40evf: regression fix - reenable interrupts in handler
Date: Fri, 6 Apr 2018 15:52:02 +0000 [thread overview]
Message-ID: <F35DEAC7BCE34641BA9FAC6BCA4A12E71AE15F64@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <53C7EC427773E548A0839C5ED9B805533D0A1AA9@IRSMSX103.ger.corp.intel.com>
> -----Original Message-----
> From: Zhang, Helin
> Sent: Friday, April 6, 2018 11:48 PM
> To: Jankowski, Konrad; Zhang, Qi Z; Dai, Wei; Xing, Beilei; Wu, Jingjing;
> dev@dpdk.org
> Subject: RE: [PATCH] net/i40evf: regression fix - reenable interrupts in handler
>
>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jankowski, Konrad
> > Sent: Thursday, April 5, 2018 1:12 AM
> > To: Zhang, Qi Z; Dai, Wei; Xing, Beilei; Wu, Jingjing; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] net/i40evf: regression fix - reenable
> > interrupts in handler
> >
> > Hi Zhang,
> >
> > If you look at the source of interrupt handlers in both the igb_uio
> > and uio_pci_generic drivers (look for irqreturn_t type), you will see
> > they can disable interrupts immediately after receipt of one.
> > It's up to the user to make sure they're re-enabled. Also please
> > compare with code in i40e_ethdev.c, which still does this correctly -
> > there's an explicit
> > rte_intr_enable(dev->intr_handle) call at the end of
> > i40e_dev_interrupt_handler().
> > Probably a cleaner approach would be to leave them disabled as is, but
> > only enable them for a once-off receipt when sending an AdminQ
> > message, maybe that was the assumption here. However I've added some
> > tracing to
> > igbuio_pci_irqcontrol() and I'm sure this isn't happening on my system.
> > (nothing is enabling those interrupts post device init). Looks like
> > code path might be different with the newest igb_uio driver and MSI
> > enabled, but the current code will still not work for all cases (like with
> uio_pci_generic).
>
> Hi Konrad
> Please don't use uio_pci_generic, as it is not supported by X710/XL710/XXV710
> series, as below. With VFIO, do you see similar issue?
> http://www.dpdk.org/doc/guides/rel_notes/known_issues.html#uio-pci-
> generic-module-bind-failed-in-x710-xl710-xxv710
>
> > There can also be cases when you have and older igb_uio driver which
> > disables interrupts in all cases and running it with with a new DPDK.
> > (vendor provided compiled driver) I think for full compatibility we
> > need to keep re-enabling those interrupts.
> >
> > Regards,
> > Konrad
> >
> > -----Original Message-----
> > From: Zhang, Qi Z
> > Sent: Wednesday, March 28, 2018 4:37 AM
> > To: Jankowski, Konrad <konrad.jankowski@intel.com>; Dai, Wei
> > <wei.dai@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Wu,
> > Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> > Subject: RE: [PATCH] net/i40evf: regression fix - reenable interrupts
> > in handler
> >
> > Hi Jankowski:
> >
> > > -----Original Message-----
> > > From: Jankowski, KonradX
> > > Sent: Thursday, February 15, 2018 2:33 AM
> > > To: Dai, Wei <wei.dai@intel.com>; Xing, Beilei
> > > <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> > > Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> > > Cc: Jankowski, KonradX <konradx.jankowski@intel.com>
> > > Subject: [PATCH] net/i40evf: regression fix - reenable interrupts in
> > > handler
Better title might be needed, with start of 'fix'. Also note that 'Fixes' line is
needed, and 'Cc' line might be needed.
> > >
> > > Commit 66b8304f removed the rte_intr_enable() call from
> > > i40evf_dev_interrupt_handler() as a "bonus". On one of my systems
> > > this causes the AdminQ messages to stop beeing delivered to the VF.
> > > This results in unability to initialize and use the port. With this
> > > patch it works
> > again.
Also please fix the checkpatch warnings on patchwork. Thanks!
/Helin
> > >
> > > System in question:
> > > Wind River OVP6 running kernel
> > > 3.10.58-ovp-rt58-WR6.0.0.13_preempt-rt
> > >
> > > Signed-off-by: Konrad Jankowski <konrad.jankowski@intel.com>
> > > ---
> > > drivers/net/i40e/i40e_ethdev_vf.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > > b/drivers/net/i40e/i40e_ethdev_vf.c
> > > index fd003fe..b927a35 100644
> > > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > > @@ -1404,6 +1404,7 @@ i40evf_dev_interrupt_handler(void *param)
> > >
> > > done:
> > > i40evf_enable_irq0(hw);
> > > + rte_intr_enable(dev->intr_handle);'
> >
> > Would you explain more about why the patch fix the issue?
> > Usually we will not accept a fix just because it work but not
> > understand the root cause.
> >
> > Regards
> > Qi
> >
> > > }
> > >
> > > static int
> > > --
> > > 2.5.5
> >
> > --------------------------------------------------------------
> > Intel Research and Development Ireland Limited Registered in Ireland
> > Registered Office: Collinstown Industrial Park, Leixlip, County
> > Kildare Registered Number: 308263
> >
> >
> > This e-mail and any attachments may contain confidential material for
> > the sole use of the intended recipient(s). Any review or distribution
> > by others is strictly prohibited. If you are not the intended
> > recipient, please contact the sender and delete all copies.
next prev parent reply other threads:[~2018-04-06 15:52 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-14 18:32 Konrad Jankowski
2018-03-28 3:36 ` Zhang, Qi Z
2018-04-04 17:12 ` Jankowski, Konrad
2018-04-06 15:47 ` Zhang, Helin
2018-04-06 15:52 ` Zhang, Helin [this message]
2018-04-10 13:26 ` Zhang, Qi Z
2018-04-10 14:06 ` Jankowski, Konrad
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=F35DEAC7BCE34641BA9FAC6BCA4A12E71AE15F64@SHSMSX103.ccr.corp.intel.com \
--to=helin.zhang@intel.com \
--cc=beilei.xing@intel.com \
--cc=dev@dpdk.org \
--cc=jingjing.wu@intel.com \
--cc=konrad.jankowski@intel.com \
--cc=qi.z.zhang@intel.com \
--cc=wei.dai@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).