Re: [PATCH v2 1/4] mac80211: fix CSA tx queue locking

[Luca Coelho <luca@coelho.fi>]

On Mon, 2014-03-24 at 13:58 +0100, Michal Kazior wrote:
> On 24 March 2014 13:25, Luca Coelho <luca@coelho.fi> wrote:
> > On Fri, 2014-03-21 at 14:31 +0100, Michal Kazior wrote:
> > Actually, in the beacon-didn't-change case, we should probably not even
> > stop the queues?
> 
> I think we should stop the queues. Take this for example:
> 
> 2 ap vifs. ap1 starts csa cs_count=100 block_tx=0, ap2 starts csa
> cs_count=0 block_tx=1.
> 
> In this case ap2 won't switch immediately but will stay around until
> ap1 interface is done.

Yes, this would be okay for count=0.  But for count=1 making ap2 wait
until ap1 is done could be problematic.  The STAs will be on the other
channel waiting after 1 beacon but it will take up to 99 more beacons
for ap2 to switch.  They'll probably drop the connection (or try to
roam) due to beacon loss.

I think it would be better to force ap2 to use the same count as ap1 (or
whatever is left in ap1's count).


> > You can call ieee80211_stop_queues_by_reason() even if it is already
> > stopped for this reason, but maybe you could call
> > ieee80211_csa_needs_block_tx() here to avoid eventually calling it
> > multiple times.  I think this is also a bit more consistent.
> >
> >         if(ieee80211_csa_needs_block_tx(...))
> >                 ieee80211_stop_queues_by_reason(...)
> 
> I fail to see how this would avoid calling it multiple times, or
> rather, where is it being called multiple number of times now that it
> can be prevented?
> 
> If you call ieee80211_csa_needs_block_tx() you check the block_tx
> anyway. Since csa_active is guaranteed to be true here already you
> just need to check block_tx.

Yeah, forget it.

--
Luca.