Re: [PATCH net v3] net: mana: Avoid waiting on dead HWC after device reset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] net: mana: premature TX NAPI deletion in
  mana_dealloc_queues()
--

commit d359cd9ade3de8d4713c17d8d3e7c311b5d6d28c
Author: Sahil Chandna <sahilchandna@linux.microsoft.com>

net: mana: Avoid waiting on dead HWC after device reset

This commit replaces the sleep-based TX drain in mana_dealloc_queues()
with synchronous CQ polling and marks the hardware channel dead after a
device reset so subsequent HWC commands fail fast.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 38249e092090a..739021aa09b38 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c

[ ... ]

> @@ -3799,42 +3809,110 @@ static int mana_dealloc_queues(struct net_device *ndev)

[ ... ]

>  		for (i = 0; i < apc->num_queues; i++) {
> +			struct napi_struct *napi;
> +			struct mana_cq *tx_cq;
> +
> +			if (!apc->tx_qp[i])
> +				continue;
> +
> +			tx_cq = &apc->tx_qp[i]->tx_cq;
> +			napi = &tx_cq->napi;
> +
>  			txq = &apc->tx_qp[i]->txq;
>  			while ((skb = skb_dequeue(&txq->pending_skbs))) {
>  				mana_unmap_skb(skb, apc);
>  				dev_kfree_skb_any(skb);
>  			}
>  			atomic_set(&txq->pending_sends, 0);
> +
> +			if (txq->napi_initialized) {
> +				netif_napi_del_locked(napi);
> +				txq->napi_initialized = false;
> +			}
>  		}
>  	}

[Severity: Medium]
Does deleting the TX NAPI and clearing txq->napi_initialized here run too
early, before mana_destroy_txq() destroys the SQ object and detaches the
TX CQ?

At this point in mana_dealloc_queues(), the SQ object is still alive and
tx_cq.gdma_cq remains published in gc->cq_table[] and eq->eq.child_cq[]
while mana_disable_vport_rx(), mana_fence_rqs(), mana_destroy_rxqs() and
mana_destroy_wq_obj() run, so a completion or GDMA_EQE_THROTTLE event on
the shared EQ can still call mana_schedule_napi() on the deleted
napi_struct.

Clearing txq->napi_initialized here also causes mana_destroy_txq() to skip
its post-mana_destroy_wq_obj() detach block:

drivers/net/ethernet/microsoft/mana/mana_en.c:mana_destroy_txq() {
    ...
		if (apc->tx_qp[i]->tx_object != INVALID_MANA_HANDLE)
			mana_destroy_wq_obj(apc, GDMA_SQ, apc->tx_qp[i]->tx_object);

		if (apc->tx_qp[i]->txq.napi_initialized) {
			/* Detach the CQ only after the WQ object is destroyed,
			 * so a completion EQE that arrives before then still
			 * finds the CQ in cq_table[] rather than hitting the
			 * WARN in mana_gd_process_eqe(). One grace period then
			 * covers both mana_gd_intr() and NAPI users.
			 */
			mana_gd_detach_cq(gc, apc->tx_qp[i]->tx_cq.gdma_cq);
			netif_napi_del_locked(napi);
			apc->tx_qp[i]->txq.napi_initialized = false;
		}

		mana_deinit_cq(apc, &apc->tx_qp[i]->tx_cq);
    ...
}

Because mana_gd_detach_cq() is skipped there, mana_deinit_cq() ->
mana_gd_destroy_queue() -> mana_gd_destroy_cq() has to detach the CQ and
wait for a second synchronize_net() grace period for every TX queue under
rtnl_lock.  Leaving netif_napi_del_locked() and txq->napi_initialized =
false to mana_destroy_txq() (and guarding the redundant napi_disable_locked()
in mana_destroy_txq()) would avoid both issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112085226.878780-1-sahilchandna@linux.microsoft.com?part=1