> -----Original Message-----
> From: Stephen Hemminger <[email protected]>
> Sent: Tuesday, September 15, 2026 12:14 AM
> To: Rita Ruvinsky <[email protected]>
> Cc: [email protected]; [email protected]; Wei Hu <[email protected]>;
> [email protected]
> Subject: [EXTERNAL] Re: [PATCH] net/mana: fix Tx stall from send queue free-
> space unit mismatch
>
> On Mon, 14 Sep 2026 13:58:08 +0300
> Rita Ruvinsky <[email protected]> wrote:
>
> > gdma_post_work_request() subtracted a unit count from an entry count:
> >
> > queue_free_units = queue->count - (queue->head - queue->tail);
> >
> > queue->count is in entries, while head and tail are in WQE alignment
> > units. On a 512-entry, 128KB send queue the check saw 512 units of
> > capacity instead of queue->size / GDMA_WQE_ALIGNMENT_UNIT_SIZE =
> 4096,
> > and returned -EBUSY with the queue one eighth full. A workload that
> > fills that window faster than it drains makes rte_eth_tx_burst()
> > return
> > 0 for long enough to look like a dead port.
> >
> > Derive the capacity from queue->size, which is also what the ring wrap
> > in gdma_get_wqe_pointer() uses. Rx is unaffected: its WQEs occupy
> > exactly one unit, so entries and units coincide.
> >
> > Fixes: 56dd45c0ce7b ("net/mana: implement hardware layer operations")
> > Cc: [email protected]
> >
> > Signed-off-by: Rita Ruvinsky <[email protected]>
> > ---
>
> Applied to next-net
>
> The long form AI review had some observations worth including:
>
> On Mon, 14 Sep 2026 13:58:08 +0300
> Rita Ruvinsky <[email protected]> wrote:
>
> > gdma_post_work_request() subtracted a unit count from an entry count:
>
> The unit analysis is right. head/tail are advanced in alignment units (queue-
> >head += wqe_size / GDMA_WQE_ALIGNMENT_UNIT_SIZE, and
> gdma_get_wqe_pointer() multiplies head by the same constant), while
> sq_count comes from rdma-core as attr->cap.max_send_wr and sq_size as
> align_hw_size(max_send_wr * get_wqe_size(max_send_sge)). Deriving the
> capacity from size is the only self-consistent choice, and it is what
> mana_gd_wq_avail_space() in the kernel driver does.
>
> Info:
>
> 1. The debug line in the -EBUSY path still reports queue->count:
>
> DP_LOG(DEBUG, "WQE size %u queue count %u head %u tail %u",
> wqe_size, queue->count, queue->head, queue->tail);
>
> After this patch count no longer takes part in the decision for the send or
> receive queue; only gdma_poll_completion_queue() still uses it, for the CQ.
> The one line printed when a post is rejected no longer shows what it was
> rejected against. Suggest:
>
> DP_LOG(DEBUG, "WQE size %u queue size %u free %u head %u
> tail %u",
> wqe_size, queue->size, queue_free_units,
> queue->head, queue->tail);
>
> 2. The comment describes the old bug rather than the invariant:
>
> /* head/tail count WQE alignment units, so the capacity they are
> * compared against must too: queue->count is in entries and
> * undercounts the queue, stalling Tx well below capacity.
> */
>
> The stall belongs in the commit message, where it already is. In the source
> the
> invariant is enough:
>
> /* head and tail are in WQE alignment units, so the capacity must
> * come from the queue size in bytes, not the entry count.
> */
>
> 3. Worth a sentence in the commit message that the kernel mana driver
> computes the same limit in mana_gd_wq_avail_space(), in bytes:
>
> u32 used_space = (wq->head - wq->tail) * GDMA_WQE_BU_SIZE;
> return wq->queue_size - used_space;
>
> It is independent confirmation of the unit convention and tells anyone
> backporting this that the two drivers now agree.
I was about to say the same. The change in DP_LOG and code comments
all make a lot of sense. Thanks for fixing this.
Wei
