Re: [PATCH 3.14] mac80211: fix AP powersave TX vs. wakeup race

[Stanislaw Gruszka <sgruszka@redhat.com>]

On Thu, Feb 20, 2014 at 10:42:48AM +0100, Johannes Berg wrote:
> On Thu, 2014-02-20 at 10:40 +0100, Stanislaw Gruszka wrote:
> 
> > > This area is a bit confusing, but I don't think WLAN_STA_PS_STA will be
> > > clear until after WLAN_STA_PS_DRIVER is?
> > 
> > Hmm, actually looks like we call ps_deliver_wakeup() with WLAN_STA_PS_STA
> > flag currently cleared.
> > 
> > tatic void sta_unblock(struct work_struct *wk)
> > {
> >         if (!test_sta_flag(sta, WLAN_STA_PS_STA)) {
> >                 local_bh_disable();
> >                 ieee80211_sta_ps_deliver_wakeup(sta);
> >                 local_bh_enable();
> > 
> > so on TX we should rather check WLAN_STA_PS_DRIVER flag .
> 
> But that flag may never be set if the driver doesn't support/use it
> (which means it's racy, but anyway), so we need to check both.
> 
> Now I'm confused though. It seems the intent here was to clear the
> WLAN_STA_PS_STA flag in something like sta_ps_end() in the RX path, but
> we don't do that. I'm not sure how it works now, but I know from testing
> that it does ;-)

LOL. There is another call to ieee80211_sta_ps_deliver_wakeup() from
sta_ps_end(), I missed this somehow. Anyway, we need to check both
flags.

> Do you think the v2 patch is fine?

Yes, I think it's ok.

Stanislaw