[PATCH] eal/interrupts: Allow UIO interrupts when using igb_uio

[PATCH] eal/interrupts: Allow UIO interrupts when using igb_uio

[Vladimir Ratnikov]

 Some drivers and devices(ex: igc + i225/i226) use
RTE_INTR_HANDLE_UIO handler when captured under igb_uio
so just let them use it.

Signed-off-by: Vladimir Ratnikov <vratnikov@netgate.com>
---
 lib/eal/linux/eal_interrupts.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
index c9881143be..037db09cfd 100644
--- a/lib/eal/linux/eal_interrupts.c
+++ b/lib/eal/linux/eal_interrupts.c
@@ -1596,6 +1596,9 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle)
 	if (rte_intr_type_get(intr_handle) == RTE_INTR_HANDLE_VDEV)
 		return 1;
 
+	if (rte_intr_type_get(intr_handle) == RTE_INTR_HANDLE_UIO)
+		return 1;
+
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH] eal/interrupts: Allow UIO interrupts when using igb_uio

[Stephen Hemminger]

On Wed, 14 Jun 2023 13:40:18 +0000
Vladimir Ratnikov <vratnikov@netgate.com> wrote:

>  Some drivers and devices(ex: igc + i225/i226) use
> RTE_INTR_HANDLE_UIO handler when captured under igb_uio
> so just let them use it.
> 
> Signed-off-by: Vladimir Ratnikov <vratnikov@netgate.com>

This doesn't look right.

With UIO only a single interrupt is possible, there is no MSI-X support.

Re: [PATCH] eal/interrupts: Allow UIO interrupts when using igb_uio

[Thomas Monjalon]

14/06/2023 18:46, Stephen Hemminger:
> On Wed, 14 Jun 2023 13:40:18 +0000
> Vladimir Ratnikov <vratnikov@netgate.com> wrote:
> 
> >  Some drivers and devices(ex: igc + i225/i226) use
> > RTE_INTR_HANDLE_UIO handler when captured under igb_uio
> > so just let them use it.
> > 
> > Signed-off-by: Vladimir Ratnikov <vratnikov@netgate.com>
> 
> This doesn't look right.
> 
> With UIO only a single interrupt is possible, there is no MSI-X support.

Please Vladimir, could you reply to Stephen?
I think he is suggesting a better fix in the igc driver.

Re: [PATCH] eal/interrupts: Allow UIO interrupts when using igb_uio

[Vladimir Ratnikov]

