netdev - Re: [PATCH iwl-next] ice: use irq_update_affinity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 28 May 2024 08:16:39 +0200
From: Michal Schmidt <mschmidt@...hat.com>
To: Jacob Keller <jacob.e.keller@...el.com>
Cc: Jesse Brandeburg <jesse.brandeburg@...el.com>, Tony Nguyen <anthony.l.nguyen@...el.com>, 
	intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org, 
	Nitesh Narayan Lal <nitesh@...hat.com>, Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH iwl-next] ice: use irq_update_affinity_hint()

On Fri, May 24, 2024 at 11:04 PM Jacob Keller <jacob.e.keller@...el.com> wrote:
> On 5/22/2024 4:12 PM, Michal Schmidt wrote:
> > irq_set_affinity_hint() is deprecated. Use irq_update_affinity_hint()
> > instead. This removes the side-effect of actually applying the affinity.
> >
> > The driver does not really need to worry about spreading its IRQs across
> > CPUs. The core code already takes care of that.
> > On the contrary, when the driver applies affinities by itself, it breaks
> > the users' expectations:
> >  1. The user configures irqbalance with IRQBALANCE_BANNED_CPULIST in
> >     order to prevent IRQs from being moved to certain CPUs that run a
> >     real-time workload.
> >  2. ice reconfigures VSIs at runtime due to a MIB change
> >     (ice_dcb_process_lldp_set_mib_change). Reopening a VSI resets the
> >     affinity in ice_vsi_req_irq_msix().
> >  3. ice has no idea about irqbalance's config, so it may move an IRQ to
> >     a banned CPU. The real-time workload suffers unacceptable latency.
> >
> > I am not sure if updating the affinity hints is at all useful, because
> > irqbalance ignores them since 2016 ([1]), but at least it's harmless.
> >
> > This ice change is similar to i40e commit d34c54d1739c ("i40e: Use
> > irq_update_affinity_hint()").
> >
> > [1] https://github.com/Irqbalance/irqbalance/commit/dcc411e7bfdd
> >
>
> This is sent tagged for next, but the commit description reads more like
> it fixes deployments running irqbalance. Would it make sense to mark
> this with Fixes and target net instead?

I chose to not add a Fixes tag here. The similar commits for i40e and
other drivers did not have them either. If I had to add Fixes, the
referenced commit could be cdedef59deb0 ("ice: Configure VSIs for
Tx/Rx"), from 2018. But that's not good, because
irq_update_affinity_hint was added in 2021 with commit 65c7cdedeb30
("genirq: Provide new interfaces for affinity hints"). I'm OK with
next.

Michal