Re: [iwl-net v2] iavf: fix VF stats not updating due to PTP command preemption

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] iavf: permanent starvation of background VF stats due to PTP worker preemption
--

commit 1a35506e39b53de13d7adda52b712e11b62285e8
Author: Tomasz Lichwala <tomasz.lichwala@linux.intel.com>

iavf: fix VF stats not updating due to PTP command preemption

This commit attempts to fix frozen VF stats by making stats requests
unconditional when the adapter is in the running state. It also sets
adapter->current_op around the PTP send to avoid dropping completion tracking.

> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index b6e10cc9faca0..830da8f1cdb62 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -2983,18 +2983,12 @@ static int iavf_watchdog_step(struct iavf_adapter *adapter)
[ ... ]
>  		} else {
> -			int ret = iavf_process_aq_command(adapter);
> -
> -			/* An error will be returned if no commands were
> -			 * processed; use this opportunity to update stats
> -			 * if the error isn't -ENOTSUPP
> -			 */
> -			if (ret && ret != -EOPNOTSUPP &&
> -			    adapter->state == __IAVF_RUNNING)
> -				iavf_request_stats(adapter);
> +			iavf_process_aq_command(adapter);
>  		}
> -		if (adapter->state == __IAVF_RUNNING)
> +		if (adapter->state == __IAVF_RUNNING) {
> +			iavf_request_stats(adapter);

[Severity: High]
Could this regression lead to permanent starvation of the background VF stats
request?

The PTP auxiliary worker runs periodically, queuing a PTP command and waking
the watchdog immediately via mod_delayed_work. Since the maximum interval
between PTP commands is around 1500ms, the watchdog's 2000ms natural idle
timer never expires. It only executes when preempted by the PTP worker.

During this PTP-awoken watchdog step, iavf_process_aq_command() processes the
PTP command. With the changes in this patch, iavf_virtchnl_send_ptp_cmd() now
explicitly sets adapter->current_op = VIRTCHNL_OP_1588_PTP_GET_TIME.

Immediately after, the watchdog unconditionally calls iavf_request_stats().
However, iavf_request_stats() checks if current_op is occupied:

drivers/net/ethernet/intel/iavf/iavf_virtchnl.c:iavf_request_stats() {
        if (adapter->current_op != VIRTCHNL_OP_UNKNOWN) {
                /* no error message, this isn't crucial */
                return;
        }
        ...
}

Because netdev_lock is held by the watchdog, iavf_adminq_task cannot run
concurrently to process the PF's reply and clear current_op. Thus, the stats
request silently aborts.

Since this cycle repeats indefinitely, will VF statistics remain permanently
frozen at zero?

>  			iavf_detect_recover_hung(&adapter->vsi);
> +		}
>  		break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015140409.3124712-1-tomasz.lichwala@linux.intel.com?part=1