On Thu, Jul 09, 2026 at 09:37:27AM +0000, Jagielski, Jedrzej wrote:
> From: Simon Horman <[email protected]>
> Sent: Wednesday, July 8, 2026 7:02 PM
>
> >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://netdev-ai.bots.linux.dev/sashiko/
> >---
> >ixgbe: E610: add MAC address runtime refresh
> >
> >This change polls RAR0 each service-task cycle on E610 and updates
> >netdev->dev_addr, hw->mac.addr and mac_table[0].addr when firmware has
> >altered the MAC. A refresh is also invoked from ixgbe_reset() so the
> >address survives RAR0 clearing during init_hw().
[...]
> >[Medium]
> >Would it make sense to hoist the E610 gating out of
> >ixgbe_mac_addr_refresh() and skip the rtnl_lock()/rtnl_unlock()
> >entirely for non-E610 devices?
> >
> >The type check lives inside the helper:
> >
> > static void ixgbe_mac_addr_refresh(struct ixgbe_adapter *adapter)
> > {
> > ...
> > if (hw->mac.type != ixgbe_mac_e610)
> > return;
> > ...
> > }
> >
> >so 82598, 82599, X540 and X550 adapters (which comprise most of the
> >ixgbe device family) acquire the global rtnl_mutex every service-task
> >cycle only to return immediately. Something like:
> >
> > if (adapter->hw.mac.type == ixgbe_mac_e610) {
> > rtnl_lock();
> > ixgbe_mac_addr_refresh(adapter);
> > rtnl_unlock();
> > }
>
> Cannot introduce rtnl locking into ixgbe_mac_addr_refresh() as this
> can be already called within rtnl-locked path
i40e does a lot of dancing for cases like this. rtnl lock is handled via
boolean provided as function arg:
static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
...
/* Release the RTNL lock before we start resetting VFs */
if (!lock_acquired)
rtnl_unlock();
...
end_unlock:
if (!lock_acquired)
rtnl_unlock();
>
> >
> >would avoid the rtnl_mutex contention on non-E610 hardware.
> >
> >> ixgbe_reset_subtask(adapter);
> >> ixgbe_phy_interrupt_subtask(adapter);
> >> ixgbe_sfp_detection_subtask(adapter);
