2023-07-12 17:50:58

by Dmitry Antipov

Subject: [PATCH] wifi: mt76: use native timestamps for RX reordering

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 <[email protected]>
---
drivers/net/wireless/mediatek/mt76/agg-rx.c | 30 ++++++++++++---------
drivers/net/wireless/mediatek/mt76/mt76.h | 7 ++---
2 files changed, 21 insertions(+), 16 deletions(-)

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;

--
2.41.0

2023-07-14 09:29:07

by Felix Fietkau

Subject: Re: [PATCH] wifi: mt76: use native timestamps for RX reordering

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 <[email protected]>
Why?

- Felix

2023-07-17 11:28:53

by Dmitry Antipov

Subject: Re: [PATCH] wifi: mt76: use native timestamps for RX reordering

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