[PATCH 3.5] mac80211: flush off-channel work when interface goes down

[PATCH 3.5] mac80211: flush off-channel work when interface goes down

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

When any interface goes down, it could be the one that
we were doing off-channel with. Thus, we need to flush
the offchannel start/stop work so they can't run when
the interface has already been removed from the driver
or even been destroyed. It's OK to flush them even if
they were for another interface, that just means that
we potentially delay the interface down a little bit.

Cc: stable@kernel.org [3.4]
Reported-by: Nirav Shah <nirav.j2.shah@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/iface.c |    2 ++
 1 file changed, 2 insertions(+)

--- a/net/mac80211/iface.c	2012-05-30 15:08:51.000000000 +0200
+++ b/net/mac80211/iface.c	2012-05-30 15:09:24.000000000 +0200
@@ -637,6 +637,8 @@ static void ieee80211_do_stop(struct iee
 		ieee80211_configure_filter(local);
 		break;
 	default:
+		flush_work(&local->hw_roc_start);
+		flush_work(&local->hw_roc_done);
 		flush_work(&sdata->work);
 		/*
 		 * When we get here, the interface is marked down.

Re: [PATCH 3.5] mac80211: flush off-channel work when interface goes down

[Eliad Peller]

On Wed, May 30, 2012 at 4:16 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> When any interface goes down, it could be the one that
> we were doing off-channel with. Thus, we need to flush
> the offchannel start/stop work so they can't run when
> the interface has already been removed from the driver
> or even been destroyed. It's OK to flush them even if
> they were for another interface, that just means that
> we potentially delay the interface down a little bit.
>
can't we check hw_roc_dev?

Eliad.

Re: [PATCH 3.5] mac80211: flush off-channel work when interface goes down

[Johannes Berg]

On Wed, 2012-05-30 at 16:44 +0300, Eliad Peller wrote:
> On Wed, May 30, 2012 at 4:16 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > When any interface goes down, it could be the one that
> > we were doing off-channel with. Thus, we need to flush
> > the offchannel start/stop work so they can't run when
> > the interface has already been removed from the driver
> > or even been destroyed. It's OK to flush them even if
> > they were for another interface, that just means that
> > we potentially delay the interface down a little bit.
> >
> can't we check hw_roc_dev?

We could, but it didn't seem worthwhile. Think I should do that instead?

johannes

Re: [PATCH 3.5] mac80211: flush off-channel work when interface goes down

[Eliad Peller]

On Wed, May 30, 2012 at 4:50 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2012-05-30 at 16:44 +0300, Eliad Peller wrote:
>> On Wed, May 30, 2012 at 4:16 PM, Johannes Berg
>> <johannes@sipsolutions.net> wrote:
>> > When any interface goes down, it could be the one that
>> > we were doing off-channel with. Thus, we need to flush
>> > the offchannel start/stop work so they can't run when
>> > the interface has already been removed from the driver
>> > or even been destroyed. It's OK to flush them even if
>> > they were for another interface, that just means that
>> > we potentially delay the interface down a little bit.
>> >
>> can't we check hw_roc_dev?
>
> We could, but it didn't seem worthwhile. Think I should do that instead?
>
hmm... i guess it doesn't really matter :)

Eliad.

Re: [PATCH 3.5] mac80211: flush off-channel work when interface goes down

[Eliad Peller]

On Wed, May 30, 2012 at 4:57 PM, Eliad Peller <eliad@wizery.com> wrote:
> On Wed, May 30, 2012 at 4:50 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
>> On Wed, 2012-05-30 at 16:44 +0300, Eliad Peller wrote:
>>> On Wed, May 30, 2012 at 4:16 PM, Johannes Berg
>>> <johannes@sipsolutions.net> wrote:
>>> > When any interface goes down, it could be the one that
>>> > we were doing off-channel with. Thus, we need to flush
>>> > the offchannel start/stop work so they can't run when
>>> > the interface has already been removed from the driver
>>> > or even been destroyed. It's OK to flush them even if
>>> > they were for another interface, that just means that
>>> > we potentially delay the interface down a little bit.
>>> >
>>> can't we check hw_roc_dev?
>>
>> We could, but it didn't seem worthwhile. Think I should do that instead?
>>
> hmm... i guess it doesn't really matter :)
>
just gave it a second thought - the driver might have not even called
ieee80211_ready_on_channel() yet, so i think it's still racy.

Eliad.

Re: [PATCH 3.5] mac80211: flush off-channel work when interface goes down

[Johannes Berg]

On Thu, 2012-05-31 at 09:07 +0300, Eliad Peller wrote:
> On Wed, May 30, 2012 at 4:57 PM, Eliad Peller <eliad@wizery.com> wrote:
> > On Wed, May 30, 2012 at 4:50 PM, Johannes Berg
> > <johannes@sipsolutions.net> wrote:
> >> On Wed, 2012-05-30 at 16:44 +0300, Eliad Peller wrote:
> >>> On Wed, May 30, 2012 at 4:16 PM, Johannes Berg
> >>> <johannes@sipsolutions.net> wrote:
> >>> > When any interface goes down, it could be the one that
> >>> > we were doing off-channel with. Thus, we need to flush
> >>> > the offchannel start/stop work so they can't run when
> >>> > the interface has already been removed from the driver
> >>> > or even been destroyed. It's OK to flush them even if
> >>> > they were for another interface, that just means that
> >>> > we potentially delay the interface down a little bit.
> >>> >
> >>> can't we check hw_roc_dev?
> >>
> >> We could, but it didn't seem worthwhile. Think I should do that instead?
> >>
> > hmm... i guess it doesn't really matter :)
> >
> just gave it a second thought - the driver might have not even called
> ieee80211_ready_on_channel() yet, so i think it's still racy.

Hmmm. Good point. For some reason I thought that wouldn't happen, but I
guess you're right.

I'll need to think more about how we can fix that though. Maybe cfg80211
could request a cancel when the corresponding interface goes down?

johannes

[PATCH v2 3.5] mac80211: clean up remain-on-channel on interface stop

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

When any interface goes down, it could be the one that we
were doing a remain-on-channel with. We therefore need to
cancel the remain-on-channel and flush the related work
structs so they don't run after the interface has been
removed or even destroyed.

It's also possible in this case that an off-channel SKB
was never transmitted, so free it if this is the case.
Note that this can also happen if the driver finishes
the off-channel period without ever starting it.

Cc: stable@kernel.org
Reported-by: Nirav Shah <nirav.j2.shah@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
v2: actually cancel the roc and clean up after it

 net/mac80211/iface.c      |   12 ++++++++++++
 net/mac80211/offchannel.c |   16 ++++++++++++++++
 2 files changed, 28 insertions(+)

--- a/net/mac80211/iface.c	2012-05-31 14:29:52.000000000 +0200
+++ b/net/mac80211/iface.c	2012-05-31 15:03:22.000000000 +0200
@@ -637,6 +637,18 @@ static void ieee80211_do_stop(struct iee
 		ieee80211_configure_filter(local);
 		break;
 	default:
+		mutex_lock(&local->mtx);
+		if (local->hw_roc_dev == sdata->dev &&
+		    local->hw_roc_channel) {
+			/* ignore return value since this is racy */
+			drv_cancel_remain_on_channel(local);
+			ieee80211_queue_work(&local->hw, &local->hw_roc_done);
+		}
+		mutex_unlock(&local->mtx);
+
+		flush_work(&local->hw_roc_start);
+		flush_work(&local->hw_roc_done);
+
 		flush_work(&sdata->work);
 		/*
 		 * When we get here, the interface is marked down.
--- a/net/mac80211/offchannel.c	2012-05-31 14:48:54.000000000 +0200
+++ b/net/mac80211/offchannel.c	2012-05-31 14:48:57.000000000 +0200
@@ -234,6 +234,22 @@ static void ieee80211_hw_roc_done(struct
 		return;
 	}
 
+	/* was never transmitted */
+	if (local->hw_roc_skb) {
+		u64 cookie;
+
+		cookie = local->hw_roc_cookie ^ 2;
+
+		cfg80211_mgmt_tx_status(local->hw_roc_dev, cookie,
+					local->hw_roc_skb->data,
+					local->hw_roc_skb->len, false,
+					GFP_KERNEL);
+
+		kfree_skb(local->hw_roc_skb);
+		local->hw_roc_skb = NULL;
+		local->hw_roc_skb_for_status = NULL;
+	}
+
 	if (!local->hw_roc_for_tx)
 		cfg80211_remain_on_channel_expired(local->hw_roc_dev,
 						   local->hw_roc_cookie,