Hi Li, thank you very much for the review. The intent is only to skip
logging for *retryable* commands that return -EAGAIN. If a
non-retryable command fails, we do want to log, even if it returns
-EAGAIN.
Jordan


On Thu, Mar 26, 2026 at 7:28 PM Li Xiasong [off-list ref] wrote:

Hi,

On 3/27/2026 6:45 AM, Harshitha Ramamurthy wrote:

quoted

From: Jordan Rhee <redacted>

AdminQ commands may return -EAGAIN under certain transient conditions.
These commands are intended to be retried by the driver, so logging
a formal error to the system log is misleading and creates
unnecessary noise.

Modify the logging logic to skip the error message when the result
is -EAGAIN.

Reviewed-by: Joshua Washington <joshwash@google.com>
Signed-off-by: Jordan Rhee <redacted>
Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
---
 drivers/net/ethernet/google/gve/gve_adminq.c | 26 +++++++++++++++-----
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
index 08587bf40ed4..c7834614c5f0 100644
--- a/drivers/net/ethernet/google/gve/gve_adminq.c
+++ b/drivers/net/ethernet/google/gve/gve_adminq.c

@@ -416,11 +416,6 @@ static bool gve_adminq_wait_for_cmd(struct gve_priv *priv, u32 prod_cnt)

 static int gve_adminq_parse_err(struct gve_priv *priv, u32 status)
 {
-     if (status != GVE_ADMINQ_COMMAND_PASSED &&
-         status != GVE_ADMINQ_COMMAND_UNSET) {
-             dev_err(&priv->pdev->dev, "AQ command failed with status %d\n", status);
-             priv->adminq_cmd_fail++;
-     }
      switch (status) {
      case GVE_ADMINQ_COMMAND_PASSED:
              return 0;

@@ -455,6 +450,16 @@ static int gve_adminq_parse_err(struct gve_priv *priv, u32 status)
      }
 }

+static bool gve_adminq_is_retryable(enum gve_adminq_opcodes opcode)
+{
+     switch (opcode) {
+     case GVE_ADMINQ_REPORT_NIC_TIMESTAMP:
+             return true;
+     default:
+             return false;
+     }
+}
+
 /* Flushes all AQ commands currently queued and waits for them to complete.
  * If there are failures, it will return the first error.
  */

@@ -482,9 +487,18 @@ static int gve_adminq_kick_and_wait(struct gve_priv *priv)
              cmd = &priv->adminq[i & priv->adminq_mask];
              status = be32_to_cpu(READ_ONCE(cmd->status));
              err = gve_adminq_parse_err(priv, status);
-             if (err)
+             if (err) {
+                     enum gve_adminq_opcodes opcode =
+                             be32_to_cpu(READ_ONCE(cmd->opcode));
+                     priv->adminq_cmd_fail++;
+                     if (!gve_adminq_is_retryable(opcode) || err != -EAGAIN)

In gve_adminq_kick_and_wait(), the condition is:

        if (!gve_adminq_is_retryable(opcode) || err != -EAGAIN)
                dev_err_ratelimited(...);

Based on the commit log, the goal is to skip logging when the result is
-EAGAIN for transient conditions. However, when gve_adminq_is_retryable()
returns false (e.g., GVE_ADMINQ_COMMAND_ERROR_ABORTED), even if err is
-EAGAIN, the condition evaluates to true and the error would still be logged.

Would it be more appropriate to use && instead of || here?

        if (!gve_adminq_is_retryable(opcode) && err != -EAGAIN)

I may be missing something, so please let me know if I've misunderstood.

quoted

+                             dev_err_ratelimited(&priv->pdev->dev,
+                                                 "AQ command %d failed with status %d\n",
+                                                 opcode, status);
+
                      // Return the first error if we failed.
                      return err;
+             }
      }

      return 0;

Best regards,
Li Xiasong