[PATCH iwl-net v2] iavf: reset the VF instead of freeing rings when queue disable fails

[PATCH iwl-net v2] iavf: reset the VF instead of freeing rings when queue disable fails

[Yuho Choi]

iavf_close() leaves the Tx/Rx rings in place because the hardware can
still DMA into them, and iavf_virtchnl_completion() frees them once the
PF has answered VIRTCHNL_OP_DISABLE_QUEUES. It does so whatever the PF
answered: for a non-zero v_retval the error is only logged and the
DISABLE_QUEUES case runs anyway.

The PF can fail the request while queues are still running. ice and
i40e give up before the Rx queues when a Tx queue cannot be stopped,
and ice also fails when an Rx queue does not stop in time. The rings
and Rx buffers are then freed while the queues still point at them, and
received packets and descriptor write-backs land in freed memory.

Keep the rings when the PF reports an error and request a VF reset
instead. The reset makes the PF stop all queues of the VF, and
iavf_reset_step() frees the rings once it has completed. Until then
iavf_close() may warn that the resources are not yet released, and
iavf_open() returns -EBUSY.

During removal no reset is scheduled, and iavf_remove() would wait for
the state to leave __IAVF_DOWN_PENDING forever. As it resets the VF
itself before it frees the rings, complete the transition to
__IAVF_DOWN in that case. If the reset never completes,
iavf_disable_vf() now frees the kept rings as it does for a running VF,
instead of leaving them with page pools that point to the q_vectors it
frees.

Fixes: e284fc88dfb6 ("i40evf: delay releasing rings")
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
v2:
- Let iavf_remove() go on when the failure arrives during removal, where
  no reset can be scheduled (sashiko).
- Free the kept rings in iavf_disable_vf() when the reset never
  completes (sashiko).
- Rebased on v7.4-rc1.

v1: https://lore.kernel.org/r/20261022192753.69765-2-oss.patchbox@gmail.com

This was found by code inspection as part of ongoing, not yet
published research. It has not been reproduced, as that needs a PF
that fails VIRTCHNL_OP_DISABLE_QUEUES.
Compile-tested only (x86_64 defconfig with IAVF, I40E and ICE, W=1).

 drivers/net/ethernet/intel/iavf/iavf_main.c    |  7 +++++--
 .../net/ethernet/intel/iavf/iavf_virtchnl.c    | 18 ++++++++++++++++--
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 2f8a80a11336..70ee5ae14a64 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -3064,9 +3064,12 @@ static void iavf_disable_vf(struct iavf_adapter *adapter)
 		iavf_napi_disable_all(adapter);
 		iavf_irq_disable(adapter);
 		iavf_free_traffic_irqs(adapter);
-		iavf_free_all_tx_resources(adapter);
-		iavf_free_all_rx_resources(adapter);
 	}
+	/* Also release rings that iavf_close() left in place because the
+	 * PF failed to disable the queues.
+	 */
+	iavf_free_all_tx_resources(adapter);
+	iavf_free_all_rx_resources(adapter);
 
 	spin_lock_bh(&adapter->mac_vlan_list_lock);
 
diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
index e6b7e8f82c7c..28818319a6df 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
@@ -2718,8 +2718,22 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
 		adapter->flags &= ~IAVF_FLAG_QUEUES_DISABLED;
 		break;
 	case VIRTCHNL_OP_DISABLE_QUEUES:
-		iavf_free_all_tx_resources(adapter);
-		iavf_free_all_rx_resources(adapter);
+		/* If the PF could not stop the queues, the hardware may still
+		 * access the rings and Rx buffers. Keep them and request a VF
+		 * reset; the reset path releases them after the PF has reset
+		 * the VF. During removal no reset is scheduled, but
+		 * iavf_remove() resets the VF itself before it releases the
+		 * rings, so let it stop waiting for the queues to go down.
+		 */
+		if (v_retval) {
+			iavf_schedule_reset(adapter, IAVF_FLAG_RESET_NEEDED);
+			if (!test_bit(__IAVF_IN_REMOVE_TASK,
+				      &adapter->crit_section))
+				break;
+		} else {
+			iavf_free_all_tx_resources(adapter);
+			iavf_free_all_rx_resources(adapter);
+		}
 		if (adapter->state == __IAVF_DOWN_PENDING) {
 			iavf_change_state(adapter, __IAVF_DOWN);
 			wake_up(&adapter->down_waitqueue);
-- 
2.43.0