Re: [Intel-wired-lan] [PATCH iwl-next v1 5/6] ixgbe: E610: add MAC address runtime refresh

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

On Thu, Jul 09, 2026 at 09:37:27AM +0000, Jagielski, Jedrzej wrote:
> From: Simon Horman <horms@kernel.org> 
> 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);