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

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

[Tomasz Lichwala]

Since the introduction of PTP support in iavf, commit 7c01dbfc8a1c
("iavf: periodically cache PHC time"), the periodic PTP clock caching
task always provides a pending admin queue command, causing
iavf_process_aq_command() to always return success and permanently
preventing the stats fallback path from executing, which results in VF
statistics remaining at zero despite traffic flowing.

Fix this by making the stats request unconditional when the adapter is in
the running state, rather than relying on it as a fallback when no other
admin queue commands were processed.

Making the stats request unconditional exposes a separate, preexisting
gap: iavf_virtchnl_send_ptp_cmd() sends VIRTCHNL_OP_1588_PTP_GET_TIME
without setting adapter->current_op. A GET_STATS request can now follow
it in the same watchdog pass, and if the PTP reply arrives first,
iavf_virtchnl_completion() unconditionally clears adapter->current_op,
dropping the completion tracking for the still-outstanding GET_STATS
reply. Fix this by setting current_op around the PTP send, matching
every other virtchnl command sender.

Fixes: 7c01dbfc8a1c ("iavf: periodically cache PHC time")
Fixes: 52e3beac764d ("iavf: add support for indirect access to PHC time")
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Tomasz Lichwala <tomasz.lichwala@linux.intel.com>
---
v2:
 - Set adapter->current_op around the PTP command send in
   iavf_virtchnl_send_ptp_cmd(), so a GET_STATS request issued right
   after it in the same watchdog pass no longer loses its completion
   tracking when the PTP reply lands first (found by Sashiko auto
   review on v1).
v1: https://lore.kernel.org/netdev/20260928230429.495442-6-anthony.l.nguyen@intel.com/

 drivers/net/ethernet/intel/iavf/iavf_main.c     | 14 ++++----------
 drivers/net/ethernet/intel/iavf/iavf_virtchnl.c |  2 ++
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index c7f69a904058..2cd01081ad85 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -2932,18 +2932,12 @@ static int iavf_watchdog_step(struct iavf_adapter *adapter)
 				iavf_send_api_ver(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);
 			iavf_detect_recover_hung(&adapter->vsi);
+		}
 		break;
 	case __IAVF_REMOVE:
 	default:
diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
index 680a28a739bf..acd1ed61af3a 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
@@ -1537,6 +1537,7 @@ void iavf_virtchnl_send_ptp_cmd(struct iavf_adapter *adapter)
 		goto out_unlock;
 	}
 
+	adapter->current_op = cmd->v_opcode;
 	err = iavf_send_pf_msg(adapter, cmd->v_opcode, cmd->msg, cmd->msglen);
 	if (!err) {
 		/* Command was sent without errors, so we can remove it from
@@ -1546,6 +1547,7 @@ void iavf_virtchnl_send_ptp_cmd(struct iavf_adapter *adapter)
 		kfree(cmd);
 	} else {
 		/* We failed to send the command, try again next cycle */
+		adapter->current_op = VIRTCHNL_OP_UNKNOWN;
 		pci_err(adapter->pdev, "Failed to send PTP command %d\n",
 			cmd->v_opcode);
 	}
-- 
2.54.0

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

[sashiko-bot]

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