[PATCH] net/iavf: fix misleading AQ failure logging

[Loftus, Ciara]

> 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: stable at dpdk.org
> 
> Signed-off-by: Anurag Mandal <anurag.mandal at intel.com>
> ---
>  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.