Re: [PATCH rdma-next 5/5] RDMA/mana_ib: Poll UD completions and flush software error QPs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] RDMA/mana_ib: Data race on shadow queue indices during QP reset
- [High] RDMA/mana_ib: Stale CQEs are misattributed to new WQEs after QP reset
--

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

RDMA/mana_ib: Poll UD completions and flush software error QPs

This commit introduces direct polling of hardware CQEs to build UD and GSI
work completions for mana_ib. It also replaces the all-QP CQ membership with
persistent send and receive error-QP lists to flush software error QPs.

> diff --git a/drivers/infiniband/hw/mana/cq.c b/drivers/infiniband/hw/mana/cq.c
> --- a/drivers/infiniband/hw/mana/cq.c
> +++ b/drivers/infiniband/hw/mana/cq.c
[ ... ]
> +static void mana_complete_send(struct mana_ib_qp *qp,
> +			       struct mana_cq_poll *poll, u32 vendor_error)
> +{
> +	struct shadow_queue *shadow = &qp->shadow_sq;
> +	struct shadow_wqe_header *wqe = shadow_queue_get_next_to_consume(shadow);
> +	struct gdma_queue *queue;
> +
> +	if (!wqe)
> +		return;
> +
> +	if (vendor_error || !(wqe->flags & MANA_WQ_NO_SIGNAL_WC))
> +		mana_fill_wc(qp, poll, wqe, wqe->send_opcode, vendor_error);
> +
> +	queue = mana_qp_get_sq(qp)->kmem;
> +	queue->tail += wqe->wqe_size_in_bu;
> +	shadow_queue_advance_consumer(shadow);
> +}

[Severity: High]
If a QP is transitioned to IB_QPS_RESET, reset_shadow_queue() rewinds
cons_idx to 0. Since the hardware GDMA CQ is not swept of stale CQEs
generated before or during the reset, will the poller later encounter these
stale CQEs when the QP is reused?

It appears mana_complete_send() might blindly pop the next shadow WQE (at
index 0) and complete it using the stale CQE data. Can a new WQE be
completed with the status, length, and immediate data of a pre-reset
operation because the CQE's tx_wqe_offset or rx_wqe_offset is not validated
against the shadow WQE?

> diff --git a/drivers/infiniband/hw/mana/qp.c b/drivers/infiniband/hw/mana/qp.c
> --- a/drivers/infiniband/hw/mana/qp.c
> +++ b/drivers/infiniband/hw/mana/qp.c
[ ... ]
> +static void mana_ib_modify_qp_state(struct ib_qp *ibqp, struct ib_qp_attr *attr,
> +				    int attr_mask, struct ib_udata *udata)
> +{
> +	struct mana_ib_qp *qp = container_of(ibqp, struct mana_ib_qp, ibqp);
> +
> +	if (udata)
> +		return;
> +
> +	if (attr_mask & IB_QP_STATE) {
> +		switch (attr->qp_state) {
> +		case IB_QPS_RESET:
> +			mana_remove_qp_from_cqs(qp);
> +			reset_shadow_queue(&qp->shadow_rq);
> +			reset_shadow_queue(&qp->shadow_sq);
> +			break;
> +		default:
> +			break;
> +		}
> +	}

[Severity: High]
When a userspace application transitions a QP to IB_QPS_RESET concurrently
with a thread polling the CQ, mana_remove_qp_from_cqs() removes it from the
software error lists but does not seem to synchronize with the active CQ
polling thread.

Does calling reset_shadow_queue() here create a data race? The function
performs unprotected plain writes of 0 to prod_idx and cons_idx:

drivers/infiniband/hw/mana/shadow_queue.h:reset_shadow_queue() {
        queue->prod_idx = 0;
        queue->cons_idx = 0;
}

The polling thread concurrently executes shadow_queue_advance_consumer(),
doing a read-modify-write on cons_idx. Can this corruption lead to
mismatched shadow WQEs and hardware completions, lost completions, or
out-of-bounds ring buffer accesses?

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