netdev - Re: [Intel-wired-lan] [PATCH iwl-net 2/2] ice: fix retry for AQ command 0x06EE

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] [day] [month] [year] [list]

Message-ID: <ff44a005-6ebf-45ed-9b84-804d44e2158c@molgen.mpg.de>
Date: Wed, 14 Jan 2026 10:16:29 +0100
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Dawid Osuchowski <dawid.osuchowski@...ux.intel.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
 Jakub Staniszewski <jakub.staniszewski@...ux.intel.com>,
 stable@...r.kernel.org, Aleksandr Loktionov <aleksandr.loktionov@...el.com>,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-net 2/2] ice: fix retry for AQ
 command 0x06EE

Dear Dawid, dear Jakub,


Thank you for your patch.

Am 13.01.26 um 20:38 schrieb Dawid Osuchowski:
> From: Jakub Staniszewski <jakub.staniszewski@...ux.intel.com>
> 
> Executing ethtool -m can fail reporting a netlink I/O error while firmware
> link management holds the i2c bus used to communicate with the module.
> 
> According to Intel(R) Ethernet Controller E810 Datasheet Rev 2.8 [1]
> Section 3.3.10.4 Read/Write SFF EEPROM (0x06EE)
> request should to be retried upon receiving EBUSY from firmware.
> 
> Commit e9c9692c8a81 ("ice: Reimplement module reads used by ethtool")
> implemented it only for part of ice_get_module_eeprom(), leaving all other
> calls to ice_aq_sff_eeprom() vulnerable to returning early on getting
> EBUSY without retrying.
> 
> Remove the retry loop from ice_get_module_eeprom() and add Admin Queue
> (AQ) command with opcode 0x06EE to the list of commands that should be
> retried on receiving EBUSY from firmware.
> 
> Cc: stable@...r.kernel.org
> Fixes: e9c9692c8a81 ("ice: Reimplement module reads used by ethtool")
> Signed-off-by: Jakub Staniszewski <jakub.staniszewski@...ux.intel.com>
> Co-developed-by: Dawid Osuchowski <dawid.osuchowski@...ux.intel.com>
> Signed-off-by: Dawid Osuchowski <dawid.osuchowski@...ux.intel.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> Link: https://www.intel.com/content/www/us/en/content-details/613875/intel-ethernet-controller-e810-datasheet.html [1]
> ---
>   drivers/net/ethernet/intel/ice/ice_common.c  |  1 +
>   drivers/net/ethernet/intel/ice/ice_ethtool.c | 35 ++++++++------------
>   2 files changed, 15 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
> index aab00c44e9b2..26eb8e05498b 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -1854,6 +1854,7 @@ static bool ice_should_retry_sq_send_cmd(u16 opcode)
>   	case ice_aqc_opc_lldp_stop:
>   	case ice_aqc_opc_lldp_start:
>   	case ice_aqc_opc_lldp_filter_ctrl:
> +	case ice_aqc_opc_sff_eeprom:
>   		return true;
>   	}
>   
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index 3565a5d96c6d..478876908db1 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -4496,7 +4496,7 @@ ice_get_module_eeprom(struct net_device *netdev,
>   	u8 addr = ICE_I2C_EEPROM_DEV_ADDR;
>   	struct ice_hw *hw = &pf->hw;
>   	bool is_sfp = false;
> -	unsigned int i, j;
> +	unsigned int i;
>   	u16 offset = 0;
>   	u8 page = 0;
>   	int status;
> @@ -4538,26 +4538,19 @@ ice_get_module_eeprom(struct net_device *netdev,
>   		if (page == 0 || !(data[0x2] & 0x4)) {
>   			u32 copy_len;
>   
> -			/* If i2c bus is busy due to slow page change or
> -			 * link management access, call can fail. This is normal.
> -			 * So we retry this a few times.
> -			 */
> -			for (j = 0; j < 4; j++) {
> -				status = ice_aq_sff_eeprom(hw, 0, addr, offset, page,
> -							   !is_sfp, value,
> -							   SFF_READ_BLOCK_SIZE,
> -							   0, NULL);
> -				netdev_dbg(netdev, "SFF %02X %02X %02X %X = %02X%02X%02X%02X.%02X%02X%02X%02X (%X)\n",
> -					   addr, offset, page, is_sfp,
> -					   value[0], value[1], value[2], value[3],
> -					   value[4], value[5], value[6], value[7],
> -					   status);
> -				if (status) {
> -					usleep_range(1500, 2500);
> -					memset(value, 0, SFF_READ_BLOCK_SIZE);
> -					continue;
> -				}
> -				break;
> +			status = ice_aq_sff_eeprom(hw, 0, addr, offset, page,
> +						   !is_sfp, value,
> +						   SFF_READ_BLOCK_SIZE,
> +						   0, NULL);
> +			netdev_dbg(netdev, "SFF %02X %02X %02X %X = %02X%02X%02X%02X.%02X%02X%02X%02X (%pe)\n",
> +				   addr, offset, page, is_sfp,
> +				   value[0], value[1], value[2], value[3],
> +				   value[4], value[5], value[6], value[7],
> +				   ERR_PTR(status));
> +			if (status) {
> +				netdev_err(netdev, "%s: error reading module EEPROM: status %pe\n",
> +					   __func__, ERR_PTR(status));
> +				return status;
>   			}
>   
>   			/* Make sure we have enough room for the new block */

Reviewed-by: Paul Menzel <pmenzel@...gen.mpg.de>


Kind regards,

Paul