1. Patchew
  2. linux
  3. [PATCH net-next v3 0/3] gve: add support for PTP gettimex64
  4. View patch

[PATCH net-next v3 1/3] gve: skip error logging for retryable AdminQ commands

Harshitha Ramamurthy posted 3 patches 1 day, 22 hours ago
[PATCH net-next v3 1/3] gve: skip error logging for retryable AdminQ commands
Posted by Harshitha Ramamurthy 1 day, 22 hours ago 
From: Jordan Rhee <jordanrhee@google.com>

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 <jordanrhee@google.com>
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)
+				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;
-- 
2.53.0.1213.gd9a14994de-goog
Re: [PATCH net-next v3 1/3] gve: skip error logging for retryable AdminQ commands
Posted by Jacob Keller 1 day, 21 hours ago 
On 4/3/2026 12:44 PM, Harshitha Ramamurthy wrote:
> From: Jordan Rhee <jordanrhee@google.com>
> 
> 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.
> 

The implementation changes from using dev_err() to using
dev_err_ratelimited(). IMO that is a good change but it would be nice if
this was mentioned in the commit message.

Thanks,
Jake

> Reviewed-by: Joshua Washington <joshwash@google.com>
> Signed-off-by: Jordan Rhee <jordanrhee@google.com>
> 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++;
> -	}

You also now log every error regardless of what the status is since this
is no longer checked?

>  	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)
> +				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;
Re: [PATCH net-next v3 1/3] gve: skip error logging for retryable AdminQ commands
Posted by Jordan Rhee 1 day, 20 hours ago 
On Fri, Apr 3, 2026 at 2:11 PM Jacob Keller <jacob.e.keller@intel.com> wrote:
>
> On 4/3/2026 12:44 PM, Harshitha Ramamurthy wrote:
> > From: Jordan Rhee <jordanrhee@google.com>
> >
> > 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.
> >
>
> The implementation changes from using dev_err() to using
> dev_err_ratelimited(). IMO that is a good change but it would be nice if
> this was mentioned in the commit message.

Ack, will do.

>
> Thanks,
> Jake
>
> > Reviewed-by: Joshua Washington <joshwash@google.com>
> > Signed-off-by: Jordan Rhee <jordanrhee@google.com>
> > 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++;
> > -     }
>
> You also now log every error regardless of what the status is since this
> is no longer checked?

PASSED gets translated to err=0, so it will not be printed.

You bring up a good point that GVE_ADMINQ_COMMAND_UNSET will now
result in two messages being printed:
     dev_err(&priv->pdev->dev, "parse_aq_err: err and status both
unset, this should not be possible.\n");
     dev_err_ratelimited(&priv->pdev->dev, "AQ command %d failed with
status %d\n", opcode, status);

The first message doesn't make sense anymore, so I'll remove it.
Thanks for catching this.

>
> >       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)
> > +                             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;
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.