> Subject: [PATCH] net/iavf: fix misleading AQ failure logging
>
> iavf_handle_virtchnl_msg() drains the admin receive queue in a loop
> until iavf_clean_arq_element() reports that no descriptors are
> pending. When the queue becomes empty, the base driver returns
> IAVF_ERR_ADMIN_QUEUE_NO_WORK (-57), which is the documented
> terminator for the drain loop, and is not an error.
>
> The current loop treats every non-IAVF_SUCCESS return as a failure
> and logs it as follows:
>
> "Failed to read msg from AdminQ, ret: -57"
>
> This message floods the logs on every interrupt cycle and misleads
> the triage during VF reset by chasing a real virtchnl problem
> seeing these spurious -57 AQ failure lines in logs and assumes
> the admin queue is broken, when in fact it has just been drained.
>
> This patch fixes the aforesaid issue by treating
> IAVF_ERR_ADMIN_QUEUE_NO_WORK in virtchnl message drain as a normal
> loop exit empty-queue condition and avoid logging it as an misleading
> AQ failure.
>
> Fixes: 02d212ca3125 ("net/iavf: rename remaining avf strings")
> Cc: [email protected]
>
> Signed-off-by: Anurag Mandal <[email protected]>
> ---
> drivers/net/intel/iavf/iavf_vchnl.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/intel/iavf/iavf_vchnl.c
> b/drivers/net/intel/iavf/iavf_vchnl.c
> index 94ccfb5d6e..870d5c1820 100644
> --- a/drivers/net/intel/iavf/iavf_vchnl.c
> +++ b/drivers/net/intel/iavf/iavf_vchnl.c
> @@ -570,7 +570,15 @@ iavf_handle_virtchnl_msg(struct rte_eth_dev *dev)
> while (pending) {
> ret = iavf_clean_arq_element(hw, &info, &pending);
>
> - if (ret != IAVF_SUCCESS) {
> + /*
> + * IAVF_ERR_ADMIN_QUEUE_NO_WORK (-57) means AQ is
> empty
> + * and is a normal way to terminate the drain loop.
> + * Log error only for genuine other failure codes.
> + * Incorrect logging like this during VF resets might
> + * mislead into chasing a non-existent AQ failure.
> + */
> + if (ret != IAVF_SUCCESS &&
> + ret != IAVF_ERR_ADMIN_QUEUE_NO_WORK) {
> PMD_DRV_LOG(INFO, "Failed to read msg from
> AdminQ,"
> "ret: %d", ret);
> break;
> --
I agree we should suppress the log on ret=NO_WORK but I
think we should still break instead of proceeding with the
message handling code. With your changes, in place of the
log:
IAVF_DRIVER: iavf_handle_virtchnl_msg(): Failed to read msg from
AdminQ,ret: -57
I now see:
IAVF_DRIVER: iavf_handle_virtchnl_msg(): Request 0 is not supported yet
I think that's because we enter the message handling code
and arrive at the default case in the switch statement.
