From: Johannes Berg <[email protected]>
mac80211 will expire normal PS-buffered frames, but
if the device rejected some frames for a sleeping
station, these won't be on the ps_tx_buf queue but
on the tx_filtered queue instead; this is done to
avoid reordering.
However, mac80211 will not expire frames from the
filtered queue, let's fix that.
Also add a more comments to what all this expiry is
doing and how it works.
Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/sta_info.c | 57 +++++++++++++++++++++++++++++++++++++++++++-----
net/mac80211/status.c | 5 ++++
2 files changed, 57 insertions(+), 5 deletions(-)
--- a/net/mac80211/sta_info.c 2011-09-09 08:58:30.000000000 +0200
+++ b/net/mac80211/sta_info.c 2011-09-09 08:58:31.000000000 +0200
@@ -698,6 +698,39 @@ static bool sta_info_cleanup_expire_buff
unsigned long flags;
struct sk_buff *skb;
+ /*
+ * First check for frames that should expire on the filtered
+ * queue. Frames here were rejected by the driver and are on
+ * a separate queue to avoid reordering with normal PS-buffered
+ * frames. They also aren't accounted for right now in the
+ * total_ps_buffered counter.
+ */
+ for (;;) {
+ spin_lock_irqsave(&sta->tx_filtered.lock, flags);
+ skb = skb_peek(&sta->tx_filtered);
+ if (sta_info_buffer_expired(sta, skb))
+ skb = __skb_dequeue(&sta->tx_filtered);
+ else
+ skb = NULL;
+ spin_unlock_irqrestore(&sta->tx_filtered.lock, flags);
+
+ /*
+ * Frames are queued in order, so if this one
+ * hasn't expired yet we can stop testing. If
+ * we actually reached the end of the queue we
+ * also need to stop, of course.
+ */
+ if (!skb)
+ break;
+ dev_kfree_skb(skb);
+ }
+
+ /*
+ * Now also check the normal PS-buffered queue, this will
+ * only find something if the filtered queue was emptied
+ * since the filtered frames are all before the normal PS
+ * buffered frames.
+ */
for (;;) {
spin_lock_irqsave(&sta->ps_tx_buf.lock, flags);
skb = skb_peek(&sta->ps_tx_buf);
@@ -707,6 +740,11 @@ static bool sta_info_cleanup_expire_buff
skb = NULL;
spin_unlock_irqrestore(&sta->ps_tx_buf.lock, flags);
+ /*
+ * frames are queued in order, so if this one
+ * hasn't expired yet (or we reached the end of
+ * the queue) we can stop testing
+ */
if (!skb)
break;
@@ -716,13 +754,22 @@ static bool sta_info_cleanup_expire_buff
sta->sta.addr);
#endif
dev_kfree_skb(skb);
-
- /* if the queue is now empty recalc TIM bit */
- if (skb_queue_empty(&sta->ps_tx_buf))
- sta_info_recalc_tim(sta);
}
- return !skb_queue_empty(&sta->ps_tx_buf);
+ /*
+ * Finally, recalculate the TIM bit for this station -- it might
+ * now be clear because the station was too slow to retrieve its
+ * frames.
+ */
+ sta_info_recalc_tim(sta);
+
+ /*
+ * Return whether there are any frames still buffered, this is
+ * used to check whether the cleanup timer still needs to run,
+ * if there are no frames we don't need to rearm the timer.
+ */
+ return !(skb_queue_empty(&sta->ps_tx_buf) &&
+ skb_queue_empty(&sta->tx_filtered));
}
static int __must_check __sta_info_destroy(struct sta_info *sta)
--- a/net/mac80211/status.c 2011-09-09 08:58:30.000000000 +0200
+++ b/net/mac80211/status.c 2011-09-09 08:58:31.000000000 +0200
@@ -107,6 +107,11 @@ static void ieee80211_handle_filtered_fr
skb_queue_len(&sta->tx_filtered) < STA_MAX_TX_BUFFER) {
skb_queue_tail(&sta->tx_filtered, skb);
sta_info_recalc_tim(sta);
+
+ if (!timer_pending(&local->sta_cleanup))
+ mod_timer(&local->sta_cleanup,
+ round_jiffies(jiffies +
+ STA_INFO_CLEANUP_INTERVAL));
return;
}
On 27 September 2011 06:30, Luis R. Rodriguez <[email protected]> wrote:
> No particular comments on the code yet but latency issues has got me
> thinking about the filtered frames stuff and if we really need it. How
> much benefit does keeping these frames give us instead of just
> dropping them?
Do you guys keep statistics about this sort of thing?
My macbook pro ends up causing my FreeBSD AP to miss TX'ing a lot of
frames, even if it's close by. This only occurs when it's on battery
power.
I have a feeling it's going to be doing aggressiveish power saving
stuff and this'll be fixed by me porting over and tidying up the
filtered frames support.
Adrian
On Tue, 2011-09-27 at 10:26 +0800, Adrian Chadd wrote:
> On 27 September 2011 06:30, Luis R. Rodriguez <[email protected]> wrote:
>
> > No particular comments on the code yet but latency issues has got me
> > thinking about the filtered frames stuff and if we really need it. How
> > much benefit does keeping these frames give us instead of just
> > dropping them?
>
> Do you guys keep statistics about this sort of thing?
Not sure.
> My macbook pro ends up causing my FreeBSD AP to miss TX'ing a lot of
> frames, even if it's close by. This only occurs when it's on battery
> power.
>
> I have a feeling it's going to be doing aggressiveish power saving
> stuff and this'll be fixed by me porting over and tidying up the
> filtered frames support.
I'm surprised you don't have filtered frames support! It's pretty
important unless you keep hardware queues almost empty since otherwise
exactly what you describe will happen. The hardware is supposed to keep
track of when a station goes to sleep (only WAKE->DOZE transition, must
rely on driver/stack for DOZE->WAKE transition to avoid packet
reordering) and reject ("filter") frames on the queue in that case.
johannes
On Thu, Sep 22, 2011 at 8:47 AM, Johannes Berg
<[email protected]> wrote:
> From: Johannes Berg <[email protected]>
>
> mac80211 will expire normal PS-buffered frames, but
> if the device rejected some frames for a sleeping
> station, these won't be on the ps_tx_buf queue but
> on the tx_filtered queue instead; this is done to
> avoid reordering.
>
> However, mac80211 will not expire frames from the
> filtered queue, let's fix that.
>
> Also add a more comments to what all this expiry is
> doing and how it works.
>
> Signed-off-by: Johannes Berg <[email protected]>
No particular comments on the code yet but latency issues has got me
thinking about the filtered frames stuff and if we really need it. How
much benefit does keeping these frames give us instead of just
dropping them?
Luis
There's sleep queue support in the net80211 stack but that doesn't
close the race for what failed to TX because the station went to sleep
with stuff in the hostap TX queue.
That small window is small for 11bg/11a rates but for 11n it could be
lots of frames.
That's what I have to port to FreeBSD's ath driver (and then when it's
all stable and sensible looking, migrate back into net80211.)
Adrian
.. the host _hardware_ TX queue. (And software TX queue, I likely have
to pause that too.)
I'm specifically only porting over small chunks to begin with so I
don't get overwhelmed with what needs to be developed, debugged and
supported.
Adrian
On Mon, 2011-09-26 at 15:30 -0700, Luis R. Rodriguez wrote:
> No particular comments on the code yet but latency issues has got me
> thinking about the filtered frames stuff and if we really need it. How
> much benefit does keeping these frames give us instead of just
> dropping them?
If the station goes to sleep, it'll incur latency by design. The
filtered frames stuff is really just a way to close the race between
giving a packet to the device and the station going to sleep -- as such
I think we should really keep it.
johannes