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]

Message-ID: <28f2d06d-7ddc-4b98-bc58-b560028611ed@intel.com>
Date: Fri, 24 May 2024 14:04:28 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Michal Schmidt <mschmidt@...hat.com>, Jesse Brandeburg
	<jesse.brandeburg@...el.com>, Tony Nguyen <anthony.l.nguyen@...el.com>,
	<intel-wired-lan@...ts.osuosl.org>
CC: <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 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?

> Signed-off-by: Michal Schmidt <mschmidt@...hat.com>