mac80211: UAPSD - first release HW buffered frames next also check mac80211 buffered frames

mac80211: UAPSD - first release HW buffered frames next also check mac80211 buffered frames

[Janusz Dziedzic]

Hello,

Seems currently we have implementation.

if(!driver_release_tids) {
  ...
} else {
  ...
}

Shouldn't we first release HW buffered frames and next check also SW
buffered frames?

Please check patch:
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 8eaa746..1303bfd 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1328,7 +1328,33 @@ ieee80211_sta_ps_deliver_response(struct sta_info *sta,
                return;
        }

-       if (!driver_release_tids) {
+       /* First release driver buffered frames */
+       if (driver_release_tids){
+               /*
+                * We need to release a frame that is buffered somewhere in the
+                * driver ... it'll have to handle that.
+                * Note that, as per the comment above, it'll also have to see
+                * if there is more than just one frame on the specific TID that
+                * we're releasing from, and it needs to set the more-data bit
+                * accordingly if we tell it that there's no more data. If we do
+                * tell it there's more data, then of course the more-data bit
+                * needs to be set anyway.
+                */
+               drv_release_buffered_frames(local, sta, driver_release_tids,
+                                           n_frames, reason, more_data);
+
+               /*
+                * Note that we don't recalculate the TIM bit here as it would
+                * most likely have no effect at all unless the driver told us
+                * that the TID became empty before returning here from the
+                * release function.
+                * Either way, however, when the driver tells us that the TID
+                * became empty we'll do the TIM recalculation.
+                */
+       }
+
+       /* Now check SW buffered frames */
+       /* This if (found) seems to be not required in final patch */
+       if (found) {
                struct sk_buff_head pending;
                struct sk_buff *skb;
                int num = 0;
@@ -1387,28 +1413,6 @@ ieee80211_sta_ps_deliver_response(struct sta_info *sta,
                ieee80211_add_pending_skbs(local, &pending);

                sta_info_recalc_tim(sta);
-       } else {
-               /*
-                * We need to release a frame that is buffered somewhere in the
-                * driver ... it'll have to handle that.
-                * Note that, as per the comment above, it'll also have to see
-                * if there is more than just one frame on the specific TID that
-                * we're releasing from, and it needs to set the more-data bit
-                * accordingly if we tell it that there's no more data. If we do
-                * tell it there's more data, then of course the more-data bit
-                * needs to be set anyway.
-                */
-               drv_release_buffered_frames(local, sta, driver_release_tids,
-                                           n_frames, reason, more_data);
-

BR
Janusz

Re: mac80211: UAPSD - first release HW buffered frames next also check mac80211 buffered frames

[Johannes Berg]

On Fri, 2011-11-04 at 16:04 +0100, Janusz Dziedzic wrote:
> Hello,
> 
> Seems currently we have implementation.
> 
> if(!driver_release_tids) {
>   ...
> } else {
>   ...
> }
> 
> Shouldn't we first release HW buffered frames and next check also SW
> buffered frames?

Only one of them can be true -- we don't release from driver & mac80211
queues at the same time. Technically we could, but the complexity would
be much higher than your simple patch for more-data handling etc.

johannes