Hi Stephen, I am happy to send these as a follow-up if you've already committed v1 or create a v2 patch.
Thanks, Rita On Mon, Sep 14, 2026 at 7:25 PM Wei Hu <[email protected]> wrote: > > > > -----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 >
