diff mbox series

[net-next,v3,1/2] igc: Link IRQs to NAPI instances

Message ID 20241018171343.314835-2-jdamato@fastly.com
State Changes Requested
Delegated to: Anthony Nguyen
Headers show
Series igc: Link IRQs and queues to NAPIs | expand

Commit Message

Joe Damato Oct. 18, 2024, 5:13 p.m. UTC
Link IRQs to NAPI instances via netdev-genl API so that users can query
this information with netlink.

Compare the output of /proc/interrupts (noting that IRQ 144 is the
"other" IRQ which does not appear to have a NAPI instance):

$ cat /proc/interrupts | grep enp86s0 | cut --delimiter=":" -f1
 128
 129
 130
 131
 132

The output from netlink shows the mapping of NAPI IDs to IRQs (again
noting that 144 is absent as it is the "other" IRQ):

$ ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/netdev.yaml \
                         --dump napi-get --json='{"ifindex": 2}'

[{'defer-hard-irqs': 0,
  'gro-flush-timeout': 0,
  'id': 8196,
  'ifindex': 2,
  'irq': 132},
 {'defer-hard-irqs': 0,
  'gro-flush-timeout': 0,
  'id': 8195,
  'ifindex': 2,
  'irq': 131},
 {'defer-hard-irqs': 0,
  'gro-flush-timeout': 0,
  'id': 8194,
  'ifindex': 2,
  'irq': 130},
 {'defer-hard-irqs': 0,
  'gro-flush-timeout': 0,
  'id': 8193,
  'ifindex': 2,
  'irq': 129}]

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 v2:
   - Line wrap at 80 characters

 drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Vinicius Costa Gomes Oct. 21, 2024, 5:48 p.m. UTC | #1
Joe Damato <jdamato@fastly.com> writes:

> Link IRQs to NAPI instances via netdev-genl API so that users can query
> this information with netlink.
>
> Compare the output of /proc/interrupts (noting that IRQ 144 is the
> "other" IRQ which does not appear to have a NAPI instance):
>
> $ cat /proc/interrupts | grep enp86s0 | cut --delimiter=":" -f1
>  128
>  129
>  130
>  131
>  132
>
> The output from netlink shows the mapping of NAPI IDs to IRQs (again
> noting that 144 is absent as it is the "other" IRQ):
>
> $ ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/netdev.yaml \
>                          --dump napi-get --json='{"ifindex": 2}'
>
> [{'defer-hard-irqs': 0,
>   'gro-flush-timeout': 0,
>   'id': 8196,
>   'ifindex': 2,
>   'irq': 132},
>  {'defer-hard-irqs': 0,
>   'gro-flush-timeout': 0,
>   'id': 8195,
>   'ifindex': 2,
>   'irq': 131},
>  {'defer-hard-irqs': 0,
>   'gro-flush-timeout': 0,
>   'id': 8194,
>   'ifindex': 2,
>   'irq': 130},
>  {'defer-hard-irqs': 0,
>   'gro-flush-timeout': 0,
>   'id': 8193,
>   'ifindex': 2,
>   'irq': 129}]
>
> Signed-off-by: Joe Damato <jdamato@fastly.com>
> ---

Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>


Cheers,
Lifshits, Vitaly Oct. 22, 2024, 7:05 a.m. UTC | #2
On 10/21/2024 8:48 PM, Vinicius Costa Gomes wrote:
> Joe Damato <jdamato@fastly.com> writes:
> 
>> Link IRQs to NAPI instances via netdev-genl API so that users can query
>> this information with netlink.
>>
>> Compare the output of /proc/interrupts (noting that IRQ 144 is the
>> "other" IRQ which does not appear to have a NAPI instance):
>>
>> $ cat /proc/interrupts | grep enp86s0 | cut --delimiter=":" -f1
>>   128
>>   129
>>   130
>>   131
>>   132
>>
>> The output from netlink shows the mapping of NAPI IDs to IRQs (again
>> noting that 144 is absent as it is the "other" IRQ):
>>
>> $ ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/netdev.yaml \
>>                           --dump napi-get --json='{"ifindex": 2}'
>>
>> [{'defer-hard-irqs': 0,
>>    'gro-flush-timeout': 0,
>>    'id': 8196,
>>    'ifindex': 2,
>>    'irq': 132},
>>   {'defer-hard-irqs': 0,
>>    'gro-flush-timeout': 0,
>>    'id': 8195,
>>    'ifindex': 2,
>>    'irq': 131},
>>   {'defer-hard-irqs': 0,
>>    'gro-flush-timeout': 0,
>>    'id': 8194,
>>    'ifindex': 2,
>>    'irq': 130},
>>   {'defer-hard-irqs': 0,
>>    'gro-flush-timeout': 0,
>>    'id': 8193,
>>    'ifindex': 2,
>>    'irq': 129}]
>>
>> Signed-off-by: Joe Damato <jdamato@fastly.com>
>> ---
> 
> Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>

