<div dir="ltr">Hi Stephen,<div>I am happy to send these as a follow-up if you've already committed v1 or create a v2 patch.</div><div> </div><div>Thanks,</div><div>Rita</div></div> <div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Mon, Sep 14, 2026 at 7:25 PM Wei Hu <<a href="mailto:weh@microsoft.com">weh@microsoft.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > -----Original Message----- > From: Stephen Hemminger <<a href="mailto:stephen@networkplumber.org" target="_blank">stephen@networkplumber.org</a>> > Sent: Tuesday, September 15, 2026 12:14 AM > To: Rita Ruvinsky <<a href="mailto:rita.ruvinsky@weka.io" target="_blank">rita.ruvinsky@weka.io</a>> > Cc: <a href="mailto:dev@dpdk.org" target="_blank">dev@dpdk.org</a>; <a href="mailto:longli@microsoft.com" target="_blank">longli@microsoft.com</a>; Wei Hu <<a href="mailto:weh@microsoft.com" target="_blank">weh@microsoft.com</a>>; > <a href="mailto:stable@dpdk.org" target="_blank">stable@dpdk.org</a> > 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 <<a href="mailto:rita.ruvinsky@weka.io" target="_blank">rita.ruvinsky@weka.io</a>> 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: <a href="mailto:stable@dpdk.org" target="_blank">stable@dpdk.org</a> > > > > Signed-off-by: Rita Ruvinsky <<a href="mailto:rita.ruvinsky@weka.io" target="_blank">rita.ruvinsky@weka.io</a>> > > --- > > 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 <<a href="mailto:rita.ruvinsky@weka.io" target="_blank">rita.ruvinsky@weka.io</a>> 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 </blockquote></div>