Re: [PATCH] wifi: ath9k_htc: bound TX aggregation to MAX_TX_BUF_SIZE
From: Toke Høiland-Jørgensen <toke@toke.dk>
Date: 2026-09-09 11:49:54
Also in:
lkml
Georgios Karantzas [off-list ref] writes:
__hif_usb_tx() dequeues up to MAX_TX_AGGR_NUM (20) frames into a single tx_buf of MAX_TX_BUF_SIZE (32768) bytes, limiting the batch by record count but never by cumulative byte length. With large frames (MTU 2304), 20 aggregated frames of 2292 bytes each exceed the allocation (20 * 2296 = 45920 bytes), so the memcpy() in the loop writes up to 13152 bytes past tx_buf->buf before usb_submit_urb(). Peek the queue head and stop before copying any record that would cross MAX_TX_BUF_SIZE, then dispatch the current batch. Leftover skbs remain queued and are drained on the next URB completion. Signed-off-by: Georgios Karantzas <redacted>
Please wrap the commit message at 72 characters.
quoted hunk ↗ jump to hunk
--- drivers/net/wireless/ath/ath9k/hif_usb.c | 29 +++++++++++++----------- 1 file changed, 16 insertions(+), 13 deletions(-)diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c index 0a3d2190b..3e9d0c59b 100644 --- a/drivers/net/wireless/ath/ath9k/hif_usb.c +++ b/drivers/net/wireless/ath/ath9k/hif_usb.c@@ -328,32 +328,35 @@ static int __hif_usb_tx(struct hif_device_usb *hif_dev) tx_skb_cnt = min_t(u16, hif_dev->tx.tx_skb_cnt, MAX_TX_AGGR_NUM); for (i = 0; i < tx_skb_cnt; i++) { - nskb = __skb_dequeue(&hif_dev->tx.tx_skb_queue); + nskb = skb_peek(&hif_dev->tx.tx_skb_queue); + if (!nskb) + break; - /* Should never be NULL */ - BUG_ON(!nskb); + if (tx_buf->offset + nskb->len + 4 > MAX_TX_BUF_SIZE) + break; + nskb = __skb_dequeue(&hif_dev->tx.tx_skb_queue); hif_dev->tx.tx_skb_cnt--;
This bit is fine...
- buf = tx_buf->buf; - buf += tx_buf->offset; + buf = tx_buf->buf + tx_buf->offset; hdr = (__le16 *)buf; *hdr++ = cpu_to_le16(nskb->len); *hdr++ = cpu_to_le16(ATH_USB_TX_STREAM_MODE_TAG); - buf += 4; - memcpy(buf, nskb->data, nskb->len); - tx_buf->len = nskb->len + 4; - - if (i < (tx_skb_cnt - 1)) - tx_buf->offset += (((tx_buf->len - 1) / 4) + 1) * 4; + memcpy(buf + 4, nskb->data, nskb->len); - if (i == (tx_skb_cnt - 1)) - tx_buf->len += tx_buf->offset; + tx_buf->len = tx_buf->offset + nskb->len + 4; + tx_buf->offset += (((tx_buf->len - 1) / 4) + 1) * 4;
But this bit seems unrelated (and wrong?); please drop.
__skb_queue_tail(&tx_buf->skb_queue, nskb);
TX_STAT_INC(hif_dev, skb_queued);
}
+ if (!i) {
+ list_move_tail(&tx_buf->list, &hif_dev->tx.tx_buf);
+ hif_dev->tx.tx_buf_cnt++;
+ return 0;
+ }It's not possible for a single packet to overflow the buffer size, so we'll never hit this. So let's not add a pointless check. -Toke