[PATCH wireless-next] wifi: brcmfmac: msgbuf: count tx status only for the current flowring
From: Kordian Chmielewski (mlody1212)
Date: Wed Nov 11 2026 - 15:24:48 EST
brcmf_proto_msgbuf_rx_trigger() reads the tx complete ring before the
control complete ring. When the firmware writes the tx status of a packet
shortly before the delete response of its flowring, the host can see the
response in one pass and the status only in the next one, after
brcmf_msgbuf_remove_flowring() has released the flowid. If a new flowring
got the flowid in the meantime, the status decrements its outstanding_tx,
which brcmf_msgbuf_schedule_txdata() and the trickle check in
brcmf_proto_msgbuf_rx_trigger() rely on. This happens whatever order the
firmware sends the two messages in.
Give each flowid a generation that goes up when the flowid is released,
and keep it in the tx pktid of every packet posted to the flowring. A tx
status of an older generation still releases its packet, but no longer
touches the counter.
Signed-off-by: Kordian Chmielewski (mlody1212) <kudzo51@xxxxxxxxx>
---
Notes:
Hi Andre,
thanks for checking the trace. Here is the patch, on top of your
[PATCH 4/4] wifi: brcmfmac: msgbuf: send the flowring delete without waiting for tx status
https://lore.kernel.org/all/20261107154810.89780-5-dev@deq.rocks/
The generation goes up when the flowid is released, not when the delete
is sent, so the debug message in your patch still counts the tx status
that comes before the response. My tree for a BCM43684 access point
bumps it when the delete is sent; this version is compile-tested only.
Feel free to take it into the next version of your series.
.../broadcom/brcm80211/brcmfmac/msgbuf.c | 34 ++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
index 8fd970571..9d8360d3e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
@@ -247,6 +247,7 @@ struct brcmf_msgbuf {
struct brcmf_commonring **commonrings;
struct brcmf_commonring **flowrings;
dma_addr_t *flowring_dma_handle;
+ u16 *flowring_gen;
u16 max_flowrings;
u16 max_submissionrings;
@@ -293,6 +294,7 @@ struct brcmf_msgbuf {
struct brcmf_msgbuf_pktid {
atomic_t allocated;
u16 data_offset;
+ u16 gen; /* tx: flowring_gen of its flowid when posted */
struct sk_buff *skb;
dma_addr_t physaddr;
};
@@ -559,6 +561,21 @@ static void brcmf_msgbuf_rxreorder(struct brcmf_if *ifp, struct sk_buff *skb)
{
}
+/*
+ * Called when a flowid is released. A tx status can still come for a packet
+ * of the old flowring after that: brcmf_proto_msgbuf_rx_trigger() reads the
+ * tx complete ring before the control complete ring, so it can see a delete
+ * response before a tx status the firmware wrote first. The flowid may have
+ * gone to a new flowring by then, and the status must not decrement the
+ * outstanding_tx of the new one.
+ */
+static void brcmf_msgbuf_flowring_next_gen(struct brcmf_msgbuf *msgbuf,
+ u16 flowid)
+{
+ WRITE_ONCE(msgbuf->flowring_gen[flowid],
+ msgbuf->flowring_gen[flowid] + 1);
+}
+
static void
brcmf_msgbuf_remove_flowring(struct brcmf_msgbuf *msgbuf, u16 flowid)
{
@@ -566,6 +583,7 @@ brcmf_msgbuf_remove_flowring(struct brcmf_msgbuf *msgbuf, u16 flowid)
void *dma_buf;
brcmf_dbg(MSGBUF, "Removing flowring %d\n", flowid);
+ brcmf_msgbuf_flowring_next_gen(msgbuf, flowid);
dma_sz = BRCMF_H2D_TXFLOWRING_MAX_ITEM * BRCMF_H2D_TXFLOWRING_ITEMSIZE;
dma_buf = msgbuf->flowrings[flowid]->buf_addr;
@@ -768,6 +786,8 @@ static void brcmf_msgbuf_txflow(struct brcmf_msgbuf *msgbuf, u16 flowid)
bphy_err(drvr, "No PKTID available !!\n");
break;
}
+ msgbuf->tx_pktids->array[pktid].gen =
+ READ_ONCE(msgbuf->flowring_gen[flowid]);
ret_ptr = brcmf_commonring_reserve_for_write(commonring);
if (!ret_ptr) {
brcmf_msgbuf_get_pktid(msgbuf->drvr->bus_if->dev,
@@ -920,10 +940,13 @@ brcmf_msgbuf_process_txstatus(struct brcmf_msgbuf *msgbuf, void *buf)
struct sk_buff *skb;
u16 flow_ring_id;
u16 flowid;
+ u16 gen = 0;
tx_status = (struct msgbuf_tx_status *)buf;
idx = le32_to_cpu(tx_status->msg.request_id) - 1;
flow_ring_id = le16_to_cpu(tx_status->compl_hdr.flow_ring_id);
+ if (idx < msgbuf->tx_pktids->array_size)
+ gen = msgbuf->tx_pktids->array[idx].gen;
skb = brcmf_msgbuf_get_pktid(msgbuf->drvr->bus_if->dev,
msgbuf->tx_pktids, idx);
if (!skb)
@@ -937,7 +960,9 @@ brcmf_msgbuf_process_txstatus(struct brcmf_msgbuf *msgbuf, void *buf)
set_bit(flowid, msgbuf->txstatus_done_map);
commonring = msgbuf->flowrings[flowid];
- atomic_dec(&commonring->outstanding_tx);
+ /* not for a packet of a flowring that had the flowid before */
+ if (gen == READ_ONCE(msgbuf->flowring_gen[flowid]))
+ atomic_dec(&commonring->outstanding_tx);
brcmf_txfinalize(brcmf_get_ifp(msgbuf->drvr, tx_status->msg.ifidx),
skb, true);
@@ -1325,6 +1350,7 @@ brcmf_msgbuf_process_flow_ring_delete_response(struct brcmf_msgbuf *msgbuf,
if (status) {
bphy_err(drvr, "Flowring deletion failed, code %d\n", status);
+ brcmf_msgbuf_flowring_next_gen(msgbuf, flowid);
brcmf_flowring_delete(msgbuf->flow, flowid);
return;
}
@@ -1681,6 +1707,10 @@ int brcmf_proto_msgbuf_attach(struct brcmf_pub *drvr)
msgbuf->max_flowrings);
if (!msgbuf->flowring_dma_handle)
goto fail;
+ msgbuf->flowring_gen = kzalloc_objs(*msgbuf->flowring_gen,
+ msgbuf->max_flowrings);
+ if (!msgbuf->flowring_gen)
+ goto fail;
msgbuf->rx_dataoffset = if_msgbuf->rx_dataoffset;
msgbuf->max_rxbufpost = if_msgbuf->max_rxbufpost;
@@ -1732,6 +1762,7 @@ int brcmf_proto_msgbuf_attach(struct brcmf_pub *drvr)
kfree(msgbuf->txstatus_done_map);
brcmf_msgbuf_release_pktids(msgbuf);
kfree(msgbuf->flowring_dma_handle);
+ kfree(msgbuf->flowring_gen);
if (msgbuf->ioctbuf)
dma_free_coherent(drvr->bus_if->dev,
BRCMF_TX_IOCTL_MAX_MSG_SIZE,
@@ -1772,6 +1803,7 @@ void brcmf_proto_msgbuf_detach(struct brcmf_pub *drvr)
msgbuf->ioctbuf, msgbuf->ioctbuf_handle);
brcmf_msgbuf_release_pktids(msgbuf);
kfree(msgbuf->flowring_dma_handle);
+ kfree(msgbuf->flowring_gen);
kfree(msgbuf);
drvr->proto->pd = NULL;
}
--
2.56.0