[dpdk-dev] [PATCH] eal: fix rte_intr_dp_is_en() check

[Liang, Cunming]

Hi Yong,

> -----Original Message-----
> From: Yong Wang [mailto:yongwang at vmware.com]
> Sent: Wednesday, July 20, 2016 5:59 AM
> To: Thomas Monjalon <thomas.monjalon at 6wind.com>
> Cc: dev at dpdk.org; Liang, Cunming <cunming.liang at intel.com>;
> david.marchand at 6wind.com
> Subject: Re: [dpdk-dev] [PATCH] eal: fix rte_intr_dp_is_en() check
> 
> 
> > On Jul 18, 2016, at 2:21 AM, Thomas Monjalon <thomas.monjalon at 6wind.com>
> wrote:
> >
> > Hi Yong,
> >
> > I think the interrupt management should be simpler.
> > If you want to invest some time to rework this API, you
> > are very welcome.
> >
> >
> > 2016-07-18 14:19, Liang, Cunming:
> >> Hi Yong,
> >>
> >> rte_intr_dp_is_en() returns true when rte_intr_efd_enable() (the way to
> >> enable data-path interrupt) sets a number of event fds.
> >> In this case, "intr_conf.rxq=1" configuration causes "nb_efd=1". The
> >> value comes from RTE_MIN($nb_efd, 1) from data-path, but not from link
> >> event.
> >> Per link event, you shouldn't use rte_intr_dp_is_en() as the indication.
> >> As igb_uio only has a single vector, when the conflict(both intr_rxq and
> >> intr_lsc turn on) happens, the intr_rxq has high priority than intr_lsc
> >> as default PMD behavior.
> >> Reference as PG 3.1.9 note in
> >> https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__dpdk.org_doc_guides_prog-5Fguide_env-5Fabstraction-
> 5Flayer.html&d=CwICAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-
> uEs&r=44mSO5N5yEs4CeCdtQE0xt0F7J0p67_mApYVAzyYms0&m=xFO005GdZMA
> 9a6IC2kvhKYaXW7Rhl3dlcom5HfOSy2g&s=yERFYJTe6TBWFCltmR3xikmxkLKutbot
> 5BfFxK30tZ0&e=
> >>
> >> Regards,
> >> Cunning
> 
> Thanks Cunming and Thomas for the feedback.  I agree with Thomas that the rx
> queue interrupt APIs could be simplified and it’s not clear to me if the API user
> needs to be concerned about rte_intr_cap_multiple(), rte_intr_allow_others(),
> rte_intr_dp_is_en(), etc. if all he needs is to enable intr with certain # of vectors
> and map the vectors to either efds or callbacks.  I feel it’s simpler just returning
> the number of vectors the system can support if it cannot satisfy nb_efd vectors
> and let the application decide how should this be handled. 

It's welcome to make a simpler intr API. At the beginning of introducing rxq intr, it
didn't attract much attention, the coordination problem of EAL intr thread remains.

Status quo:

- The 3 APIs you talked is the helper function for PMD developer, not for end user.
   EAL provides efds/vectors mapping, hides vfio/uio difference. The causes/vectors
   mapping is handled by PMD. Then it's not necessary for the end user to aware
   intr vector, just take aware of event by queue id.

   Your idea on exposing vectors to end user is an option. In terms of it, the APP
   has to understand the causes of vectors, misc(e.g. lsc, mb, etc) vs non-misc(rxq).
   In particular case(e.g. mb), it seems APP is not the right person to turn on/off.
   If all are acceptable, ethdev can provide new API for causes to vector mapping.

   There's one note for replacing the 3 API provided to PMD. PMD is blind
   to the user space IO infrastructure, vfio or uio. The way for PMD to configure
   mapping for legacy/MSI(e.g. uio case) and MSIX(e.g. vfio case) isn't the same.

>The link event and rxq
> interrupt init/uninit path also looks quite different and some unification will be
> helpful.

- 'intr_conf' and interrupt thread for misc
   To turn on lsc intr, it requires 'intr_conf.lsc=1'. It cases ethdev to register handler
   for further processing in the interrupt thread. It's not necessary for rxq intr.

   Probably a clean way is not to handle device external interrupt event in EAL
   interrupt thread (intr mb may have some problem). The EAL interrupt thread is
   only used to postpone the delay execution or other background interrupt
   (e.g. alarm). Then misc/non-misc can be combined,  while requiring APP to
   detect the interrupt causes. 

> 
> The API documentation can also use some more clarification.  For example,
> without reading the program guide, it’s easy to miss the fact that when the
> device is bound to UIO, the implementation will try to enable rxq instead of lsc
> when both intr_conf.lsc and intr_conf.rxq is set to 1. I can imagine there are cases
> where an application would prefer to enable link event instead of rxq interrupt in
> such cases and currently there is no easy way to achieve that.  Or is there any
> particular reason such a preference is chosen?

In uio, if do care lsc. The suggestion is to only turn on intr_conf.lsc. It's just a default
behavior definition for the single vector tradeoff. Which don't want to introduce
another configuration. The uio msix patch was expected to upstream on that time...
The uio single vector problem probably will remain for a long time. So welcome to
rework these stuffs. 

Thanks,
Cunming
 
> 
> >>
> >> On 7/15/2016 8:36 AM, Yong Wang wrote:
> >>> When binding a device to igb_uio with intr_conf.rxq set to 1, nb_efd
> >>> is 1 (for link event) but rte_intr_dp_is_en() will still return true.
> >>> rte_intr_dp_is_en() should also consider intr_handle type in addition
> >>> to nb_efd.
> >>>
> >>> Signed-off-by: Yong Wang <yongwang at vmware.com>
> >