[PATCH wireless-drivers] mt76: usb: fix buffer allocation for scatter-gather capable devices

[PATCH wireless-drivers] mt76: usb: fix buffer allocation for scatter-gather capable devices

[Lorenzo Bianconi]

Partially revert commit f8f527b16db5 ("mt76: usb: use EP max packet
aligned buffer sizes for rx") since it breaks A-MSDU support.
When A-MSDU is enable the device can receive frames up to
q->buf_size but they will be discarded in mt76u_process_rx_entry
since there is no enough room for skb_shared_info.
Fix it by introducing q->data_size and take info account
skb_shared_info size in q->buf_size
Moreover increase buffer size even for legacy mode (scatter-gather not
available)

Fixes: f8f527b16db5 ("mt76: usb: use EP max packet aligned buffer sizes for rx")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/mt76.h |  4 ++++
 drivers/net/wireless/mediatek/mt76/usb.c  | 26 ++++++++++++-----------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 8ecbf81a906f..f118919ca5ff 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -31,6 +31,9 @@
 #define MT_MCU_RING_SIZE    32
 #define MT_RX_BUF_SIZE      2048
 
+#define MT_BUF_WITH_OVERHEAD(x) \
+	((x) + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
+
 struct mt76_dev;
 struct mt76_wcid;
 
@@ -124,6 +127,7 @@ struct mt76_queue {
 	u16 tail;
 	int ndesc;
 	int queued;
+	int data_size;
 	int buf_size;
 	bool stopped;
 
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index bbaa1365bbda..9e328e4532b3 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -299,7 +299,7 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76_queue *q, struct urb *urb,
 
 		page = virt_to_head_page(data);
 		offset = data - page_address(page);
-		sg_set_page(&urb->sg[i], page, q->buf_size, offset);
+		sg_set_page(&urb->sg[i], page, q->data_size, offset);
 	}
 
 	if (i < nsgs) {
@@ -311,7 +311,7 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76_queue *q, struct urb *urb,
 	}
 
 	urb->num_sgs = max_t(int, i, urb->num_sgs);
-	urb->transfer_buffer_length = urb->num_sgs * q->buf_size,
+	urb->transfer_buffer_length = urb->num_sgs * q->data_size;
 	sg_init_marker(urb->sg, urb->num_sgs);
 
 	return i ? : -ENOMEM;
@@ -322,14 +322,13 @@ mt76u_refill_rx(struct mt76_dev *dev, struct urb *urb, int nsgs, gfp_t gfp)
 {
 	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
 
-	if (dev->usb.sg_en) {
+	if (dev->usb.sg_en)
 		return mt76u_fill_rx_sg(dev, q, urb, nsgs, gfp);
-	} else {
-		urb->transfer_buffer_length = q->buf_size;
-		urb->transfer_buffer = page_frag_alloc(&q->rx_page,
-						       q->buf_size, gfp);
-		return urb->transfer_buffer ? 0 : -ENOMEM;
-	}
+
+	urb->transfer_buffer_length = q->data_size;
+	urb->transfer_buffer = page_frag_alloc(&q->rx_page, q->buf_size, gfp);
+
+	return urb->transfer_buffer ? 0 : -ENOMEM;
 }
 
 static int
@@ -446,8 +445,9 @@ mt76u_process_rx_entry(struct mt76_dev *dev, struct urb *urb)
 		return 0;
 
 	data_len = min_t(int, len, data_len - MT_DMA_HDR_LEN);
-	if (MT_DMA_HDR_LEN + data_len > SKB_WITH_OVERHEAD(q->buf_size)) {
-		dev_err_ratelimited(dev->dev, "rx data too big %d\n", data_len);
+	if (MT_DMA_HDR_LEN + data_len > q->data_size) {
+		dev_err_ratelimited(dev->dev, "rx data too big %d\n",
+				    data_len);
 		return 0;
 	}
 
@@ -577,8 +577,10 @@ static int mt76u_alloc_rx(struct mt76_dev *dev)
 	if (!q->entry)
 		return -ENOMEM;
 
-	q->buf_size = dev->usb.sg_en ? MT_RX_BUF_SIZE : PAGE_SIZE;
+	q->data_size = dev->usb.sg_en ? MT_RX_BUF_SIZE : PAGE_SIZE;
+	q->buf_size = MT_BUF_WITH_OVERHEAD(q->data_size);
 	q->ndesc = MT_NUM_RX_ENTRIES;
+
 	for (i = 0; i < q->ndesc; i++) {
 		err = mt76u_rx_urb_alloc(dev, &q->entry[i]);
 		if (err < 0)
-- 
2.21.0

Re: [PATCH wireless-drivers] mt76: usb: fix buffer allocation for scatter-gather capable devices

[Kalle Valo]

Lorenzo Bianconi <lorenzo@kernel.org> writes:

> Partially revert commit f8f527b16db5 ("mt76: usb: use EP max packet
> aligned buffer sizes for rx") since it breaks A-MSDU support.
> When A-MSDU is enable the device can receive frames up to
> q->buf_size but they will be discarded in mt76u_process_rx_entry
> since there is no enough room for skb_shared_info.
> Fix it by introducing q->data_size and take info account
> skb_shared_info size in q->buf_size
> Moreover increase buffer size even for legacy mode (scatter-gather not
> available)
>
> Fixes: f8f527b16db5 ("mt76: usb: use EP max packet aligned buffer sizes for rx")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Felix, can I take this directly to wireless-drivers?

-- 
Kalle Valo

Re: [PATCH wireless-drivers] mt76: usb: fix buffer allocation for scatter-gather capable devices

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 968 bytes --]

> Lorenzo Bianconi <lorenzo@kernel.org> writes:
> 
> > Partially revert commit f8f527b16db5 ("mt76: usb: use EP max packet
> > aligned buffer sizes for rx") since it breaks A-MSDU support.
> > When A-MSDU is enable the device can receive frames up to
> > q->buf_size but they will be discarded in mt76u_process_rx_entry
> > since there is no enough room for skb_shared_info.
> > Fix it by introducing q->data_size and take info account
> > skb_shared_info size in q->buf_size
> > Moreover increase buffer size even for legacy mode (scatter-gather not
> > available)
> >
> > Fixes: f8f527b16db5 ("mt76: usb: use EP max packet aligned buffer sizes for rx")
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> Felix, can I take this directly to wireless-drivers?

Hi Kalle,

please hold on with this patch, I will post a new one with a different
approach based on what Felix has suggested me

Regards,
Lorenzo

> 
> -- 
> Kalle Valo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH wireless-drivers] mt76: usb: fix buffer allocation for scatter-gather capable devices

[Kalle Valo]

Lorenzo Bianconi <lorenzo@kernel.org> writes:

>> Lorenzo Bianconi <lorenzo@kernel.org> writes:
>> 
>> > Partially revert commit f8f527b16db5 ("mt76: usb: use EP max packet
>> > aligned buffer sizes for rx") since it breaks A-MSDU support.
>> > When A-MSDU is enable the device can receive frames up to
>> > q->buf_size but they will be discarded in mt76u_process_rx_entry
>> > since there is no enough room for skb_shared_info.
>> > Fix it by introducing q->data_size and take info account
>> > skb_shared_info size in q->buf_size
>> > Moreover increase buffer size even for legacy mode (scatter-gather not
>> > available)
>> >
>> > Fixes: f8f527b16db5 ("mt76: usb: use EP max packet aligned buffer sizes for rx")
>> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
>> 
>> Felix, can I take this directly to wireless-drivers?
>
> Hi Kalle,
>
> please hold on with this patch, I will post a new one with a different
> approach based on what Felix has suggested me

Ok, thanks for letting me know.

-- 
Kalle Valo