Reviewed-by: Vitaly Lifshits <vitaly.lifshits@intel.com>

> 
> 
> Cheers,
Jacob Keller Oct. 22, 2024, 6:50 p.m. UTC | #3
On 10/18/2024 10:13 AM, Joe Damato wrote:
> Link IRQs to NAPI instances via netdev-genl API so that users can query
> this information with netlink.
> 
> Compare the output of /proc/interrupts (noting that IRQ 144 is the
> "other" IRQ which does not appear to have a NAPI instance):
> 

Minor nit: 144 doesn't appear in either output, and it seems like this
intended to indicate 128?

We think its a typo as the 144 appears in the data from the second commit.

I can make a note here to fix this typo when sending after we finish
validation, if there's no other issues.

Thanks,
Jake

> $ cat /proc/interrupts | grep enp86s0 | cut --delimiter=":" -f1
>  128
>  129
>  130
>  131
>  132
> 
> The output from netlink shows the mapping of NAPI IDs to IRQs (again
> noting that 144 is absent as it is the "other" IRQ):
> 
> $ ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/netdev.yaml \
>                          --dump napi-get --json='{"ifindex": 2}'
> 
> [{'defer-hard-irqs': 0,
>   'gro-flush-timeout': 0,
>   'id': 8196,
>   'ifindex': 2,
>   'irq': 132},
>  {'defer-hard-irqs': 0,
>   'gro-flush-timeout': 0,
>   'id': 8195,
>   'ifindex': 2,
>   'irq': 131},
>  {'defer-hard-irqs': 0,
>   'gro-flush-timeout': 0,
>   'id': 8194,
>   'ifindex': 2,
>   'irq': 130},
>  {'defer-hard-irqs': 0,
>   'gro-flush-timeout': 0,
>   'id': 8193,
>   'ifindex': 2,
>   'irq': 129}]
> 
> Signed-off-by: Joe Damato <jdamato@fastly.com>
> ---
Joe Damato Oct. 22, 2024, 7:54 p.m. UTC | #4
On Tue, Oct 22, 2024 at 11:50:15AM -0700, Jacob Keller wrote:
> 
> 
> On 10/18/2024 10:13 AM, Joe Damato wrote:
> > Link IRQs to NAPI instances via netdev-genl API so that users can query
> > this information with netlink.
> > 
> > Compare the output of /proc/interrupts (noting that IRQ 144 is the
> > "other" IRQ which does not appear to have a NAPI instance):
> > 
> 
> Minor nit: 144 doesn't appear in either output, and it seems like this
> intended to indicate 128?
> 
> We think its a typo as the 144 appears in the data from the second commit.
> 
> I can make a note here to fix this typo when sending after we finish
> validation, if there's no other issues.

Yes, that's an error on my part. Sorry about that. I re-ran the
patch after updating it and amended the commit message, but forgot
to update '144' to be '128'.

Based on the e1000 bug report that came in [1], I'm going to take
another look at the igc patches to make sure the paths where the
queue mapping happens (in Patch 2) are all in paths where rtnl is
held as I attempted to do for e1000 [2].

[1]: https://lore.kernel.org/netdev/8cf62307-1965-46a0-a411-ff0080090ff9@yandex.ru/
[2]: https://lore.kernel.org/netdev/20241022172153.217890-1-jdamato@fastly.com/T/#u
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 6e70bca15db1..7964bbedb16c 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -5576,6 +5576,9 @@  static int igc_request_msix(struct igc_adapter *adapter)
 				  q_vector);
 		if (err)
 			goto err_free;
+
+		netif_napi_set_irq(&q_vector->napi,
+				   adapter->msix_entries[vector].vector);
 	}
 
 	igc_configure_msix(adapter);