Re: [Intel-wired-lan] [PATCH iwl-net v2] idpf: do not perform flow ops when netdev is detached
From: Jacob Keller
Date: Thu Apr 23 2026 - 14:41:08 EST
On 4/23/2026 9:33 AM, Simon Horman wrote:
> From: 'Simon Horman' <horms@xxxxxxxxxx>
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev
> ---
> idpf: do not perform flow ops when netdev is detached
>
> This commit adds checks to prevent crashes when out-of-tree modules bypass
> ethtool core locks and call flow operations on detached netdevs during a
> hardware reset. It introduces netdev and netif_device_present checks in
> idpf_get_rxnfc and idpf_set_rxnfc.
>
>> Even though commit 2e281e1155fc ("idpf: detach and close netdevs while
>> handling a reset") prevents ethtool -N/-n operations to operate on
>> detached netdevs, we found that out-of-tree workflows like OpenOnload
>> can bypass ethtool core locks and call idpf_set_rxnfc directly during
>> an idpf HW reset.
>
> Is it acceptable to add defensive checks upstream solely to accommodate
> out-of-tree modules that bypass core kernel locking?
>
> The core ethtool infrastructure manages locks like rtnl_lock for in-tree
> callers to guarantee safety. Modifying the upstream driver to paper over
> locking violations in out-of-tree modules might contradict upstream policies.
>
> Simon says: If this is to address OOT module behaviour then I am concerned.
> And I would suggest that the correct approach is to fix
> the OOT kernel module.
>
The commit message says: "OpenOnload can bypass ethtool core locks and
call idpf_set_rxnfc directly". But if it somehow calls
idpf_set_rxnfc_directly without holding the appropriate locks, then no
amount of changes to the idpf driver will fix that issue. It is simply
unsafe for that to be occurring, and this feels like it just patches one
specific problem without addressing the root cause that something is
calling the drivers ethtool function without correctly holding the
expected locks.
@Li Li, could you please explain more details about the workflow that
triggers these behaviors? If it can't be reproduced with in-tree modules
then I don't think we can accept this fix.