Re: [Intel-wired-lan] [PATCH net] ice: fix netdev bring-up and bring-down in self-test

[Tony Nguyen <anthony.l.nguyen@intel.com>]

On 3/27/2026 12:23 AM, Aleksandr Loktionov wrote:
> From: Konstantin Ilichev <konstantin.ilichev@intel.com>
> 
> When an offline self-test is initiated with ethtool -t, any ongoing
> traffic could get stuck because ice_stop() and ice_open() are called
> without letting the OS know about state transitions.  In most cases
> a write() system call would block.
> 
> Fix this by calling dev_change_flags() to bring the netdev up and
> down, which ensures ndo_open()/ndo_stop() are called and all watchers
> are notified correctly.

+ Olek

AI review reports:

The ethtool core acquires the per-netdev mutex via netdev_lock_ops(dev) 
before invoking the driver's .self_test callback. dev_change_flags() is 
an exported API that explicitly re-acquires this exact same lock:
net/core/dev_api.c:dev_change_flags() {
	...
	netdev_lock_ops(dev);
	ret = netif_change_flags(dev, flags, extack);
	netdev_unlock_ops(dev);
	...
}
Because dev->lock is a standard, non-recursive mutex, this will result 
in a hard deadlock for any driver that opts into request_ops_lock. While 
ice might not currently set this flag, introducing nested lock 
acquisitions of the same mutex guarantees a deadlock as the subsystem 
migrates toward per-netdev locking.


With ice netdev lock changes in progress [1], this would soon become an 
issue.

Thanks,
Tony

[1] 
https://lore.kernel.org/netdev/20260325200644.2528726-4-anthony.l.nguyen@intel.com/

> Fixes: 0e674aeb0b77 ("ice: Add handler for ethtool selftest")
> Cc: stable@vger.kernel.org
> Co-developed-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> Signed-off-by: Konstantin Ilichev <konstantin.ilichev@intel.com>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
> 
>   drivers/net/ethernet/intel/ice/ice_ethtool.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index 96d95af..2a4f06f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -1416,7 +1416,7 @@ ice_self_test(struct net_device *netdev, struct ethtool_test *eth_test,
>   		/* If the device is online then take it offline */
>   		if (if_running)
>   			/* indicate we're in test mode */
> -			ice_stop(netdev);
> +			dev_change_flags(netdev, netdev->flags & ~IFF_UP, NULL);
>   
>   		data[ICE_ETH_TEST_LINK] = ice_link_test(netdev);
>   		data[ICE_ETH_TEST_EEPROM] = ice_eeprom_test(netdev);
> @@ -1434,10 +1434,12 @@ ice_self_test(struct net_device *netdev, struct ethtool_test *eth_test,
>   		clear_bit(ICE_TESTING, pf->state);
>   
>   		if (if_running) {
> -			int status = ice_open(netdev);
> +			int status = dev_change_flags(netdev,
> +						      netdev->flags | IFF_UP,
> +						      NULL);
>   
>   			if (status) {
> -				dev_err(dev, "Could not open device %s, err %d\n",
> +				dev_err(dev, "Could not bring up device %s, err %d\n",
>   					pf->int_name, status);
>   			}
>   		}