[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAMzD94TJ7pg8gVbUCAhtfkpL4+FtSfepqd=CJ-imNFq-A5W0gw@mail.gmail.com>
Date: Mon, 26 Jan 2026 16:55:53 -0500
From: Brian Vazquez <brianvv@...gle.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Eric Dumazet <edumazet@...gle.com>, Jacob Keller <jacob.e.keller@...el.com>,
Brian Vazquez <brianvv.kernel@...il.com>, Tony Nguyen <anthony.l.nguyen@...el.com>,
Przemek Kitszel <przemyslaw.kitszel@...el.com>, "David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, intel-wired-lan@...ts.osuosl.org,
David Decotigny <decot@...gle.com>, Li Li <boolli@...gle.com>,
Anjali Singhai <anjali.singhai@...el.com>, Sridhar Samudrala <sridhar.samudrala@...el.com>,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
emil.s.tantilov@...el.com, Brett Creeley <brett.creeley@....com>,
Aleksandr Loktionov <aleksandr.loktionov@...el.com>
Subject: Re: [Intel-wired-lan] [iwl-net PATCH v2] idpf: change IRQ naming to
match netdev and ethtool queue numbering
Sure, sending v3 to add more details about the ABI.
On Mon, Jan 26, 2026 at 4:12 PM Andrew Lunn <andrew@...n.ch> wrote:
>
> On Mon, Jan 26, 2026 at 09:51:10PM +0100, Eric Dumazet wrote:
> > On Mon, Jan 26, 2026 at 9:46 PM Jacob Keller <jacob.e.keller@...el.com> wrote:
> > >
> > >
> > >
> > > On 1/26/2026 9:53 AM, Andrew Lunn wrote:
> > > > On Mon, Jan 26, 2026 at 12:40:15PM -0500, Brian Vazquez wrote:
> > > >> On Mon, Jan 26, 2026 at 11:24 AM Andrew Lunn <andrew@...n.ch> wrote:
> > > >>>
> > > >>> On Mon, Jan 26, 2026 at 02:46:24PM +0000, Brian Vazquez wrote:
> > > >>>> The code uses the vidx for the IRQ name but that doesn't match ethtool
> > > >>>> reporting or netdev naming, this makes it hard to tune the device and
> > > >>>> associate queues with IRQs. Sequentially requesting irqs starting from
> > > >>>> '0' makes the output consistent.
> > > >>>>
> > > >>>> Before:
> > > >>>>
> > > >>>> ethtool -L eth1 tx 1 combined 3
> > > >>>>
> > > >>>> grep . /proc/irq/*/*idpf*/../smp_affinity_list
> > > >>>> /proc/irq/67/idpf-Mailbox-0/../smp_affinity_list:0-55,112-167
> > > >>>> /proc/irq/68/idpf-eth1-TxRx-1/../smp_affinity_list:0
> > > >>>> /proc/irq/70/idpf-eth1-TxRx-3/../smp_affinity_list:1
> > > >>>> /proc/irq/71/idpf-eth1-TxRx-4/../smp_affinity_list:2
> > > >>>> /proc/irq/72/idpf-eth1-Tx-5/../smp_affinity_list:3
> > > >>>>
> > > >>>> ethtool -S eth1 | grep -v ': 0'
> > > >>>> NIC statistics:
> > > >>>> tx_q-0_pkts: 1002
> > > >>>> tx_q-1_pkts: 2679
> > > >>>> tx_q-2_pkts: 1113
> > > >>>> tx_q-3_pkts: 1192 <----- tx_q-3 vs idpf-eth1-Tx-5
> > > >>>> rx_q-0_pkts: 1143
> > > >>>> rx_q-1_pkts: 3172
> > > >>>> rx_q-2_pkts: 1074
> > > >>>>
> > > >>>> After:
> > > >>>>
> > > >>>> ethtool -L eth1 tx 1 combined 3
> > > >>>>
> > > >>>> grep . /proc/irq/*/*idpf*/../smp_affinity_list
> > > >>>>
> > > >>>> /proc/irq/67/idpf-Mailbox-0/../smp_affinity_list:0-55,112-167
> > > >>>> /proc/irq/68/idpf-eth1-TxRx-0/../smp_affinity_list:0
> > > >>>> /proc/irq/70/idpf-eth1-TxRx-1/../smp_affinity_list:1
> > > >>>> /proc/irq/71/idpf-eth1-TxRx-2/../smp_affinity_list:2
> > > >>>> /proc/irq/72/idpf-eth1-Tx-3/../smp_affinity_list:3
> > > >>>>
> > > >>>> ethtool -S eth1 | grep -v ': 0'
> > > >>>> NIC statistics:
> > > >>>> tx_q-0_pkts: 118
> > > >>>> tx_q-1_pkts: 134
> > > >>>> tx_q-2_pkts: 228
> > > >>>> tx_q-3_pkts: 138 <--- tx_q-3 matches idpf-eth1-Tx-3
> > > >>>> rx_q-0_pkts: 111
> > > >>>> rx_q-1_pkts: 366
> > > >>>> rx_q-2_pkts: 120
> > > >>>
> > > >>> Are there any ABI issues here?
> > > >>
> > > >> The patch doesn't change the format, it just fixes the numbering in
> > > >> the name to make it consistent with other reporting tools. It
> > > >> shouldn't break any library.
> > > >
> > > > But is the numbering part of the ABI?
> > > >
> > > > Making a comment about ABI in the commit message makes it clear it is
> > > > something you have considered, and you have decided it is not an
> > > > issue. If there is no such comment, reviewers probably should ask.
> > > >
> > > > Andrew
> > >
> > > I don't see how an application can depend on the name if it can't
> > > correlate it to anything meaningful. The change fixes the ID values used
> > > so that they *do* correlate. If an application was previously assuming
> > > it correlated to the queue ID, it would incorrect associate the IRQ with
> > > the wrong queue.
> >
> > Agree, this mismatch caused me some surprises, we can not keep a broken
> > ABI (which has been broken by accident)
>
> So everybody is agreed, great. I just think it would of been good to
> mention ABI in the commit message, to show due diligence has been
> done.
>
> Andrew
Powered by blists - more mailing lists