Skip to contentSkip to conversation

eal/interrupts: Allow UIO interrupts when using igb_uio

PATCH In review Vladimir Ratnikov 4 participants 9 messages Jun 14, 2023 – Oct 31, 2023

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 eal/interrupts: Allow UIO interrupts when using igb_uio

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.

---
 lib/eal/linux/eal_interrupts.c | 3 +++
 1 file changed, 3 insertions(+)
lib/eal/linux/eal_interrupts.c +3 -0
index c9881143be..037db09cfd 100644
@@ -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;
 }
 
Signature · 1 line
2.34.1
#1 lore raw
Quoted 5 lines · On Wed, 14 Jun 2023 13:40:18 +0000 Vladimir Ratnikov <[email protected]>…

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.

This doesn't look right.

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

#2 lore raw

14/06/2023 18:46, Stephen Hemminger:

Quoted 7 lines
Quoted 4 lines · On Wed, 14 Jun 2023 13:40:18 +0000 Vladimir Ratnikov <[email protected]>…

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.

Quoted 5 lines
Quoted 1 line

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.

#3 lore raw

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

Quoted 19 lines · Thomas Monjalon

14/06/2023 18:46, Stephen Hemminger:

Quoted 7 lines
Quoted 4 lines · On Wed, 14 Jun 2023 13:40:18 +0000 Vladimir Ratnikov <[email protected]>…

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.

Quoted 5 lines
Quoted 1 line

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.

#4 lore raw
Quoted 10 lines · On Tue, 4 Jul 2023 12:45:54 +0200 Vladimir Ratnikov <[email protected]>…

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) ?

Quoted 2 lines

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.

#5 lore raw

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)

Quoted 23 lines · Stephen Hemminger
Quoted 6 lines · On Tue, 4 Jul 2023 12:45:54 +0200 Vladimir Ratnikov <[email protected]>…

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

Quoted 1 line

for MSI-X and just do the same(check capability, allocate interrupt

vector

Quoted 5 lines

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.

#6 lore raw
Quoted 7 lines · On Tue, 4 Jul 2023 20:19:05 +0200 Vladimir Ratnikov <[email protected]>…

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.

#7 lore raw
In reply to Vladimir Ratnikov
Quoted 6 lines · On Tue, 4 Jul 2023 20:19:05 +0200 Vladimir Ratnikov <[email protected]>…

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.

Reviewed-by on v1 1/1
#8 lore raw
In reply to Vladimir Ratnikov
Quoted 5 lines

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.

Thanks
Harman

Quoted 21 lines
---
 lib/eal/linux/eal_interrupts.c | 3 +++
 1 file changed, 3 insertions(+)
lib/eal/linux/eal_interrupts.c +3 -1
index c9881143be..037db09cfd 100644
@@ -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