eal/interrupts: Allow UIO interrupts when using igb_uio

Message ID 20230614134018.2344-1-vratnikov@netgate.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers
Series eal/interrupts: Allow UIO interrupts when using igb_uio |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/intel-Functional success Functional PASS
ci/iol-abi-testing success Testing PASS
ci/iol-aarch-unit-testing success Testing PASS
ci/iol-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

Vladimir Ratnikov June 14, 2023, 1:40 p.m. UTC
  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(+)
  

Comments

Stephen Hemminger June 14, 2023, 4:46 p.m. UTC | #1
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.
  
Harman Kalra July 3, 2023, 7:19 a.m. UTC | #2
>  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
  
Thomas Monjalon July 3, 2023, 3:32 p.m. UTC | #3
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.
  
Vladimir Ratnikov July 4, 2023, 10:45 a.m. UTC | #4
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.
>
>
>
  
Stephen Hemminger July 4, 2023, 3:55 p.m. UTC | #5
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.
  
Vladimir Ratnikov July 4, 2023, 6:19 p.m. UTC | #6
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.
>
  
Stephen Hemminger July 5, 2023, 11:27 p.m. UTC | #7
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.
  
Stephen Hemminger Oct. 31, 2023, 5:17 p.m. UTC | #8
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.
  

Patch

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;
 }