linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Luis R. Rodriguez" <lrodriguez@atheros.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: Luis Rodriguez <Luis.Rodriguez@Atheros.com>,
	"linville@tuxdriver.com" <linville@tuxdriver.com>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"stable@kernel.org" <stable@kernel.org>, Jouni Malinen <j@w1.fi>,
	Paul Stewart <pstew@google.com>,
	Amod Bodas <Amod.Bodas@Atheros.com>,
	Vasanth Thiagarajan <Vasanth.Thiagarajan@Atheros.com>
Subject: Re: [PATCH v2 2/3] mac80211: wait until completely disassociated before new association
Date: Mon, 4 Oct 2010 13:55:51 -0700	[thread overview]
Message-ID: <20101004205551.GR2105@tux> (raw)
In-Reply-To: <1286217898.3620.54.camel@jlt3.sipsolutions.net>

On Mon, Oct 04, 2010 at 11:44:58AM -0700, Johannes Berg wrote:
> On Mon, 2010-10-04 at 11:04 -0700, Luis R. Rodriguez wrote:
> > On Mon, Oct 04, 2010 at 09:39:52AM -0700, Johannes Berg wrote:
> > > On Mon, 2010-10-04 at 09:36 -0700, Luis R. Rodriguez wrote:
> > > 
> > > > > > +wait:
> > > > > >  	wk->timeout = jiffies + IEEE80211_ASSOC_TIMEOUT;
> > > > > >  	run_again(local, wk->timeout);
> > > > > 
> > > > > But you'll be staying off-channel for the wait period, so what does this
> > > > > really help?
> > > > 
> > > > I totally missed this what locks us offchannel here, I though we just re-arm
> > > > the timer, and come back offchannel at a later time. What is it that locks
> > > > us offchannel until the timer runs again?
> > > 
> > > I believe we stay off-channel as long as the work item is active, after
> > > it has been activated, no?
> > 
> > Well I don't see that, the problem here was the assumption that within a work
> > item we can try to transmit a frame for our home channel without changing it.
> 
> Yes, that's true, but we do try to stop most things ... we just miss
> some :-)

That's one way of putting it, that's fine, I was under the impression
we did want to send the data though, but if the goal is to *stop* this
that's fine too.

> > If that is desired we must move back to the home channel as I did, but I can
> > see how we'd need more work than what I did, we'd need to start the queues,
> > get out of PS state with the AP and then TX... unless TX already handles
> > that for us.
> 
> We don't need to go out of PS state to just TX, but we'd need to be
> careful to TX with asleep bit.

I got what you meant here.

> That said, we don't TX data frames then.

But not here. Right now I am going to assume that we actually are
transmitting some frames for the delba when we try to tear down
the BA agreements with the old AP and the new AP are on the
same band, we just likely transmit it on the wrong channel.

> > ieee80211_work_work() just iterates over all work items, and then bails out.
> > The work loop is protected against local->mtx, and if we call work_work
> > when we either add new work, purge work, or hit a timer. We *try* to prevent
> > frames from being sent on the home channel by calling
> > ieee80211_offchannel_stop_station() but notice how we only stop the queues
> > for NL80211_IFTYPE_STATION interfaces.
> 
> No, we just stop the station ones differently from all the others.

Ah and also we did call ieee80211_offchannel_stop_beaconing() prior
to processing work_work stuff so that should take care of stopping
beaconing but that also turns off all TX queues... so yeah you're
right. The race here was just within work items assuming they can
transmit on other channels than the wk->chan.

> > Also this seems buggy, we do not take into consideration how much offchannel
> > work we are doing in consideration against the current AP's DTIM interval as
> > we do when going offchannel for scan work. We should merge that code for
> > this offchannel work_work loop.
> 
> True, we don't do _any_ timing here.

We can resolve that later, I'll add that to the TODO list.

  Luis

  reply	other threads:[~2010-10-04 20:55 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-01 20:33 [PATCH v2 0/3] mac80211: fix rate_control_send_low warnings for delbas Luis R. Rodriguez
2010-10-01 20:33 ` [PATCH v2 1/3] mac80211: fix channel assumption for association done work Luis R. Rodriguez
2010-10-04 13:12   ` Johannes Berg
2010-10-01 20:33 ` [PATCH v2 2/3] mac80211: wait until completely disassociated before new association Luis R. Rodriguez
2010-10-04 13:14   ` Johannes Berg
2010-10-04 16:36     ` Luis R. Rodriguez
2010-10-04 16:39       ` Johannes Berg
2010-10-04 18:04         ` Luis R. Rodriguez
2010-10-04 18:44           ` Johannes Berg
2010-10-04 20:55             ` Luis R. Rodriguez [this message]
2010-10-05  7:59               ` Johannes Berg
2010-10-05 19:36                 ` Luis R. Rodriguez
2010-10-05 19:43                   ` Johannes Berg
2010-10-01 20:33 ` [PATCH v2 3/3] mac80211: move to the home channel for disassociation when roaming Luis R. Rodriguez
2010-10-04 13:15   ` Johannes Berg
2010-10-04 16:38     ` Luis R. Rodriguez
2010-10-04 16:41       ` Johannes Berg
2010-10-04 17:23         ` Luis R. Rodriguez
2010-10-04 18:39           ` Johannes Berg
2010-10-04 21:03             ` Luis R. Rodriguez
2010-10-01 20:50 ` [PATCH v2 0/3] mac80211: fix rate_control_send_low warnings for delbas Luis R. Rodriguez
2010-10-04 13:27 ` Johannes Berg
2010-10-04 16:41   ` Luis R. Rodriguez
2010-10-04 16:42     ` Johannes Berg
2010-10-04 17:25       ` Luis R. Rodriguez
2010-10-04 17:30         ` Johannes Berg
2010-10-04 17:35           ` Luis R. Rodriguez
2010-10-04 18:39             ` Johannes Berg
2010-10-04 20:39             ` Jouni Malinen
2010-10-04 21:07               ` Luis R. Rodriguez
2010-10-04 22:47                 ` Jouni Malinen
2010-10-04 23:50                   ` Luis R. Rodriguez
2010-10-05  7:57                     ` Johannes Berg
2010-10-05 17:03                       ` Luis R. Rodriguez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101004205551.GR2105@tux \
    --to=lrodriguez@atheros.com \
    --cc=Amod.Bodas@Atheros.com \
    --cc=Luis.Rodriguez@Atheros.com \
    --cc=Vasanth.Thiagarajan@Atheros.com \
    --cc=j@w1.fi \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=pstew@google.com \
    --cc=stable@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).