[dpdk-dev,v6,2/8] eal/linux: add rx queue interrupt FDs to intr handle struct

  From: David Marchand [mailto:david.marchand@6wind.com]

Sent: Friday, February 27, 2015 6:33 PM
To: Liang, Cunming
Cc: dev@dpdk.org; Stephen Hemminger; Thomas Monjalon; Zhou, Danny
Subject: Re: [PATCH v6 2/8] eal/linux: add rx queue interrupt FDs to intr handle struct

Hello,

On Fri, Feb 27, 2015 at 5:56 AM, Cunming Liang <cunming.liang@intel.com<mailto:cunming.liang@intel.com>> wrote:
Per vector event fd will store in rte_intr_handle during init.
Device drivers take responsibility to fill queue-vec mapping table(vec_num[]).

Signed-off-by: Danny Zhou <danny.zhou@intel.com<mailto:danny.zhou@intel.com>>

Signed-off-by: Cunming Liang <cunming.liang@intel.com<mailto:cunming.liang@intel.com>>

---
v6 changes:
 - add mapping table between irq vector number and queue id.


Ok, you will store vfio eventfds here, but vfio is an implementation, not the abstraction.
[Liang, Cunming] If looking at the rte_intr_hanle_type, it includes UIO/VFIO_LEGACY/VFIO_MSI/VFIO_MSIX.
I agree, VFIO is an implementation, but the different type combination is a kind of ‘abstraction’.
So in rte_intr_handle (like a multiplexing), some specified field for vfio interrupter mapping, I feel it’s reasonable.


--
David Marchand
  

2015-02-27 11:28, Liang, Cunming:
> From: David Marchand [mailto:david.marchand@6wind.com]
> Sent: Friday, February 27, 2015 6:33 PM
> On Fri, Feb 27, 2015 at 5:56 AM, Cunming Liang <cunming.liang@intel.com<mailto:cunming.liang@intel.com>> wrote:
> > --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> > +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> > @@ -38,6 +38,9 @@
> > 
> >  #ifndef _RTE_LINUXAPP_INTERRUPTS_H_
> >  #define _RTE_LINUXAPP_INTERRUPTS_H_
> > 
> > +#define VFIO_MAX_RXTX_INTR_ID        32
> > +#define VFIO_MAX_QUEUE_ID            VFIO_MAX_RXTX_INTR_ID> 
> This is a little weird to talk about vfio here.
> This file is "generic".
> 
> Ok, you will store vfio eventfds here, but vfio is an implementation, not the abstraction.
> [Liang, Cunming] If looking at the rte_intr_hanle_type, it includes UIO/VFIO_LEGACY/VFIO_MSI/VFIO_MSIX.
> I agree, VFIO is an implementation, but the different type combination is a kind of ‘abstraction’.
> So in rte_intr_handle (like a multiplexing), some specified field for vfio interrupter mapping, I feel it’s reasonable.
> 
> 
> --
> David Marchand
>
  

2015-02-27 11:28, Liang, Cunming:
> From: David Marchand [mailto:david.marchand@6wind.com]
> Sent: Friday, February 27, 2015 6:33 PM
> > On Fri, Feb 27, 2015 at 5:56 AM, Cunming Liang wrote:
> > > --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> > > +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> > > @@ -38,6 +38,9 @@
> > > 
> > >  #ifndef _RTE_LINUXAPP_INTERRUPTS_H_
> > >  #define _RTE_LINUXAPP_INTERRUPTS_H_
> > > 
> > > +#define VFIO_MAX_RXTX_INTR_ID        32
> > > +#define VFIO_MAX_QUEUE_ID            VFIO_MAX_RXTX_INTR_ID
> > 
> > This is a little weird to talk about vfio here.
> > This file is "generic".
> > 
> > Ok, you will store vfio eventfds here, but vfio is an implementation,
> > not the abstraction.
> 
> [Liang, Cunming] If looking at the rte_intr_hanle_type, it includes UIO/VFIO_LEGACY/VFIO_MSI/VFIO_MSIX.
> I agree, VFIO is an implementation, but the different type combination is a kind of ‘abstraction’.
> So in rte_intr_handle (like a multiplexing), some specified field for vfio interrupter mapping, I feel it’s reasonable.

Not sure to understand. Are we trying to mask the different kernel drivers
from an application point of view, and provide a generic interrupt mechanism?
If yes, why some VFIO constants are needed?
I'm not saying that the current implementation is perfect, but we should try
to improve it.

Thanks
  

> -----Original Message-----

> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]

> Sent: Friday, February 27, 2015 10:52 PM

> To: Liang, Cunming

> Cc: David Marchand; dev@dpdk.org; Stephen Hemminger; Zhou, Danny

> Subject: Re: [PATCH v6 2/8] eal/linux: add rx queue interrupt FDs to intr handle

> struct

> 

> 2015-02-27 11:28, Liang, Cunming:

> > From: David Marchand [mailto:david.marchand@6wind.com]

> > Sent: Friday, February 27, 2015 6:33 PM

> > > On Fri, Feb 27, 2015 at 5:56 AM, Cunming Liang wrote:

> > > > --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h

> > > > +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h

> > > > @@ -38,6 +38,9 @@

> > > >

> > > >  #ifndef _RTE_LINUXAPP_INTERRUPTS_H_

> > > >  #define _RTE_LINUXAPP_INTERRUPTS_H_

> > > >

> > > > +#define VFIO_MAX_RXTX_INTR_ID        32

> > > > +#define VFIO_MAX_QUEUE_ID            VFIO_MAX_RXTX_INTR_ID

> > >

> > > This is a little weird to talk about vfio here.

> > > This file is "generic".

> > >

> > > Ok, you will store vfio eventfds here, but vfio is an implementation,

> > > not the abstraction.

> >

> > [Liang, Cunming] If looking at the rte_intr_hanle_type, it includes

> UIO/VFIO_LEGACY/VFIO_MSI/VFIO_MSIX.

> > I agree, VFIO is an implementation, but the different type combination is a kind

> of ‘abstraction’.

> > So in rte_intr_handle (like a multiplexing), some specified field for vfio

> interrupter mapping, I feel it’s reasonable.

> 

> Not sure to understand. Are we trying to mask the different kernel drivers

> from an application point of view, and provide a generic interrupt mechanism?

> If yes, why some VFIO constants are needed?

> I'm not saying that the current implementation is perfect, but we should try

> to improve it.

[LCM] VFIO_MAX_RXTX_INTR_ID is easy to fix, it can move to a private interrupt header file, as only be used inside EAL.
VFIO_MAX_QUEUE_ID can be removed, so vec_num[] dynamic creation by the device driver. Sounds good ?
> 

> Thanks