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 <[email protected]> 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/[email protected]?part=1
