* [PATCH] net/mana: fix Tx stall from send queue free-space unit mismatch
@ 2026-09-14 10:58 Rita Ruvinsky
2026-09-14 16:14 ` Stephen Hemminger
2026-09-17 6:23 ` [PATCH v2] " Rita Ruvinsky
0 siblings, 2 replies; 6+ messages in thread
From: Rita Ruvinsky @ 2026-09-14 10:58 UTC (permalink / raw)
To: dev; +Cc: longli, weh, stable, Rita Ruvinsky
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: stable@dpdk.org
Signed-off-by: Rita Ruvinsky <rita.ruvinsky@weka.io>
---
Independent of patchwork 167383 and 167384 (net/mana MR length
truncation / Rx WQE double free) from the same author, currently in
awaiting-upstream: this touches gdma.c only and applies cleanly with or
without them.
drivers/net/mana/gdma.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mana/gdma.c b/drivers/net/mana/gdma.c
index 7f66a7a7cf..c80fc25f21 100644
--- a/drivers/net/mana/gdma.c
+++ b/drivers/net/mana/gdma.c
@@ -138,7 +138,12 @@ gdma_post_work_request(struct mana_gdma_queue *queue,
client_oob_size + sgl_data_size,
GDMA_WQE_ALIGNMENT_UNIT_SIZE);
uint8_t *wq_buffer_pointer;
- uint32_t queue_free_units = queue->count - (queue->head - queue->tail);
+ /* 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.
+ */
+ uint32_t queue_free_units = queue->size / GDMA_WQE_ALIGNMENT_UNIT_SIZE -
+ (queue->head - queue->tail);
if (wqe_size / GDMA_WQE_ALIGNMENT_UNIT_SIZE > queue_free_units) {
DP_LOG(DEBUG, "WQE size %u queue count %u head %u tail %u",
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] net/mana: fix Tx stall from send queue free-space unit mismatch 2026-09-14 10:58 [PATCH] net/mana: fix Tx stall from send queue free-space unit mismatch Rita Ruvinsky @ 2026-09-14 16:14 ` Stephen Hemminger 2026-09-14 16:24 ` [EXTERNAL] " Wei Hu 2026-09-17 6:23 ` [PATCH v2] " Rita Ruvinsky 1 sibling, 1 reply; 6+ messages in thread From: Stephen Hemminger @ 2026-09-14 16:14 UTC (permalink / raw) To: Rita Ruvinsky; +Cc: dev, longli, weh, stable On Mon, 14 Sep 2026 13:58:08 +0300 Rita Ruvinsky <rita.ruvinsky@weka.io> 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: stable@dpdk.org > > Signed-off-by: Rita Ruvinsky <rita.ruvinsky@weka.io> > --- 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 <rita.ruvinsky@weka.io> 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXTERNAL] Re: [PATCH] net/mana: fix Tx stall from send queue free-space unit mismatch 2026-09-14 16:14 ` Stephen Hemminger @ 2026-09-14 16:24 ` Wei Hu 2026-09-16 11:12 ` Rita Ruvinsky 0 siblings, 1 reply; 6+ messages in thread From: Wei Hu @ 2026-09-14 16:24 UTC (permalink / raw) To: Stephen Hemminger, Rita Ruvinsky Cc: dev@dpdk.org, longli@microsoft.com, stable@dpdk.org > -----Original Message----- > From: Stephen Hemminger <stephen@networkplumber.org> > Sent: Tuesday, September 15, 2026 12:14 AM > To: Rita Ruvinsky <rita.ruvinsky@weka.io> > Cc: dev@dpdk.org; longli@microsoft.com; Wei Hu <weh@microsoft.com>; > stable@dpdk.org > 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 <rita.ruvinsky@weka.io> 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: stable@dpdk.org > > > > Signed-off-by: Rita Ruvinsky <rita.ruvinsky@weka.io> > > --- > > 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 <rita.ruvinsky@weka.io> 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] net/mana: fix Tx stall from send queue free-space unit mismatch 2026-09-14 16:24 ` [EXTERNAL] " Wei Hu @ 2026-09-16 11:12 ` Rita Ruvinsky 2026-09-16 15:17 ` Stephen Hemminger 0 siblings, 1 reply; 6+ messages in thread From: Rita Ruvinsky @ 2026-09-16 11:12 UTC (permalink / raw) To: Wei Hu Cc: Stephen Hemminger, dev@dpdk.org, longli@microsoft.com, stable@dpdk.org [-- Attachment #1: Type: text/plain, Size: 4346 bytes --] 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 <weh@microsoft.com> wrote: > > > > -----Original Message----- > > From: Stephen Hemminger <stephen@networkplumber.org> > > Sent: Tuesday, September 15, 2026 12:14 AM > > To: Rita Ruvinsky <rita.ruvinsky@weka.io> > > Cc: dev@dpdk.org; longli@microsoft.com; Wei Hu <weh@microsoft.com>; > > stable@dpdk.org > > 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 <rita.ruvinsky@weka.io> 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: stable@dpdk.org > > > > > > Signed-off-by: Rita Ruvinsky <rita.ruvinsky@weka.io> > > > --- > > > > 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 <rita.ruvinsky@weka.io> 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 > [-- Attachment #2: Type: text/html, Size: 5989 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] net/mana: fix Tx stall from send queue free-space unit mismatch 2026-09-16 11:12 ` Rita Ruvinsky @ 2026-09-16 15:17 ` Stephen Hemminger 0 siblings, 0 replies; 6+ messages in thread From: Stephen Hemminger @ 2026-09-16 15:17 UTC (permalink / raw) To: Rita Ruvinsky; +Cc: Wei Hu, dev@dpdk.org, longli@microsoft.com, stable@dpdk.org On Wed, 16 Sep 2026 14:12:12 +0300 Rita Ruvinsky <rita.ruvinsky@weka.io> wrote: > 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 It is in next-net branch now. But if you send V2 I can swap to new version. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] net/mana: fix Tx stall from send queue free-space unit mismatch 2026-09-14 10:58 [PATCH] net/mana: fix Tx stall from send queue free-space unit mismatch Rita Ruvinsky 2026-09-14 16:14 ` Stephen Hemminger @ 2026-09-17 6:23 ` Rita Ruvinsky 1 sibling, 0 replies; 6+ messages in thread From: Rita Ruvinsky @ 2026-09-17 6:23 UTC (permalink / raw) To: dev; +Cc: stephen, weh, stable, Rita Ruvinsky 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. The size is the authoritative capacity: rdma-core derives sq_size from sq_count as align_hw_size(max_send_wr * get_wqe_size(max_send_sge)), and the kernel mana driver computes the same limit in bytes in mana_gd_wq_avail_space(). Derive it 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. Also report the size and computed free space in the -EBUSY debug line, since queue->count no longer takes part in the decision. Fixes: 56dd45c0ce7b ("net/mana: implement hardware layer operations") Cc: stable@dpdk.org Signed-off-by: Rita Ruvinsky <rita.ruvinsky@weka.io> --- v2: - comment states the invariant rather than the old bug (Stephen Hemminger) - report queue->size and the computed free space in the -EBUSY debug line (Stephen Hemminger, Wei Hu) - reference mana_gd_wq_avail_space() in the commit message (Stephen Hemminger) Independent of patchwork 167383 and 167384 from the same author. drivers/net/mana/gdma.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/net/mana/gdma.c b/drivers/net/mana/gdma.c index 7f66a7a7cf..a92aef59d2 100644 --- a/drivers/net/mana/gdma.c +++ b/drivers/net/mana/gdma.c @@ -138,11 +138,16 @@ gdma_post_work_request(struct mana_gdma_queue *queue, client_oob_size + sgl_data_size, GDMA_WQE_ALIGNMENT_UNIT_SIZE); uint8_t *wq_buffer_pointer; - uint32_t queue_free_units = queue->count - (queue->head - queue->tail); + /* head and tail are in WQE alignment units, so the capacity must + * come from the queue size in bytes, not the entry count. + */ + uint32_t queue_free_units = queue->size / GDMA_WQE_ALIGNMENT_UNIT_SIZE - + (queue->head - queue->tail); if (wqe_size / GDMA_WQE_ALIGNMENT_UNIT_SIZE > queue_free_units) { - DP_LOG(DEBUG, "WQE size %u queue count %u head %u tail %u", - wqe_size, queue->count, queue->head, queue->tail); + 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); return -EBUSY; } -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-17 6:24 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-14 10:58 [PATCH] net/mana: fix Tx stall from send queue free-space unit mismatch Rita Ruvinsky 2026-09-14 16:14 ` Stephen Hemminger 2026-09-14 16:24 ` [EXTERNAL] " Wei Hu 2026-09-16 11:12 ` Rita Ruvinsky 2026-09-16 15:17 ` Stephen Hemminger 2026-09-17 6:23 ` [PATCH v2] " Rita Ruvinsky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox