Re: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: reintroduce retry mechanism for indirect AQ
> -Original Message- > From: Intel-wired-lan On Behalf Of Dawid > Osuchowski > Sent: 14 January 2026 01:08 > To: [email protected] > Cc: [email protected]; [email protected]; Loktionov, Aleksandr > ; Jakub Staniszewski > ; Kitszel, Przemyslaw > ; Dawid Osuchowski > > Subject: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: reintroduce retry > mechanism for indirect AQ > > From: Jakub Staniszewski > > Add retry mechanism for indirect Admin Queue (AQ) commands. To do so we need > to keep the command buffer. > > This technically reverts commit 43a630e37e25 > ("ice: remove unused buffer copy code in ice_sq_send_cmd_retry()"), but > combines it with a fix in the logic by using a kmemdup() call, making it more > robust and less likely to break in the future due to programmer error. > > Cc: Michal Schmidt > Cc: [email protected] > Fixes: 3056df93f7a8 ("ice: Re-send some AQ commands, as result of EBUSY AQ > error") > Signed-off-by: Jakub Staniszewski > Co-developed-by: Dawid Osuchowski > Signed-off-by: Dawid Osuchowski > Reviewed-by: Aleksandr Loktionov > Reviewed-by: Przemek Kitszel > --- > Ccing Michal, given they are the author of the "reverted" commit. > > drivers/net/ethernet/intel/ice/ice_common.c | 12 +--- > 1 file changed, 9 insertions(+), 3 deletions(-) > Tested-by: Rinitha S (A Contingent worker at Intel)
Re: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: reintroduce retry mechanism for indirect AQ
Hey Paul,
On 2026-01-13 11:31 PM, Paul Menzel wrote:
[Cc: +Michal]
Dear Dawid, dear Jakub,
...
Am 13.01.26 um 20:38 schrieb Dawid Osuchowski:
Ccing Michal, given they are the author of the "reverted" commit.
At least Michal was not in the (visible) Cc: list
Interesting. I was using 'git send-email' without any suppression of Cc
or similar options. In the direct email sent from me Michal is in Cc,
seems the mailing list for some reason stripped him...
drivers/net/ethernet/intel/ice/ice_common.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
...
do {
status = ice_sq_send_cmd(hw, cq, desc, buf, buf_size, cd);
if (!is_cmd_for_retry || !status ||
hw->adminq.sq_last_status != LIBIE_AQ_RC_EBUSY)
break;
+ if (buf_cpy)
+ memcpy(buf, buf_cpy, buf_size);
memcpy(desc, &desc_cpy, sizeof(desc_cpy));
-
Unrelated change?
During internal review it was pointed out that this function contains a
lot of empty lines, this was my feeble attempt to at least partially
reduce their count.
msleep(ICE_SQ_SEND_DELAY_TIME_MS);
} while (++idx < ICE_SQ_SEND_MAX_EXECUTE);
+ kfree(buf_cpy);
return status;
}
The diff looks good otherwise.
Reviewed-by: Paul Menzel
Kind regards,
Paul
Thanks,
Dawid
Re: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: reintroduce retry mechanism for indirect AQ
[Cc: +Michal]
Dear Dawid, dear Jakub,
Am 13.01.26 um 20:38 schrieb Dawid Osuchowski:
From: Jakub Staniszewski
Add retry mechanism for indirect Admin Queue (AQ) commands. To do so we
need to keep the command buffer.
This technically reverts commit 43a630e37e25
("ice: remove unused buffer copy code in ice_sq_send_cmd_retry()"),
but combines it with a fix in the logic by using a kmemdup() call,
making it more robust and less likely to break in the future due to
programmer error.
Cc: Michal Schmidt
Cc: [email protected]
Fixes: 3056df93f7a8 ("ice: Re-send some AQ commands, as result of EBUSY AQ
error")
Signed-off-by: Jakub Staniszewski
Co-developed-by: Dawid Osuchowski
Signed-off-by: Dawid Osuchowski
Reviewed-by: Aleksandr Loktionov
Reviewed-by: Przemek Kitszel
---
Ccing Michal, given they are the author of the "reverted" commit.
At least Michal was not in the (visible) Cc: list
drivers/net/ethernet/intel/ice/ice_common.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
b/drivers/net/ethernet/intel/ice/ice_common.c
index a400bf4f239a..aab00c44e9b2 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -1879,34 +1879,40 @@ ice_sq_send_cmd_retry(struct ice_hw *hw, struct
ice_ctl_q_info *cq,
{
struct libie_aq_desc desc_cpy;
bool is_cmd_for_retry;
+ u8 *buf_cpy = NULL;
u8 idx = 0;
u16 opcode;
int status;
opcode = le16_to_cpu(desc->opcode);
is_cmd_for_retry = ice_should_retry_sq_send_cmd(opcode);
memset(&desc_cpy, 0, sizeof(desc_cpy));
if (is_cmd_for_retry) {
- /* All retryable cmds are direct, without buf. */
- WARN_ON(buf);
+ if (buf) {
+ buf_cpy = kmemdup(buf, buf_size, GFP_KERNEL);
+ if (!buf_cpy)
+ return -ENOMEM;
+ }
memcpy(&desc_cpy, desc, sizeof(desc_cpy));
}
do {
status = ice_sq_send_cmd(hw, cq, desc, buf, buf_size, cd);
if (!is_cmd_for_retry || !status ||
hw->adminq.sq_last_status != LIBIE_AQ_RC_EBUSY)
break;
+ if (buf_cpy)
+ memcpy(buf, buf_cpy, buf_size);
memcpy(desc, &desc_cpy, sizeof(desc_cpy));
-
Unrelated change?
msleep(ICE_SQ_SEND_DELAY_TIME_MS);
} while (++idx < ICE_SQ_SEND_MAX_EXECUTE);
+ kfree(buf_cpy);
return status;
}
The diff looks good otherwise.
Reviewed-by: Paul Menzel
Kind regards,
Paul