[-- Attachment #1: Type: text/plain, Size: 1031 bytes --]

Sorry for a long reply, sure.

Stephen,
am I right that the most concern is about a place where interrupt
capabilities check appears for non MSI-X support?
What if having dedicated rte_intr_cap_single analog when there's no support
for MSI-X and just do the same(check capability, allocate interrupt vector
etc) ?


Regards,
-Vladimir

On Mon, Jul 3, 2023 at 5:32 PM Thomas Monjalon <thomas@monjalon.net> wrote:

> 14/06/2023 18:46, Stephen Hemminger:
> > On Wed, 14 Jun 2023 13:40:18 +0000
> > Vladimir Ratnikov <vratnikov@netgate.com> wrote:
> >
> > >  Some drivers and devices(ex: igc + i225/i226) use
> > > RTE_INTR_HANDLE_UIO handler when captured under igb_uio
> > > so just let them use it.
> > >
> > > Signed-off-by: Vladimir Ratnikov <vratnikov@netgate.com>
> >
> > This doesn't look right.
> >
> > With UIO only a single interrupt is possible, there is no MSI-X support.
>
> Please Vladimir, could you reply to Stephen?
> I think he is suggesting a better fix in the igc driver.
>
>
>

[-- Attachment #2: Type: text/html, Size: 1955 bytes --]

Re: [PATCH] eal/interrupts: Allow UIO interrupts when using igb_uio

[Stephen Hemminger]

On Tue, 4 Jul 2023 12:45:54 +0200
Vladimir Ratnikov <vratnikov@netgate.com> wrote:

> Sorry for a long reply, sure.
> 
> Stephen,
> am I right that the most concern is about a place where interrupt
> capabilities check appears for non MSI-X support?
> What if having dedicated rte_intr_cap_single analog when there's no support
> for MSI-X and just do the same(check capability, allocate interrupt vector
> etc) ?
> 
> 
> Regards,
> -Vladimir

With single interrupt, only link state interrupt is possible.
Does that work with igb_uio?  It should, if not then yes rte_intr_cap_single
or something like that is needed.

Re: [PATCH] eal/interrupts: Allow UIO interrupts when using igb_uio

[Vladimir Ratnikov]

[-- Attachment #1: Type: text/plain, Size: 1124 bytes --]

On systems with I225 interfaces it works in interrupt mode(rx), so not only
LSE interrupts are supported.
I could try add  rte_intr_cap_single functionality and recheck it twice(if
several interfaces works in rx_mode=interrupt)
But actually it worked with changes above(CPU utilization close to the
zero, data passes through the interface etc)

On Tue, Jul 4, 2023 at 5:55 PM Stephen Hemminger <stephen@networkplumber.org>
wrote:

> On Tue, 4 Jul 2023 12:45:54 +0200
> Vladimir Ratnikov <vratnikov@netgate.com> wrote:
>
> > Sorry for a long reply, sure.
> >
> > Stephen,
> > am I right that the most concern is about a place where interrupt
> > capabilities check appears for non MSI-X support?
> > What if having dedicated rte_intr_cap_single analog when there's no
> support
> > for MSI-X and just do the same(check capability, allocate interrupt
> vector
> > etc) ?
> >
> >
> > Regards,
> > -Vladimir
>
> With single interrupt, only link state interrupt is possible.
> Does that work with igb_uio?  It should, if not then yes
> rte_intr_cap_single
> or something like that is needed.
>

[-- Attachment #2: Type: text/html, Size: 1547 bytes --]

Re: [PATCH] eal/interrupts: Allow UIO interrupts when using igb_uio

[Stephen Hemminger]

On Tue, 4 Jul 2023 20:19:05 +0200
Vladimir Ratnikov <vratnikov@netgate.com> wrote:

> On systems with I225 interfaces it works in interrupt mode(rx), so not only
> LSE interrupts are supported.
> I could try add  rte_intr_cap_single functionality and recheck it twice(if
> several interfaces works in rx_mode=interrupt)
> But actually it worked with changes above(CPU utilization close to the
> zero, data passes through the interface etc)
> 

But this will cause mess with other devices.
For example igb has code that does:

        /* check and configure queue intr-vector mapping */                                          
        if ((rte_intr_cap_multiple(intr_handle) ||                                                   
             !RTE_ETH_DEV_SRIOV(dev).active) &&                                                      
            dev->data->dev_conf.intr_conf.rxq != 0) {                                                
                intr_vector = dev->data->nb_rx_queues;                                               
                if (rte_intr_efd_enable(intr_handle, intr_vector))                                   
                        return -1;                                                                   
        }                                                                                            
                                                                                                     
        /* Allocate the vector list */                                                               
        if (rte_intr_dp_is_en(intr_handle)) {                                                        
                if (rte_intr_vec_list_alloc(intr_handle, "intr_vec",                                 
                                                   dev->data->nb_rx_queues)) {                       
                        PMD_INIT_LOG(ERR, "Failed to allocate %d rx_queues"                          
                                     " intr_vec", dev->data->nb_rx_queues);                          
                        return -ENOMEM;                                                              
                }                                                                                    
        }                                                                                            
                                                                                                     
        /* configure MSI-X for Rx interrupt */                                                       
        eth_igb_configure_msix_intr(dev);   

MSI-X won't work with igb_uio because the interrupt vector region is not shared with userspace.
                                                         
                                                 

Re: [PATCH] eal/interrupts: Allow UIO interrupts when using igb_uio

[Stephen Hemminger]

On Tue, 4 Jul 2023 20:19:05 +0200
Vladimir Ratnikov <vratnikov@netgate.com> wrote:

> On systems with I225 interfaces it works in interrupt mode(rx), so not only
> LSE interrupts are supported.
> I could try add  rte_intr_cap_single functionality and recheck it twice(if
> several interfaces works in rx_mode=interrupt)
> But actually it worked with changes above(CPU utilization close to the
> zero, data passes through the interface etc)

If you want to use interrupts please use VFIO where it is possible to
support MSI-X correctly.

In the past, there was a proposed patch to handle multiple IRQ vectors
with igb_uio but it needed other kernel changes to work. These changes were
rejected upstream and led to the changes to VFIO to work without IOMMU.
That is a better and supported upstream.

RE: [EXT] [PATCH] eal/interrupts: Allow UIO interrupts when using igb_uio

[Harman Kalra]

>  Some drivers and devices(ex: igc + i225/i226) use RTE_INTR_HANDLE_UIO
> handler when captured under igb_uio so just let them use it.
> 
> Signed-off-by: Vladimir Ratnikov <vratnikov@netgate.com>


Reviewed-by: Harman Kalra <hkalra@marvell.com>

Thanks
Harman 

> ---
>  lib/eal/linux/eal_interrupts.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
> index c9881143be..037db09cfd 100644
> --- a/lib/eal/linux/eal_interrupts.c
> +++ b/lib/eal/linux/eal_interrupts.c
> @@ -1596,6 +1596,9 @@ rte_intr_cap_multiple(struct rte_intr_handle
> *intr_handle)
>  	if (rte_intr_type_get(intr_handle) == RTE_INTR_HANDLE_VDEV)
>  		return 1;
> 
> +	if (rte_intr_type_get(intr_handle) == RTE_INTR_HANDLE_UIO)
> +		return 1;
> +
>  	return 0;
>  }
> 
> --
> 2.34.1