diff mbox series

wifi: mt76: use native timestamps for RX reordering

Message ID 20230712173437.162921-1-dmantipov@yandex.ru
State New
Headers show
Series wifi: mt76: use native timestamps for RX reordering | expand

Commit Message

Dmitry Antipov July 12, 2023, 5:34 p.m. UTC 
Prefer native (i.e. unsigned long) timestamps for RX reordering. Since
'struct mt76_rx_status' with native timestamp becomes too large to fit
into 48-byte 'cb' area of 'struct sk_buff', use separate timestamps in
reorder array of 'struct mt76_rx_tid' instead.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 drivers/net/wireless/mediatek/mt76/agg-rx.c | 30 ++++++++++++---------
 drivers/net/wireless/mediatek/mt76/mt76.h   |  7 ++---
 2 files changed, 21 insertions(+), 16 deletions(-)

Comments

Felix Fietkau July 14, 2023, 9:10 a.m. UTC | #1 
On 12.07.23 19:34, Dmitry Antipov wrote:
> Prefer native (i.e. unsigned long) timestamps for RX reordering. Since
> 'struct mt76_rx_status' with native timestamp becomes too large to fit
> into 48-byte 'cb' area of 'struct sk_buff', use separate timestamps in
> reorder array of 'struct mt76_rx_tid' instead.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
Why?

- Felix
Dmitry Antipov July 17, 2023, 11:16 a.m. UTC | #2 
On 7/14/23 12:10, Felix Fietkau wrote:

> Why?

1) 'unsigned long' is a native kernel way to manage jiffies and 2) comment
above 'time_after32()' states that this is a quirk for the cases where some
data format explicitly dictates using of 32-bit values.

Dmitry
diff mbox series

Patch

diff --git a/drivers/net/wireless/mediatek/mt76/agg-rx.c b/drivers/net/wireless/mediatek/mt76/agg-rx.c
index 10cbd9e560e7..3b9eeb38e118 100644
--- a/drivers/net/wireless/mediatek/mt76/agg-rx.c
+++ b/drivers/net/wireless/mediatek/mt76/agg-rx.c
@@ -19,11 +19,13 @@  mt76_aggr_release(struct mt76_rx_tid *tid, struct sk_buff_head *frames, int idx)
 
 	tid->head = ieee80211_sn_inc(tid->head);
 
-	skb = tid->reorder_buf[idx];
+	skb = tid->reorder[idx].skb;
 	if (!skb)
 		return;
 
-	tid->reorder_buf[idx] = NULL;
+	tid->reorder[idx].skb = NULL;
+	tid->reorder[idx].timestamp = 0;
+
 	tid->nframes--;
 	__skb_queue_tail(frames, skb);
 }
@@ -46,7 +48,7 @@  mt76_rx_aggr_release_head(struct mt76_rx_tid *tid, struct sk_buff_head *frames)
 {
 	int idx = tid->head % tid->size;
 
-	while (tid->reorder_buf[idx]) {
+	while (tid->reorder[idx].skb) {
 		mt76_aggr_release(tid, frames, idx);
 		idx = tid->head % tid->size;
 	}
@@ -70,15 +72,15 @@  mt76_rx_aggr_check_release(struct mt76_rx_tid *tid, struct sk_buff_head *frames)
 	for (idx = (tid->head + 1) % tid->size;
 	     idx != start && nframes;
 	     idx = (idx + 1) % tid->size) {
-		skb = tid->reorder_buf[idx];
+		skb = tid->reorder[idx].skb;
 		if (!skb)
 			continue;
 
 		nframes--;
 		status = (struct mt76_rx_status *)skb->cb;
-		if (!time_after32(jiffies,
-				  status->reorder_time +
-				  mt76_aggr_tid_to_timeo(tid->num)))
+		if (!time_after(jiffies,
+				tid->reorder[idx].timestamp +
+				mt76_aggr_tid_to_timeo(tid->num)))
 			continue;
 
 		mt76_rx_aggr_release_frames(tid, frames, status->seqno);
@@ -222,13 +224,13 @@  void mt76_rx_aggr_reorder(struct sk_buff *skb, struct sk_buff_head *frames)
 	idx = seqno % size;
 
 	/* Discard if the current slot is already in use */
-	if (tid->reorder_buf[idx]) {
+	if (tid->reorder[idx].skb) {
 		dev_kfree_skb(skb);
 		goto out;
 	}
 
-	status->reorder_time = jiffies;
-	tid->reorder_buf[idx] = skb;
+	tid->reorder[idx].timestamp = jiffies;
+	tid->reorder[idx].skb = skb;
 	tid->nframes++;
 	mt76_rx_aggr_release_head(tid, frames);
 
@@ -246,7 +248,7 @@  int mt76_rx_aggr_start(struct mt76_dev *dev, struct mt76_wcid *wcid, u8 tidno,
 
 	mt76_rx_aggr_stop(dev, wcid, tidno);
 
-	tid = kzalloc(struct_size(tid, reorder_buf, size), GFP_KERNEL);
+	tid = kzalloc(struct_size(tid, reorder, size), GFP_KERNEL);
 	if (!tid)
 		return -ENOMEM;
 
@@ -272,12 +274,14 @@  static void mt76_rx_aggr_shutdown(struct mt76_dev *dev, struct mt76_rx_tid *tid)
 
 	tid->stopped = true;
 	for (i = 0; tid->nframes && i < size; i++) {
-		struct sk_buff *skb = tid->reorder_buf[i];
+		struct sk_buff *skb = tid->reorder[i].skb;
 
 		if (!skb)
 			continue;
 
-		tid->reorder_buf[i] = NULL;
+		tid->reorder[i].skb = NULL;
+		tid->reorder[i].timestamp = 0;
+
 		tid->nframes--;
 		dev_kfree_skb(skb);
 	}
diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 6b07b8fafec2..f38da680dc54 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -372,7 +372,10 @@  struct mt76_rx_tid {
 
 	u8 started:1, stopped:1, timer_pending:1;
 
-	struct sk_buff *reorder_buf[];
+	struct {
+		struct sk_buff *skb;
+		unsigned long timestamp;
+	} reorder[];
 };
 
 #define MT_TX_CB_DMA_DONE		BIT(0)
@@ -606,8 +609,6 @@  struct mt76_rx_status {
 		u16 wcid_idx;
 	};
 
-	u32 reorder_time;
-
 	u32 ampdu_ref;
 	u32 timestamp;