Re: [PATCH rdma-next v6 09/10] RDMA/mana_ib: Handle error CQEs for RC QPs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] RDMA/mana_ib: handle_error_cqe() leaves active GSI QPs on CQ error lists
--

commit 1bb70d4df95cbf22f7fdb1b2cc6c47bc5b5ed6d5
Author: Konstantin Taranov <kotaranov@microsoft.com>

RDMA/mana_ib: Handle error CQEs for RC QPs

This commit adds handling for hardware error CQEs on RC QPs, deferring the
transition to the ERR state via a workqueue so that other work queues can be
flushed. It also cleans pending CQEs upon QP reset and tracks deferred error
state to avoid transitioning reset and reused QPs to ERR.

> diff --git a/drivers/infiniband/hw/mana/cq.c b/drivers/infiniband/hw/mana/cq.c
> index 74597c07f1cba..a707664f5ab8b 100644
> --- a/drivers/infiniband/hw/mana/cq.c
> +++ b/drivers/infiniband/hw/mana/cq.c

[ ... ]

> @@ -516,6 +594,79 @@ static bool handle_psn_cqe(struct mana_ib_qp *qp, u32 psn,
>  	return consumed;
>  }
>  

[ ... ]

> +static bool handle_error_cqe(struct mana_ib_cq *cq, struct mana_ib_qp *qp,
> +			     struct gdma_comp *cqe, struct mana_cq_poll *poll)
> +{
> +	bool mmq = cqe->is_sq && (cqe->wq_num & MANA_QID_SUBTYPE_MASK) == MANA_QID_SUBTYPE_MMQ;
> +	struct mana_rdma_cqe *rdma_cqe = (struct mana_rdma_cqe *)cqe->cqe_data;
> +	bool send = mana_cqe_is_send(qp, cqe);
> +	u32 vendor_err = rdma_cqe->error.vendor_error;
> +	struct shadow_wqe_header *shadow_wqe;
> +	struct shadow_queue *shadow_q;
> +
> +	if (send && qp->ibqp.qp_type == IB_QPT_RC) {
> +		/* The error PSN/FSN belongs to the failing WQE, not a success. */
> +		if (!mana_advance_send(qp, mmq, rdma_cqe->error.psn - 1, poll))
> +			return false;
> +		if (poll->produced == poll->budget)
> +			return false;
> +	}
> +
> +	if (!send)
> +		shadow_q = &qp->shadow_rq;
> +	else if (mmq)
> +		shadow_q = &qp->shadow_mmq;
> +	else
> +		shadow_q = &qp->shadow_sq;
> +
> +	shadow_wqe = shadow_queue_get_next_to_consume(shadow_q);
> +	if (shadow_wqe) {
> +		if (send) {
> +			if (!mana_complete_send_dependencies(qp, mmq, shadow_wqe, poll))
> +				return false;
> +
> +			mana_complete_send(qp, mmq, poll, vendor_err);
> +		} else {
> +			mana_fill_wc(qp, poll, shadow_wqe, IB_WC_RECV, vendor_err);
> +			shadow_queue_advance_consumer(shadow_q);
> +		}
> +	}
> +
> +	/* Hardware has stopped DMA for the queues flushed below. */
> +	if (send) {
> +		if (list_empty(&qp->send_err_node))
> +			list_add_tail(&qp->send_err_node, &cq->send_err_qp_list);
> +	} else {
> +		if (list_empty(&qp->recv_err_node))
> +			list_add_tail(&qp->recv_err_node, &cq->recv_err_qp_list);
> +	}
> +	if (qp->ibqp.qp_type != IB_QPT_GSI)
> +		mana_ib_schedule_qp_error(qp);

[Severity: High]
As reported by Sashiko on v5 [1], in handle_error_cqe(), mana_ib_schedule_qp_error()
is skipped when qp->ibqp.qp_type == IB_QPT_GSI, but qp->send_err_node or
qp->recv_err_node is still unconditionally added to cq->send_err_qp_list or
cq->recv_err_qp_list.

QPs are only removed from those error lists in mana_remove_qp_from_cqs() on a
transition to IB_QPS_RESET in mana_ib_modify_qp_state(), or during QP
destruction:

drivers/infiniband/hw/mana/qp.c:mana_ib_modify_qp_state() {
	if (attr_mask & IB_QP_STATE) {
		switch (attr->qp_state) {
		case IB_QPS_RESET:
			mana_remove_qp_from_cqs(qp, true);
			...
			break;
		default:
			break;
		}
}

However, the MAD layer never transitions a GSI QP to IB_QPS_RESET. On a GSI
send error, ib_mad_send_done() -> ib_mad_send_error() transitions the QP from
IB_QPS_SQE back to IB_QPS_RTS and reposts the WRs via ib_post_send() ->
mana_ib_post_send_ud():

drivers/infiniband/hw/mana/wr.c:mana_ib_post_send_ud() {
	...
	if (ring_sq) {
		mana_gd_wq_ring_doorbell(mdev_to_gc(mdev), queue);
		mana_ib_notify_flush(qp->ibqp.send_cq, &qp->send_err_node);
	}
}

Because qp->send_err_node remains on cq->send_err_qp_list while the GSI QP is
active in IB_QPS_RTS, won't mana_flush_completions() immediately software-flush
all in-flight and newly posted GSI WRs with IB_WC_WR_FLUSH_ERR, causing
ib_mad_send_done() to DMA-unmap buffers while hardware still owns them and
permanently breaking GSI traffic on the port? Similarly, if a receive error
occurs while the GSI RQ remains active in RTS, adding qp->recv_err_node would
flush all pre-posted GSI receive WRs.

Should adding the QP to cq->send_err_qp_list and cq->recv_err_qp_list also be
skipped for IB_QPT_GSI (or cleaned up when transitioning back to IB_QPS_RTS)?

[1] https://lore.kernel.org/r/20261017064413.995985-10-kotaranov@linux.microsoft.com

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